mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-07-12 21:02:17 +02:00
fix(x): surface coding tool errors
This commit is contained in:
parent
36c0cca3f9
commit
4a2fe30153
11 changed files with 138 additions and 18 deletions
|
|
@ -22,7 +22,7 @@ import {
|
||||||
import { type ComponentProps, type ReactNode, isValidElement, useState } from "react";
|
import { type ComponentProps, type ReactNode, isValidElement, useState } from "react";
|
||||||
import { AnimatePresence, motion } from "motion/react";
|
import { AnimatePresence, motion } from "motion/react";
|
||||||
import type { ToolCall, ToolGroup as ToolGroupType } from "@/lib/chat-conversation";
|
import type { ToolCall, ToolGroup as ToolGroupType } from "@/lib/chat-conversation";
|
||||||
import { getToolActionsSummary, getToolDisplayName, getToolGroupSummary, toToolState } from "@/lib/chat-conversation";
|
import { getToolActionsSummary, getToolDisplayName, getToolErrorText, getToolGroupSummary, toToolState } from "@/lib/chat-conversation";
|
||||||
|
|
||||||
const formatToolValue = (value: unknown) => {
|
const formatToolValue = (value: unknown) => {
|
||||||
if (typeof value === "string") return value;
|
if (typeof value === "string") return value;
|
||||||
|
|
@ -329,7 +329,7 @@ export const ToolGroupComponent = ({ group, isToolOpen, onToolOpenChange }: Tool
|
||||||
<ToolTabbedContent
|
<ToolTabbedContent
|
||||||
input={tool.input as ToolUIPart["input"]}
|
input={tool.input as ToolUIPart["input"]}
|
||||||
output={tool.result as ToolUIPart["output"]}
|
output={tool.result as ToolUIPart["output"]}
|
||||||
errorText={tool.status === 'error' ? 'Tool error' : undefined}
|
errorText={getToolErrorText(tool)}
|
||||||
/>
|
/>
|
||||||
</ToolContent>
|
</ToolContent>
|
||||||
</Tool>
|
</Tool>
|
||||||
|
|
|
||||||
|
|
@ -51,6 +51,7 @@ import {
|
||||||
getWebSearchCardData,
|
getWebSearchCardData,
|
||||||
getComposioConnectCardData,
|
getComposioConnectCardData,
|
||||||
getToolDisplayName,
|
getToolDisplayName,
|
||||||
|
getToolErrorText,
|
||||||
groupConversationItems,
|
groupConversationItems,
|
||||||
isChatMessage,
|
isChatMessage,
|
||||||
isErrorMessage,
|
isErrorMessage,
|
||||||
|
|
@ -484,7 +485,7 @@ export function ChatSidebar({
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
const toolTitle = getToolDisplayName(item)
|
const toolTitle = getToolDisplayName(item)
|
||||||
const errorText = item.status === 'error' ? 'Tool error' : ''
|
const errorText = getToolErrorText(item)
|
||||||
const output = normalizeToolOutput(item.result, item.status)
|
const output = normalizeToolOutput(item.result, item.status)
|
||||||
const input = normalizeToolInput(item.input)
|
const input = normalizeToolInput(item.input)
|
||||||
return (
|
return (
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ import { Conversation, ConversationContent, ConversationScrollButton } from '@/c
|
||||||
import { MessageResponse } from '@/components/ai-elements/message'
|
import { MessageResponse } from '@/components/ai-elements/message'
|
||||||
import { Shimmer } from '@/components/ai-elements/shimmer'
|
import { Shimmer } from '@/components/ai-elements/shimmer'
|
||||||
import { Tool, ToolContent, ToolHeader } from '@/components/ai-elements/tool'
|
import { Tool, ToolContent, ToolHeader } from '@/components/ai-elements/tool'
|
||||||
import { toToolState, getToolDisplayName, getWebSearchCardData, type ToolCall } from '@/lib/chat-conversation'
|
import { toToolState, getToolDisplayName, getToolErrorText, getWebSearchCardData, type ToolCall } from '@/lib/chat-conversation'
|
||||||
import { CodeRunPermissionRequest, CodingRunTimeline } from '@/components/coding-run'
|
import { CodeRunPermissionRequest, CodingRunTimeline } from '@/components/coding-run'
|
||||||
import { PermissionRequest } from '@/components/ai-elements/permission-request'
|
import { PermissionRequest } from '@/components/ai-elements/permission-request'
|
||||||
import { AskHumanRequest } from '@/components/ai-elements/ask-human-request'
|
import { AskHumanRequest } from '@/components/ai-elements/ask-human-request'
|
||||||
|
|
@ -84,7 +84,7 @@ function RowboatToolCall({ item, onOpenDiff }: { item: ToolCall; onOpenDiff: (pa
|
||||||
<Tool open={open || item.status === 'running'} onOpenChange={setOpen}>
|
<Tool open={open || item.status === 'running'} onOpenChange={setOpen}>
|
||||||
<ToolHeader title={AGENT_LABEL[agent ?? ''] ?? 'Coding agent'} type="tool-code_agent_run" state={toToolState(item.status)} />
|
<ToolHeader title={AGENT_LABEL[agent ?? ''] ?? 'Coding agent'} type="tool-code_agent_run" state={toToolState(item.status)} />
|
||||||
<ToolContent>
|
<ToolContent>
|
||||||
<CodingRunTimeline events={item.codeRunEvents ?? []} onOpenDiff={onOpenDiff} />
|
<CodingRunTimeline events={item.codeRunEvents ?? []} error={getToolErrorText(item)} onOpenDiff={onOpenDiff} />
|
||||||
</ToolContent>
|
</ToolContent>
|
||||||
</Tool>
|
</Tool>
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
import { useMemo, useState } from 'react'
|
import { useMemo, useState } from 'react'
|
||||||
import {
|
import {
|
||||||
|
AlertCircle,
|
||||||
CheckCircle2,
|
CheckCircle2,
|
||||||
Circle,
|
Circle,
|
||||||
CircleDot,
|
CircleDot,
|
||||||
|
|
@ -17,7 +18,7 @@ import {
|
||||||
import type { CodeRunEvent, PermissionAsk, PermissionDecision } from '@x/shared/src/code-mode.js'
|
import type { CodeRunEvent, PermissionAsk, PermissionDecision } from '@x/shared/src/code-mode.js'
|
||||||
import { cn } from '@/lib/utils'
|
import { cn } from '@/lib/utils'
|
||||||
import { Tool, ToolContent, ToolHeader } from '@/components/ai-elements/tool'
|
import { Tool, ToolContent, ToolHeader } from '@/components/ai-elements/tool'
|
||||||
import { toToolState, type ToolCall } from '@/lib/chat-conversation'
|
import { getToolErrorText, toToolState, type ToolCall } from '@/lib/chat-conversation'
|
||||||
|
|
||||||
// ── Timeline reduction ──────────────────────────────────────────────
|
// ── Timeline reduction ──────────────────────────────────────────────
|
||||||
// The raw ACP stream is a flat list of events; collapse it into ordered rows,
|
// The raw ACP stream is a flat list of events; collapse it into ordered rows,
|
||||||
|
|
@ -111,14 +112,26 @@ const basename = (p: string) => p.split(/[\\/]/).pop() || p
|
||||||
|
|
||||||
export function CodingRunTimeline({
|
export function CodingRunTimeline({
|
||||||
events,
|
events,
|
||||||
|
error,
|
||||||
onOpenDiff,
|
onOpenDiff,
|
||||||
}: {
|
}: {
|
||||||
events: CodeRunEvent[]
|
events: CodeRunEvent[]
|
||||||
|
error?: string
|
||||||
// When set, changed-file names become clickable (the Code section opens the diff).
|
// When set, changed-file names become clickable (the Code section opens the diff).
|
||||||
onOpenDiff?: (path: string) => void
|
onOpenDiff?: (path: string) => void
|
||||||
}) {
|
}) {
|
||||||
const rows = useMemo(() => reduceEvents(events), [events])
|
const rows = useMemo(() => reduceEvents(events), [events])
|
||||||
if (rows.length === 0) {
|
if (rows.length === 0) {
|
||||||
|
if (error) {
|
||||||
|
return (
|
||||||
|
<div className="px-4 py-3">
|
||||||
|
<div className="flex items-start gap-2 rounded-md border border-destructive/30 bg-destructive/5 px-3 py-2 text-sm text-destructive">
|
||||||
|
<AlertCircle className="mt-0.5 size-3.5 shrink-0" />
|
||||||
|
<span className="min-w-0 whitespace-pre-wrap break-words">{error}</span>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
return <div className="px-4 py-3 text-xs text-muted-foreground">Starting the agent…</div>
|
return <div className="px-4 py-3 text-xs text-muted-foreground">Starting the agent…</div>
|
||||||
}
|
}
|
||||||
return (
|
return (
|
||||||
|
|
@ -133,12 +146,17 @@ export function CodingRunTimeline({
|
||||||
}
|
}
|
||||||
if (row.kind === 'tool') {
|
if (row.kind === 'tool') {
|
||||||
const running = row.status !== 'completed' && row.status !== 'failed'
|
const running = row.status !== 'completed' && row.status !== 'failed'
|
||||||
|
const failed = row.status === 'failed'
|
||||||
return (
|
return (
|
||||||
<div key={row.id} className="flex flex-col gap-1">
|
<div key={row.id} className="flex flex-col gap-1">
|
||||||
<div className="flex items-center gap-2 text-sm">
|
<div className="flex items-center gap-2 text-sm">
|
||||||
{running
|
{running ? (
|
||||||
? <Loader className="size-3.5 shrink-0 animate-spin text-muted-foreground" />
|
<Loader className="size-3.5 shrink-0 animate-spin text-muted-foreground" />
|
||||||
: <CheckCircle2 className="size-3.5 shrink-0 text-green-600" />}
|
) : failed ? (
|
||||||
|
<AlertCircle className="size-3.5 shrink-0 text-destructive" />
|
||||||
|
) : (
|
||||||
|
<CheckCircle2 className="size-3.5 shrink-0 text-green-600" />
|
||||||
|
)}
|
||||||
{toolKindIcon(row.toolKind, row.title)}
|
{toolKindIcon(row.toolKind, row.title)}
|
||||||
<span className="truncate text-foreground/90">{row.title ?? row.toolKind ?? 'Tool call'}</span>
|
<span className="truncate text-foreground/90">{row.title ?? row.toolKind ?? 'Tool call'}</span>
|
||||||
</div>
|
</div>
|
||||||
|
|
@ -189,6 +207,12 @@ export function CodingRunTimeline({
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
})}
|
})}
|
||||||
|
{error && (
|
||||||
|
<div className="flex items-start gap-2 rounded-md border border-destructive/30 bg-destructive/5 px-3 py-2 text-sm text-destructive">
|
||||||
|
<AlertCircle className="mt-0.5 size-3.5 shrink-0" />
|
||||||
|
<span className="min-w-0 whitespace-pre-wrap break-words">{error}</span>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
@ -258,12 +282,13 @@ export function CodingRunBlock({
|
||||||
(item.result as { agent?: string } | undefined)?.agent ??
|
(item.result as { agent?: string } | undefined)?.agent ??
|
||||||
(item.input as { agent?: string } | undefined)?.agent
|
(item.input as { agent?: string } | undefined)?.agent
|
||||||
const title = AGENT_LABEL[agent ?? ''] ?? 'Coding agent'
|
const title = AGENT_LABEL[agent ?? ''] ?? 'Coding agent'
|
||||||
|
const error = getToolErrorText(item)
|
||||||
return (
|
return (
|
||||||
<>
|
<>
|
||||||
<Tool open={open} onOpenChange={onOpenChange}>
|
<Tool open={open} onOpenChange={onOpenChange}>
|
||||||
<ToolHeader title={title} type="tool-code_agent_run" state={toToolState(item.status)} />
|
<ToolHeader title={title} type="tool-code_agent_run" state={toToolState(item.status)} />
|
||||||
<ToolContent>
|
<ToolContent>
|
||||||
<CodingRunTimeline events={item.codeRunEvents ?? []} />
|
<CodingRunTimeline events={item.codeRunEvents ?? []} error={error} />
|
||||||
</ToolContent>
|
</ToolContent>
|
||||||
</Tool>
|
</Tool>
|
||||||
{item.pendingCodePermission && (
|
{item.pendingCodePermission && (
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ import {
|
||||||
isErrorMessage,
|
isErrorMessage,
|
||||||
isToolCall,
|
isToolCall,
|
||||||
getToolDisplayName,
|
getToolDisplayName,
|
||||||
|
getToolErrorText,
|
||||||
toToolState,
|
toToolState,
|
||||||
normalizeToolOutput,
|
normalizeToolOutput,
|
||||||
} from '@/lib/chat-conversation'
|
} from '@/lib/chat-conversation'
|
||||||
|
|
@ -62,7 +63,7 @@ function CompactToolRow({ tool }: { tool: ToolCall }) {
|
||||||
const [open, setOpen] = useState(false)
|
const [open, setOpen] = useState(false)
|
||||||
const title = getToolDisplayName(tool)
|
const title = getToolDisplayName(tool)
|
||||||
const state = toToolState(tool.status)
|
const state = toToolState(tool.status)
|
||||||
const errorText = tool.status === 'error' && typeof tool.result === 'string' ? tool.result : undefined
|
const errorText = getToolErrorText(tool)
|
||||||
return (
|
return (
|
||||||
<Tool open={open} onOpenChange={setOpen} className="mb-0 text-xs">
|
<Tool open={open} onOpenChange={setOpen} className="mb-0 text-xs">
|
||||||
<ToolHeader title={title} type={`tool-${tool.name}` as `tool-${string}`} state={state} />
|
<ToolHeader title={title} type={`tool-${tool.name}` as `tool-${string}`} state={state} />
|
||||||
|
|
|
||||||
|
|
@ -121,6 +121,19 @@ export const normalizeToolOutput = (
|
||||||
return output
|
return output
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export const getToolErrorText = (tool: ToolCall): string | undefined => {
|
||||||
|
if (tool.status !== 'error') return undefined
|
||||||
|
if (typeof tool.result === 'string' && tool.result.trim()) return tool.result
|
||||||
|
if (tool.result !== undefined) {
|
||||||
|
try {
|
||||||
|
return JSON.stringify(tool.result, null, 2)
|
||||||
|
} catch {
|
||||||
|
// Fall through to the generic label for non-serializable legacy values.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 'Tool error'
|
||||||
|
}
|
||||||
|
|
||||||
export type WebSearchCardResult = { title: string; url: string; description: string }
|
export type WebSearchCardResult = { title: string; url: string; description: string }
|
||||||
|
|
||||||
export type WebSearchCardData = {
|
export type WebSearchCardData = {
|
||||||
|
|
|
||||||
|
|
@ -190,6 +190,35 @@ describe('buildTurnConversation', () => {
|
||||||
expect(items.map((i) => i.status)).toEqual(['pending', 'running'])
|
expect(items.map((i) => i.status)).toEqual(['pending', 'running'])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('uses the tool result envelope to determine error status', () => {
|
||||||
|
const state = reduceTurn([
|
||||||
|
created(T1, S1),
|
||||||
|
requested(T1, 0),
|
||||||
|
completed(
|
||||||
|
T1,
|
||||||
|
0,
|
||||||
|
assistantCalls(
|
||||||
|
toolCallPart('flagged', 'echo'),
|
||||||
|
toolCallPart('normal', 'echo'),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
invocation(T1, 'flagged', 'echo'),
|
||||||
|
{
|
||||||
|
type: 'tool_result',
|
||||||
|
turnId: T1,
|
||||||
|
ts: TS,
|
||||||
|
toolCallId: 'flagged',
|
||||||
|
toolName: 'echo',
|
||||||
|
source: 'sync',
|
||||||
|
result: { output: 'boom', isError: true },
|
||||||
|
},
|
||||||
|
invocation(T1, 'normal', 'echo'),
|
||||||
|
toolResult(T1, 'normal', 'echo', { success: false, message: 'payload data' }),
|
||||||
|
])
|
||||||
|
const items = buildTurnConversation(state).filter(isToolCall)
|
||||||
|
expect(items.map((i) => i.status)).toEqual(['error', 'completed'])
|
||||||
|
})
|
||||||
|
|
||||||
it('renders user attachments and a failed turn as an error item', () => {
|
it('renders user attachments and a failed turn as an error item', () => {
|
||||||
const input = {
|
const input = {
|
||||||
role: 'user' as const,
|
role: 'user' as const,
|
||||||
|
|
|
||||||
|
|
@ -200,7 +200,7 @@ function extractAttachments(content: UserContent): MessageAttachment[] | undefin
|
||||||
}
|
}
|
||||||
|
|
||||||
function toolStatus(tc: ToolCallState): ToolCall['status'] {
|
function toolStatus(tc: ToolCallState): ToolCall['status'] {
|
||||||
if (tc.result) return 'completed'
|
if (tc.result) return tc.result.result.isError ? 'error' : 'completed'
|
||||||
if (tc.permission && !tc.permission.resolved) return 'pending'
|
if (tc.permission && !tc.permission.resolved) return 'pending'
|
||||||
return 'running'
|
return 'running'
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -79,7 +79,7 @@ After \`code_agent_run\` returns:
|
||||||
- Pass through the agent's \`summary\` as-is. Do not rewrite it.
|
- Pass through the agent's \`summary\` as-is. Do not rewrite it.
|
||||||
- Refer to file paths as plain text. Do NOT use \`\`\`file:path\`\`\` reference blocks. (This overrides the global "always wrap paths in filepath blocks" rule — for code-mode output, plain text.)
|
- Refer to file paths as plain text. Do NOT use \`\`\`file:path\`\`\` reference blocks. (This overrides the global "always wrap paths in filepath blocks" rule — for code-mode output, plain text.)
|
||||||
- Only add your own explanation if it failed:
|
- Only add your own explanation if it failed:
|
||||||
- \`success: false\` with a message — surface the message. If it mentions the agent isn't installed or signed in, tell the user to install or sign in via **Settings → Code Mode**.
|
- A tool error with a message — surface the message. If it mentions the agent isn't installed or signed in, tell the user to install or sign in via **Settings → Code Mode**.
|
||||||
- \`stopReason: "cancelled"\` — the run was stopped; acknowledge briefly and ask if they want to continue.
|
- \`stopReason: "cancelled"\` — the run was stopped; acknowledge briefly and ask if they want to continue.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,54 @@
|
||||||
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import container from "../../di/container.js";
|
||||||
|
import { InMemoryAbortRegistry } from "../../runs/abort-registry.js";
|
||||||
|
import { BuiltinTools } from "./builtin-tools.js";
|
||||||
|
import type { ToolContext } from "./exec-tool.js";
|
||||||
|
|
||||||
|
function context(signal: AbortSignal): ToolContext {
|
||||||
|
return {
|
||||||
|
runId: "turn-1",
|
||||||
|
toolCallId: "tool-1",
|
||||||
|
signal,
|
||||||
|
abortRegistry: new InMemoryAbortRegistry(),
|
||||||
|
publish: async () => {},
|
||||||
|
codePolicy: "ask",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function mockCodeServices(runPrompt: () => Promise<never>): void {
|
||||||
|
vi.spyOn(container, "resolve").mockImplementation(((name: string) => {
|
||||||
|
if (name === "codeModeManager") return { runPrompt };
|
||||||
|
if (name === "codePermissionRegistry") {
|
||||||
|
return { cancelRun: vi.fn(), request: vi.fn() };
|
||||||
|
}
|
||||||
|
throw new Error(`Unexpected dependency: ${name}`);
|
||||||
|
}) as typeof container.resolve);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("code_agent_run", () => {
|
||||||
|
afterEach(() => vi.restoreAllMocks());
|
||||||
|
|
||||||
|
it("throws genuine coding-agent failures for the runtime to mark as errors", async () => {
|
||||||
|
mockCodeServices(async () => {
|
||||||
|
throw new Error("spawn Electron ENOENT");
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(BuiltinTools.code_agent_run.execute(
|
||||||
|
{ agent: "codex", cwd: "/repo", prompt: "Fix it" },
|
||||||
|
context(new AbortController().signal),
|
||||||
|
)).rejects.toThrow("Coding agent failed: spawn Electron ENOENT");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns an ordinary cancellation result when the turn was aborted", async () => {
|
||||||
|
const controller = new AbortController();
|
||||||
|
controller.abort();
|
||||||
|
mockCodeServices(async () => {
|
||||||
|
throw new Error("cancelled");
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(BuiltinTools.code_agent_run.execute(
|
||||||
|
{ agent: "codex", cwd: "/repo", prompt: "Fix it" },
|
||||||
|
context(controller.signal),
|
||||||
|
)).resolves.toMatchObject({ success: false, stopReason: "cancelled" });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -868,7 +868,7 @@ export const BuiltinTools: z.infer<typeof BuiltinToolsSchema> = {
|
||||||
}),
|
}),
|
||||||
execute: async ({ agent, cwd, prompt }: { agent: 'claude' | 'codex', cwd: string, prompt: string }, ctx?: ToolContext) => {
|
execute: async ({ agent, cwd, prompt }: { agent: 'claude' | 'codex', cwd: string, prompt: string }, ctx?: ToolContext) => {
|
||||||
if (!ctx) {
|
if (!ctx) {
|
||||||
return { success: false, message: 'code_agent_run requires run context (runId / streaming).' };
|
throw new Error('code_agent_run requires run context (runId / streaming).');
|
||||||
}
|
}
|
||||||
// The composer chip is the source of truth for the agent. The model's `agent`
|
// The composer chip is the source of truth for the agent. The model's `agent`
|
||||||
// argument is only a fallback for the ask-human flow (code mode not active, no
|
// argument is only a fallback for the ask-human flow (code mode not active, no
|
||||||
|
|
@ -955,10 +955,7 @@ export const BuiltinTools: z.infer<typeof BuiltinToolsSchema> = {
|
||||||
changedFiles: [...changedFiles],
|
changedFiles: [...changedFiles],
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
return {
|
throw new Error(`Coding agent failed: ${error instanceof Error ? error.message : String(error)}`);
|
||||||
success: false,
|
|
||||||
message: `Coding agent failed: ${error instanceof Error ? error.message : String(error)}`,
|
|
||||||
};
|
|
||||||
} finally {
|
} finally {
|
||||||
ctx.signal.removeEventListener('abort', onAbort);
|
ctx.signal.removeEventListener('abort', onAbort);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue