From 1fcb4002d7c7a0a13b2c7cad00b1eff5ecbf9381 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Mon, 1 Jun 2026 21:25:29 +1000 Subject: [PATCH] fix: show execute tool errors (#25886) Execute tool failures that only return an `error` field, such as stopped-workspace connection failures, were rendered as a generic failed command without showing the backend detail. Normalize execute results into transcript blocks so shell output and tool errors both render in the *expanded* command transcript, and add Storybook coverage for connection errors plus output-with-error cases. edit: i've dropped the red on the danger icon, though it was pre-existing. no point alerting the user to an error the model will handle. Closes CODAGT-530 --- .../tools/ExecuteTool.stories.tsx | 126 +++++++++++++----- .../ChatElements/tools/ExecuteTool.tsx | 27 ++-- .../components/ChatElements/tools/Tool.tsx | 7 +- .../ChatElements/tools/toolVisibility.test.ts | 41 +++++- .../ChatElements/tools/toolVisibility.ts | 19 ++- 5 files changed, 171 insertions(+), 49 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx index d9b469ff9c..54a47fe050 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.stories.tsx @@ -1,10 +1,13 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { userEvent, within } from "storybook/test"; +import { expect, userEvent, within } from "storybook/test"; import { ExecuteTool } from "./ExecuteTool"; const longCommand = "find /home/coder/project/src -type f -name '*.ts' -not -path '*/node_modules/*' -not -path '*/.git/*' | xargs grep -l 'deprecated' | sort | head -50"; +const stoppedWorkspaceError = + "workspace has no running agent: the workspace is likely stopped. Use the start_workspace tool to start it"; + const meta: Meta = { title: "components/ai-elements/tool/ExecuteTool", component: ExecuteTool, @@ -18,7 +21,7 @@ const meta: Meta = { args: { status: "completed", isError: false, - output: "", + transcriptBlocks: [], }, }; export default meta; @@ -28,7 +31,7 @@ type Story = StoryObj; export const ShortCommand: Story = { args: { command: "git status", - output: "", + transcriptBlocks: [], }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -43,7 +46,7 @@ export const RunningWithoutCommand: Story = { args: { command: "", status: "running", - output: "", + transcriptBlocks: [], }, }; @@ -57,7 +60,7 @@ export const LongCommand: Story = { ], args: { command: longCommand, - output: "", + transcriptBlocks: [], }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); @@ -72,14 +75,19 @@ export const LongCommand: Story = { export const LongCommandWithOutput: Story = { args: { command: longCommand, - output: [ - "src/api/legacyClient.ts", - "src/components/OldTable/OldTable.tsx", - "src/hooks/useObsoleteAuth.ts", - "src/pages/SettingsPage/DeprecatedPanel.tsx", - "src/utils/formatDate.ts", - "src/utils/legacyHelpers.ts", - ].join("\n"), + transcriptBlocks: [ + { + kind: "output", + text: [ + "src/api/legacyClient.ts", + "src/components/OldTable/OldTable.tsx", + "src/hooks/useObsoleteAuth.ts", + "src/pages/SettingsPage/DeprecatedPanel.tsx", + "src/utils/formatDate.ts", + "src/utils/legacyHelpers.ts", + ].join("\n"), + }, + ], }, }; @@ -87,19 +95,24 @@ export const LongCommandWithOutput: Story = { export const WithOutput: Story = { args: { command: "docker ps --format 'table {{.Names}}\t{{.Status}}\t{{.Ports}}'", - output: [ - "NAMES STATUS PORTS", - "coder-gateway Up 3 hours 0.0.0.0:3000->3000/tcp", - "coder-database Up 3 hours 0.0.0.0:5432->5432/tcp", - "coder-provisioner Up 3 hours", - "redis-cache Up 3 hours 0.0.0.0:6379->6379/tcp", - "nginx-proxy Up 2 hours 0.0.0.0:80->80/tcp, 0.0.0.0:443->443/tcp", - "prometheus Up 2 hours 0.0.0.0:9090->9090/tcp", - "grafana Up 2 hours 0.0.0.0:3001->3001/tcp", - "jaeger Up 1 hour 0.0.0.0:16686->16686/tcp", - "otel-collector Up 1 hour 0.0.0.0:4317->4317/tcp", - "loki Up 1 hour 0.0.0.0:3100->3100/tcp", - ].join("\n"), + transcriptBlocks: [ + { + kind: "output", + text: [ + "NAMES STATUS PORTS", + "coder-gateway Up 3 hours 0.0.0.0:3000->3000/tcp", + "coder-database Up 3 hours 0.0.0.0:5432->5432/tcp", + "coder-provisioner Up 3 hours", + "redis-cache Up 3 hours 0.0.0.0:6379->6379/tcp", + "nginx-proxy Up 2 hours 0.0.0.0:80->80/tcp, 0.0.0.0:443->443/tcp", + "prometheus Up 2 hours 0.0.0.0:9090->9090/tcp", + "grafana Up 2 hours 0.0.0.0:3001->3001/tcp", + "jaeger Up 1 hour 0.0.0.0:16686->16686/tcp", + "otel-collector Up 1 hour 0.0.0.0:4317->4317/tcp", + "loki Up 1 hour 0.0.0.0:3100->3100/tcp", + ].join("\n"), + }, + ], }, }; @@ -108,8 +121,12 @@ export const Running: Story = { args: { command: "go test -race -count=1 ./coderd/...", status: "running", - output: - "=== RUN TestWorkspaceAgent\n--- PASS: TestWorkspaceAgent (0.42s)", + transcriptBlocks: [ + { + kind: "output", + text: "=== RUN TestWorkspaceAgent\n--- PASS: TestWorkspaceAgent (0.42s)", + }, + ], }, }; @@ -119,11 +136,54 @@ export const ErrorOutput: Story = { command: "make build", status: "completed", isError: true, - output: [ - "coderd/workspaces.go:142:6: cannot use ws (variable of type *database.Workspace) as database.Store value in argument to api.Authorize", - "coderd/workspaces.go:155:19: ws.OwnerID undefined (type *database.Workspace has no field or method OwnerID)", - "make: *** [build] Error 1", - ].join("\n"), + transcriptBlocks: [ + { + kind: "output", + text: [ + "coderd/workspaces.go:142:6: cannot use ws (variable of type *database.Workspace) as database.Store value in argument to api.Authorize", + "coderd/workspaces.go:155:19: ws.OwnerID undefined (type *database.Workspace has no field or method OwnerID)", + "make: *** [build] Error 1", + ].join("\n"), + }, + ], + }, +}; + +/** A connection error renders its details inside the expanded transcript. */ +export const ConnectionError: Story = { + args: { + command: "ls -la", + status: "error", + isError: true, + shellToolDisplayMode: "auto", + transcriptBlocks: [{ kind: "error", text: stoppedWorkspaceError }], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const errorMessage = await canvas.findByText( + /workspace has no running agent/i, + ); + await expect(errorMessage).toBeVisible(); + }, +}; + +/** A timed-out command can return partial output plus an execute error. */ +export const OutputWithError: Story = { + args: { + command: "go test ./...", + status: "completed", + isBackgrounded: true, + transcriptBlocks: [ + { + kind: "output", + text: [ + "=== RUN TestWorkspaceAgent", + "--- PASS: TestWorkspaceAgent (0.42s)", + "=== RUN TestWorkspaceBuild", + ].join("\n"), + }, + { kind: "error", text: "command timed out after 10s" }, + ], }, }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx index 6f0a3b18f0..70f2c9fddf 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ExecuteTool.tsx @@ -27,6 +27,7 @@ import { resolveAgentDisplayState, } from "./displayMode"; import { ToolIcon } from "./ToolIcon"; +import type { ExecuteTranscriptBlock } from "./toolVisibility"; import { formatShellDurationMs, sanitizeExecuteModelIntent, @@ -37,7 +38,7 @@ import { type ExecuteToolProps = { command: string; - output: string; + transcriptBlocks: readonly ExecuteTranscriptBlock[]; status: ToolStatus; isError: boolean; durationMs?: number; @@ -53,8 +54,9 @@ type ExecuteToolInnerProps = ExecuteToolProps & { }; export const ExecuteTool: React.FC = (props) => { + const hasTranscriptBlocks = props.transcriptBlocks.length > 0; const autoDisplayState: AgentDisplayState = - props.output.length > 0 || + hasTranscriptBlocks || props.status === "running" || props.isBackgrounded || !!props.killedBySignal @@ -75,7 +77,7 @@ export const ExecuteTool: React.FC = (props) => { const ExecuteToolInner: React.FC = ({ command, - output, + transcriptBlocks, status, isError, durationMs, @@ -127,7 +129,7 @@ const ExecuteToolInner: React.FC = ({ @@ -168,7 +170,7 @@ const ExecuteToolInner: React.FC = ({ {outputOpen && ( )} @@ -217,9 +219,9 @@ const ShellCommandLine: React.FC<{ const ShellTranscriptBody: React.FC<{ command: string; - output: string; + transcriptBlocks: readonly ExecuteTranscriptBlock[]; isError: boolean; -}> = ({ command, output, isError }) => { +}> = ({ command, transcriptBlocks, isError }) => { return ( {" "} {command} - {output.length > 0 && ( + {transcriptBlocks.map((block) => ( - {output} + {block.text} - )} + ))} ); diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx index 82c18f6fe7..5fce31cff8 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx @@ -231,12 +231,15 @@ const ExecuteRenderer: FC = ({ shellToolDisplayMode, }) => { const data = getExecuteRenderData(args, result); + const outputBlock = data.transcriptBlocks.find( + (block) => block.kind === "output", + ); if (data.authenticateURL) { return ( @@ -245,7 +248,7 @@ const ExecuteRenderer: FC = ({ return ( { describe("getExecuteRenderData", () => { it("parses execute output and auth metadata from result payloads", () => { @@ -18,13 +21,49 @@ describe("toolVisibility", () => { ), ).toEqual({ command: "git fetch origin", - output: "fetched", + transcriptBlocks: [{ kind: "output", text: "fetched" }], durationMs: 47200, isBackgrounded: true, authenticateURL: "https://example.com/auth", providerLabel: "GitHub", }); }); + + it("normalizes execute error results into transcript blocks", () => { + const data = getExecuteRenderData( + { command: "ls -la" }, + { error: stoppedWorkspaceError }, + ); + + expect(data.command).toBe("ls -la"); + expect(data.transcriptBlocks).toEqual([ + { kind: "error", text: stoppedWorkspaceError }, + ]); + expect( + data.transcriptBlocks.map((block) => block.text).join("\n"), + ).toContain("workspace has no running agent"); + }); + + it("keeps output before error when both fields exist", () => { + expect( + getExecuteRenderData( + { command: "make build" }, + { output: " compiling ", error: " failed " }, + ).transcriptBlocks, + ).toEqual([ + { kind: "output", text: "compiling" }, + { kind: "error", text: "failed" }, + ]); + }); + + it("uses message as an error fallback when error is blank", () => { + expect( + getExecuteRenderData( + { command: "coder login" }, + { error: " ", message: " auth required " }, + ).transcriptBlocks, + ).toEqual([{ kind: "error", text: "auth required" }]); + }); }); describe("shouldRenderTool", () => { diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts index daaca9e269..32c17772fd 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/toolVisibility.ts @@ -8,9 +8,14 @@ import { toProviderLabel, } from "./utils"; +export type ExecuteTranscriptBlock = { + kind: "output" | "error"; + text: string; +}; + type ExecuteRenderData = { command: string; - output: string; + transcriptBlocks: ExecuteTranscriptBlock[]; durationMs?: number; isBackgrounded: boolean; authenticateURL: string; @@ -29,6 +34,16 @@ export const getExecuteRenderData = ( const command = parsedArgs ? asString(parsedArgs.command) : ""; const rec = asRecord(result); const output = rec ? asString(rec.output).trim() : ""; + const error = rec ? asString(rec.error).trim() : ""; + const fallbackMessage = rec && !error ? asString(rec.message).trim() : ""; + const errorText = error || fallbackMessage; + const transcriptBlocks: ExecuteTranscriptBlock[] = []; + if (output) { + transcriptBlocks.push({ kind: "output", text: output }); + } + if (errorText) { + transcriptBlocks.push({ kind: "error", text: errorText }); + } const durationMs = rec ? (asNumber(rec.wall_duration_ms, { parseString: true }) ?? asNumber(rec.duration_ms, { parseString: true })) @@ -47,7 +62,7 @@ export const getExecuteRenderData = ( return { command, - output, + transcriptBlocks, durationMs, isBackgrounded, authenticateURL,
- {output} + {block.text}