From 3e0cc50da0ac074339da1fd5eef5320ee97a90ad Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 07:58:31 -0600 Subject: [PATCH 01/10] Fix sub-agent row clipping and add a connected-agent callout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Sub Agents list used shadcn's ScrollArea, whose Radix viewport wraps content in a display:table element that sizes to the content's natural (unshrunk) width instead of the container's. A long child name never actually truncated — it just grew the row until the sidebar's overflow-hidden clipped the REVIEW badge, terminal icon, and overflow menu off past the right edge entirely. Switch to a plain overflow-y-auto div, matching the pattern the outer agent list already uses for the same reason. Also move the REVIEW badge out of the shrinking name label and into the row's right-side action cluster (grouped with the terminal/menu buttons) so the label is the only thing that gives way to a long name. Add a clearer visual callout for the sub agent currently connected in the terminal: a left accent bar + primary-tinted background/border on the row, plus a small presence dot on its AgentTypeIcon. Previously this was only a faint bg-muted tint, easy to miss at a glance. --- apps/web/src/components/app/agent-card.tsx | 15 +- .../src/components/app/child-agent-row.tsx | 282 ++++++++++-------- 2 files changed, 164 insertions(+), 133 deletions(-) diff --git a/apps/web/src/components/app/agent-card.tsx b/apps/web/src/components/app/agent-card.tsx index 21ced193..8e29b895 100644 --- a/apps/web/src/components/app/agent-card.tsx +++ b/apps/web/src/components/app/agent-card.tsx @@ -14,7 +14,6 @@ import { useCopyText } from "@/hooks/use-copy"; import { SessionSettingsDialog } from "@/components/app/session-settings-dialog"; import { type Agent, type AgentVisualState } from "@/components/app/types"; import { Badge } from "@/components/ui/badge"; -import { ScrollArea } from "@/components/ui/scroll-area"; import { AnimatePresence, motion } from "framer-motion"; import { type AgentType } from "@/lib/agent-types"; @@ -224,7 +223,17 @@ export function AgentCard({ {childAgents.length} - + {/* + A plain scrollable div, not the shadcn ScrollArea: Radix's + ScrollArea wraps its viewport content in a `display: table` + element that sizes to the content's natural (unshrunk) + width instead of the container's — so a long child name + never actually truncates, it just grows the row until an + ancestor's overflow-hidden clips it off past the sidebar's + edge. The outer agent list avoids the same trap the same + way (agent-sidebar.tsx's "agent-sidebar-scroll" div). + */} +
{childAgents.map((child) => ( ))}
- +
) : null} diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index 04ca749d..416261f6 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -101,7 +101,12 @@ export function ChildAgentRow({ className={cn( "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", + // Connected row: a filled left accent bar plus a primary-tinted + // background/border, so the currently-viewed sub agent reads clearly + // at a glance instead of blending into the row list (see also the + // presence dot on its AgentTypeIcon below). + isConnected && + "border-l-2 border-l-primary border-primary/50 bg-primary/[0.07]", // 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", @@ -127,13 +132,22 @@ export function ChildAgentRow({ Open submitted review ) : null} - + + + {isConnected ? ( +
{displayName} - {isReviewAgent ? ( - - Review - - ) : null}
{statusLabel} @@ -167,132 +169,152 @@ export function ChildAgentRow({ ) : null}
- {isStopped ? ( - - - - - Resume child agent - - ) : isConnected ? ( - - - - - Detach - - ) : ( - - + {/* + Right-side action cluster: REVIEW badge, terminal icon, 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. + */} +
+ {isReviewAgent ? ( + + Review + + ) : null} + {isStopped ? ( + + + + + Resume child agent + + ) : isConnected ? ( + + + + + Detach + + ) : ( + + + + + 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. + */} + + - - 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 ? ( + + + {!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 settings - ) : 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 - - - + + +
); From a9dc07baeee6cbfb142d50d56f5b183d0fb4b162 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 08:18:37 -0600 Subject: [PATCH 02/10] Redesign connected callout, fix click-through and ScrollArea placement Per Brad's feedback, the connected sub-agent indicator is now a solid right-edge border (border-r-4, mirroring the top-level agent card's own borderForAgentState treatment) instead of a left border + presence dot: the dot was easy to miss, the thin border got visually lost against the animated review-active gradient, and toggling the border on/off shifted the row. Width is now reserved unconditionally via border-r-4/border-r-transparent so only the color ever changes. Also addresses persona review findings: - The right-side action cluster (REVIEW badge, terminal button, overflow menu) had default pointer-events, creating dead click zones over the row-wide "open submitted review" overlay button on ready-to-open rows. Cluster is now pointer-events-none with [&_button]:pointer-events-auto so only real controls are clickable. - Moved the ScrollArea display:table workaround into a shared `fitContentWidth` prop on components/ui/scroll-area.tsx (with the root-cause comment there) instead of a hand-rolled div, since the same workaround already existed ad hoc in activity-pane.tsx and the bug likely affects other ScrollArea call sites with shrinking content. - Removed now-inert `relative z-20` / redundant `shrink-0` left over from the badge/action-cluster restructuring. - Test coverage: badge groups with the action cluster (not the truncating label), the cluster's pointer-events guard, the mutually-exclusive ready-vs-connected border treatment, and that toggling isConnected never changes border-r-width. --- apps/web/src/components/app/agent-card.tsx | 19 ++--- .../components/app/child-agent-row.test.tsx | 58 ++++++++++++++- .../src/components/app/child-agent-row.tsx | 73 +++++++++---------- apps/web/src/components/ui/scroll-area.tsx | 24 +++++- 4 files changed, 119 insertions(+), 55 deletions(-) diff --git a/apps/web/src/components/app/agent-card.tsx b/apps/web/src/components/app/agent-card.tsx index 8e29b895..1d2ee83e 100644 --- a/apps/web/src/components/app/agent-card.tsx +++ b/apps/web/src/components/app/agent-card.tsx @@ -14,6 +14,7 @@ import { useCopyText } from "@/hooks/use-copy"; import { SessionSettingsDialog } from "@/components/app/session-settings-dialog"; import { type Agent, type AgentVisualState } from "@/components/app/types"; import { Badge } from "@/components/ui/badge"; +import { ScrollArea } from "@/components/ui/scroll-area"; import { AnimatePresence, motion } from "framer-motion"; import { type AgentType } from "@/lib/agent-types"; @@ -223,17 +224,11 @@ export function AgentCard({ {childAgents.length} - {/* - A plain scrollable div, not the shadcn ScrollArea: Radix's - ScrollArea wraps its viewport content in a `display: table` - element that sizes to the content's natural (unshrunk) - width instead of the container's — so a long child name - never actually truncates, it just grows the row until an - ancestor's overflow-hidden clips it off past the sidebar's - edge. The outer agent list avoids the same trap the same - way (agent-sidebar.tsx's "agent-sidebar-scroll" div). - */} -
+
{childAgents.map((child) => ( ))}
-
+ ) : null} 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..19788746 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -48,7 +48,9 @@ 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,6 +83,9 @@ function renderRow( setDeleteTarget, setDeleteConfirmOpen, onEditSettings, + rerenderWith: ( + elementOverrides: Partial> + ) => rerender(buildElement(elementOverrides)), }; } @@ -97,7 +103,6 @@ describe("ChildAgentRow", () => { const badge = screen.getByText("Review"); expect(badge.className.split(/\s+/)).toEqual( expect.arrayContaining([ - "ml-auto", "border-primary", "bg-background", "text-foreground", @@ -111,6 +116,26 @@ describe("ChildAgentRow", () => { expect(row.className).toContain("child-agent-review-active-row"); }); + it("groups the REVIEW badge with the terminal/menu controls, not the truncating name label", () => { + renderRow(baseAgent); + + const badge = screen.getByText("Review"); + const menuButton = screen.getByTestId("child-agent-menu-agt_child"); + // The badge and the overflow menu button should share an immediate + // parent (the right-side action cluster) rather than the badge living + // inside the name label's min-w-0/flex-1/truncate wrapper. + expect(badge.parentElement).toBe(menuButton.parentElement); + }); + + it("keeps the action cluster transparent to clicks except its own controls", () => { + renderRow(baseAgent); + + const menuButton = screen.getByTestId("child-agent-menu-agt_child"); + const cluster = menuButton.parentElement; + expect(cluster?.className).toContain("pointer-events-none"); + expect(cluster?.className).toContain("[&_button]:pointer-events-auto"); + }); + it("stops chasing after the initial review is submitted", () => { renderRow(baseAgent, { isInitialReviewActive: false }); @@ -154,10 +179,35 @@ describe("ChildAgentRow", () => { const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-primary/45"); + expect(row.className).toContain("border-r-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"); + // Ready-to-open wins: the connected row's own right-edge accent must not + // also be present, or the two would visually fight on the same edge. + expect(row.className).not.toContain("border-r-status-done"); + }); + + it("shows the connected right-edge accent when not also ready to open", () => { + renderRow(baseAgent, { isConnected: true }); + + const row = screen.getByTestId("child-agent-row-agt_child"); + expect(row.className).toContain("border-r-status-done"); + expect(row.className).not.toContain("border-primary/45"); + expect(row.className).not.toContain("cursor-pointer"); + }); + + it("reserves the connected accent's width so attaching never shifts the row", () => { + const { rerenderWith } = renderRow(baseAgent, { isConnected: false }); + const row = screen.getByTestId("child-agent-row-agt_child"); + expect(row.className).toContain("border-r-4"); + expect(row.className).toContain("border-r-transparent"); + + rerenderWith({ isConnected: true }); + // Same border-r-4 width both before and after — only the color class + // toggles (see the two tests above), so the box never resizes. + expect(row.className).toContain("border-r-4"); + expect(row.className).toContain("border-r-status-done"); }); it("opens a submitted review from the row without attaching its terminal", () => { diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index 416261f6..a7ba0049 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -99,17 +99,24 @@ export function ChildAgentRow({ data-review-active={showReviewActivity ? "true" : "false"} data-review-ready={canOpenSubmittedReview ? "true" : "false"} className={cn( - "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", + "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-lg border border-border/60 border-r-4 border-r-transparent bg-background/30 px-2 py-1 sm:py-1.5", "transition-colors hover:bg-muted/35", - // Connected row: a filled left accent bar plus a primary-tinted - // background/border, so the currently-viewed sub agent reads clearly - // at a glance instead of blending into the row list (see also the - // presence dot on its AgentTypeIcon below). - isConnected && - "border-l-2 border-l-primary border-primary/50 bg-primary/[0.07]", - // 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", + // Ready-to-open wins over connected styling: it is the actionable + // state. Both branches set border-r-{color} explicitly (not just + // the general border-{color} shorthand) — twMerge only dedupes a + // directional override against the base border-r-transparent when + // one is present, so leaving the ready branch to the general + // border-primary/45 alone would have left the reserved 4px right + // edge transparent even on a ready-to-open row. + canOpenSubmittedReview + ? "cursor-pointer border-primary/45 border-r-primary/45 bg-primary/[0.06] hover:bg-primary/10" + : // 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. The width is reserved + // by border-r-4 above and only the color toggles, so this never + // shifts the row's layout the way an added/removed border would. + isConnected && "border-r-status-done", isStopped && "opacity-65", canOpenSubmittedReview && "opacity-100", showReviewActivity && "child-agent-review-active-row" @@ -132,22 +139,13 @@ export function ChildAgentRow({ Open submitted review ) : null} - - - {isConnected ? ( - +
+
{isReviewAgent ? ( { if (closeOnSessionAction) onRequestClose?.(); void startAgent(agent); @@ -217,7 +215,7 @@ export function ChildAgentRow({ data-agent-control="true" data-testid={`child-agent-detach-${agent.id}`} aria-label={`Detach from ${displayName}`} - className="relative z-20 h-11 w-11 shrink-0 text-muted-foreground hover:text-foreground sm:h-7 sm:w-7" + className="h-11 w-11 text-muted-foreground hover:text-foreground sm:h-7 sm:w-7" onClick={detachTerminal} > @@ -234,7 +232,7 @@ export function ChildAgentRow({ data-agent-control="true" data-testid={`child-agent-attach-${agent.id}`} aria-label={`Attach to ${displayName}`} - className="relative z-20 h-11 w-11 shrink-0 text-muted-foreground hover:text-foreground sm:h-7 sm:w-7" + className="h-11 w-11 text-muted-foreground hover:text-foreground sm:h-7 sm:w-7" onClick={() => { if (closeOnSessionAction) onRequestClose?.(); void attachToAgent(agent); @@ -247,11 +245,12 @@ export function ChildAgentRow({ )} {/* - 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. - */} + 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. + */} diff --git a/apps/web/src/components/ui/scroll-area.tsx b/apps/web/src/components/ui/scroll-area.tsx index d543caf7..6059ae49 100644 --- a/apps/web/src/components/ui/scroll-area.tsx +++ b/apps/web/src/components/ui/scroll-area.tsx @@ -7,11 +7,31 @@ const ScrollArea = React.forwardRef< React.ElementRef, React.ComponentPropsWithoutRef & { horizontal?: boolean; + /** + * Radix's Viewport wraps its children in an internal div styled + * `min-width: 100%; display: table` (see @radix-ui/react-scroll-area's + * Viewport implementation) so it can measure content for horizontal + * scrolling. Table sizing means that wrapper grows to fit its content's + * natural width instead of the container's — so flex-shrink/min-w-0/ + * truncate on descendants never actually engage, and overflow just + * spills past the Root's overflow-hidden instead of scrolling or + * eliding. Pass `fitContentWidth` for a vertical list of items that are + * meant to shrink-to-fit (truncated names, wrapping rows, etc.) rather + * than to be horizontally scrollable — it forces that wrapper back to + * `display: block` so it sizes to the container like a normal element. + * Don't set it on a ScrollArea that intentionally relies on the table + * sizing for horizontal content (the `horizontal` prop above). + */ + fitContentWidth?: boolean; } ->(({ className, children, horizontal, ...props }, ref) => ( +>(({ className, children, horizontal, fitContentWidth, ...props }, ref) => ( [data-radix-scroll-area-viewport]>div]:!block", + className + )} {...props} > From 98eb9c37fc9ebdd728ffae5a2c51dd1afa2e5a7c Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 08:27:19 -0600 Subject: [PATCH 03/10] Square right corners, scope ready-state border to avoid connected echo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Brad's live feedback on the screenshots: - Ready-to-open rows were showing a muted version of the same right-edge border treatment as connected rows, reading as a confusing partial "connected" signal on a row that wasn't. The ready branch now reserves the same border-r-4 width as every row (so it never mis-aligns with or jumps against its neighbors — caught live by both reviewers) but explicitly keeps that edge transparent, so only the actually-connected row ever shows color there. - Squared off the row's right corners (rounded-l-lg instead of rounded-lg) so the connected accent reads as one continuous bar instead of being interrupted by a rounded corner partway through. Also fixes two more review findings: - The connected border now keys off `state === "active"` (matching the top-level card's own agentVisualState condition) instead of the bare isConnected prop, so a paused-but-still-attached agent or a dropped terminal socket don't disagree with the top-level card about whether the same agent is "connected." - Fixed a rerenderWith test helper bug that silently dropped props set at initial render time on rerender. --- .../components/app/child-agent-row.test.tsx | 32 ++++++++++--- .../src/components/app/child-agent-row.tsx | 46 +++++++++++++------ 2 files changed, 57 insertions(+), 21 deletions(-) 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 19788746..157fd94e 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -85,7 +85,7 @@ function renderRow( onEditSettings, rerenderWith: ( elementOverrides: Partial> - ) => rerender(buildElement(elementOverrides)), + ) => rerender(buildElement({ ...overrides, ...elementOverrides })), }; } @@ -179,17 +179,20 @@ describe("ChildAgentRow", () => { const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-primary/45"); - expect(row.className).toContain("border-r-primary/45"); expect(row.className).toContain("bg-primary/[0.06]"); expect(row.className).toContain("hover:bg-primary/10"); expect(row.className).toContain("cursor-pointer"); - // Ready-to-open wins: the connected row's own right-edge accent must not - // also be present, or the two would visually fight on the same edge. + // Ready-to-open reserves the same border-r-4 width as every other row + // (so a row's right-side controls never mis-align with its neighbors, + // and never jump when this state flips) but keeps that edge + // transparent — it doesn't opt into the connected accent's color, so a + // ready-but-not-connected row can't echo even a muted version of it. + expect(row.className).toContain("border-r-4"); expect(row.className).not.toContain("border-r-status-done"); }); it("shows the connected right-edge accent when not also ready to open", () => { - renderRow(baseAgent, { isConnected: true }); + renderRow(baseAgent, { isConnected: true, state: "active" }); const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-r-status-done"); @@ -197,13 +200,28 @@ describe("ChildAgentRow", () => { expect(row.className).not.toContain("cursor-pointer"); }); + it("does not light the connected accent for a paused agent that's still attached", () => { + // state tracks agentVisualState (running/creating AND actually + // connected), which can diverge from the raw isConnected prop — e.g. a + // paused agent you're still attached to. The accent should follow + // state, matching the top-level card's own condition, not isConnected. + renderRow(baseAgent, { isConnected: true, state: "stopped" }); + + const row = screen.getByTestId("child-agent-row-agt_child"); + expect(row.className).not.toContain("border-r-status-done"); + expect(row.className).toContain("border-r-transparent"); + }); + it("reserves the connected accent's width so attaching never shifts the row", () => { - const { rerenderWith } = renderRow(baseAgent, { isConnected: false }); + const { rerenderWith } = renderRow(baseAgent, { + isConnected: false, + state: "idle", + }); const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-r-4"); expect(row.className).toContain("border-r-transparent"); - rerenderWith({ isConnected: true }); + rerenderWith({ isConnected: true, state: "active" }); // Same border-r-4 width both before and after — only the color class // toggles (see the two tests above), so the box never resizes. expect(row.className).toContain("border-r-4"); diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index a7ba0049..01fc9f45 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -99,24 +99,42 @@ export function ChildAgentRow({ data-review-active={showReviewActivity ? "true" : "false"} data-review-ready={canOpenSubmittedReview ? "true" : "false"} className={cn( - "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-lg border border-border/60 border-r-4 border-r-transparent bg-background/30 px-2 py-1 sm:py-1.5", + // Right corners stay square (a deliberate visual choice, not literal + // edge-flushing — the row still sits inset inside the card's px-2, + // the list's pr-2, etc.): the connected accent below is meant to + // read as one continuous bar rather than one interrupted partway + // through by a rounded corner, echoing the top-level card's own + // border-r-4 treatment. border-r-4/border-r-transparent are in the + // BASE, unconditionally — every row reserves the same 4px right + // edge, so ready/connected/plain rows all keep identical internal + // geometry (a ready row that instead fell back to a 1px border + // measurably misaligned its controls against the rows above and + // below it, and jumped 3px when a review's ready state flipped). + "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-l-lg border border-border/60 border-r-4 border-r-transparent bg-background/30 px-2 py-1 sm:py-1.5", "transition-colors hover:bg-muted/35", - // Ready-to-open wins over connected styling: it is the actionable - // state. Both branches set border-r-{color} explicitly (not just - // the general border-{color} shorthand) — twMerge only dedupes a - // directional override against the base border-r-transparent when - // one is present, so leaving the ready branch to the general - // border-primary/45 alone would have left the reserved 4px right - // edge transparent even on a ready-to-open row. + // Ready-to-open wins over connected styling and keeps its own + // long-standing look (a colored border on all sides, tint, + // cursor-pointer) — but re-asserts border-r-transparent explicitly + // so the general border-primary/45 above doesn't bleed onto the + // reserved right edge (twMerge only drops a directional class for + // a later *general* one, not the reverse) and echo even a muted + // version of the connected accent on a row that isn't connected. canOpenSubmittedReview - ? "cursor-pointer border-primary/45 border-r-primary/45 bg-primary/[0.06] hover:bg-primary/10" + ? "cursor-pointer border-primary/45 border-r-transparent bg-primary/[0.06] hover:bg-primary/10" : // 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. The width is reserved - // by border-r-4 above and only the color toggles, so this never - // shifts the row's layout the way an added/removed border would. - isConnected && "border-r-status-done", + // consistently across both list levels — and exclusively means + // "connected," not shared with any other state. 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). + // isConnected alone would keep the accent lit for a + // paused-but-still-attached agent, or after the terminal + // socket drops — both cases where the top-level card already + // goes transparent, so the two would disagree about the same + // fact. + state === "active" && "border-r-status-done", isStopped && "opacity-65", canOpenSubmittedReview && "opacity-100", showReviewActivity && "child-agent-review-active-row" @@ -129,7 +147,7 @@ export function ChildAgentRow({ type="button" data-testid={`child-agent-open-review-${agent.id}`} aria-label={`Open submitted review from ${displayName}`} - className="absolute inset-0 z-0 rounded-lg outline-none focus-visible:ring-2 focus-visible:ring-ring" + className="absolute inset-0 z-0 rounded-l-lg outline-none focus-visible:ring-2 focus-visible:ring-ring" onClick={() => { if (closeOnSessionAction) onRequestClose?.(); openSubmittedReview(agent); From 3a8491a5800bb7dc24f14bab35fee41088efb538 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 17:24:52 -0600 Subject: [PATCH 04/10] Close every row's border instead of leaving the right edge transparent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brad's feedback on a screenshot: the ready-to-open row's right edge had no border (it was intentionally transparent, to avoid echoing the connected accent's color) — but that reads as the border looking cut off / broken, not as "correctly not connected." Both concerns turn out to be solvable at once: give every row's right edge a real color, always, matching its own other three sides (neutral border-border/60 by default, primary/45 when ready-to-open) — so every row's border closes into a complete shape. The connected state is the one exception: it colors ONLY the right edge, differently from the other three, which is what makes it read as "this edge means something" — and now it's the ONLY state that does that, since ready's border is uniform on all sides instead of singling out the right edge. Width (border-r-4) stays constant in every state as before, so alignment and no-layout-shift both still hold. --- .../components/app/child-agent-row.test.tsx | 17 +++++---- .../src/components/app/child-agent-row.tsx | 38 ++++++++++--------- 2 files changed, 31 insertions(+), 24 deletions(-) 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 157fd94e..defffa09 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -179,14 +179,15 @@ describe("ChildAgentRow", () => { const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-primary/45"); + expect(row.className).toContain("border-r-primary/45"); expect(row.className).toContain("bg-primary/[0.06]"); expect(row.className).toContain("hover:bg-primary/10"); expect(row.className).toContain("cursor-pointer"); - // Ready-to-open reserves the same border-r-4 width as every other row - // (so a row's right-side controls never mis-align with its neighbors, - // and never jump when this state flips) but keeps that edge - // transparent — it doesn't opt into the connected accent's color, so a - // ready-but-not-connected row can't echo even a muted version of it. + // Ready-to-open closes its border on all four sides at the same color + // (border-r-primary/45 matches the other three) rather than leaving the + // right edge a different color or transparent — a uniform closed border + // reads as its own distinct treatment, not an asymmetric single-edge + // accent, so it can't be mistaken for a muted version of "connected." expect(row.className).toContain("border-r-4"); expect(row.className).not.toContain("border-r-status-done"); }); @@ -209,7 +210,9 @@ describe("ChildAgentRow", () => { const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).not.toContain("border-r-status-done"); - expect(row.className).toContain("border-r-transparent"); + // Falls back to the neutral default, matching every other unconnected + // row's closed border — not a transparent gap. + expect(row.className).toContain("border-r-border/60"); }); it("reserves the connected accent's width so attaching never shifts the row", () => { @@ -219,7 +222,7 @@ describe("ChildAgentRow", () => { }); const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-r-4"); - expect(row.className).toContain("border-r-transparent"); + expect(row.className).toContain("border-r-border/60"); rerenderWith({ isConnected: true, state: "active" }); // Same border-r-4 width both before and after — only the color class diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index 01fc9f45..ddfe6a98 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -104,29 +104,33 @@ export function ChildAgentRow({ // the list's pr-2, etc.): the connected accent below is meant to // read as one continuous bar rather than one interrupted partway // through by a rounded corner, echoing the top-level card's own - // border-r-4 treatment. border-r-4/border-r-transparent are in the - // BASE, unconditionally — every row reserves the same 4px right - // edge, so ready/connected/plain rows all keep identical internal - // geometry (a ready row that instead fell back to a 1px border - // measurably misaligned its controls against the rows above and - // below it, and jumped 3px when a review's ready state flipped). - "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-l-lg border border-border/60 border-r-4 border-r-transparent bg-background/30 px-2 py-1 sm:py-1.5", + // border-r-4 treatment. border-r-4 is in the BASE, unconditionally, + // and every row's right edge is always painted some color (never + // literally transparent) — so every row's border closes into a + // complete shape instead of reading as cut open on one side, and + // ready/connected/plain rows all keep identical internal geometry + // (a row whose border-r width varied by state measurably + // misaligned its controls against its neighbors). + "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-l-lg border border-border/60 border-r-4 border-r-border/60 bg-background/30 px-2 py-1 sm:py-1.5", "transition-colors hover:bg-muted/35", // Ready-to-open wins over connected styling and keeps its own - // long-standing look (a colored border on all sides, tint, - // cursor-pointer) — but re-asserts border-r-transparent explicitly - // so the general border-primary/45 above doesn't bleed onto the - // reserved right edge (twMerge only drops a directional class for - // a later *general* one, not the reverse) and echo even a muted - // version of the connected accent on a row that isn't connected. + // long-standing look: a colored, CLOSED border on all four sides + // (border-r-primary/45 matches the other three, not the neutral + // default) plus its tint and cursor-pointer. Uniformly coloring + // every side — rather than singling the right edge out, the way + // the connected treatment below does — is what keeps this from + // reading as a muted version of "connected": that signal is + // specifically an asymmetric one edge lit up against the rest, + // and a ready row's border never does that. canOpenSubmittedReview - ? "cursor-pointer border-primary/45 border-r-transparent bg-primary/[0.06] hover:bg-primary/10" + ? "cursor-pointer border-primary/45 border-r-primary/45 bg-primary/[0.06] hover:bg-primary/10" : // 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 exclusively means - // "connected," not shared with any other state. state === - // "active" (not the bare isConnected prop) so this exactly + // consistently across both list levels — and, being the only + // state that colors just the right edge differently from the + // row's other three sides, exclusively means "connected." 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). // isConnected alone would keep the accent lit for a From 13360d644b011e2f0b9908a83529f3e9e8273a9c Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 17:56:15 -0600 Subject: [PATCH 05/10] Click-to-connect sub agent rows; move review-opening to the overflow menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Brad's feedback: the row itself is now click-to-attach/detach, mirroring the top-level agent card's own row-click behavior (agent-card-header.tsx) exactly — same data-agent-control="true" + closest() opt-out convention for interactive descendants. The dedicated terminal/detach icon buttons are gone; a stopped agent still gets a dedicated Resume control since it isn't click-to-attach either (matches the top-level card there too). Opening a submitted review moves from a full-row overlay button into its own "Open review" item at the top of the overflow menu. This was originally implemented as "click connects AND opens the review" (one combined action), but that races: attachToAgent's and openSubmittedReview's navigate() calls target different routes (the agent's own page vs. its parent's page with ?expandReview=), and whichever fires last silently wins, stripping the other's URL state. Separating them avoids the race entirely and gives each action its own clear trigger — confirmed no live call site in this app actually chains attach->navigate together for this reason. Also: - Removed the pointer-events-none/z-index scaffolding that existed only to let clicks pass through decorative content to the old absolutely-positioned overlay button — now unnecessary, since normal DOM bubbling reaches the row's own onClick directly. DropdownMenuContent is portaled, but React's synthetic events bubble through the *component* tree regardless, so it gets its own data-agent-control marker (verified live: without it, opening the menu also fired the row's attach/detach). - Updated the "review-row-open" tip copy and moved its TipSpot to wrap the badge specifically, since that's the row's only always-visible "this review is ready" affordance now. - e2e: the "open submitted review" flow now opens the row's overflow menu first. --- .../components/app/child-agent-row.test.tsx | 188 +++++++------ .../src/components/app/child-agent-row.tsx | 256 +++++++++--------- apps/web/src/lib/tips/tips.ts | 2 +- e2e/review-agent-ui.spec.ts | 5 + 4 files changed, 250 insertions(+), 201 deletions(-) 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 defffa09..ae0f1dd1 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -89,6 +89,13 @@ function renderRow( }; } +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({ @@ -116,7 +123,7 @@ describe("ChildAgentRow", () => { expect(row.className).toContain("child-agent-review-active-row"); }); - it("groups the REVIEW badge with the terminal/menu controls, not the truncating name label", () => { + it("groups the REVIEW badge with the overflow menu control, not the truncating name label", () => { renderRow(baseAgent); const badge = screen.getByText("Review"); @@ -124,16 +131,9 @@ describe("ChildAgentRow", () => { // The badge and the overflow menu button should share an immediate // parent (the right-side action cluster) rather than the badge living // inside the name label's min-w-0/flex-1/truncate wrapper. - expect(badge.parentElement).toBe(menuButton.parentElement); - }); - - it("keeps the action cluster transparent to clicks except its own controls", () => { - renderRow(baseAgent); - - const menuButton = screen.getByTestId("child-agent-menu-agt_child"); - const cluster = menuButton.parentElement; - expect(cluster?.className).toContain("pointer-events-none"); - expect(cluster?.className).toContain("[&_button]:pointer-events-auto"); + expect(badge.closest("div.flex.shrink-0")).toBe( + menuButton.closest("div.flex.shrink-0") + ); }); it("stops chasing after the initial review is submitted", () => { @@ -144,17 +144,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("keeps the badge 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"); + expect(badge.querySelector("svg")).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("marks the badge ready with a checkmark once the review can be opened, without a row border", () => { renderRow( { ...baseAgent, status: "stopped", submittedReviewId: 42 }, { state: "stopped", isInitialReviewActive: false } @@ -162,34 +167,45 @@ 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"); + // "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 badge's + // checkmark 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 badge = screen.getByText("Review"); expect(badge.className).toContain("bg-primary"); expect(badge.className).toContain("text-primary-foreground"); + expect(badge.querySelector("svg")).not.toBeNull(); }); - it("keeps the ready treatment when the row is terminal-connected", () => { - renderRow( + it("opens the submitted review from the overflow menu, independent of connecting", () => { + const submittedAgent = { ...baseAgent, submittedReviewId: 42 }; + const { attachToAgent, openSubmittedReview } = renderRow(submittedAgent, { + isInitialReviewActive: false, + }); + + openMenu(); + fireEvent.click(screen.getByTestId("child-agent-open-review-agt_child")); + expect(openSubmittedReview).toHaveBeenCalledWith(submittedAgent); + expect(attachToAgent).not.toHaveBeenCalled(); + }); + + 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 }, - { isConnected: true, isInitialReviewActive: false } + { isInitialReviewActive: false, isConnected: false, state: "idle" } ); - const row = screen.getByTestId("child-agent-row-agt_child"); - expect(row.className).toContain("border-primary/45"); - expect(row.className).toContain("border-r-primary/45"); - expect(row.className).toContain("bg-primary/[0.06]"); - expect(row.className).toContain("hover:bg-primary/10"); - expect(row.className).toContain("cursor-pointer"); - // Ready-to-open closes its border on all four sides at the same color - // (border-r-primary/45 matches the other three) rather than leaving the - // right edge a different color or transparent — a uniform closed border - // reads as its own distinct treatment, not an asymmetric single-edge - // accent, so it can't be mistaken for a muted version of "connected." - expect(row.className).toContain("border-r-4"); - expect(row.className).not.toContain("border-r-status-done"); + 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", () => { @@ -198,7 +214,6 @@ describe("ChildAgentRow", () => { const row = screen.getByTestId("child-agent-row-agt_child"); expect(row.className).toContain("border-r-status-done"); expect(row.className).not.toContain("border-primary/45"); - expect(row.className).not.toContain("cursor-pointer"); }); it("does not light the connected accent for a paused agent that's still attached", () => { @@ -231,35 +246,6 @@ describe("ChildAgentRow", () => { expect(row.className).toContain("border-r-status-done"); }); - it("opens a submitted review from the row without attaching its terminal", () => { - const submittedAgent = { - ...baseAgent, - submittedReviewId: 42, - }; - const { attachToAgent, openSubmittedReview } = renderRow(submittedAgent, { - isInitialReviewActive: false, - }); - - fireEvent.click(screen.getByTestId("child-agent-open-review-agt_child")); - expect(openSubmittedReview).toHaveBeenCalledWith(submittedAgent); - expect(attachToAgent).not.toHaveBeenCalled(); - }); - - it("keeps the terminal control independent from review navigation", () => { - const { attachToAgent, openSubmittedReview } = renderRow({ - ...baseAgent, - submittedReviewId: 42, - }); - - 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(); - }); - it("does not infer review purpose from a persona", () => { renderRow({ ...baseAgent, role: "standard" }); @@ -269,27 +255,77 @@ describe("ChildAgentRow", () => { 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" }, + { isConnected: false, 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" }, + { isConnected: true, 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" }, + { isConnected: false, 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 ddfe6a98..148e2652 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -1,12 +1,13 @@ import { Archive, + Check, + Eye, MoreVertical, Pause, Pencil, Play, - Terminal, - Unplug, } from "lucide-react"; +import type { ReactNode } from "react"; import { latestEventColor, @@ -31,6 +32,27 @@ 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; @@ -98,77 +120,69 @@ export function ChildAgentRow({ data-agent-role={agent.role ?? "standard"} data-review-active={showReviewActivity ? "true" : "false"} data-review-ready={canOpenSubmittedReview ? "true" : "false"} + onClick={(event) => { + // 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 below — not tied to this click at all, so there's + // no race between the two actions' navigation. + const target = event.target as HTMLElement; + if (target.closest("[data-agent-control='true']")) return; + if (isStopped) return; + if (isConnected) { + detachTerminal(); + return; + } + if (closeOnSessionAction) onRequestClose?.(); + void attachToAgent(agent); + }} className={cn( - // Right corners stay square (a deliberate visual choice, not literal - // edge-flushing — the row still sits inset inside the card's px-2, - // the list's pr-2, etc.): the connected accent below is meant to - // read as one continuous bar rather than one interrupted partway - // through by a rounded corner, echoing the top-level card's own - // border-r-4 treatment. border-r-4 is in the BASE, unconditionally, - // and every row's right edge is always painted some color (never - // literally transparent) — so every row's border closes into a - // complete shape instead of reading as cut open on one side, and - // ready/connected/plain rows all keep identical internal geometry - // (a row whose border-r width varied by state measurably + // No rounded corners at all (a single rounded side read as odd): + // the connected accent below is meant to read as one continuous + // bar, uninterrupted by any corner treatment, echoing the + // top-level card's own border-r-4 treatment. border-r-4 is in the + // BASE, unconditionally, and every row's right edge is always + // painted some color (never literally transparent) — so every + // row's border closes into a complete shape instead of reading as + // cut open on one side, and every row keeps identical internal + // geometry (a row whose border-r width varied by state measurably // misaligned its controls against its neighbors). - "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-l-lg border border-border/60 border-r-4 border-r-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-none border border-border/60 border-r-4 border-r-border/60 bg-background/30 px-2 py-1 sm:py-1.5", "transition-colors hover:bg-muted/35", - // Ready-to-open wins over connected styling and keeps its own - // long-standing look: a colored, CLOSED border on all four sides - // (border-r-primary/45 matches the other three, not the neutral - // default) plus its tint and cursor-pointer. Uniformly coloring - // every side — rather than singling the right edge out, the way - // the connected treatment below does — is what keeps this from - // reading as a muted version of "connected": that signal is - // specifically an asymmetric one edge lit up against the rest, - // and a ready row's border never does that. - canOpenSubmittedReview - ? "cursor-pointer border-primary/45 border-r-primary/45 bg-primary/[0.06] hover:bg-primary/10" - : // 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 colors just the right edge differently from the - // row's other three sides, exclusively means "connected." 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). - // isConnected alone would keep the accent lit for a - // paused-but-still-attached agent, or after the terminal - // socket drops — both cases where the top-level card already - // goes transparent, so the two would disagree about the same - // fact. - state === "active" && "border-r-status-done", + !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 colors just the right edge differently from the row's other + // three sides, exclusively means "connected." A ready-to-open + // review no longer has its own border treatment at all (that + // signal now lives on the REVIEW badge's checkmark 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). + // isConnected alone would keep the accent lit for a + // paused-but-still-attached agent, or after the terminal socket + // drops — both cases where the top-level card already goes + // transparent, so the two would disagree about the same fact. + state === "active" && "border-r-status-done", isStopped && "opacity-65", canOpenSubmittedReview && "opacity-100", showReviewActivity && "child-agent-review-active-row" )} > - {canOpenSubmittedReview ? ( - - - - - Detach - - ) : ( - - - - - View terminal - - )} + ) : null} {/* Session lifecycle controls. A sub agent used to offer only terminal attach and resume, so moving plain children into this section would @@ -286,7 +282,29 @@ export function ChildAgentRow({ - + {/* + data-agent-control marks the whole content, not just the + trigger button: Radix portals this out of the row's DOM + subtree, but React's synthetic events still bubble through the + *component* tree, so a click inside would otherwise also reach + the row's own onClick above. target.closest() here walks the + real (portaled) DOM, where this attribute is an actual ancestor + of every item's click target. + */} + + {canOpenSubmittedReview ? ( + { + if (closeOnSessionAction) onRequestClose?.(); + openSubmittedReview(agent); + }} + > + + Open review + + ) : null} {!isStopped && !isArchiving ? ( ); - if (!canOpenSubmittedReview) return row; - return ( - - {row} - - ); + return row; } diff --git a/apps/web/src/lib/tips/tips.ts b/apps/web/src/lib/tips/tips.ts index d0fc084a..f679366d 100644 --- a/apps/web/src/lib/tips/tips.ts +++ b/apps/web/src/lib/tips/tips.ts @@ -199,7 +199,7 @@ export const tips: Tip[] = [ { id: "review-row-open", title: "Open Submitted Reviews", - body: "A review agent's row lights up once its review is submitted. Click the row to open the review and work through its feedback.", + body: "A review agent's badge gets a checkmark once its review is submitted. Open the row's overflow menu and choose Open review to work through its feedback.", docsSection: "personas", since: "0.29.1", surfaces: ["inline"], diff --git a/e2e/review-agent-ui.spec.ts b/e2e/review-agent-ui.spec.ts index b906bedc..123ef5df 100644 --- a/e2e/review-agent-ui.spec.ts +++ b/e2e/review-agent-ui.spec.ts @@ -72,6 +72,11 @@ test.describe("Review agent UI", () => { page.getByTestId(`child-agent-row-${fixture.activeAgentId}`) ).toHaveAttribute("data-review-ready", "false"); + // "Open review" lives in the row's overflow menu now, decoupled from + // connecting to the agent's terminal. + await page + .getByTestId(`child-agent-menu-${fixture.openReviewAgentId}`) + .click(); await page .getByTestId(`child-agent-open-review-${fixture.openReviewAgentId}`) .click(); From cce8c939f6ddc07f5bb1bed471dc7c0e5c028324 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 18:13:02 -0600 Subject: [PATCH 06/10] Fix portal event bubbling, restore keyboard access, make badge clickable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 5 findings from a fresh frontend-ux-review round on the click-to-connect/menu-based-review redesign: - The row's data-agent-control guard only checked closest() from the click target, which walks the real (portaled) DOM — but React's synthetic events bubble through the *component* tree regardless of where content is portaled to. TipSpot's popover (wrapping the REVIEW badge) is a portal too, and wasn't marked, so dismissing the tip or clicking "Learn more" also fired the row's attach/detach — the exact navigate() race this PR was built to eliminate, just via a different path. Fixed with one general guard (event.currentTarget.contains()) that covers every portal, not just the ones explicitly marked; data-agent-control on DropdownMenuContent stays as a second, redundant-but-harmless guard. - Removing the dedicated terminal/detach buttons left no keyboard or screen-reader path to a sub agent's terminal at all — the row's click-to-connect has no non-mouse equivalent, and nothing replaced the old buttons' accessible names. Added a "View terminal"/"Detach" item at the top of the overflow menu, restoring that access. - Clicking the ready-to-open REVIEW badge attached the terminal instead of opening the review — confusing, since the badge is the one element that visibly lights up for exactly this action, and the worst case for a stopped reviewer (the common end state), whose row click is otherwise a dead end. The badge is now its own + ) : ( + + Review + + )} ) : null} {/* @@ -285,13 +311,39 @@ export function ChildAgentRow({ {/* data-agent-control marks the whole content, not just the trigger button: Radix portals this out of the row's DOM - subtree, but React's synthetic events still bubble through the - *component* tree, so a click inside would otherwise also reach - the row's own onClick above. target.closest() here walks the - real (portaled) DOM, where this attribute is an actual ancestor - of every item's click target. + 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 ? ( Date: Tue, 18 Aug 2026 18:19:35 -0600 Subject: [PATCH 07/10] Drop the now-unused isConnected prop from ChildAgentRow Per review follow-up: isConnected is no longer read anywhere in child-agent-row.tsx (state === "active" replaced it entirely). Removed from ChildAgentRowProps and its only call site (agent-card.tsx). --- apps/web/src/components/app/agent-card.tsx | 1 - apps/web/src/components/app/child-agent-row.test.tsx | 5 ----- apps/web/src/components/app/child-agent-row.tsx | 1 - 3 files changed, 7 deletions(-) diff --git a/apps/web/src/components/app/agent-card.tsx b/apps/web/src/components/app/agent-card.tsx index 1d2ee83e..a4d08277 100644 --- a/apps/web/src/components/app/agent-card.tsx +++ b/apps/web/src/components/app/agent-card.tsx @@ -239,7 +239,6 @@ export function AgentCard({ child.role === "review" && child.submittedReviewId == null } - isConnected={connectedAgentId === child.id} attachToAgent={attachToAgent} detachTerminal={detachTerminal} startAgent={startAgent} 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 b2473523..64d65812 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -57,11 +57,6 @@ function renderRow( agent={agent} state="idle" isInitialReviewActive={true} - // The component itself no longer reads isConnected — connection - // state comes entirely from `state === "active"` now — but the - // prop stays required in the type since agent-card.tsx still - // passes it. Kept here only to satisfy that type. - isConnected={false} attachToAgent={attachToAgent} detachTerminal={detachTerminal} startAgent={startAgent} diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index 9af9eed4..1f964f4f 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -59,7 +59,6 @@ export type ChildAgentRowProps = { agent: Agent; state: AgentVisualState; isInitialReviewActive: boolean; - isConnected: boolean; attachToAgent: (agent: Agent) => Promise; detachTerminal: () => void; startAgent: (agent: Agent) => Promise; From 3e86e803c67b87328d570c55842a2ae698907c19 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Tue, 18 Aug 2026 18:49:42 -0600 Subject: [PATCH 08/10] Restore rounded corners, thick border exclusive to connected; icon review status; Session details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes from live feedback: 1. Rounded corners are back on every sub-agent row. The thick right-edge accent is now exclusive to the connected row — unconnected rows get an ordinary matching 1px border on all four sides (not a permanently reserved thick-but-transparent or thick-but-muted edge, both of which read as visually broken). The outer row footprint never changes size either way; only the connected row's own border-r width/color differ from the rest. 2. The REVIEW text badge is replaced with themed iconography: a muted ClipboardList icon while a review is still in progress, swapping to a colored ClipboardCheck once it's ready to open. The ready color is status-working (green) specifically, not status-done/primary (blue) — deliberately a different family from the connected accent so the two signals never compete, the same concern that shaped the earlier border redesign. 3. "Session settings" becomes "Session details" everywhere (the dialog title, both its trigger labels, and the docs), and the dialog now shows the same read-only info a parent agent's own expanded card already shows in the sidebar — branch/worktree, IDE launch links, sandbox state, and the latest event — by reusing the exact same components (AgentCardDetails, AgentCardLatestEvent, AgentCardPhaseStatus) rather than a second implementation. Previously a sub agent's row showed only name/status/time with no way to see any of this. The supporting hooks (useAgentDiffStats, useCopyText) and the isFullAccessEnabled pure function are all agent-id/agent-object-only, so the dialog stands them up itself instead of threading more props through agent-card.tsx. --- .../src/components/app/agent-card-actions.tsx | 4 +- apps/web/src/components/app/agent-card.tsx | 2 + .../components/app/child-agent-row.test.tsx | 76 ++++++------ .../src/components/app/child-agent-row.tsx | 108 ++++++++++-------- .../components/app/docs-sections/agents.tsx | 4 +- .../app/session-settings-dialog.tsx | 91 +++++++++++---- e2e/review-agent-ui.spec.ts | 10 +- 7 files changed, 169 insertions(+), 126 deletions(-) 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 a4d08277..4e36ba37 100644 --- a/apps/web/src/components/app/agent-card.tsx +++ b/apps/web/src/components/app/agent-card.tsx @@ -281,6 +281,7 @@ export function AgentCard({ agent={agent} open={settingsOpen} onOpenChange={setSettingsOpen} + enabledIdes={enabledIdes} /> { if (!open) setSettingsChild(null); }} + enabledIdes={enabledIdes} /> ); 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 64d65812..0d556f25 100644 --- a/apps/web/src/components/app/child-agent-row.test.tsx +++ b/apps/web/src/components/app/child-agent-row.test.tsx @@ -106,15 +106,8 @@ describe("ChildAgentRow", () => { }, }); - const badge = screen.getByText("Review"); - expect(badge.className.split(/\s+/)).toEqual( - expect.arrayContaining([ - "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"); @@ -122,15 +115,15 @@ describe("ChildAgentRow", () => { expect(row.className).toContain("child-agent-review-active-row"); }); - it("groups the REVIEW badge with the overflow menu control, not the truncating name label", () => { + it("groups the review indicator with the overflow menu control, not the truncating name label", () => { renderRow(baseAgent); - const badge = screen.getByText("Review"); + const indicator = screen.getByRole("img", { name: "Review in progress" }); const menuButton = screen.getByTestId("child-agent-menu-agt_child"); - // The badge and the overflow menu button should share an immediate - // parent (the right-side action cluster) rather than the badge living - // inside the name label's min-w-0/flex-1/truncate wrapper. - expect(badge.closest("div.flex.shrink-0")).toBe( + // 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") ); }); @@ -143,14 +136,14 @@ describe("ChildAgentRow", () => { expect(row.className).not.toContain("child-agent-review-active-row"); }); - it("keeps the badge 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"); - const badge = screen.getByText("Review"); - expect(badge.className).toContain("bg-background"); - expect(badge.querySelector("svg")).toBeNull(); + 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( @@ -158,7 +151,7 @@ describe("ChildAgentRow", () => { ).toBeNull(); }); - it("marks the badge ready with a checkmark once the review can be opened, without a row border", () => { + 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 } @@ -169,15 +162,19 @@ describe("ChildAgentRow", () => { expect(row.className).toContain("opacity-100"); expect(row.className).not.toContain("opacity-65"); // "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 badge's - // checkmark is the sole carrier of that signal, and opening it moves to - // the overflow menu (tested below), decoupled from connecting. + // 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 badge = screen.getByText("Review"); - expect(badge.className).toContain("bg-primary"); - expect(badge.className).toContain("text-primary-foreground"); - expect(badge.querySelector("svg")).not.toBeNull(); + 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("opens the submitted review from the overflow menu, independent of connecting", () => { @@ -292,6 +289,7 @@ describe("ChildAgentRow", () => { 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"); }); @@ -303,29 +301,19 @@ describe("ChildAgentRow", () => { 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"); - // Falls back to the neutral default, matching every other unconnected - // row's closed border — not a transparent gap. - expect(row.className).toContain("border-r-border/60"); - }); - - it("reserves the connected accent's width so attaching never shifts the row", () => { - const { rerenderWith } = renderRow(baseAgent, { state: "idle" }); - const row = screen.getByTestId("child-agent-row-agt_child"); - expect(row.className).toContain("border-r-4"); - expect(row.className).toContain("border-r-border/60"); - - rerenderWith({ state: "active" }); - // Same border-r-4 width both before and after — only the color class - // toggles (see the two tests above), so the box never resizes. - expect(row.className).toContain("border-r-4"); - expect(row.className).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("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"); diff --git a/apps/web/src/components/app/child-agent-row.tsx b/apps/web/src/components/app/child-agent-row.tsx index 1f964f4f..1e9384fe 100644 --- a/apps/web/src/components/app/child-agent-row.tsx +++ b/apps/web/src/components/app/child-agent-row.tsx @@ -1,6 +1,7 @@ import { Archive, - Check, + ClipboardCheck, + ClipboardList, Eye, MoreVertical, Pause, @@ -18,7 +19,6 @@ import { 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, @@ -153,36 +153,28 @@ export function ChildAgentRow({ void attachToAgent(agent); }} className={cn( - // No rounded corners at all (a single rounded side read as odd): - // the connected accent below is meant to read as one continuous - // bar, uninterrupted by any corner treatment, echoing the - // top-level card's own border-r-4 treatment. border-r-4 is in the - // BASE, unconditionally, and every row's right edge is always - // painted some color (never literally transparent) — so every - // row's border closes into a complete shape instead of reading as - // cut open on one side, and every row keeps identical internal - // geometry (a row whose border-r width varied by state measurably - // misaligned its controls against its neighbors). - "group relative flex min-h-11 w-full min-w-0 items-center gap-2 rounded-none border border-border/60 border-r-4 border-r-border/60 bg-background/30 px-2 py-1 sm:py-1.5", + // 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", !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 colors just the right edge differently from the row's other - // three sides, exclusively means "connected." A ready-to-open - // review no longer has its own border treatment at all (that - // signal now lives on the REVIEW badge's checkmark 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 + // 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). - // isConnected alone would keep the accent lit for a - // paused-but-still-attached agent, or after the terminal socket - // drops — both cases where the top-level card already goes - // transparent, so the two would disagree about the same fact. - state === "active" && "border-r-status-done", + state === "active" && "border-r-4 border-r-status-done", isStopped && "opacity-65", canOpenSubmittedReview && "opacity-100", showReviewActivity && "child-agent-review-active-row" @@ -231,31 +223,47 @@ export function ChildAgentRow({ {canOpenSubmittedReview ? ( // A real button, not inert decoration: it's the one element - // in the row that visibly lights up (checkmark + fill), 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. - + // 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 + ) : ( - - Review - + // Decorative only — the row's own status line already says + // "Working"/etc.; this just marks the agent as a reviewer, + // muted, with no checkmark yet. + + )} ) : null} @@ -388,7 +396,7 @@ export function ChildAgentRow({ onSelect={() => onEditSettings(agent)} > - Session settings + Session details

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.

@@ -406,7 +406,7 @@ export function AgentsContent() { 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{" "} + Pause, Session details, and{" "} Archive. Selecting a sub agent expands the card it lives in.

diff --git a/apps/web/src/components/app/session-settings-dialog.tsx b/apps/web/src/components/app/session-settings-dialog.tsx index a03fc26d..bca8c61c 100644 --- a/apps/web/src/components/app/session-settings-dialog.tsx +++ b/apps/web/src/components/app/session-settings-dialog.tsx @@ -2,6 +2,12 @@ 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 { + AgentCardLatestEvent, + AgentCardPhaseStatus, +} from "@/components/app/agent-card-status"; +import { isFullAccessEnabled } from "@/components/app/agents-view-utils"; import { type Agent } from "@/components/app/types"; import { Button } from "@/components/ui/button"; import { @@ -11,7 +17,10 @@ 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 { type IdeType } from "@/lib/ide-types"; const MAX_NAME_LENGTH = 120; @@ -19,18 +28,39 @@ 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(); + useEffect(() => { if (open && agent) { setName(agent.name); @@ -67,36 +97,55 @@ export function SessionSettingsDialog({ return ( - + - Session settings + Session details
{ e.preventDefault(); void handleSave(); }} > -
- - setName(e.target.value)} - maxLength={MAX_NAME_LENGTH} - autoFocus - /> - - {name.length}/{MAX_NAME_LENGTH} characters - +
+ {agent ? ( +
+ + + +
+ ) : null} +
+ + setName(e.target.value)} + maxLength={MAX_NAME_LENGTH} + autoFocus + /> + + {name.length}/{MAX_NAME_LENGTH} characters + +
-
+
+ Open submitted review ) : ( // Decorative only — the row's own status line already says - // "Working"/etc.; this just marks the agent as a reviewer, - // muted, with no checkmark yet. + // "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.