diff --git a/apps/web/src/components/app/agent-card-actions.tsx b/apps/web/src/components/app/agent-card-actions.tsx index ec61ef85..b2f42497 100644 --- a/apps/web/src/components/app/agent-card-actions.tsx +++ b/apps/web/src/components/app/agent-card-actions.tsx @@ -93,8 +93,8 @@ export function AgentCardActions({ variant="ghost" className="h-8 w-8 rounded-full border border-blue-500/35 bg-blue-500/10 p-0 text-blue-400 hover:bg-blue-500/15 hover:text-blue-300" data-testid={`agent-session-settings-${agent.id}`} - aria-label="Edit session settings" - title="Edit session settings" + aria-label="Session details" + title="Session details" onClick={onEditSettings} > diff --git a/apps/web/src/components/app/agent-card.tsx b/apps/web/src/components/app/agent-card.tsx index 21ced193..4e36ba37 100644 --- a/apps/web/src/components/app/agent-card.tsx +++ b/apps/web/src/components/app/agent-card.tsx @@ -224,7 +224,11 @@ export function AgentCard({ {childAgents.length} - +
{childAgents.map((child) => ( { if (!open) setSettingsChild(null); }} + enabledIdes={enabledIdes} /> ); diff --git a/apps/web/src/components/app/agent-event-utils.ts b/apps/web/src/components/app/agent-event-utils.ts index 6bf01980..20164e6e 100644 --- a/apps/web/src/components/app/agent-event-utils.ts +++ b/apps/web/src/components/app/agent-event-utils.ts @@ -17,3 +17,37 @@ export function latestEventColor(type: EventType): string { if (type === "done") return "text-status-done"; return "text-foreground/80"; } + +/** + * The row's own current-status label/color — "Stopped"/"Error" when the + * agent isn't actively running, otherwise the latest event's own + * label/color. `isStopped` is a caller-supplied condition (rather than + * derived here from `agent.status`) so it can key off whatever "stopped" + * means in context — e.g. ChildAgentRow's `state === "stopped"`, which + * covers more than a literal `status === "stopped"`. + * + * This exists because the event's own label/color describes the latest + * EVENT, not the agent's current status — a stopped or errored agent can + * still have a stale "Working" event, which would otherwise render in the + * active-work color even though the agent isn't running anymore. + */ +export function describeAgentStatus( + agent: Pick, + isStopped: boolean +): { label: string; colorClass: string } { + const label = + agent.status === "error" + ? "Error" + : isStopped + ? "Stopped" + : agent.latestEvent + ? latestEventLabel(agent.latestEvent.type) + : "Running"; + const colorClass = + agent.status === "error" + ? "text-status-blocked" + : agent.latestEvent && !isStopped + ? latestEventColor(agent.latestEvent.type) + : "text-muted-foreground"; + return { label, colorClass }; +} diff --git a/apps/web/src/components/app/child-agent-row.test.tsx b/apps/web/src/components/app/child-agent-row.test.tsx index 20c480bc..7653336b 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -48,14 +48,15 @@ function renderRow( const setDeleteTarget = vi.fn(); const setDeleteConfirmOpen = vi.fn(); const onEditSettings = vi.fn(); - render( + const buildElement = ( + elementOverrides: Partial> = {} + ) => ( ); + const { rerender } = render(buildElement(overrides)); return { attachToAgent, detachTerminal, @@ -80,9 +82,19 @@ function renderRow( setDeleteTarget, setDeleteConfirmOpen, onEditSettings, + rerenderWith: ( + elementOverrides: Partial> + ) => rerender(buildElement({ ...overrides, ...elementOverrides })), }; } +function openMenu(agentId = "agt_child") { + fireEvent.pointerDown( + screen.getByTestId(`child-agent-menu-${agentId}`), + new MouseEvent("pointerdown", { bubbles: true, button: 0 }) + ); +} + describe("ChildAgentRow", () => { it("labels review agents and chases before their initial review is submitted", () => { renderRow({ @@ -94,16 +106,8 @@ describe("ChildAgentRow", () => { }, }); - const badge = screen.getByText("Review"); - expect(badge.className.split(/\s+/)).toEqual( - expect.arrayContaining([ - "ml-auto", - "border-primary", - "bg-background", - "text-foreground", - ]) - ); - expect(badge.className).not.toContain("violet"); + const indicator = screen.getByRole("img", { name: "Review in progress" }); + expect(indicator.className).toContain("text-muted-foreground"); const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("min-h-11"); expect(row.dataset.agentRole).toBe("review"); @@ -111,6 +115,19 @@ describe("ChildAgentRow", () => { expect(row.className).toContain("child-agent-review-active-row"); }); + it("groups the review indicator with the overflow menu control, not the truncating name label", () => { + renderRow(baseAgent); + + const indicator = screen.getByRole("img", { name: "Review in progress" }); + const menuButton = screen.getByTestId("child-agent-menu-agt_child"); + // The indicator and the overflow menu button should share an immediate + // parent (the right-side action cluster) rather than living inside the + // name label's min-w-0/flex-1/truncate wrapper. + expect(indicator.closest("div.flex.shrink-0")).toBe( + menuButton.closest("div.flex.shrink-0") + ); + }); + it("stops chasing after the initial review is submitted", () => { renderRow(baseAgent, { isInitialReviewActive: false }); @@ -119,17 +136,22 @@ describe("ChildAgentRow", () => { expect(row.className).not.toContain("child-agent-review-active-row"); }); - it("keeps the row muted until a review has been submitted", () => { + it("shows the muted clipboard-list indicator 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"); + const indicator = screen.getByRole("img", { name: "Review in progress" }); + expect(indicator.querySelector("svg.lucide-clipboard-list")).not.toBeNull(); + expect(indicator.querySelector("svg.lucide-clipboard-check")).toBeNull(); + // "Open review" only makes sense once a review exists. + openMenu(); + expect( + screen.queryByTestId("child-agent-open-review-agt_child") + ).toBeNull(); }); - it("lights up the row once the review can be opened", () => { + it("swaps to a colored clipboard-check indicator once the review can be opened, without a row border", () => { renderRow( { ...baseAgent, status: "stopped", submittedReviewId: 42 }, { state: "stopped", isInitialReviewActive: false } @@ -137,88 +159,255 @@ describe("ChildAgentRow", () => { 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"); + // "Ready to open" no longer gets its own row-wide border/tint (it used + // to read as a muted echo of the connected accent) — the indicator's + // color/icon swap is the sole carrier of that signal, and opening it + // moves to the overflow menu (tested below), decoupled from connecting. + expect(row.className).not.toContain("border-primary/45"); + expect(row.className).not.toContain("bg-primary/[0.06]"); + const trigger = screen.getByTestId( + "child-agent-open-review-badge-agt_child" + ); + // status-working (green), deliberately not the same color family as the + // connected accent (status-done/primary, blue in this theme) — the two + // signals must never look like variants of each other. + expect(trigger.className).toContain("text-status-working"); + expect(trigger.querySelector("svg.lucide-clipboard-check")).not.toBeNull(); }); - it("keeps the ready treatment when the row is terminal-connected", () => { - renderRow( - { ...baseAgent, submittedReviewId: 42 }, - { isConnected: true, isInitialReviewActive: false } - ); + it("opens the submitted review from the overflow menu, independent of connecting", () => { + const submittedAgent = { ...baseAgent, submittedReviewId: 42 }; + const { attachToAgent, openSubmittedReview } = renderRow(submittedAgent, { + 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"); + openMenu(); + fireEvent.click(screen.getByTestId("child-agent-open-review-agt_child")); + expect(openSubmittedReview).toHaveBeenCalledWith(submittedAgent); + // Also proves the portal-bubbling fix: DropdownMenuContent is portaled + // outside the row's real DOM, but React's synthetic events still + // bubble through the *component* tree — without the row's + // currentTarget.contains() guard, this click would also attach. + expect(attachToAgent).not.toHaveBeenCalled(); }); - it("opens a submitted review from the row without attaching its terminal", () => { - const submittedAgent = { - ...baseAgent, - submittedReviewId: 42, - }; + it("opens the submitted review by clicking its own badge, not the row", () => { + const submittedAgent = { ...baseAgent, submittedReviewId: 42 }; const { attachToAgent, openSubmittedReview } = renderRow(submittedAgent, { isInitialReviewActive: false, }); - fireEvent.click(screen.getByTestId("child-agent-open-review-agt_child")); + fireEvent.click( + screen.getByTestId("child-agent-open-review-badge-agt_child") + ); expect(openSubmittedReview).toHaveBeenCalledWith(submittedAgent); expect(attachToAgent).not.toHaveBeenCalled(); }); - it("keeps the terminal control independent from review navigation", () => { - const { attachToAgent, openSubmittedReview } = renderRow({ + it("keeps the badge trigger reachable on a stopped, ready-to-open row", () => { + // The row's own click-to-connect is a dead end here (isStopped bails + // out), so the badge is the only way to reach the review without + // opening the overflow menu — worth pinning explicitly. + const submittedAgent = { ...baseAgent, + status: "stopped" as const, submittedReviewId: 42, + }; + const { openSubmittedReview } = renderRow(submittedAgent, { + state: "stopped", + isInitialReviewActive: false, }); - const attachButton = screen.getByTestId("child-agent-attach-agt_child"); - expect(attachButton.className).toContain("h-11"); - expect(attachButton.className).toContain("w-11"); - expect(attachButton.className).toContain("sm:h-7"); - fireEvent.click(attachButton); - expect(attachToAgent).toHaveBeenCalledOnce(); - expect(openSubmittedReview).not.toHaveBeenCalled(); + fireEvent.click( + screen.getByTestId("child-agent-open-review-badge-agt_child") + ); + expect(openSubmittedReview).toHaveBeenCalledWith(submittedAgent); + }); + + it("renders the badge plain (not a button) before a review is submitted", () => { + renderRow(baseAgent); + + expect( + screen.queryByTestId("child-agent-open-review-badge-agt_child") + ).toBeNull(); + }); + + describe("keyboard/screen-reader terminal access (the overflow menu's View terminal / Detach item)", () => { + it("attaches from the menu when not connected", () => { + const { attachToAgent } = renderRow( + { ...baseAgent, role: "standard" }, + { state: "idle" } + ); + + openMenu(); + fireEvent.click(screen.getByTestId("child-agent-terminal-agt_child")); + expect(attachToAgent).toHaveBeenCalledWith( + expect.objectContaining({ id: "agt_child" }) + ); + }); + + it("detaches from the menu when connected", () => { + const { detachTerminal } = renderRow( + { ...baseAgent, role: "standard" }, + { state: "active" } + ); + + openMenu(); + const item = screen.getByTestId("child-agent-terminal-agt_child"); + expect(item.textContent).toContain("Detach"); + fireEvent.click(item); + expect(detachTerminal).toHaveBeenCalledOnce(); + }); + + it("is absent for a stopped agent, which uses Resume instead", () => { + const stopped = { ...baseAgent, status: "stopped" as const }; + renderRow(stopped, { state: "stopped" }); + + openMenu(); + expect(screen.queryByTestId("child-agent-terminal-agt_child")).toBeNull(); + }); + }); + + it("still attaches by clicking a ready-to-open row's body, same as any other row", () => { + // Opening the review is a menu action now — the row itself has no + // special case for a ready-to-open review, it's click-to-connect like + // every other row. + const { attachToAgent } = renderRow( + { ...baseAgent, submittedReviewId: 42 }, + { isInitialReviewActive: false, state: "idle" } + ); + + fireEvent.click(screen.getByTestId("child-agent-row-agt_child")); + expect(attachToAgent).toHaveBeenCalledWith( + expect.objectContaining({ id: "agt_child" }) + ); + }); + + it("shows the connected right-edge accent when not also ready to open", () => { + renderRow(baseAgent, { state: "active" }); + + const row = screen.getByTestId("child-agent-row-agt_child"); + expect(row.className).toContain("border-r-4"); + expect(row.className).toContain("border-r-status-done"); + expect(row.className).not.toContain("border-primary/45"); + }); + + it("does not light the connected accent for a paused agent that's still attached", () => { + // state tracks agentVisualState (running/creating AND actually + // connected) — the accent (and the row's click-to-detach) follow it, + // not any looser notion of "was ever attached." + renderRow(baseAgent, { state: "stopped" }); + + const row = screen.getByTestId("child-agent-row-agt_child"); + expect(row.className).not.toContain("border-r-4"); + expect(row.className).not.toContain("border-r-status-done"); + // A normal 1px border matching the row's other sides — not a thick + // reserved edge (muted or invisible), just an ordinary bordered pill. + expect(row.className).toContain("border-border/60"); + }); + + it("describes a paused reviewer's pending indicator differently from an active one", () => { + // canOpenSubmittedReview is just "no submission yet" — much broader + // than "actively working." A stopped reviewer never submitted, so + // "Review in progress" would misdescribe it. + renderRow( + { ...baseAgent, status: "stopped" }, + { state: "stopped", isInitialReviewActive: false } + ); + + expect( + screen.queryByRole("img", { name: "Review in progress" }) + ).toBeNull(); + // Throws (failing the test) if not found — this is the assertion. + screen.getByRole("img", { + name: "Review agent — paused, no review submitted", + }); }); it("does not infer review purpose from a persona", () => { renderRow({ ...baseAgent, role: "standard" }); - expect(screen.queryByText("Review")).toBeNull(); + expect( + screen.queryByRole("img", { name: "Review in progress" }) + ).toBeNull(); const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.dataset.reviewActive).toBe("false"); expect(row.className).not.toContain("child-agent-review-active-row"); }); - it("detaches to the detached state without attaching another agent", () => { - const { attachToAgent, detachTerminal } = renderRow(baseAgent, { - state: "active", - isConnected: true, + describe("click-to-connect (mirrors the top-level agent card)", () => { + it("attaches by clicking anywhere on the row", () => { + const { attachToAgent, detachTerminal } = renderRow( + { ...baseAgent, role: "standard" }, + { state: "idle" } + ); + + fireEvent.click(screen.getByTestId("child-agent-row-agt_child")); + expect(attachToAgent).toHaveBeenCalledWith( + expect.objectContaining({ id: "agt_child" }) + ); + expect(detachTerminal).not.toHaveBeenCalled(); }); - fireEvent.click(screen.getByTestId("child-agent-detach-agt_child")); - expect(detachTerminal).toHaveBeenCalledOnce(); - expect(attachToAgent).not.toHaveBeenCalled(); + it("detaches by clicking an already-connected row", () => { + const { attachToAgent, detachTerminal } = renderRow( + { ...baseAgent, role: "standard" }, + { state: "active" } + ); + + fireEvent.click(screen.getByTestId("child-agent-row-agt_child")); + expect(detachTerminal).toHaveBeenCalledOnce(); + expect(attachToAgent).not.toHaveBeenCalled(); + }); + + it("does not attach or detach by clicking a stopped row", () => { + const stopped = { + ...baseAgent, + role: "standard" as const, + status: "stopped" as const, + }; + const { attachToAgent, detachTerminal } = renderRow(stopped, { + state: "stopped", + }); + + const row = screen.getByTestId("child-agent-row-agt_child"); + expect(row.className).not.toContain("cursor-pointer"); + fireEvent.click(row); + expect(attachToAgent).not.toHaveBeenCalled(); + expect(detachTerminal).not.toHaveBeenCalled(); + }); + + it("does not attach when clicking the overflow menu button", () => { + const { attachToAgent } = renderRow( + { ...baseAgent, role: "standard" }, + { state: "idle" } + ); + + fireEvent.click(screen.getByTestId("child-agent-menu-agt_child")); + expect(attachToAgent).not.toHaveBeenCalled(); + }); + + it("does not attach when clicking the resume button on a stopped row", () => { + const stopped = { + ...baseAgent, + role: "standard" as const, + status: "stopped" as const, + }; + const { attachToAgent, startAgent } = renderRow(stopped, { + state: "stopped", + }); + + fireEvent.click(screen.getByTestId("child-agent-resume-agt_child")); + expect(startAgent).toHaveBeenCalledWith(stopped); + expect(attachToAgent).not.toHaveBeenCalled(); + }); }); describe("session actions", () => { // Plain children now live in this section too, so the row has to carry the // lifecycle controls an agent card's footer offers. - function openMenu() { - fireEvent.pointerDown( - screen.getByTestId("child-agent-menu-agt_child"), - new MouseEvent("pointerdown", { bubbles: true, button: 0 }) - ); - } - it("archives the sub agent through the shared confirmation dialog", () => { const { setDeleteTarget, setDeleteConfirmOpen } = renderRow(baseAgent); diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index 04ca749d..a15cdd56 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -1,5 +1,8 @@ import { Archive, + ClipboardCheck, + ClipboardList, + Eye, MoreVertical, Pause, Pencil, @@ -7,15 +10,12 @@ import { Terminal, Unplug, } from "lucide-react"; +import type { ReactNode } from "react"; -import { - latestEventColor, - latestEventLabel, -} from "@/components/app/agent-event-utils"; +import { describeAgentStatus } 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 { DropdownMenu, @@ -31,11 +31,31 @@ import { import { formatRelativeTime } from "@/lib/format"; import { cn } from "@/lib/utils"; +/** + * Wraps the REVIEW badge in the "open review from the menu" tip once its + * review is ready to open — otherwise renders the badge plain. A tiny local + * component (rather than an inline ternary) so TipSpot's own eligibility + * check still only ever mounts for a badge that's actually ready. + */ +function ReviewBadge({ + tip, + children, +}: { + tip: boolean; + children: ReactNode; +}): JSX.Element { + if (!tip) return <>{children}; + return ( + + {children} + + ); +} + export type ChildAgentRowProps = { agent: Agent; state: AgentVisualState; isInitialReviewActive: boolean; - isConnected: boolean; attachToAgent: (agent: Agent) => Promise; detachTerminal: () => void; startAgent: (agent: Agent) => Promise; @@ -53,7 +73,6 @@ export function ChildAgentRow({ agent, state, isInitialReviewActive, - isConnected, attachToAgent, detachTerminal, startAgent, @@ -67,6 +86,12 @@ export function ChildAgentRow({ closeOnSessionAction = false, }: ChildAgentRowProps): JSX.Element { const isStopped = state === "stopped"; + // Not the raw isConnected/connectedAgentId-equality prop: that stays true + // through a mid-reconnect or a dropped socket, which would make a click + // silently detach a row that visually reads as "not connected" (its + // accent already follows this same condition, below). state === "active" + // is what use-agents.ts's agentVisualState actually calls "connected." + const isConnectedActive = state === "active"; const isArchiving = agent.status === "archiving"; // The shared DropdownMenuItem is a plain block styled for destructive items; // these need inline icons and the normal foreground colour. @@ -77,20 +102,20 @@ export function ChildAgentRow({ isReviewAgent && agent.submittedReviewId != null; const showReviewActivity = isReviewAgent && agent.status === "running" && isInitialReviewActive; - const displayName = agent.persona ?? agent.name; - const statusLabel = isStopped - ? "Stopped" - : agent.status === "error" - ? "Error" - : agent.latestEvent - ? latestEventLabel(agent.latestEvent.type) - : "Running"; - const statusColor = + // canOpenSubmittedReview is just "no submission yet" — much broader than + // "actively working," so a paused or errored reviewer needs its own + // wording rather than a blanket "Review in progress." + const reviewPendingLabel = agent.status === "error" - ? "text-status-blocked" - : agent.latestEvent && !isStopped - ? latestEventColor(agent.latestEvent.type) - : "text-muted-foreground"; + ? "Review agent — no review submitted" + : isStopped + ? "Review agent — paused, no review submitted" + : "Review in progress"; + const displayName = agent.persona ?? agent.name; + const { label: statusLabel, colorClass: statusColor } = describeAgentStatus( + agent, + isStopped + ); const row = (
{ + // Mirrors the top-level agent card's row-click-to-attach/detach + // (agent-card-header.tsx): a data-agent-control="true" marker plus + // closest() lets interactive descendants (the overflow menu, the + // resume button) opt out of the row's own click, the same + // convention that file uses instead of stopPropagation. Opening a + // submitted review is a separate action, reached through the + // overflow menu and the badge below — not tied to this click at + // all, so there's no race between the two actions' navigation. + const target = event.target as HTMLElement; + // Radix (DropdownMenuContent, TipSpot's Popover, Tooltip content) + // portals its content to document.body — outside this row's real + // DOM subtree — but React's synthetic events still bubble through + // the *component* tree regardless of where they're portaled to. + // contains() walks the real DOM, so this is the one check that + // actually catches every portal, not just the ones marked below. + if (!event.currentTarget.contains(target)) return; + if (target.closest("[data-agent-control='true']")) return; + if (isStopped) return; + if (isConnectedActive) { + detachTerminal(); + return; + } + if (closeOnSessionAction) onRequestClose?.(); + void attachToAgent(agent); + }} className={cn( + // Rounded on every corner, like an ordinary pill, with a normal + // matching 1px border on all four sides at rest — no permanently + // reserved thick edge (that read as either a muted "always-there" + // border, or, fully transparent, as no border at all on one side). + // Only the connected row below adds border-r-4 on top of this, so + // "thick right edge" exclusively means "this one's connected," and + // every other row just looks like an ordinary bordered pill. "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", - 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 && "cursor-pointer", + // Connected row: the same solid right-edge border treatment the + // top-level agent card uses for "this is what's connected" + // (agents-view.tsx's borderForAgentState), so the signal reads + // consistently across both list levels — and, being the only state + // that thickens this edge at all, exclusively means "connected." A + // ready-to-open review no longer has its own border treatment at + // all (that signal now lives on the review indicator's icon/color + // swap below), so it can't compete with or dilute this one. state === + // "active" (not the bare isConnected prop) so this exactly matches + // the top-level card's own condition (use-agents.ts's + // agentVisualState: running/creating AND actually connected). + state === "active" && "border-r-4 border-r-status-done", isStopped && "opacity-65", canOpenSubmittedReview && "opacity-100", showReviewActivity && "child-agent-review-active-row" )} > - {canOpenSubmittedReview ? ( - - - - - Resume child agent - - ) : isConnected ? ( - - - - - Detach - - ) : ( - - + {/* + Right-side action cluster: REVIEW badge, resume button (stopped + agents only), overflow menu. Grouped in one shrink-0 flex container + (rather than the badge living inside the shrinking label) so the + label is the only thing that gives way to a long name — this + cluster never competes with it for space. Plain elements here (no + pointer-events tricks needed): a click anywhere in the cluster that + isn't a real control just bubbles up to the row's own onClick, same + as clicking blank space anywhere else in the row. + */} +
+ {isReviewAgent ? ( + + {canOpenSubmittedReview ? ( + // A real button, not inert decoration: it's the one element + // in the row that visibly lights up (a filled clipboard-check + // in the "done" green, distinct from the connected accent's + // blue so the two signals never compete), so it's also the + // thing a user is most likely to click or tap aiming to open + // the review — including on a stopped row, whose + // click-to-connect is otherwise a dead end. Its own trigger + // (not the row's) so it can't race attachToAgent's navigate + // the way a combined click used to. + + + + + Open submitted review + + ) : ( + // Decorative only — the row's own status line already says + // "Working"/etc.; this just marks the agent as a reviewer with + // no submission yet. Label follows the same stopped/error + // condition the status line uses, not a blanket "in progress" + // that would misdescribe a paused or errored reviewer. + + + )} + + ) : null} + {/* + Attach/detach no longer have their own buttons — clicking + anywhere on the row does it (mirrors the top-level agent card, + see the row's onClick above). A stopped agent isn't click-to- + attach, though (the row's onClick bails out via isStopped), so + it keeps a dedicated Resume control, same as the top-level card. + */} + {isStopped ? ( + + + + + Resume child agent + + ) : null} + {/* + Session lifecycle controls. A sub agent used to offer only terminal + attach and resume, so moving plain children into this section would + have stripped the pause/rename/archive an agent card carries in its + footer. They live behind an overflow menu because the row has one + action slot. + */} + + - - View terminal - - )} - {/* - Session lifecycle controls. A sub agent used to offer only terminal - attach and resume, so moving plain children into this section would have - stripped the pause/rename/archive an agent card carries in its footer. - They live behind an overflow menu because the row has one action slot. - */} - - - - - - {!isStopped && !isArchiving ? ( + + {/* + data-agent-control marks the whole content, not just the + trigger button: Radix portals this out of the row's DOM + subtree, so it's not a real ancestor of an item's click target + in the row's own onClick's closest() check. The row's own + currentTarget.contains() guard already catches this (and every + other portal, e.g. the tip popover) on its own — this stays as + a second, belt-and-braces guard. + */} + + {!isStopped ? ( + // The keyboard/screen-reader path to connect — the row's own + // click-to-attach has no non-mouse equivalent, so this is the + // only accessible way to reach a sub agent's terminal. Label + // and action both follow isConnectedActive, matching what the + // row's own accent and click already mean by "connected." + { + if (isConnectedActive) { + detachTerminal(); + return; + } + if (closeOnSessionAction) onRequestClose?.(); + void attachToAgent(agent); + }} + > + {isConnectedActive ? ( + + ) : ( + + )} + {isConnectedActive ? "Detach" : "View terminal"} + + ) : null} + {canOpenSubmittedReview ? ( + { + if (closeOnSessionAction) onRequestClose?.(); + openSubmittedReview(agent); + }} + > + + Open review + + ) : null} + {!isStopped && !isArchiving ? ( + { + setStopTarget(agent); + setStopConfirmOpen(true); + }} + > + + Pause + + ) : null} + {isStopped && !isArchiving ? ( + { + if (closeOnSessionAction) onRequestClose?.(); + void startAgent(agent); + }} + > + + Resume + + ) : null} { - setStopTarget(agent); - setStopConfirmOpen(true); - }} + data-testid={`child-agent-settings-${agent.id}`} + onSelect={() => onEditSettings(agent)} > - - Pause + + Session details - ) : null} - {isStopped && !isArchiving ? ( { - if (closeOnSessionAction) onRequestClose?.(); - void startAgent(agent); + setDeleteTarget(agent); + setDeleteConfirmOpen(true); }} > - - Resume + + Archive - ) : null} - onEditSettings(agent)} - > - - Session settings - - { - setDeleteTarget(agent); - setDeleteConfirmOpen(true); - }} - > - - Archive - - - + + +
); - if (!canOpenSubmittedReview) return row; - return ( - - {row} - - ); + return row; } diff --git a/apps/web/src/components/app/docs-sections/agents.tsx b/apps/web/src/components/app/docs-sections/agents.tsx index e33034fc..30d63c41 100644 --- a/apps/web/src/components/app/docs-sections/agents.tsx +++ b/apps/web/src/components/app/docs-sections/agents.tsx @@ -371,7 +371,7 @@ export function AgentsContent() {

To rename any agent yourself, expand its sidebar card and click the - edit button to open the Session settings dialog and + edit button to open the Session details dialog and type a new name.

@@ -396,19 +396,22 @@ export function AgentsContent() { directory and full-access mode, and renders as a row in the{" "} Sub Agents list inside the parent's expanded card rather than as a card of its own. Persona reviewers appear in the same - list, marked with a Review badge. Passing{" "} - child: false launches an independent agent instead — it - gets its own top-level card, but Dispatch still records who launched - it, so the launcher can message and archive it. + list, marked with a clipboard icon that turns into a green checkmark + once its review is submitted — click it to open the review directly. + Passing child: false launches an independent agent + instead — it gets its own top-level card, but Dispatch still records + who launched it, so the launcher can message and archive it.

Nesting stops at one level: a sub agent can only launch independent - agents, not children or persona reviews of its own. Sub agent rows - carry the same session controls a card does — attach or detach the - terminal, resume a stopped session, and an overflow menu with{" "} - Pause, Session settings, and{" "} - Archive. Selecting a sub agent expands the card it - lives in. + agents, not children or persona reviews of its own. Clicking a sub + agent row's own body connects or disconnects its terminal, the same + way a top-level card's row does. An overflow menu carries the rest of + its session controls: View terminal/ + Detach, Open review (once a review + is submitted), Pause/Resume,{" "} + Session details, and Archive. + Selecting a sub agent expands the card it lives in.

Each child is told which agent launched it and can coordinate back diff --git a/apps/web/src/components/app/session-settings-dialog.tsx b/apps/web/src/components/app/session-settings-dialog.tsx index a03fc26d..8d5ef29f 100644 --- a/apps/web/src/components/app/session-settings-dialog.tsx +++ b/apps/web/src/components/app/session-settings-dialog.tsx @@ -2,6 +2,10 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; +import { AgentCardDetails } from "@/components/app/agent-card-details"; +import { AgentCardPhaseStatus } from "@/components/app/agent-card-status"; +import { describeAgentStatus } from "@/components/app/agent-event-utils"; +import { isFullAccessEnabled } from "@/components/app/agents-view-utils"; import { type Agent } from "@/components/app/types"; import { Button } from "@/components/ui/button"; import { @@ -11,7 +15,12 @@ import { DialogTitle, } from "@/components/ui/dialog"; import { Input } from "@/components/ui/input"; +import { useAgentDiffStats } from "@/hooks/use-agent-diff-stats"; +import { useCopyText } from "@/hooks/use-copy"; import { api } from "@/lib/api"; +import { formatRelativeTime } from "@/lib/format"; +import { type IdeType } from "@/lib/ide-types"; +import { cn } from "@/lib/utils"; const MAX_NAME_LENGTH = 120; @@ -19,18 +28,52 @@ type SessionSettingsDialogProps = { agent: Agent | null; open: boolean; onOpenChange: (open: boolean) => void; + enabledIdes: IdeType[]; }; +/** + * "Session details": the rename form this dialog has always offered, plus — + * for a sub agent, which otherwise has no way to see this — the same + * read-only info a parent agent's own expanded card shows in the sidebar + * (branch/worktree, IDE links, sandbox state, latest event). Reuses the + * exact same components (AgentCardDetails, AgentCardLatestEvent, + * AgentCardPhaseStatus) a parent card renders, rather than a second + * implementation of the same information. + */ export function SessionSettingsDialog({ agent, open, onOpenChange, + enabledIdes, }: SessionSettingsDialogProps) { const [name, setName] = useState(""); const [saving, setSaving] = useState(false); const inputRef = useRef(null); const queryClient = useQueryClient(); + // These hooks power AgentCardDetails exactly the way a parent card's own + // instance is powered (agent-card.tsx) — they're generic enough (a bare + // agent id + an "enabled" bool, or no agent dependency at all) to stand up + // fresh here rather than threading yet more props down from the card. + const { diffStats, refresh: refreshDiffStats } = useAgentDiffStats( + agent?.id ?? "", + open && agent != null + ); + const [worktreePathCopied, copyWorktreePath] = useCopyText(); + + // Mirrors ChildAgentRow's own derivation (same shared helper): the latest + // event's own label/color describes the EVENT, not the agent's current + // status, so a stopped or errored agent needs this rather than + // AgentCardLatestEvent's summary alone — that only reflects the event + // type, and would show a stale "Working" in the active-work color for an + // agent that has since stopped. + const isStopped = agent + ? agent.status !== "running" && agent.status !== "creating" + : false; + const { label: statusLabel, colorClass: statusColor } = agent + ? describeAgentStatus(agent, isStopped) + : { label: "", colorClass: "" }; + useEffect(() => { if (open && agent) { setName(agent.name); @@ -67,36 +110,85 @@ export function SessionSettingsDialog({ return (

- + { + // Radix's default autofocus lands on the first non-link tabbable + // in the dialog — for a worktree agent, that's now the "Copy + // worktree path" button (AgentCardDetails renders ahead of the + // name field), not the Input's own `autoFocus`. That both drops + // focus from the rename field this dialog exists to let you use + // quickly, and immediately opens that button's tooltip on top of + // the branch info. Force focus onto the name field explicitly. + e.preventDefault(); + inputRef.current?.focus(); + inputRef.current?.select(); + }} + > - Session settings + Session details
{ e.preventDefault(); void handleSave(); }} > -
- - setName(e.target.value)} - maxLength={MAX_NAME_LENGTH} - autoFocus - /> - - {name.length}/{MAX_NAME_LENGTH} characters - +
+ {agent ? ( +
+ +
+ + {statusLabel} + + {agent.latestEvent?.updatedAt ? ( + <> + • + + {formatRelativeTime(agent.latestEvent.updatedAt)} + + + ) : null} +
+ {agent.latestEvent?.message ? ( +
+ {agent.latestEvent.message} +
+ ) : null} + +
+ ) : null} +
+ + setName(e.target.value)} + maxLength={MAX_NAME_LENGTH} + /> + + {name.length}/{MAX_NAME_LENGTH} characters + +
-
+