Light up review agent rows and add a contextual tip once their review is ready

apps/web/src/components/app/child-agent-row.test.tsx

@@ -1,6 +1,7 @@
 // @vitest-environment jsdom
 import { cleanup, fireEvent, render, screen } from "@testing-library/react";
 import type { ComponentProps } from "react";
+import { MemoryRouter } from "react-router-dom";
 import { afterEach, describe, expect, it, vi } from "vitest";
 
 import type { Agent } from "@/components/app/types";
@@ -42,19 +43,21 @@ function renderRow(
   const startAgent = vi.fn().mockResolvedValue(undefined);
   const openSubmittedReview = vi.fn();
   render(
-    <TooltipProvider>
-      <ChildAgentRow
-        agent={agent}
-        state="idle"
-        isInitialReviewActive={true}
-        isConnected={false}
-        attachToAgent={attachToAgent}
-        detachTerminal={detachTerminal}
-        startAgent={startAgent}
-        openSubmittedReview={openSubmittedReview}
-        {...overrides}
-      />
-    </TooltipProvider>
+    <MemoryRouter>
+      <TooltipProvider>
+        <ChildAgentRow
+          agent={agent}
+          state="idle"
+          isInitialReviewActive={true}
+          isConnected={false}
+          attachToAgent={attachToAgent}
+          detachTerminal={detachTerminal}
+          startAgent={startAgent}
+          openSubmittedReview={openSubmittedReview}
+          {...overrides}
+        />
+      </TooltipProvider>
+    </MemoryRouter>
   );
   return {
     attachToAgent,
@@ -100,6 +103,47 @@ describe("ChildAgentRow", () => {
     expect(row.className).not.toContain("child-agent-review-active-row");
   });
 
+  it("keeps the row muted until a review has been submitted", () => {
+    renderRow(baseAgent);
+
+    const row = screen.getByTestId("child-agent-row-agt_child");
+    expect(row.dataset.reviewReady).toBe("false");
+    expect(row.className).not.toContain("cursor-pointer");
+    const badge = screen.getByText("Review");
+    expect(badge.className).toContain("bg-background");
+  });
+
+  it("lights up the row once the review can be opened", () => {
+    renderRow(
+      { ...baseAgent, status: "stopped", submittedReviewId: 42 },
+      { state: "stopped", isInitialReviewActive: false }
+    );
+
+    const row = screen.getByTestId("child-agent-row-agt_child");
+    expect(row.dataset.reviewReady).toBe("true");
+    expect(row.className).toContain("cursor-pointer");
+    expect(row.className).toContain("border-primary/45");
+    expect(row.className).toContain("opacity-100");
+    expect(row.className).not.toContain("opacity-65");
+    const badge = screen.getByText("Review");
+    expect(badge.className).toContain("bg-primary");
+    expect(badge.className).toContain("text-primary-foreground");
+  });
+
+  it("keeps the ready treatment when the row is terminal-connected", () => {
+    renderRow(
+      { ...baseAgent, submittedReviewId: 42 },
+      { isConnected: true, isInitialReviewActive: false }
+    );
+
+    const row = screen.getByTestId("child-agent-row-agt_child");
+    expect(row.className).toContain("border-primary/45");
+    expect(row.className).toContain("bg-primary/[0.06]");
+    expect(row.className).toContain("hover:bg-primary/10");
+    expect(row.className).toContain("cursor-pointer");
+    expect(row.className).not.toContain("border-primary/35");
+  });
+
   it("opens a submitted review from the row without attaching its terminal", () => {
     const submittedAgent = {
       ...baseAgent,

apps/web/src/components/app/child-agent-row.tsx

@@ -7,6 +7,7 @@ import {
 } from "@/components/app/agent-event-utils";
 import { AgentTypeIcon } from "@/components/app/agent-type-icon";
 import { type Agent, type AgentVisualState } from "@/components/app/types";
+import { TipSpot } from "@/components/tips/tip-spot";
 import { Badge } from "@/components/ui/badge";
 import { Button } from "@/components/ui/button";
 import {
@@ -62,17 +63,21 @@ export function ChildAgentRow({
         ? latestEventColor(agent.latestEvent.type)
         : "text-muted-foreground";
 
-  return (
+  const row = (
     <div
       data-testid={`child-agent-row-${agent.id}`}
       data-agent-role={agent.role ?? "standard"}
       data-review-active={showReviewActivity ? "true" : "false"}
+      data-review-ready={canOpenSubmittedReview ? "true" : "false"}
       className={cn(
-        "group relative flex min-h-11 min-w-0 items-center gap-2 rounded-lg border border-border/60 bg-background/30 px-2 py-1 sm:py-1.5",
+        "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-lg border border-border/60 bg-background/30 px-2 py-1 sm:py-1.5",
         "transition-colors hover:bg-muted/35",
-        canOpenSubmittedReview && "cursor-pointer",
         isConnected && "border-primary/35 bg-muted/40",
+        // Ready-to-open wins over connected styling: it is the actionable state.
+        canOpenSubmittedReview &&
+          "cursor-pointer border-primary/45 bg-primary/[0.06] hover:bg-primary/10",
         isStopped && "opacity-65",
+        canOpenSubmittedReview && "opacity-100",
         showReviewActivity && "child-agent-review-active-row"
       )}
     >
@@ -109,7 +114,14 @@ export function ChildAgentRow({
             {displayName}
           </span>
           {isReviewAgent ? (
-            <Badge className="ml-auto h-4 shrink-0 border-primary bg-background px-1 text-[8px] font-semibold uppercase tracking-wide text-foreground">
+            <Badge
+              className={cn(
+                "ml-auto h-4 shrink-0 border-primary px-1 text-[8px] font-semibold uppercase tracking-wide",
+                canOpenSubmittedReview
+                  ? "bg-primary text-primary-foreground"
+                  : "bg-background text-foreground"
+              )}
+            >
               Review
             </Badge>
           ) : null}
@@ -186,4 +198,16 @@ export function ChildAgentRow({
       )}
     </div>
   );
+
+  if (!canOpenSubmittedReview) return row;
+  return (
+    <TipSpot
+      tipId="review-row-open"
+      side="bottom"
+      align="center"
+      triggerClassName="flex w-full"
+    >
+      {row}
+    </TipSpot>
+  );
 }

apps/web/src/components/tips/tip-queue-context.ts

@@ -2,8 +2,8 @@ import { createContext, useContext } from "react";
 
 export type TipQueueContextValue = {
   activeTipId: string | null;
-  requestOpen: (tipId: string) => boolean;
-  release: (tipId: string) => void;
+  requestOpen: (tipId: string, instanceId: string) => boolean;
+  release: (tipId: string, instanceId: string) => void;
 };
 
 export const TipQueueContext = createContext<TipQueueContextValue>({

apps/web/src/components/tips/tip-queue-provider.test.tsx

@@ -0,0 +1,118 @@
+// @vitest-environment jsdom
+import { act, cleanup, render } from "@testing-library/react";
+import { afterEach, describe, expect, it } from "vitest";
+
+import { useTipQueue } from "./tip-queue-context";
+import { TipQueueProvider } from "./tip-queue-provider";
+
+type QueueHandle = ReturnType<typeof useTipQueue>;
+
+function Capture({ handle }: { handle: { current: QueueHandle | null } }) {
+  handle.current = useTipQueue();
+  return null;
+}
+
+function renderQueue() {
+  const handle: { current: QueueHandle | null } = { current: null };
+  render(
+    <TipQueueProvider>
+      <Capture handle={handle} />
+    </TipQueueProvider>
+  );
+  return handle as { current: QueueHandle };
+}
+
+afterEach(cleanup);
+
+describe("TipQueueProvider", () => {
+  it("grants the first instance and denies a second instance of the same tip", () => {
+    const queue = renderQueue();
+
+    let first = false;
+    act(() => {
+      first = queue.current.requestOpen("review-row-open", "instance-a");
+    });
+    expect(first).toBe(true);
+
+    let second = true;
+    act(() => {
+      second = queue.current.requestOpen("review-row-open", "instance-b");
+    });
+    expect(second).toBe(false);
+
+    // The owner keeps its grant on re-request (effect re-runs).
+    let ownerAgain = false;
+    act(() => {
+      ownerAgain = queue.current.requestOpen("review-row-open", "instance-a");
+    });
+    expect(ownerAgain).toBe(true);
+  });
+
+  it("does not hand the same tip to another instance after the owner releases", () => {
+    const queue = renderQueue();
+
+    act(() => {
+      queue.current.requestOpen("review-row-open", "instance-a");
+    });
+    act(() => {
+      queue.current.requestOpen("review-row-open", "instance-b");
+    });
+    act(() => {
+      queue.current.release("review-row-open", "instance-a");
+    });
+
+    expect(queue.current.activeTipId).toBeNull();
+  });
+
+  it("queues a different tip and grants it after release", () => {
+    const queue = renderQueue();
+
+    act(() => {
+      queue.current.requestOpen("review-row-open", "instance-a");
+    });
+    let other = true;
+    act(() => {
+      other = queue.current.requestOpen("split-tabs", "instance-c");
+    });
+    expect(other).toBe(false);
+
+    act(() => {
+      queue.current.release("review-row-open", "instance-a");
+    });
+    expect(queue.current.activeTipId).toBe("split-tabs");
+
+    let granted = false;
+    act(() => {
+      granted = queue.current.requestOpen("split-tabs", "instance-c");
+    });
+    expect(granted).toBe(true);
+  });
+
+  it("denies a second same-tick grant before state commits", () => {
+    const queue = renderQueue();
+
+    let first = false;
+    let second = true;
+    act(() => {
+      // Both calls happen in one tick, before the first grant's state commit.
+      first = queue.current.requestOpen("review-row-open", "instance-a");
+      second = queue.current.requestOpen("review-row-open", "instance-b");
+    });
+
+    expect(first).toBe(true);
+    expect(second).toBe(false);
+  });
+
+  it("ignores a release from a non-owning instance", () => {
+    const queue = renderQueue();
+
+    act(() => {
+      queue.current.requestOpen("review-row-open", "instance-a");
+    });
+    act(() => {
+      queue.current.release("review-row-open", "instance-b");
+    });
+
+    expect(queue.current.activeTipId).toBe("review-row-open");
+  });
+});

apps/web/src/components/tips/tip-queue-provider.tsx

@@ -2,36 +2,63 @@ import { useCallback, useMemo, useRef, useState } from "react";
 
 import { TipQueueContext } from "@/components/tips/tip-queue-context";
 
+type TipClaim = { tipId: string; instanceId: string };
+
 export function TipQueueProvider({ children }: { children: React.ReactNode }) {
-  const [activeTipId, setActiveTipId] = useState<string | null>(null);
-  const queueRef = useRef<string[]>([]);
+  const [active, setActive] = useState<TipClaim | null>(null);
+  // Synchronous source of truth: two open timers can fire in the same tick,
+  // before the `active` state from the first grant has committed.
+  const activeRef = useRef<TipClaim | null>(null);
+  const queueRef = useRef<TipClaim[]>([]);
 
+  // `active` is an intentional dep even though reads go through activeRef:
+  // its identity change re-runs waiting TipSpot effects so a queued tip
+  // re-requests (and wins) after the previous owner releases.
   const requestOpen = useCallback(
-    (tipId: string): boolean => {
-      if (activeTipId === null) {
-        setActiveTipId(tipId);
+    (tipId: string, instanceId: string): boolean => {
+      const current = activeRef.current;
+      if (current === null) {
+        const claim = { tipId, instanceId };
+        activeRef.current = claim;
+        setActive(claim);
         return true;
       }
-      if (activeTipId === tipId) return true;
-      if (!queueRef.current.includes(tipId)) {
-        queueRef.current.push(tipId);
+      if (current.tipId === tipId) {
+        // Only the owning instance holds the grant. Another instance of the
+        // same tip is denied without queueing — after the owner dismisses,
+        // the tip lands in dismissedTips and no other instance should open.
+        return current.instanceId === instanceId;
+      }
+      if (
+        !queueRef.current.some(
+          (claim) => claim.tipId === tipId && claim.instanceId === instanceId
+        )
+      ) {
+        queueRef.current.push({ tipId, instanceId });
       }
       return false;
     },
-    [activeTipId]
+    // eslint-disable-next-line react-hooks/exhaustive-deps -- see comment above
+    [active]
   );
 
-  const release = useCallback((tipId: string) => {
-    setActiveTipId((current) => {
-      if (current !== tipId) return current;
-      const next = queueRef.current.shift() ?? null;
-      return next;
-    });
+  const release = useCallback((tipId: string, instanceId: string) => {
+    const current = activeRef.current;
+    if (
+      current === null ||
+      current.tipId !== tipId ||
+      current.instanceId !== instanceId
+    ) {
+      return;
+    }
+    const next = queueRef.current.shift() ?? null;
+    activeRef.current = next;
+    setActive(next);
   }, []);
 
   const value = useMemo(
-    () => ({ activeTipId, requestOpen, release }),
-    [activeTipId, requestOpen, release]
+    () => ({ activeTipId: active?.tipId ?? null, requestOpen, release }),
+    [active, requestOpen, release]
   );
 
   return (