From 1b67b461c2961179476698fcc917beac2772eb5c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 4 Sep 2026 12:04:38 +0000 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20make=20outgoing=20age?= =?UTF-8?q?nt=20messages=20readable=20communication=20cards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give agent reports and task messages matching cards with readable prose, recipient metadata, accessible expansion, and delivery-aware status. Preserve legacy reports and task navigation, with unit and full-app Storybook coverage. Remove one obsolete Config test fixture field to unblock the existing baseline typecheck failure. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$21.44`_ --- .../Messages/MessageRenderer.stories.tsx | 23 +-- .../Tools/AgentReportToolCall.test.tsx | 54 ++++++- .../features/Tools/AgentReportToolCall.tsx | 85 +++++------ .../Tools/Shared/AgentCommunicationCard.tsx | 93 ++++++++++++ .../features/Tools/TaskToolCall.test.tsx | 74 +++++++++- src/browser/features/Tools/TaskToolCall.tsx | 103 ++++++++----- .../App.agentCommunication.stories.tsx | 138 ++++++++++++++++++ src/node/services/workspaceService.test.ts | 1 - 8 files changed, 466 insertions(+), 105 deletions(-) create mode 100644 src/browser/features/Tools/Shared/AgentCommunicationCard.tsx create mode 100644 src/browser/stories/App.agentCommunication.stories.tsx diff --git a/src/browser/features/Messages/MessageRenderer.stories.tsx b/src/browser/features/Messages/MessageRenderer.stories.tsx index bbdc303e17f..ffb65fcfc1e 100644 --- a/src/browser/features/Messages/MessageRenderer.stories.tsx +++ b/src/browser/features/Messages/MessageRenderer.stories.tsx @@ -212,10 +212,10 @@ The report inherited transcript-sized markdown styles instead of compact task ch title: "Agent update", reportMarkdown: `## Agent update -The same compact report typography applies to incremental agent findings. +Incremental findings read as outgoing communication, separate from compact task details. -- Body and inline \`code\` remain aligned with tool chrome. -- Headings retain a modest hierarchy.`, +- Body and inline \`code\` stay legible alongside transcript messages. +- Headings retain a clear hierarchy.`, }, { success: true } ), @@ -238,7 +238,7 @@ The same compact report typography applies to incremental agent findings. const agentReportCard = await waitFor(() => { const card = canvasElement.querySelector( - '[data-component="AgentReportToolCall"]' + '[data-component="AgentCommunicationCard"]' ); if (!card) throw new Error("Agent report card not rendered"); return card; @@ -287,20 +287,21 @@ The same compact report typography applies to incremental agent findings. }); await waitFor(() => { - const report = agentReportCard.querySelector(".compact-report-markdown"); + const report = agentReportCard.querySelector(".markdown-content"); const heading = report?.querySelector("h2"); const code = report?.querySelector("code"); if (!report || !heading || !code) { throw new Error("Expanded agent report markdown not rendered"); } - if (Number.parseFloat(getComputedStyle(report).fontSize) > 11) { - throw new Error("Agent report body text is larger than compact tool chrome"); + const bodyFontSize = Number.parseFloat(getComputedStyle(report).fontSize); + if (bodyFontSize < 14) { + throw new Error("Agent report body text is smaller than transcript prose"); } - if (Number.parseFloat(getComputedStyle(heading).fontSize) > 13) { - throw new Error("Agent report heading is too large for compact tool chrome"); + if (Number.parseFloat(getComputedStyle(heading).fontSize) <= bodyFontSize) { + throw new Error("Agent report heading has lost its hierarchy"); } - if (Number.parseFloat(getComputedStyle(code).fontSize) > 11) { - throw new Error("Agent report inline code is larger than compact tool chrome"); + if (Number.parseFloat(getComputedStyle(code).fontSize) < 12) { + throw new Error("Agent report inline code is too small to read"); } if (agentReportCard.scrollWidth > agentReportCard.clientWidth) { throw new Error( diff --git a/src/browser/features/Tools/AgentReportToolCall.test.tsx b/src/browser/features/Tools/AgentReportToolCall.test.tsx index d0a3dbe2dd7..642651eeff0 100644 --- a/src/browser/features/Tools/AgentReportToolCall.test.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.test.tsx @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { GlobalWindow } from "happy-dom"; -import { cleanup, render } from "@testing-library/react"; +import { cleanup, fireEvent, render } from "@testing-library/react"; import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; import { AgentReportToolCall } from "./AgentReportToolCall"; @@ -77,4 +77,56 @@ describe("AgentReportToolCall", () => { expect(view.getByText(/Report file: report\.md/)).toBeTruthy(); }); + + test("collapses the body while retaining the title, then reopens it", () => { + const view = render( + + + + ); + const toggle = view.getByRole("button", { name: "Recovery audit" }); + expect(toggle.getAttribute("aria-expanded")).toBe("true"); + fireEvent.click(toggle); + expect(toggle.getAttribute("aria-expanded")).toBe("false"); + expect(view.queryByText("Detailed findings")).toBeNull(); + fireEvent.click(toggle); + expect(view.getByText(/Detailed findings/)).toBeTruthy(); + }); + + test("keeps validation failures visible when collapsed instead of claiming delivery", () => { + const view = render( + + + + ); + fireEvent.click(view.getByRole("button", { name: "Audit" })); + expect(view.getByRole("alert").textContent).toContain("Report exceeds limit"); + expect(view.getByRole("status").className).toContain("text-danger"); + }); + + test("prefers the submitted report over the draft arguments", () => { + const view = render( + + + + ); + expect(view.queryByText("Draft report")).toBeNull(); + expect(view.getByText("Accepted report")).toBeTruthy(); + }); }); diff --git a/src/browser/features/Tools/AgentReportToolCall.tsx b/src/browser/features/Tools/AgentReportToolCall.tsx index 2e692e47cc8..3c3a1fc92c8 100644 --- a/src/browser/features/Tools/AgentReportToolCall.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.tsx @@ -2,22 +2,9 @@ import React from "react"; import type { AgentReportToolArgs, AgentReportToolResult } from "@/common/types/tools"; -import { - ToolContainer, - ToolHeader, - ExpandIcon, - ToolName, - StatusIndicator, - ToolDetails, - ToolIcon, - ErrorBox, -} from "./Shared/ToolPrimitives"; -import { - useToolExpansion, - getStatusDisplay, - isToolErrorResult, - type ToolStatus, -} from "./Shared/toolUtils"; +import { ErrorBox } from "./Shared/ToolPrimitives"; +import { AgentCommunicationCard } from "./Shared/AgentCommunicationCard"; +import { isToolErrorResult, type ToolStatus } from "./Shared/toolUtils"; import { MarkdownRenderer } from "../Messages/MarkdownRenderer"; interface LegacyAgentReportFileArgs { @@ -47,42 +34,38 @@ function getSubmittedReportMarkdown( return `Report file: ${args.reportMarkdownPath ?? "report.md"}`; } -export const AgentReportToolCall: React.FC = ({ - args, - result, - status = "pending", -}) => { - // Default to expanded so incremental findings are visible when they wake the parent. - const { expanded, toggleExpanded } = useToolExpansion(true); - - const errorResult = isToolErrorResult(result) ? result : null; - - const title = args.title ?? "Agent update"; - const reportMarkdown = getSubmittedReportMarkdown(args, result); - - // Show a small preview when collapsed so the card still has some useful context. - const firstLine = reportMarkdown.trim().split("\n")[0] ?? ""; - const preview = firstLine.length > 80 ? firstLine.slice(0, 80).trim() + "…" : firstLine; +export const AgentReportToolCall: React.FC = (props) => { + const reportMarkdown = getSubmittedReportMarkdown(props.args, props.result); + const failedResult = props.result?.success === false ? props.result : null; return ( - - - ▶ - - {title} - {getStatusDisplay(status)} - - - {expanded && ( - - - {errorResult && {errorResult.error}} - - )} - - {!expanded && preview && ( -
{preview}
- )} - + + {isToolErrorResult(failedResult) ? ( + failedResult.error + ) : ( + <> + {failedResult.message} + {failedResult.errors.map((error, index) => ( +
+ {error.path}: {error.message} +
+ ))} + + )} + + ) + } + > + + ); }; diff --git a/src/browser/features/Tools/Shared/AgentCommunicationCard.tsx b/src/browser/features/Tools/Shared/AgentCommunicationCard.tsx new file mode 100644 index 00000000000..25479fb712a --- /dev/null +++ b/src/browser/features/Tools/Shared/AgentCommunicationCard.tsx @@ -0,0 +1,93 @@ +import type { ReactNode } from "react"; +import { ChevronRight, CircleAlert, CircleCheck, Clock3 } from "lucide-react"; +import { cn } from "@/common/lib/utils"; +import { ToolChrome, ToolContainer, ToolDetails, ToolIcon } from "./ToolPrimitives"; +import { useToolExpansion, type ToolStatus } from "./toolUtils"; + +interface AgentCommunicationCardProps { + toolName: "agent_report" | "task_send_message"; + title: string; + destination: ReactNode; + status: ToolStatus; + statusLabel?: string; + preview: string; + initiallyExpanded: boolean; + children: ReactNode; + error?: ReactNode; +} + +const DELIVERY_LABELS: Record = { + pending: "Pending", + executing: "Sending…", + completed: "Sent", + failed: "Not sent", + interrupted: "Interrupted", + backgrounded: "Queued", + redacted: "Redacted", +}; + +/** Outgoing agent communication should read like the received report, not a command log. */ +export function AgentCommunicationCard(props: AgentCommunicationCardProps) { + const { expanded, toggleExpanded } = useToolExpansion(props.initiallyExpanded); + const StatusIcon = + props.status === "completed" + ? CircleCheck + : props.status === "failed" || props.status === "interrupted" + ? CircleAlert + : Clock3; + + return ( + + + +
+ {props.destination} + + + +
+ + {expanded ? ( + + {props.children} + + ) : ( + + {props.preview.trim().split("\n")[0]} + + )} + {props.error} + + ); +} diff --git a/src/browser/features/Tools/TaskToolCall.test.tsx b/src/browser/features/Tools/TaskToolCall.test.tsx index 4bc799d27f2..5145cd445dd 100644 --- a/src/browser/features/Tools/TaskToolCall.test.tsx +++ b/src/browser/features/Tools/TaskToolCall.test.tsx @@ -750,11 +750,81 @@ describe("TaskSendMessageToolCall", () => { ); - expect(view.getByText("queued")).toBeDefined(); - fireEvent.click(view.getByText("task_send_message")); + expect(view.getByRole("status").textContent).toBe("Queued"); + fireEvent.click(view.getByRole("button", { name: "Message to agent" })); expect(view.getByText("child-task")).toBeDefined(); expect(view.getByText("Use the corrected API shape.")).toBeDefined(); }); + + test.each([ + { status: "accepted", taskId: "child-task", targetRelation: "ancestor" }, + { status: "reactivated", taskId: "child-task" }, + { status: "queued", taskId: "child-task", targetRelation: "sibling" }, + { status: "not_found", taskId: "child-task" }, + { status: "invalid_scope", taskId: "child-task" }, + { status: "not_active", taskId: "child-task", taskStatus: "reported", error: "Inactive peer" }, + { status: "error", taskId: "child-task", error: "Delivery failed" }, + { status: "refused", taskId: "child-task", reason: "Message already queued" }, + { status: "rate_limited", taskId: "child-task", retryAfterMs: 1200 }, + ] as const)("uses the delivery outcome instead of generic tool completion: $status", (result) => { + const view = render( + + + + ); + const status = view.getByRole("status"); + const sent = result.status === "accepted" || result.status === "reactivated"; + expect(status.className.includes("text-success")).toBe(sent); + expect(status.className.includes("text-danger")).toBe(!sent && result.status !== "queued"); + if (result.error != null) expect(view.getByRole("alert").textContent).toBe(result.error); + if (result.reason != null) expect(view.getByRole("alert").textContent).toBe(result.reason); + if (result.targetRelation != null) + expect(view.getByText(`To ${result.targetRelation}`)).toBeTruthy(); + if (result.retryAfterMs != null) expect(view.getByText("Retry in 2s")).toBeTruthy(); + }); + + test("shows transport failures even while collapsed", () => { + const view = render( + + + + ); + expect(view.getByRole("alert").textContent).toBe("Connection lost"); + expect( + view.getByRole("button", { name: "Message to agent" }).getAttribute("aria-expanded") + ).toBe("false"); + }); + + test("opens the recipient workspace without toggling the message", () => { + const workspace = createWorkspaceMetadata({ id: "child-task", title: "Reviewer" }); + const select = mock(() => undefined); + workspaceContextMock = { + workspaceMetadata: new Map([[workspace.id, workspace]]), + setSelectedWorkspace: select, + }; + try { + const view = render( + + + + ); + fireEvent.click(view.getByRole("button", { name: "child-task" })); + expect(select).toHaveBeenCalledWith(workspace); + expect( + view.getByRole("button", { name: "Message to Reviewer" }).getAttribute("aria-expanded") + ).toBe("false"); + } finally { + workspaceContextMock = null; + } + }); }); const taskTerminateArgs = { task_ids: ["wfr_x"] }; diff --git a/src/browser/features/Tools/TaskToolCall.tsx b/src/browser/features/Tools/TaskToolCall.tsx index 90bc362ab73..48f5294257a 100644 --- a/src/browser/features/Tools/TaskToolCall.tsx +++ b/src/browser/features/Tools/TaskToolCall.tsx @@ -17,6 +17,7 @@ import { isToolErrorResult, type ToolStatus, } from "./Shared/toolUtils"; +import { AgentCommunicationCard } from "./Shared/AgentCommunicationCard"; import { MarkdownRenderer } from "../Messages/MarkdownRenderer"; import { useOptionalMessageListContext } from "../Messages/MessageListContext"; import { useStickyExpand } from "../Messages/useStickyExpand"; @@ -1706,53 +1707,77 @@ const TaskListItem: React.FC<{ interface TaskSendMessageToolCallProps { args: TaskSendMessageToolArgs; - result?: TaskSendMessageToolSuccessResult; + result?: TaskSendMessageToolSuccessResult | ToolErrorResult; status?: ToolStatus; } +const MESSAGE_DELIVERY: Record< + TaskSendMessageToolSuccessResult["status"], + { status: ToolStatus; label: string } +> = { + accepted: { status: "completed", label: "Accepted" }, + queued: { status: "backgrounded", label: "Queued" }, + reactivated: { status: "completed", label: "Sent · Agent reactivated" }, + not_found: { status: "failed", label: "Target not found" }, + invalid_scope: { status: "failed", label: "Invalid target" }, + not_active: { status: "failed", label: "Agent inactive" }, + error: { status: "failed", label: "Not sent" }, + refused: { status: "failed", label: "Refused" }, + rate_limited: { status: "failed", label: "Rate limited" }, +}; + export const TaskSendMessageToolCall: React.FC = (props) => { - const { expanded, toggleExpanded } = useToolExpansion(false); - const status = props.status ?? "pending"; - const summary = props.result?.status ?? "sending"; + const workspaceContext = useOptionalWorkspaceContext(); + const workspace = findWorkspaceForTaskTarget( + workspaceContext?.workspaceMetadata, + props.args.task_id + ); + const result = props.result; + const toolError = isToolErrorResult(result); + // A finished tool call can still mean delivery was refused or queued, not sent. + const delivery = result && !toolError ? MESSAGE_DELIVERY[result.status] : undefined; + const relation = result && "targetRelation" in result ? result.targetRelation : undefined; + const error = result && "error" in result ? result.error : undefined; return ( - - - ▶ - - task_send_message - {summary} - {getStatusDisplay(status)} - - - {expanded && ( - -
-
- - {props.result && } - {props.result && "targetRelation" in props.result && props.result.targetRelation && ( - to {props.result.targetRelation} - )} -
-
- {props.args.message} -
- {props.result && "error" in props.result && props.result.error && ( -
{props.result.error}
- )} - {props.result?.status === "refused" && ( -
{props.result.reason}
- )} - {props.result?.status === "rate_limited" && props.result.retryAfterMs != null && ( -
- Retry in {Math.ceil(props.result.retryAfterMs / 1000)}s + + To {relation && `${relation} `} + + + } + status={toolError ? "failed" : (delivery?.status ?? props.status ?? "pending")} + statusLabel={delivery?.label} + preview={props.args.message} + initiallyExpanded={false} + error={ + <> + {error && ( + + {error} + + )} + {result && "status" in result && result.status === "refused" && ( + + {result.reason} + + )} + {result && + "status" in result && + result.status === "rate_limited" && + result.retryAfterMs != null && ( +
+ Retry in {Math.ceil(result.retryAfterMs / 1000)}s
)} -
- - )} - + + } + > +
{props.args.message}
+ ); }; diff --git a/src/browser/stories/App.agentCommunication.stories.tsx b/src/browser/stories/App.agentCommunication.stories.tsx new file mode 100644 index 00000000000..0df80821e1a --- /dev/null +++ b/src/browser/stories/App.agentCommunication.stories.tsx @@ -0,0 +1,138 @@ +import { expect, userEvent, within } from "@storybook/test"; +import { getAutoExpandPrefsKey } from "@/common/constants/storage"; +import { updatePersistedState } from "@/browser/hooks/usePersistedState"; +import { appMeta, AppWithMocks, type AppStory } from "./meta.js"; +import { setupSimpleChatStory } from "./helpers/chatSetup"; +import { PhoneSubagentReportDecorator } from "./helpers/subagentReportStory"; +import { collapseLeftSidebar, collapseRightSidebar } from "./helpers/uiState"; +import { createAssistantMessage } from "./mocks/messages"; +import { createWorkspace, STABLE_TIMESTAMP } from "./mocks/workspaces"; + +const WORKSPACE_ID = "ws-agent-communication"; +const REPORT_TITLE = "Preserving active-pointer crash recovery"; +const REPORT = + "One final generation-guard audit caught an edge not covered by existing tests: startup’s authoritative newer-handle selection must be allowed to replace a stale **active** child pointer, not only terminal pointers.\n\nI’m adding a compare-with-snapshotted previous execution ID permission only for startup reconciliation, plus a test for the old running status."; +const MESSAGE = + "Focused follow-up before finalizing: verify the native canvas clipping behavior and the parent popup-blocked failure path.\n\nCheck handshake ordering in src/browser/features/Tools/SubagentTranscriptDialog.tsx and keep the change scoped to the existing behavior."; + +function setupCommunicationStory(failed = false) { + // Separate transcript stores so switching stories cannot retain a successful delivery. + const workspaceId = failed ? `${WORKSPACE_ID}-failed` : WORKSPACE_ID; + collapseLeftSidebar(); + collapseRightSidebar(); + updatePersistedState(getAutoExpandPrefsKey(workspaceId), {}); + return setupSimpleChatStory({ + workspaceId, + workspaceName: "agent-communication", + projectName: "mux", + messages: [ + createAssistantMessage("outgoing-updates", "", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP, + toolCalls: [ + { + type: "dynamic-tool", + toolCallId: "outgoing-report", + toolName: "agent_report", + state: "output-available", + input: { title: REPORT_TITLE, reportMarkdown: REPORT }, + output: failed + ? { success: false, error: "The parent workspace is unavailable." } + : { success: true }, + }, + { + type: "dynamic-tool", + toolCallId: "outgoing-message", + toolName: "task_send_message", + state: "output-available", + input: { task_id: "b3947e259a", message: MESSAGE }, + output: failed + ? { + status: "refused", + taskId: "b3947e259a", + reason: "This message is already queued.", + } + : { status: "reactivated", taskId: "b3947e259a" }, + }, + ], + }), + ], + additionalWorkspaces: [ + createWorkspace({ + id: "b3947e259a", + name: "reviewer", + title: "Reviewer", + projectName: "mux", + parentWorkspaceId: workspaceId, + taskStatus: "running", + }), + ], + }); +} + +export default { + ...appMeta, + title: "App/AgentCommunication", +}; + +export const Outgoing: AppStory = { + render: () => , + parameters: { + ...appMeta.parameters, + pixel: { matrix: { themes: ["dark", "light"], viewports: ["laptop"] } }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const toggle = await canvas.findByRole("button", { name: "Message to Reviewer" }); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(toggle); + await expect(toggle).toHaveAttribute("aria-expanded", "true"); + // Prove keyboard collapse/expand as well as pointer interaction. + toggle.focus(); + await userEvent.keyboard("{Enter}"); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + await userEvent.keyboard(" "); + await expect(toggle).toHaveAttribute("aria-expanded", "true"); + await new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ); + }, +}; + +export const Phone: AppStory = { + ...Outgoing, + // Pin both the manager/Pixel viewport and the test-runner's otherwise desktop-width canvas. + globals: { viewport: { value: "mobile1", isRotated: false } }, + decorators: [PhoneSubagentReportDecorator], + parameters: { + ...appMeta.parameters, + pixel: { matrix: { themes: ["dark", "light"], viewports: ["phone"] } }, + }, + play: async (context) => { + await Outgoing.play?.(context); + const cards = context.canvasElement.querySelectorAll( + '[data-component="AgentCommunicationCard"]' + ); + await expect(cards.length).toBe(2); + for (const card of cards) { + await expect(card.clientWidth).toBeLessThan(390); + await expect(card.scrollWidth).toBeLessThanOrEqual(card.clientWidth); + } + }, +}; + +export const DeliveryFailures: AppStory = { + render: () => setupCommunicationStory(true)} />, + parameters: { + ...appMeta.parameters, + pixel: { matrix: { themes: ["dark", "light"], viewports: ["laptop"] } }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(await canvas.findAllByRole("alert")).toHaveLength(2); + const toggle = canvas.getByRole("button", { name: REPORT_TITLE }); + await userEvent.click(toggle); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + await expect(canvas.getAllByRole("alert")).toHaveLength(2); + }, +}; diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index 12e71b83f30..3cf746ef273 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -13733,7 +13733,6 @@ describe("WorkspaceService reorderPinned across projects", () => { const mockConfig: Partial = { srcDir: "/tmp/src", - getSessionDir: mock(() => "/tmp/test/sessions"), findWorkspace: mock((id: string) => { const found = findEntry(id); if (!found) return null; From fb91ab2596b4da886979024bbf308e51309f80de Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 4 Sep 2026 12:21:55 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20harden=20outgoing=20c?= =?UTF-8?q?ommunication=20cards=20after=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validate persisted report and message results before rendering, keep transport errors separate from untrusted delivery fields, restore bounded keyboard-accessible message scrolling, and name blank-title report toggles. Add malformed-history and desktop/phone scrolling tests. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$21.44`_ --- .../Tools/AgentReportToolCall.test.tsx | 27 ++++++++++++ .../features/Tools/AgentReportToolCall.tsx | 21 ++++++--- .../features/Tools/TaskToolCall.test.tsx | 39 ++++++++++++++++ src/browser/features/Tools/TaskToolCall.tsx | 44 ++++++++++++------- .../App.agentCommunication.stories.tsx | 38 ++++++++++++++-- 5 files changed, 143 insertions(+), 26 deletions(-) diff --git a/src/browser/features/Tools/AgentReportToolCall.test.tsx b/src/browser/features/Tools/AgentReportToolCall.test.tsx index 642651eeff0..731f16f8ae5 100644 --- a/src/browser/features/Tools/AgentReportToolCall.test.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.test.tsx @@ -129,4 +129,31 @@ describe("AgentReportToolCall", () => { expect(view.queryByText("Draft report")).toBeNull(); expect(view.getByText("Accepted report")).toBeTruthy(); }); + + test.each(["", " "])("keeps the report toggle named for a blank title: %j", (title) => { + const view = render( + + + + ); + expect(view.getByRole("button", { name: /\S/ })).toBeTruthy(); + }); + + test.each( + ["legacy output", 42, true, [], { success: false, message: "Missing errors" }].map( + (result) => ({ result }) + ) + )("keeps malformed persisted report results renderable: %j", ({ result }) => { + const view = render( + + + + ); + expect(view.getByText("Preserved findings")).toBeTruthy(); + expect(view.getByRole("status").className).not.toContain("text-success"); + }); }); diff --git a/src/browser/features/Tools/AgentReportToolCall.tsx b/src/browser/features/Tools/AgentReportToolCall.tsx index 3c3a1fc92c8..5cf3fa19f80 100644 --- a/src/browser/features/Tools/AgentReportToolCall.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.tsx @@ -1,6 +1,7 @@ import React from "react"; import type { AgentReportToolArgs, AgentReportToolResult } from "@/common/types/tools"; +import { AgentReportToolResultSchema } from "@/common/utils/tools/toolDefinitions"; import { ErrorBox } from "./Shared/ToolPrimitives"; import { AgentCommunicationCard } from "./Shared/AgentCommunicationCard"; @@ -17,7 +18,7 @@ type AgentReportRenderableArgs = AgentReportToolArgs | LegacyAgentReportFileArgs interface AgentReportToolCallProps { args: AgentReportRenderableArgs; - result?: AgentReportToolResult; + result?: unknown; status?: ToolStatus; } @@ -35,15 +36,25 @@ function getSubmittedReportMarkdown( } export const AgentReportToolCall: React.FC = (props) => { - const reportMarkdown = getSubmittedReportMarkdown(props.args, props.result); - const failedResult = props.result?.success === false ? props.result : null; + // Persisted results bypass input-schema validation and may be malformed. + const parsed = AgentReportToolResultSchema.safeParse(props.result); + const result = isToolErrorResult(props.result) + ? props.result + : parsed.success + ? parsed.data + : undefined; + const invalidResult = props.result != null && result == null; + const reportMarkdown = getSubmittedReportMarkdown(props.args, result); + const failedResult = result?.success === false ? result : null; + const title = props.args.title?.trim() ?? ""; return ( 0 ? title : "Agent update"} destination="To parent" - status={failedResult ? "failed" : (props.status ?? "pending")} + status={failedResult || invalidResult ? "failed" : (props.status ?? "pending")} + statusLabel={invalidResult ? "Result unavailable" : undefined} preview={reportMarkdown} initiallyExpanded error={ diff --git a/src/browser/features/Tools/TaskToolCall.test.tsx b/src/browser/features/Tools/TaskToolCall.test.tsx index 5145cd445dd..e547ebb25ca 100644 --- a/src/browser/features/Tools/TaskToolCall.test.tsx +++ b/src/browser/features/Tools/TaskToolCall.test.tsx @@ -825,6 +825,45 @@ describe("TaskSendMessageToolCall", () => { workspaceContextMock = null; } }); + + test.each( + [ + "legacy output", + 42, + true, + [], + { status: "refused", reason: {} }, + { status: "constructor" }, + ].map((result) => ({ result })) + )("keeps malformed persisted message results renderable: %j", ({ result }) => { + const view = render( + + + + ); + expect(view.getByRole("status").className).not.toContain("text-success"); + fireEvent.click(view.getByRole("button", { name: "Message to agent" })); + expect(view.getByText(taskSendMessageArgs.message)).toBeTruthy(); + }); + + test("does not render unvalidated fields carried beside a transport error", () => { + const view = render( + + + + ); + expect(view.getByRole("alert").textContent).toBe("Connection lost"); + }); }); const taskTerminateArgs = { task_ids: ["wfr_x"] }; diff --git a/src/browser/features/Tools/TaskToolCall.tsx b/src/browser/features/Tools/TaskToolCall.tsx index 48f5294257a..9c3027d98b3 100644 --- a/src/browser/features/Tools/TaskToolCall.tsx +++ b/src/browser/features/Tools/TaskToolCall.tsx @@ -18,6 +18,7 @@ import { type ToolStatus, } from "./Shared/toolUtils"; import { AgentCommunicationCard } from "./Shared/AgentCommunicationCard"; +import { TaskSendMessageToolResultSchema } from "@/common/utils/tools/toolDefinitions"; import { MarkdownRenderer } from "../Messages/MarkdownRenderer"; import { useOptionalMessageListContext } from "../Messages/MessageListContext"; import { useStickyExpand } from "../Messages/useStickyExpand"; @@ -1707,7 +1708,7 @@ const TaskListItem: React.FC<{ interface TaskSendMessageToolCallProps { args: TaskSendMessageToolArgs; - result?: TaskSendMessageToolSuccessResult | ToolErrorResult; + result?: unknown; status?: ToolStatus; } @@ -1732,12 +1733,15 @@ export const TaskSendMessageToolCall: React.FC = ( workspaceContext?.workspaceMetadata, props.args.task_id ); - const result = props.result; - const toolError = isToolErrorResult(result); + // Persisted output is unknown even when the tool arguments have passed validation. + const parsed = TaskSendMessageToolResultSchema.safeParse(props.result); + const result = parsed.success ? parsed.data : undefined; + const toolError = isToolErrorResult(props.result) ? props.result : undefined; + const invalidResult = props.result != null && result == null && toolError == null; // A finished tool call can still mean delivery was refused or queued, not sent. - const delivery = result && !toolError ? MESSAGE_DELIVERY[result.status] : undefined; + const delivery = result ? MESSAGE_DELIVERY[result.status] : undefined; const relation = result && "targetRelation" in result ? result.targetRelation : undefined; - const error = result && "error" in result ? result.error : undefined; + const error = toolError?.error ?? (result && "error" in result ? result.error : undefined); return ( = ( } - status={toolError ? "failed" : (delivery?.status ?? props.status ?? "pending")} - statusLabel={delivery?.label} + status={ + toolError || invalidResult ? "failed" : (delivery?.status ?? props.status ?? "pending") + } + statusLabel={invalidResult ? "Result unavailable" : delivery?.label} preview={props.args.message} initiallyExpanded={false} error={ @@ -1760,23 +1766,27 @@ export const TaskSendMessageToolCall: React.FC = ( {error} )} - {result && "status" in result && result.status === "refused" && ( + {result?.status === "refused" && ( {result.reason} )} - {result && - "status" in result && - result.status === "rate_limited" && - result.retryAfterMs != null && ( -
- Retry in {Math.ceil(result.retryAfterMs / 1000)}s -
- )} + {result?.status === "rate_limited" && result.retryAfterMs != null && ( +
+ Retry in {Math.ceil(result.retryAfterMs / 1000)}s +
+ )} } > -
{props.args.message}
+
+ {props.args.message} +
); }; diff --git a/src/browser/stories/App.agentCommunication.stories.tsx b/src/browser/stories/App.agentCommunication.stories.tsx index 0df80821e1a..6b287ae3b3f 100644 --- a/src/browser/stories/App.agentCommunication.stories.tsx +++ b/src/browser/stories/App.agentCommunication.stories.tsx @@ -1,4 +1,4 @@ -import { expect, userEvent, within } from "@storybook/test"; +import { expect, userEvent, waitFor, within } from "@storybook/test"; import { getAutoExpandPrefsKey } from "@/common/constants/storage"; import { updatePersistedState } from "@/browser/hooks/usePersistedState"; import { appMeta, AppWithMocks, type AppStory } from "./meta.js"; @@ -15,9 +15,9 @@ const REPORT = const MESSAGE = "Focused follow-up before finalizing: verify the native canvas clipping behavior and the parent popup-blocked failure path.\n\nCheck handshake ordering in src/browser/features/Tools/SubagentTranscriptDialog.tsx and keep the change scoped to the existing behavior."; -function setupCommunicationStory(failed = false) { +function setupCommunicationStory(failed = false, longMessage = false) { // Separate transcript stores so switching stories cannot retain a successful delivery. - const workspaceId = failed ? `${WORKSPACE_ID}-failed` : WORKSPACE_ID; + const workspaceId = `${WORKSPACE_ID}-${failed ? "failed" : longMessage ? "long" : "sent"}`; collapseLeftSidebar(); collapseRightSidebar(); updatePersistedState(getAutoExpandPrefsKey(workspaceId), {}); @@ -45,7 +45,14 @@ function setupCommunicationStory(failed = false) { toolCallId: "outgoing-message", toolName: "task_send_message", state: "output-available", - input: { task_id: "b3947e259a", message: MESSAGE }, + input: { + task_id: "b3947e259a", + message: longMessage + ? Array.from({ length: 200 }, (_, index) => `Check ${index + 1}: ${MESSAGE}`).join( + "\n\n" + ) + : MESSAGE, + }, output: failed ? { status: "refused", @@ -136,3 +143,26 @@ export const DeliveryFailures: AppStory = { await expect(canvas.getAllByRole("alert")).toHaveLength(2); }, }; + +export const LongMessage: AppStory = { + ...Outgoing, + render: () => setupCommunicationStory(false, true)} />, + play: async (context) => { + await Outgoing.play?.(context); + const content = within(context.canvasElement).getByRole("region", { name: "Message content" }); + await expect(content.scrollHeight).toBeGreaterThan(content.clientHeight); + await expect(content.clientHeight).toBeLessThanOrEqual(window.innerHeight * 0.4 + 1); + content.focus(); + await expect(content).toHaveFocus(); + content.scrollTo({ top: content.scrollHeight }); + await waitFor(() => expect(content.scrollTop).toBeGreaterThan(0)); + await expect(content.scrollWidth).toBeLessThanOrEqual(content.clientWidth); + }, +}; + +export const LongMessagePhone: AppStory = { + ...LongMessage, + globals: Phone.globals, + decorators: Phone.decorators, + parameters: Phone.parameters, +}; From febac7e67c2f971ab3df72c4fb96ca9f3d2197b0 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 4 Sep 2026 12:39:21 +0000 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=A4=96=20tests:=20supply=20archive=20?= =?UTF-8?q?state=20in=20flat-sidebar=20fixtures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the five existing flat-sidebar tests that failed in CI because their workspace-actions fixtures omitted archivingWorkspaceIds. This only completes the test data; production sidebar behavior is unchanged. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$21.44`_ --- .../components/ProjectSidebar/ProjectSidebar.test.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx b/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx index e5bf32afd53..36e52ffab41 100644 --- a/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx +++ b/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx @@ -959,6 +959,7 @@ describe("ProjectSidebar flat chat list", () => { ({ selectedWorkspace: null, setSelectedWorkspace: () => undefined, + archivingWorkspaceIds: new Set(), preflightArchiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "ready" } }), archiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "archived" } }), @@ -1010,6 +1011,7 @@ describe("ProjectSidebar flat chat list", () => { ({ selectedWorkspace: null, setSelectedWorkspace: () => undefined, + archivingWorkspaceIds: new Set(), preflightArchiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "ready" } }), archiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "archived" } }), @@ -1100,6 +1102,7 @@ describe("ProjectSidebar flat chat list", () => { ({ selectedWorkspace: null, setSelectedWorkspace: () => undefined, + archivingWorkspaceIds: new Set(), preflightArchiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "ready" } }), archiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "archived" } }), @@ -1280,6 +1283,7 @@ describe("ProjectSidebar flat chat list", () => { ({ selectedWorkspace: null, setSelectedWorkspace: () => undefined, + archivingWorkspaceIds: new Set(), preflightArchiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "ready" } }), archiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "archived" } }), @@ -1335,6 +1339,7 @@ describe("ProjectSidebar flat chat list", () => { ({ selectedWorkspace: null, setSelectedWorkspace: () => undefined, + archivingWorkspaceIds: new Set(), preflightArchiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "ready" } }), archiveWorkspace: () => Promise.resolve({ success: true, data: { kind: "archived" } }), From 5a7a36db8e72a0697c210046cdea34653f8b0f71 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 4 Sep 2026 12:53:29 +0000 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20preserve=20hook-wrapp?= =?UTF-8?q?ed=20agent=20message=20results?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normalize known hook/UI metadata before strict render-time validation, without mutating the original output. Preserve bare pre-hook blocking errors and add frozen hooked-result regression cases for both cards. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$21.44`_ --- .../Tools/AgentReportToolCall.test.tsx | 36 ++++++++++++++++ .../features/Tools/AgentReportToolCall.tsx | 13 ++++-- .../features/Tools/Shared/toolUtils.tsx | 16 +++++++ .../features/Tools/TaskToolCall.test.tsx | 42 +++++++++++++++++++ src/browser/features/Tools/TaskToolCall.tsx | 6 ++- 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/src/browser/features/Tools/AgentReportToolCall.test.tsx b/src/browser/features/Tools/AgentReportToolCall.test.tsx index 731f16f8ae5..59ecde64113 100644 --- a/src/browser/features/Tools/AgentReportToolCall.test.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.test.tsx @@ -156,4 +156,40 @@ describe("AgentReportToolCall", () => { expect(view.getByText("Preserved findings")).toBeTruthy(); expect(view.getByRole("status").className).not.toContain("text-success"); }); + + test("accepts report results decorated by post hooks without mutating hook output", () => { + const result = Object.freeze({ + success: true, + report: { reportMarkdown: "Submitted findings" }, + hook_output: "Formatter completed", + hook_duration_ms: 20, + hook_path: ".xum/tool_post", + ui_only: {}, + }); + const view = render( + + + + ); + expect(view.getByText("Submitted findings")).toBeTruthy(); + expect(view.getByRole("status").className).toContain("text-success"); + }); + + test("shows bare pre-hook blocking errors", () => { + const view = render( + + + + ); + expect(view.getByRole("alert").textContent).toBe("Blocked by project hook"); + expect(view.getByRole("status").className).toContain("text-danger"); + }); }); diff --git a/src/browser/features/Tools/AgentReportToolCall.tsx b/src/browser/features/Tools/AgentReportToolCall.tsx index 5cf3fa19f80..740ab1a0c92 100644 --- a/src/browser/features/Tools/AgentReportToolCall.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.tsx @@ -5,7 +5,11 @@ import { AgentReportToolResultSchema } from "@/common/utils/tools/toolDefinition import { ErrorBox } from "./Shared/ToolPrimitives"; import { AgentCommunicationCard } from "./Shared/AgentCommunicationCard"; -import { isToolErrorResult, type ToolStatus } from "./Shared/toolUtils"; +import { + isToolErrorResult, + normalizeToolResultForRendering, + type ToolStatus, +} from "./Shared/toolUtils"; import { MarkdownRenderer } from "../Messages/MarkdownRenderer"; interface LegacyAgentReportFileArgs { @@ -37,9 +41,10 @@ function getSubmittedReportMarkdown( export const AgentReportToolCall: React.FC = (props) => { // Persisted results bypass input-schema validation and may be malformed. - const parsed = AgentReportToolResultSchema.safeParse(props.result); - const result = isToolErrorResult(props.result) - ? props.result + const normalizedResult = normalizeToolResultForRendering(props.result); + const parsed = AgentReportToolResultSchema.safeParse(normalizedResult); + const result = isToolErrorResult(normalizedResult) + ? normalizedResult : parsed.success ? parsed.data : undefined; diff --git a/src/browser/features/Tools/Shared/toolUtils.tsx b/src/browser/features/Tools/Shared/toolUtils.tsx index 885ba006379..37bddd8665f 100644 --- a/src/browser/features/Tools/Shared/toolUtils.tsx +++ b/src/browser/features/Tools/Shared/toolUtils.tsx @@ -1,6 +1,7 @@ import React from "react"; import { AlertTriangle, Check, CircleDot, EyeOff, X } from "lucide-react"; import type { ToolErrorResult } from "@/common/types/tools"; +import { isPlainObject } from "@/common/utils/isPlainObject"; import { useStickyExpand, type UseStickyExpandOptions, @@ -165,6 +166,21 @@ export function unwrapResult(result: unknown): unknown { return result; } +/** Preserve wrapper compatibility before strict result validation without mutating hook/UI output. */ +export function normalizeToolResultForRendering(result: unknown): unknown { + if (!isPlainObject(result)) return result; + const core = { ...result }; + delete core.hook_output; + delete core.hook_duration_ms; + delete core.hook_path; + delete core.ui_only; + // Blocking pre-hooks return a bare error instead of the tool's result schema. + if (typeof core.error === "string" && !("success" in core) && !("status" in core)) { + return { success: false, error: core.error }; + } + return core; +} + /** * Type guard for ToolErrorResult shape: { success: false, error: string }. * Use this when you need type narrowing to access error. diff --git a/src/browser/features/Tools/TaskToolCall.test.tsx b/src/browser/features/Tools/TaskToolCall.test.tsx index e547ebb25ca..beade8fa20b 100644 --- a/src/browser/features/Tools/TaskToolCall.test.tsx +++ b/src/browser/features/Tools/TaskToolCall.test.tsx @@ -864,6 +864,48 @@ describe("TaskSendMessageToolCall", () => { ); expect(view.getByRole("alert").textContent).toBe("Connection lost"); }); + + test.each(["accepted", "queued", "reactivated"] as const)( + "preserves hooked delivery outcome: %s", + (status) => { + const view = render( + + + + ); + expect(view.getByRole("status").className).toContain( + status === "queued" ? "text-backgrounded" : "text-success" + ); + } + ); + + test("shows bare pre-hook errors alongside their metadata", () => { + const view = render( + + + + ); + expect(view.getByRole("alert").textContent).toBe("Blocked by project hook"); + expect(view.getByRole("status").className).toContain("text-danger"); + }); }); const taskTerminateArgs = { task_ids: ["wfr_x"] }; diff --git a/src/browser/features/Tools/TaskToolCall.tsx b/src/browser/features/Tools/TaskToolCall.tsx index 9c3027d98b3..9e2da85143e 100644 --- a/src/browser/features/Tools/TaskToolCall.tsx +++ b/src/browser/features/Tools/TaskToolCall.tsx @@ -15,6 +15,7 @@ import { useToolExpansion, getStatusDisplay, isToolErrorResult, + normalizeToolResultForRendering, type ToolStatus, } from "./Shared/toolUtils"; import { AgentCommunicationCard } from "./Shared/AgentCommunicationCard"; @@ -1734,9 +1735,10 @@ export const TaskSendMessageToolCall: React.FC = ( props.args.task_id ); // Persisted output is unknown even when the tool arguments have passed validation. - const parsed = TaskSendMessageToolResultSchema.safeParse(props.result); + const normalizedResult = normalizeToolResultForRendering(props.result); + const parsed = TaskSendMessageToolResultSchema.safeParse(normalizedResult); const result = parsed.success ? parsed.data : undefined; - const toolError = isToolErrorResult(props.result) ? props.result : undefined; + const toolError = isToolErrorResult(normalizedResult) ? normalizedResult : undefined; const invalidResult = props.result != null && result == null && toolError == null; // A finished tool call can still mean delivery was refused or queued, not sent. const delivery = result ? MESSAGE_DELIVERY[result.status] : undefined; From 3cfd3cb73f4648e8c5def9b484441b40229ed3a3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 4 Sep 2026 13:15:46 +0000 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20handle=20missing=20an?= =?UTF-8?q?d=20SDK-wrapped=20communication=20results?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unwrap supported SDK JSON results before hook normalization and schema validation. Treat absent completed outputs as unavailable rather than sent while preserving pending, executing, failed, and interrupted states. Cover both cards with missing-result, wrapper, hook, and error cases. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$21.44`_ --- .../Tools/AgentReportToolCall.test.tsx | 65 +++++++++++++++++++ .../features/Tools/AgentReportToolCall.tsx | 2 +- .../features/Tools/Shared/toolUtils.tsx | 5 +- .../features/Tools/TaskToolCall.test.tsx | 64 ++++++++++++++++++ src/browser/features/Tools/TaskToolCall.tsx | 3 +- 5 files changed, 135 insertions(+), 4 deletions(-) diff --git a/src/browser/features/Tools/AgentReportToolCall.test.tsx b/src/browser/features/Tools/AgentReportToolCall.test.tsx index 59ecde64113..8d9ee7969f7 100644 --- a/src/browser/features/Tools/AgentReportToolCall.test.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.test.tsx @@ -192,4 +192,69 @@ describe("AgentReportToolCall", () => { expect(view.getByRole("alert").textContent).toBe("Blocked by project hook"); expect(view.getByRole("status").className).toContain("text-danger"); }); + + test.each([null, undefined, { type: "json", value: null }].map((result) => ({ result })))( + "does not claim delivery for a missing completed result: %j", + ({ result }) => { + const view = render( + + + + ); + expect(view.getByRole("status").className).not.toContain("text-success"); + expect(view.getByRole("status").textContent).toBe("Result unavailable"); + } + ); + + test.each([ + { status: "pending", label: "Pending" }, + { status: "executing", label: "Sending…" }, + { status: "failed", label: "Not sent" }, + { status: "interrupted", label: "Interrupted" }, + ] as const)("preserves lifecycle status without a result: $status", ({ status, label }) => { + const view = render( + + + + ); + expect(view.getByRole("status").textContent).toBe(label); + }); + + test("accepts SDK-wrapped results with inner and outer hook metadata", () => { + const view = render( + + + + ); + expect(view.getByRole("status").className).toContain("text-success"); + }); + + test("shows SDK-wrapped blocking errors", () => { + const view = render( + + + + ); + expect(view.getByRole("alert").textContent).toBe("Wrapped blocking error"); + }); }); diff --git a/src/browser/features/Tools/AgentReportToolCall.tsx b/src/browser/features/Tools/AgentReportToolCall.tsx index 740ab1a0c92..fef7d65f651 100644 --- a/src/browser/features/Tools/AgentReportToolCall.tsx +++ b/src/browser/features/Tools/AgentReportToolCall.tsx @@ -48,7 +48,7 @@ export const AgentReportToolCall: React.FC = (props) = : parsed.success ? parsed.data : undefined; - const invalidResult = props.result != null && result == null; + const invalidResult = (props.result != null || props.status === "completed") && result == null; const reportMarkdown = getSubmittedReportMarkdown(props.args, result); const failedResult = result?.success === false ? result : null; const title = props.args.title?.trim() ?? ""; diff --git a/src/browser/features/Tools/Shared/toolUtils.tsx b/src/browser/features/Tools/Shared/toolUtils.tsx index 37bddd8665f..61b5587565b 100644 --- a/src/browser/features/Tools/Shared/toolUtils.tsx +++ b/src/browser/features/Tools/Shared/toolUtils.tsx @@ -168,8 +168,9 @@ export function unwrapResult(result: unknown): unknown { /** Preserve wrapper compatibility before strict result validation without mutating hook/UI output. */ export function normalizeToolResultForRendering(result: unknown): unknown { - if (!isPlainObject(result)) return result; - const core = { ...result }; + const unwrapped = unwrapResult(result); + if (!isPlainObject(unwrapped)) return unwrapped; + const core = { ...unwrapped }; delete core.hook_output; delete core.hook_duration_ms; delete core.hook_path; diff --git a/src/browser/features/Tools/TaskToolCall.test.tsx b/src/browser/features/Tools/TaskToolCall.test.tsx index beade8fa20b..74052f3bfc4 100644 --- a/src/browser/features/Tools/TaskToolCall.test.tsx +++ b/src/browser/features/Tools/TaskToolCall.test.tsx @@ -906,6 +906,70 @@ describe("TaskSendMessageToolCall", () => { expect(view.getByRole("alert").textContent).toBe("Blocked by project hook"); expect(view.getByRole("status").className).toContain("text-danger"); }); + + test.each([null, undefined, { type: "json", value: null }].map((result) => ({ result })))( + "does not claim delivery for a missing completed result: %j", + ({ result }) => { + const view = render( + + + + ); + expect(view.getByRole("status").className).not.toContain("text-success"); + expect(view.getByRole("status").textContent).toBe("Result unavailable"); + } + ); + + test.each([ + { status: "pending", label: "Pending" }, + { status: "executing", label: "Sending…" }, + { status: "failed", label: "Not sent" }, + { status: "interrupted", label: "Interrupted" }, + ] as const)("preserves lifecycle status without a result: $status", ({ status, label }) => { + const view = render( + + + + ); + expect(view.getByRole("status").textContent).toBe(label); + }); + + test("accepts SDK-wrapped results with inner and outer hook metadata", () => { + const view = render( + + + + ); + expect(view.getByRole("status").className).toContain("text-success"); + }); + + test("shows SDK-wrapped blocking errors", () => { + const view = render( + + + + ); + expect(view.getByRole("alert").textContent).toBe("Wrapped blocking error"); + }); }); const taskTerminateArgs = { task_ids: ["wfr_x"] }; diff --git a/src/browser/features/Tools/TaskToolCall.tsx b/src/browser/features/Tools/TaskToolCall.tsx index 9e2da85143e..45ba395fc3b 100644 --- a/src/browser/features/Tools/TaskToolCall.tsx +++ b/src/browser/features/Tools/TaskToolCall.tsx @@ -1739,7 +1739,8 @@ export const TaskSendMessageToolCall: React.FC = ( const parsed = TaskSendMessageToolResultSchema.safeParse(normalizedResult); const result = parsed.success ? parsed.data : undefined; const toolError = isToolErrorResult(normalizedResult) ? normalizedResult : undefined; - const invalidResult = props.result != null && result == null && toolError == null; + const invalidResult = + (props.result != null || props.status === "completed") && result == null && toolError == null; // A finished tool call can still mean delivery was refused or queued, not sent. const delivery = result ? MESSAGE_DELIVERY[result.status] : undefined; const relation = result && "targetRelation" in result ? result.targetRelation : undefined;