mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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. <img width="832" height="482" alt="image" src="https://github.com/user-attachments/assets/50b04b9a-b153-48e5-ab5e-6c2fa000f21e" /> 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
This commit is contained in:
@@ -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<typeof ExecuteTool> = {
|
||||
title: "components/ai-elements/tool/ExecuteTool",
|
||||
component: ExecuteTool,
|
||||
@@ -18,7 +21,7 @@ const meta: Meta<typeof ExecuteTool> = {
|
||||
args: {
|
||||
status: "completed",
|
||||
isError: false,
|
||||
output: "",
|
||||
transcriptBlocks: [],
|
||||
},
|
||||
};
|
||||
export default meta;
|
||||
@@ -28,7 +31,7 @@ type Story = StoryObj<typeof ExecuteTool>;
|
||||
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" },
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -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<ExecuteToolProps> = (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<ExecuteToolProps> = (props) => {
|
||||
|
||||
const ExecuteToolInner: React.FC<ExecuteToolInnerProps> = ({
|
||||
command,
|
||||
output,
|
||||
transcriptBlocks,
|
||||
status,
|
||||
isError,
|
||||
durationMs,
|
||||
@@ -127,7 +129,7 @@ const ExecuteToolInner: React.FC<ExecuteToolInnerProps> = ({
|
||||
<span
|
||||
aria-label="Command failed"
|
||||
role="img"
|
||||
className="flex shrink-0 text-content-destructive"
|
||||
className="flex shrink-0 text-content-secondary"
|
||||
>
|
||||
<TriangleAlertIcon aria-hidden className="size-3.5 shrink-0" />
|
||||
</span>
|
||||
@@ -168,7 +170,7 @@ const ExecuteToolInner: React.FC<ExecuteToolInnerProps> = ({
|
||||
{outputOpen && (
|
||||
<ShellTranscriptBody
|
||||
command={command}
|
||||
output={output}
|
||||
transcriptBlocks={transcriptBlocks}
|
||||
isError={isError}
|
||||
/>
|
||||
)}
|
||||
@@ -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 (
|
||||
<ScrollArea
|
||||
className="col-start-1 col-span-2 mt-2 rounded-xl bg-surface-secondary/60 text-2xs"
|
||||
@@ -233,16 +235,19 @@ const ShellTranscriptBody: React.FC<{
|
||||
</span>{" "}
|
||||
{command}
|
||||
</pre>
|
||||
{output.length > 0 && (
|
||||
{transcriptBlocks.map((block) => (
|
||||
<pre
|
||||
key={block.kind}
|
||||
className={cn(
|
||||
"m-0 mt-4 whitespace-pre-wrap break-words border-0 bg-transparent p-0 font-mono text-xs font-normal leading-5",
|
||||
isError ? "text-content-destructive" : "text-content-secondary",
|
||||
block.kind === "error" || isError
|
||||
? "text-content-destructive"
|
||||
: "text-content-secondary",
|
||||
)}
|
||||
>
|
||||
{output}
|
||||
{block.text}
|
||||
</pre>
|
||||
)}
|
||||
))}
|
||||
</div>
|
||||
</ScrollArea>
|
||||
);
|
||||
|
||||
@@ -231,12 +231,15 @@ const ExecuteRenderer: FC<ToolRendererProps> = ({
|
||||
shellToolDisplayMode,
|
||||
}) => {
|
||||
const data = getExecuteRenderData(args, result);
|
||||
const outputBlock = data.transcriptBlocks.find(
|
||||
(block) => block.kind === "output",
|
||||
);
|
||||
|
||||
if (data.authenticateURL) {
|
||||
return (
|
||||
<ExecuteAuthRequiredTool
|
||||
command={data.command}
|
||||
output={data.output}
|
||||
output={outputBlock?.text ?? ""}
|
||||
authenticateURL={data.authenticateURL}
|
||||
providerLabel={data.providerLabel}
|
||||
/>
|
||||
@@ -245,7 +248,7 @@ const ExecuteRenderer: FC<ToolRendererProps> = ({
|
||||
return (
|
||||
<ExecuteToolComponent
|
||||
command={data.command}
|
||||
output={data.output}
|
||||
transcriptBlocks={data.transcriptBlocks}
|
||||
status={status}
|
||||
isError={isError}
|
||||
durationMs={data.durationMs}
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { getExecuteRenderData, shouldRenderTool } from "./toolVisibility";
|
||||
|
||||
const stoppedWorkspaceError =
|
||||
"workspace has no running agent: the workspace is likely stopped. Use the start_workspace tool to start it";
|
||||
|
||||
describe("toolVisibility", () => {
|
||||
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", () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user