From 21192e2056d5731fffef374d626b1a20d89c0b1d Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 5 Jan 2026 04:44:43 +0100 Subject: [PATCH] refactor: consolidated tool execution params into details object - Changed isError property from required to optional across ToolResultMessage and tool event interfaces. - Added SpinnerType variants with distinct animation frames per symbol preset. - Fixed spinner animation crash when frames array is empty by adding guard clause. - Refactored agent-loop tool execution to consolidate toolCallId, toolName, and isError into details object. --- packages/agent/src/agent-loop.ts | 15 ++++--- packages/agent/src/types.ts | 2 +- packages/agent/test/e2e.test.ts | 1 - packages/ai/CHANGELOG.md | 3 ++ packages/ai/README.md | 3 -- packages/ai/src/types.ts | 2 +- packages/ai/test/handoff.test.ts | 4 -- packages/ai/test/image-tool-result.test.ts | 2 - packages/ai/test/stream.test.ts | 1 - packages/ai/test/unicode-surrogate.test.ts | 3 -- packages/coding-agent/CHANGELOG.md | 4 ++ .../src/core/hooks/tool-wrapper.ts | 1 - packages/coding-agent/src/core/hooks/types.ts | 2 +- .../interactive/components/tool-execution.ts | 8 ++-- .../src/modes/interactive/theme/theme.ts | 40 +++++++++++++------ packages/web-ui/src/utils/test-sessions.ts | 33 --------------- 16 files changed, 49 insertions(+), 75 deletions(-) diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index 7153ee86a..e89ddb143 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -322,8 +322,11 @@ async function executeToolCalls( }); let result: AgentToolResult; - let isError = false; + const details: { toolCallId: string; toolName: string; isError?: boolean } = { + toolCallId: toolCall.id, + toolName: toolCall.name, + }; try { if (!tool) throw new Error(`Tool ${toolCall.name} not found`); @@ -350,25 +353,21 @@ async function executeToolCalls( content: [{ type: "text", text: e instanceof Error ? e.message : String(e) }], details: {}, }; - isError = true; + details.isError = true; } stream.push({ type: "tool_execution_end", - toolCallId: toolCall.id, - toolName: toolCall.name, result, - isError, + ...details, }); const toolResultMessage: ToolResultMessage = { role: "toolResult", - toolCallId: toolCall.id, - toolName: toolCall.name, content: result.content, details: result.details, - isError, timestamp: Date.now(), + ...details, }; results.push(toolResultMessage); diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index c24d4dd41..953b8df39 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -217,4 +217,4 @@ export type AgentEvent = // Tool execution lifecycle | { type: "tool_execution_start"; toolCallId: string; toolName: string; args: any } | { type: "tool_execution_update"; toolCallId: string; toolName: string; args: any; partialResult: any } - | { type: "tool_execution_end"; toolCallId: string; toolName: string; result: any; isError: boolean }; + | { type: "tool_execution_end"; toolCallId: string; toolName: string; result: any; isError?: boolean }; diff --git a/packages/agent/test/e2e.test.ts b/packages/agent/test/e2e.test.ts index 80ad67cbc..8ad424568 100644 --- a/packages/agent/test/e2e.test.ts +++ b/packages/agent/test/e2e.test.ts @@ -455,7 +455,6 @@ describe("Agent.continue()", () => { toolCallId: "calc-1", toolName: "calculate", content: [{ type: "text", text: "5 + 3 = 8" }], - isError: false, timestamp: Date.now(), }; diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index f423ebc31..33c883347 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Changed + +- Made `isError` field optional in `ToolResultMessage` interface, defaulting to non-error state ## [3.14.0] - 2026-01-04 diff --git a/packages/ai/README.md b/packages/ai/README.md index 1e9fb6dae..b4b0f7ca4 100644 --- a/packages/ai/README.md +++ b/packages/ai/README.md @@ -121,7 +121,6 @@ for (const call of toolCalls) { toolCallId: call.id, toolName: call.name, content: [{ type: "text", text: result }], - isError: false, timestamp: Date.now(), }); } @@ -209,7 +208,6 @@ for (const block of response.content) { toolCallId: block.id, toolName: block.name, content: [{ type: "text", text: JSON.stringify(result) }], - isError: false, timestamp: Date.now(), }); } @@ -225,7 +223,6 @@ context.messages.push({ { type: "text", text: "Generated chart showing temperature trends" }, { type: "image", data: imageBuffer.toString("base64"), mimeType: "image/png" }, ], - isError: false, timestamp: Date.now(), }); ``` diff --git a/packages/ai/src/types.ts b/packages/ai/src/types.ts index cb1851904..fd41c2781 100644 --- a/packages/ai/src/types.ts +++ b/packages/ai/src/types.ts @@ -138,7 +138,7 @@ export interface ToolResultMessage { toolName: string; content: (TextContent | ImageContent)[]; // Supports text and images details?: TDetails; - isError: boolean; + isError?: boolean; timestamp: number; // Unix timestamp in milliseconds } diff --git a/packages/ai/test/handoff.test.ts b/packages/ai/test/handoff.test.ts index 0414434e5..e895be801 100644 --- a/packages/ai/test/handoff.test.ts +++ b/packages/ai/test/handoff.test.ts @@ -57,7 +57,6 @@ const providerContexts = { toolCallId: "toolu_01abc123", toolName: "get_weather", content: [{ type: "text", text: "Weather in Tokyo: 18°C, partly cloudy" }], - isError: false, timestamp: Date.now(), } satisfies ToolResultMessage, facts: { @@ -109,7 +108,6 @@ const providerContexts = { toolCallId: "call_gemini_123", toolName: "get_weather", content: [{ type: "text", text: "Weather in Berlin: 22°C, sunny" }], - isError: false, timestamp: Date.now(), } satisfies ToolResultMessage, facts: { @@ -160,7 +158,6 @@ const providerContexts = { toolCallId: "call_abc123", toolName: "get_weather", content: [{ type: "text", text: "Weather in London: 15°C, rainy" }], - isError: false, timestamp: Date.now(), } satisfies ToolResultMessage, facts: { @@ -213,7 +210,6 @@ const providerContexts = { toolCallId: "call_789_item_012", // Match the updated ID format toolName: "get_weather", content: [{ type: "text", text: "Weather in Sydney: 25°C, clear" }], - isError: false, timestamp: Date.now(), } satisfies ToolResultMessage, facts: { diff --git a/packages/ai/test/image-tool-result.test.ts b/packages/ai/test/image-tool-result.test.ts index 894f51149..6279cf814 100644 --- a/packages/ai/test/image-tool-result.test.ts +++ b/packages/ai/test/image-tool-result.test.ts @@ -81,7 +81,6 @@ async function handleToolWithImageResult(model: Model, o mimeType: "image/png", }, ], - isError: false, timestamp: Date.now(), }; @@ -174,7 +173,6 @@ async function handleToolWithTextAndImageResult(model: Model(model: Model, options?: Options toolCallId: block.id, toolName: block.name, content: [{ type: "text", text: `${result}` }], - isError: false, timestamp: Date.now(), }); } diff --git a/packages/ai/test/unicode-surrogate.test.ts b/packages/ai/test/unicode-surrogate.test.ts index a482e3dff..8486927e4 100644 --- a/packages/ai/test/unicode-surrogate.test.ts +++ b/packages/ai/test/unicode-surrogate.test.ts @@ -93,7 +93,6 @@ async function testEmojiInToolResults(llm: Model, option - Special quotes: "curly" 'quotes'`, }, ], - isError: false, timestamp: Date.now(), }; @@ -182,7 +181,6 @@ Unanswered Comments: 2 }`, }, ], - isError: false, timestamp: Date.now(), }; @@ -254,7 +252,6 @@ async function testUnpairedHighSurrogate(llm: Model, opt toolCallId: "test_2", toolName: "test_tool", content: [{ type: "text", text: `Text with unpaired surrogate: ${unpairedSurrogate} <- should be sanitized` }], - isError: false, timestamp: Date.now(), }; diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 16ed576b5..3f7ab55ba 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,10 @@ # Changelog ## [Unreleased] + ### Added +- Added spinner type variants (status and activity) with distinct animation frames per symbol preset - Added animated spinner for task tool progress display during subagent execution - Added language/file type icons for read tool output with support for 35+ file types - Added async cleanup registry for graceful session flush on SIGINT, SIGTERM, and SIGHUP signals @@ -27,6 +29,7 @@ ### Changed +- Changed `isError` property in tool result events to be optional instead of required - Changed `SessionManager.open()` and `SessionManager.continueRecent()` to async methods for proper initialization - Changed session file writes to use atomic rename pattern with fsync for crash-safe persistence - Changed read tool display to show file type icons and metadata inline with path @@ -68,6 +71,7 @@ ### Fixed +- Fixed spinner animation crash when spinner frames array is empty by adding length check - Fixed session persistence to properly await all queued writes before closing or switching sessions - Fixed session persistence to truncate oversized content blocks before writing to prevent memory exhaustion - Fixed extension list and inspector panel to use correct symbols for disabled and shadowed states instead of reusing unrelated status icons diff --git a/packages/coding-agent/src/core/hooks/tool-wrapper.ts b/packages/coding-agent/src/core/hooks/tool-wrapper.ts index d2afa1c69..37a19defd 100644 --- a/packages/coding-agent/src/core/hooks/tool-wrapper.ts +++ b/packages/coding-agent/src/core/hooks/tool-wrapper.ts @@ -59,7 +59,6 @@ export function wrapToolWithHooks(tool: AgentTool, hookRunner: HookRu input: params, content: result.content, details: result.details, - isError: false, })) as ToolResultEventResult | undefined; // Apply modifications if any diff --git a/packages/coding-agent/src/core/hooks/types.ts b/packages/coding-agent/src/core/hooks/types.ts index 5cff754a1..b5992ee54 100644 --- a/packages/coding-agent/src/core/hooks/types.ts +++ b/packages/coding-agent/src/core/hooks/types.ts @@ -410,7 +410,7 @@ interface ToolResultEventBase { /** Full content array (text and images) */ content: (TextContent | ImageContent)[]; /** Whether the tool execution was an error */ - isError: boolean; + isError?: boolean; } /** Tool result event for bash tool */ diff --git a/packages/coding-agent/src/modes/interactive/components/tool-execution.ts b/packages/coding-agent/src/modes/interactive/components/tool-execution.ts index bfa598d6a..436ba4ed7 100644 --- a/packages/coding-agent/src/modes/interactive/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/interactive/components/tool-execution.ts @@ -346,7 +346,7 @@ export class ToolExecutionComponent extends Container { private cwd: string; private result?: { content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>; - isError: boolean; + isError?: boolean; details?: any; }; // Cached edit diff preview (computed when args arrive, before tool executes) @@ -439,7 +439,7 @@ export class ToolExecutionComponent extends Container { result: { content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>; details?: any; - isError: boolean; + isError?: boolean; }, isPartial = false, ): void { @@ -456,7 +456,9 @@ export class ToolExecutionComponent extends Container { const needsSpinner = this.isPartial && this.toolName === "task"; if (needsSpinner && !this.spinnerInterval) { this.spinnerInterval = setInterval(() => { - this.spinnerFrame = (this.spinnerFrame + 1) % 10; + const frameCount = theme.spinnerFrames.length; + if (frameCount === 0) return; + this.spinnerFrame = (this.spinnerFrame + 1) % frameCount; this.updateDisplay(); this.ui.requestRender(); }, 80); diff --git a/packages/coding-agent/src/modes/interactive/theme/theme.ts b/packages/coding-agent/src/modes/interactive/theme/theme.ts index 72deabe59..ae68c1926 100644 --- a/packages/coding-agent/src/modes/interactive/theme/theme.ts +++ b/packages/coding-agent/src/modes/interactive/theme/theme.ts @@ -127,8 +127,6 @@ export type SymbolKey = | "md.quoteBorder" | "md.hrChar" | "md.bullet" - // Spinner frames (array stored as comma-separated string) - | "spinner.frames" // Language/file type icons | "lang.default" | "lang.typescript" @@ -369,8 +367,6 @@ const UNICODE_SYMBOLS: SymbolMap = { "md.hrChar": "─", // pick: • | alt: · ▪ ◦ "md.bullet": "•", - // Spinner frames (pulsing dot) - "spinner.frames": "·,•,●,•", // Language icons (unicode uses code symbol prefix) "lang.default": "❖", "lang.typescript": "❖ ts", @@ -610,8 +606,6 @@ const NERD_SYMBOLS: SymbolMap = { "md.hrChar": "\u2500", // pick:  | alt:  • "md.bullet": "\uf111", - // Spinner frames (nerd font circles) - "spinner.frames": "󰪥,󰪤,󰪣,󰪢,󰪡,󰪠,󰪟,󰪞,󰪝,󰪜,󰪛,󰪥", // Language icons (nerd font devicons) "lang.default": "", "lang.typescript": "", @@ -757,8 +751,6 @@ const ASCII_SYMBOLS: SymbolMap = { "md.quoteBorder": "|", "md.hrChar": "-", "md.bullet": "*", - // Spinner frames (ASCII) - "spinner.frames": "|,/,-,\\", // Language icons (ASCII uses abbreviations) "lang.default": "code", "lang.typescript": "ts", @@ -804,6 +796,23 @@ const SYMBOL_PRESETS: Record = { ascii: ASCII_SYMBOLS, }; +export type SpinnerType = "status" | "activity"; + +const SPINNER_FRAMES: Record> = { + unicode: { + status: ["·", "•", "●", "•"], + activity: ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"], + }, + nerd: { + status: ["󰪥", "󰪤", "󰪣", "󰪢", "󰪡", "󰪠", "󰪟", "󰪞", "󰪥"], + activity: ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"], + }, + ascii: { + status: ["|", "/", "-", "\\"], + activity: ["-", "\\", "|", "/"], + }, +}; + // ============================================================================ // Types & Schema // ============================================================================ @@ -1537,10 +1546,17 @@ export class Theme { } /** - * Get spinner frames as an array (stored as comma-separated string in symbols). + * Default spinner frames (status spinner). */ get spinnerFrames(): string[] { - return this.symbols["spinner.frames"].split(","); + return this.getSpinnerFrames(); + } + + /** + * Get spinner frames by type. + */ + getSpinnerFrames(type: SpinnerType = "status"): string[] { + return SPINNER_FRAMES[this.symbolPreset][type]; } /** @@ -2117,8 +2133,6 @@ export function getLanguageFromPath(filePath: string): string | undefined { export function getSymbolTheme(): SymbolTheme { const preset = theme.getSymbolPreset(); - const spinnerFrames = - preset === "ascii" ? ["-", "\\", "|", "/"] : ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; return { cursor: theme.nav.cursor, @@ -2129,7 +2143,7 @@ export function getSymbolTheme(): SymbolTheme { table: theme.boxSharp, quoteBorder: theme.md.quoteBorder, hrChar: theme.md.hrChar, - spinnerFrames, + spinnerFrames: theme.getSpinnerFrames("activity"), }; } diff --git a/packages/web-ui/src/utils/test-sessions.ts b/packages/web-ui/src/utils/test-sessions.ts index 5d54c0933..03b7470b5 100644 --- a/packages/web-ui/src/utils/test-sessions.ts +++ b/packages/web-ui/src/utils/test-sessions.ts @@ -73,7 +73,6 @@ export const simpleHtml = { toolCallId: "toolu_01Tu6wbnPMHtBKj9B7TMos1x", toolName: "artifacts", output: "Created file index.html", - isError: false, }, { role: "assistant", @@ -180,7 +179,6 @@ export const longSession = { toolCallId: "toolu_01Y3hvzepDjUWnHF8bdmgMSA", toolName: "artifacts", output: "Created file index.html", - isError: false, }, { role: "assistant", @@ -264,7 +262,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -344,7 +341,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -428,7 +424,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -512,7 +507,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -558,7 +552,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -676,7 +669,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -723,7 +715,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -806,7 +797,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -889,7 +879,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1002,7 +991,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1085,7 +1073,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1161,7 +1148,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1208,7 +1194,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1292,7 +1277,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1339,7 +1323,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1419,7 +1402,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1536,7 +1518,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1583,7 +1564,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1625,7 +1605,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1667,7 +1646,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1714,7 +1692,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1761,7 +1738,6 @@ export const longSession = { toolCallId: "toolu_01BtH9H2BvwxvKjLw5iHXcZC", toolName: "artifacts", output: "Created file news_today.md", - isError: false, }, { role: "assistant", @@ -1845,7 +1821,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1892,7 +1867,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1939,7 +1913,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -1986,7 +1959,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -2033,7 +2005,6 @@ export const longSession = { details: { files: [], }, - isError: false, }, { role: "assistant", @@ -2080,7 +2051,6 @@ export const longSession = { toolCallId: "toolu_01J26GUEiATmMTYeMDJnmAFN", toolName: "artifacts", output: "Created file liveblog_ukraine.md", - isError: false, }, { role: "assistant", @@ -2160,7 +2130,6 @@ export const longSession = { toolCallId: "toolu_012E1DjRwg1wgZhD38mBNpSv", toolName: "artifacts", output: "Created file minimal.html", - isError: false, }, { role: "assistant", @@ -2241,7 +2210,6 @@ export const longSession = { toolName: "artifacts", output: "Updated file index.html\n\nExecution timed out. Partial logs:\n[log] Page loaded successfully!\n[log] Welcome to the simple HTML page", - isError: false, }, { role: "assistant", @@ -2323,7 +2291,6 @@ export const longSession = { toolName: "artifacts", output: "Updated file index.html\n\nExecution timed out. Partial logs:\n[log] Page loaded successfully!\n[log] Welcome to the simple HTML page\n[log] Third console log added!", - isError: false, }, { role: "assistant",