From 8720b5d1ef14f7367edc6f8b4da07c2923ab47a2 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Mon, 6 Jul 2026 23:16:39 -0700 Subject: [PATCH] fix(desktop): paint ACP summary titles on action-bearing tool rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marge's review caught that the friendly summaries never reached the screen for the tools that matter most: getCompactToolActionLabel took the structured action-descriptor branch first and never consulted the label where summaryTitle lived, so shell/file-read rows kept rendering raw 'Ran ' descriptors after the friendly title landed. buildCompactToolSummary now exposes summaryTitle (null when the row failed) and failed alongside label, and CompactToolSummaryRow renders the friendly phrase as the visible row label ahead of the action descriptor. The exact command/path receipt stays on the hover title and on expand. Failed rows bypass the descriptor branch entirely so the failure label is painted instead of the descriptor repainting 'Ran ' over it — this also fixes the previously-dead failure precedence for action-bearing tools that Marge flagged as a minor. New render-layer tests (renderToStaticMarkup through the real buildCompactToolSummary pipeline) assert the user-visible HTML: the friendly title paints for shell and file-read rows, failure beats the friendly phrase, and rows without a summary keep today's descriptor label. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../CompactToolSummaryRow.render.test.mjs | 121 ++++++++++++++++++ .../CompactToolSummaryRow.tsx | 26 +++- .../ui/AgentSessionToolItem/ToolItem.tsx | 2 + .../agents/ui/agentSessionToolSummary.ts | 14 +- 4 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.render.test.mjs diff --git a/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.render.test.mjs b/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.render.test.mjs new file mode 100644 index 0000000000..6ffba76212 --- /dev/null +++ b/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.render.test.mjs @@ -0,0 +1,121 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import React from "react"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { buildCompactToolSummary } from "../agentSessionToolSummary.ts"; +import { CompactToolSummaryRow } from "./CompactToolSummaryRow.tsx"; + +const baseTimestamp = "2026-06-14T19:00:00.000Z"; + +function makeTool(overrides = {}) { + return { + id: "tool:1", + type: "tool", + title: "Tool call", + toolName: "shell", + buzzToolName: null, + status: "completed", + args: {}, + result: "", + isError: false, + timestamp: baseTimestamp, + startedAt: baseTimestamp, + completedAt: "2026-06-14T19:00:01.000Z", + ...overrides, + }; +} + +/** Build the row via the real summary pipeline, then render it to HTML. */ +function renderRow(item) { + const summary = buildCompactToolSummary(item); + return renderToStaticMarkup( + React.createElement(CompactToolSummaryRow, { + action: summary.action, + duration: null, + failed: summary.failed, + fileEditSummary: summary.fileEditSummary, + kind: summary.kind, + label: summary.label, + preview: summary.preview, + summaryTitle: summary.summaryTitle, + thumbnailSrc: summary.thumbnailSrc, + }), + ); +} + +test("render: friendly summary title is the visible label for an action-bearing shell row", () => { + const html = renderRow( + makeTool({ + toolName: "developer__shell", + args: { command: "git status" }, + summaryTitle: "checking repository state", + }), + ); + + assert.ok( + html.includes("checking repository state"), + `friendly title must be painted, got: ${html}`, + ); + // The structured "Ran " descriptor must not win the row label. + assert.ok( + !html.includes(">Ran<"), + `descriptor verb must not render: ${html}`, + ); + // Receipt stays available: the exact command rides the hover title. + assert.ok( + html.includes('title="git status"'), + `exact command receipt must stay on hover: ${html}`, + ); +}); + +test("render: friendly summary title is the visible label for an action-bearing file-read row", () => { + const html = renderRow( + makeTool({ + toolName: "read_file", + args: { path: "crates/buzz-agent/src/agent.rs" }, + summaryTitle: "reading agent source module", + }), + ); + + assert.ok(html.includes("reading agent source module")); + assert.ok( + !html.includes(">Read<"), + `descriptor verb must not render: ${html}`, + ); +}); + +test("render: failed shell row ignores the friendly title and paints the failure label", () => { + const html = renderRow( + makeTool({ + toolName: "developer__shell", + args: { command: "false" }, + status: "failed", + isError: true, + result: "exit 1", + summaryTitle: "running a quick command", + }), + ); + + assert.ok( + !html.includes("running a quick command"), + `friendly phrase must not mask a failure: ${html}`, + ); + assert.ok( + html.includes("failed"), + `failure label must be visible, got: ${html}`, + ); +}); + +test("render: rows without a summary keep today's descriptor label", () => { + const html = renderRow( + makeTool({ + toolName: "developer__shell", + args: { command: "git status" }, + }), + ); + + assert.ok(html.includes(">Ran<"), `descriptor verb should render: ${html}`); + assert.ok(html.includes("git status")); +}); diff --git a/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.tsx b/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.tsx index 2feeb8f971..39b2d06006 100644 --- a/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.tsx +++ b/desktop/src/features/agents/ui/AgentSessionToolItem/CompactToolSummaryRow.tsx @@ -22,18 +22,22 @@ export function compactSummaryTone() { export function CompactToolSummaryRow({ action, duration, + failed, fileEditSummary, kind, label, preview, + summaryTitle, thumbnailSrc, }: { action: AgentActivityAction | null; duration: string | null; + failed: boolean; fileEditSummary: CompactFileEditSummary | null; kind: CompactToolKind; label: string; preview: string | null; + summaryTitle: string | null; thumbnailSrc: string | null; }) { const [thumbnailFailed, setThumbnailFailed] = React.useState(false); @@ -44,14 +48,30 @@ export function CompactToolSummaryRow({ if (!thumbnailSrc || thumbnailFailed) return null; return resolveToolImageSrc(thumbnailSrc); }, [thumbnailFailed, thumbnailSrc]); - const actionLabel = fileEditSummary - ? null - : getCompactToolActionLabel(action, kind, label, preview); + // Failed rows bypass the structured action descriptor entirely: the + // descriptor would repaint "Ran " and swallow the failure label. + const actionLabel = + fileEditSummary || failed + ? null + : getCompactToolActionLabel(action, kind, label, preview); return ( <> {fileEditSummary ? ( + ) : summaryTitle && !failed ? ( + // The agent-provided friendly phrase wins the visible row label. + // The exact command/path receipt stays on hover and on expand. + + {summaryTitle} + ) : actionLabel ? ( diff --git a/desktop/src/features/agents/ui/agentSessionToolSummary.ts b/desktop/src/features/agents/ui/agentSessionToolSummary.ts index 3e72e93304..c180f81e0a 100644 --- a/desktop/src/features/agents/ui/agentSessionToolSummary.ts +++ b/desktop/src/features/agents/ui/agentSessionToolSummary.ts @@ -42,6 +42,14 @@ export type CompactToolSummary = { action: AgentActivityAction | null; kind: CompactToolKind; label: string; + /** + * Agent-provided friendly phrase (Buzz ACP tool summary) when it won the + * row label. Null for failed rows — failure labels always win. The render + * layer must prefer this over the structured `action` descriptor. + */ + summaryTitle: string | null; + /** True when the tool failed; the render layer must keep failure visible. */ + failed: boolean; preview: string | null; fileEditSummary: FileEditDiffSummary | null; fileEditDiff: FileEditDiff | null; @@ -81,12 +89,14 @@ export function buildCompactToolSummary(item: ToolItem): CompactToolSummary { const statusLabel = labelForStatus(descriptor, item.status, failed, running); // Prefer the agent-provided friendly phrase (Buzz ACP tool summary) as the // row label — but failure labels always win so errors stay unmistakable. - const summaryTitle = item.summaryTitle?.trim(); - const label = !failed && summaryTitle ? summaryTitle : statusLabel; + const summaryTitle = (!failed && item.summaryTitle?.trim()) || null; + const label = summaryTitle ?? statusLabel; return { action: descriptor.action ?? null, kind: descriptor.renderClass, label, + summaryTitle, + failed, preview: fileEditSummary?.filename ?? descriptor.preview, fileEditSummary, fileEditDiff,