From d627b8c5e95be31ac03923ef67d91748ec8564c5 Mon Sep 17 00:00:00 2001 From: zhangyc Date: Mon, 13 Apr 2026 23:03:01 +0800 Subject: [PATCH] fix: guard tool info rendering when tool_use input is undefined (#536) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Guard `toolInfoFromToolUse()` against `undefined` input for Read, Glob, Grep, WebSearch, and TodoWrite cases - Make `planEntries()` resilient to `undefined` input (returns `[]` instead of crashing) - Guard TodoWrite plan updates in `streamEventToAcpNotifications()` with optional chaining on `chunk.input?.todos` ## Why this change Claude `tool_use` blocks can temporarily carry `input: undefined` during streaming (before `input_json_delta` arrives), and historical `tool_use` messages with incomplete input may be replayed when a JetBrains IDEA session is resumed. Without these guards, ACP crashes with: ``` TypeError: Cannot read properties of undefined (reading 'limit') TypeError: Cannot read properties of undefined (reading '-i') TypeError: Cannot read properties of undefined (reading 'todos') ``` All three crashes share the same root cause: the ACP bridge assumes `tool_use.input` is always defined, but it can be `undefined` during streaming early chunks or session history replay. ## Test plan - [x] Added regression tests for `toolInfoFromToolUse()` with `input: undefined` (Read, Grep, Glob, WebSearch, TodoWrite) - [x] Added regression test for `planEntries(undefined)` — returns `[]` - [x] Added regression test for `planEntries({})` — graceful handling - [x] Added regression test for `toAcpNotifications()` with TodoWrite + undefined input — no crash, no spurious plan update - [x] Added regression test verifying TodoWrite + valid input still emits plan update - [x] `npm run build` passes with no TypeScript errors - [x] All 167 tests pass (10 new) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Ben Brandt --- src/acp-agent.ts | 4 +- src/tests/tools.test.ts | 116 +++++++++++++++++++++++++++++++++++++++- src/tools.ts | 72 ++++++++++++------------- 3 files changed, 153 insertions(+), 39 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index ed0c404f..f8832e87 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -2038,8 +2038,8 @@ export function toAcpNotifications( const alreadyCached = chunk.id in toolUseCache; toolUseCache[chunk.id] = chunk; if (chunk.name === "TodoWrite") { - // @ts-expect-error - sometimes input is empty object - if (Array.isArray(chunk.input.todos)) { + // @ts-expect-error - sometimes input is empty object or undefined + if (Array.isArray(chunk.input?.todos)) { update = { sessionUpdate: "plan", entries: planEntries(chunk.input as { todos: ClaudePlanEntry[] }), diff --git a/src/tests/tools.test.ts b/src/tests/tools.test.ts index a2a62180..96c7262e 100644 --- a/src/tests/tools.test.ts +++ b/src/tests/tools.test.ts @@ -11,7 +11,12 @@ import { BetaBashCodeExecutionToolResultBlockParam, } from "@anthropic-ai/sdk/resources/beta.mjs"; import { toAcpNotifications, ToolUseCache, Logger } from "../acp-agent.js"; -import { toolUpdateFromToolResult, createPostToolUseHook, toolInfoFromToolUse } from "../tools.js"; +import { + toolUpdateFromToolResult, + createPostToolUseHook, + toolInfoFromToolUse, + planEntries, +} from "../tools.js"; describe("rawOutput in tool call updates", () => { const mockClient = {} as AgentSideConnection; @@ -1271,3 +1276,112 @@ describe("toolInfoFromToolUse - ExitPlanMode", () => { expect(info.content).toEqual([]); }); }); + +describe("toolInfoFromToolUse - undefined input regression", () => { + it("Read with undefined input should not throw", () => { + const toolUse = { name: "Read", id: "toolu_read_undef", input: undefined }; + const info = toolInfoFromToolUse(toolUse, false); + expect(info.title).toBe("Read File"); + expect(info.locations).toEqual([]); + }); + + it("Grep with undefined input should not throw", () => { + const toolUse = { name: "Grep", id: "toolu_grep_undef", input: undefined }; + const info = toolInfoFromToolUse(toolUse, false); + expect(info.title).toBe("grep"); + }); + + it("Glob with undefined input should not throw", () => { + const toolUse = { name: "Glob", id: "toolu_glob_undef", input: undefined }; + const info = toolInfoFromToolUse(toolUse, false); + expect(info.title).toBe("Find"); + expect(info.locations).toEqual([]); + }); + + it("WebSearch with undefined input should not throw", () => { + const toolUse = { name: "WebSearch", id: "toolu_ws_undef", input: undefined }; + const info = toolInfoFromToolUse(toolUse, false); + expect(info.title).toBe("Web search"); + }); + + it("TodoWrite with undefined input should not throw", () => { + const toolUse = { name: "TodoWrite", id: "toolu_todo_undef", input: undefined }; + const info = toolInfoFromToolUse(toolUse, false); + expect(info.title).toBe("Update TODOs"); + }); +}); + +describe("planEntries - undefined input regression", () => { + it("should return empty array when input is undefined", () => { + expect(planEntries(undefined)).toEqual([]); + }); + + it("should return empty array when input has no todos", () => { + expect(planEntries({} as any)).toEqual([]); + }); + + it("should still map valid todos correctly", () => { + const result = planEntries({ + todos: [ + { content: "Task 1", status: "pending", activeForm: "" }, + { content: "Task 2", status: "completed", activeForm: "" }, + ], + }); + expect(result).toEqual([ + { content: "Task 1", status: "pending", priority: "medium" }, + { content: "Task 2", status: "completed", priority: "medium" }, + ]); + }); +}); + +describe("toAcpNotifications - TodoWrite with undefined input regression", () => { + const mockClient = {} as AgentSideConnection; + const mockLogger: Logger = { log: () => {}, error: () => {} }; + + it("should not throw when TodoWrite tool_use has undefined input", () => { + const toolUseCache: ToolUseCache = {}; + + const notifications = toAcpNotifications( + [ + { + type: "tool_use" as const, + id: "toolu_todo_undef", + name: "TodoWrite", + input: undefined as any, + }, + ], + "assistant", + "test-session", + toolUseCache, + mockClient, + mockLogger, + ); + + // TodoWrite with undefined input should not crash, and should not emit plan update + const planUpdates = notifications.filter((n) => (n.update as any).sessionUpdate === "plan"); + expect(planUpdates).toHaveLength(0); + }); + + it("should still emit plan update when TodoWrite has valid input", () => { + const toolUseCache: ToolUseCache = {}; + + const notifications = toAcpNotifications( + [ + { + type: "tool_use" as const, + id: "toolu_todo_valid", + name: "TodoWrite", + input: { todos: [{ content: "Do X", status: "pending" }] }, + }, + ], + "assistant", + "test-session", + toolUseCache, + mockClient, + mockLogger, + ); + + const planUpdates = notifications.filter((n) => (n.update as any).sessionUpdate === "plan"); + expect(planUpdates).toHaveLength(1); + }); +}); diff --git a/src/tools.ts b/src/tools.ts index 5ccce7a9..af9d6fa1 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -128,7 +128,7 @@ export function toolInfoFromToolUse( switch (name) { case "Agent": case "Task": { - const input = toolUse.input as AgentInput | BashInput; + const input = toolUse.input as AgentInput | BashInput | undefined; return { title: input?.description ? input.description : "Task", kind: "think", @@ -145,7 +145,7 @@ export function toolInfoFromToolUse( } case "Bash": { - const input = toolUse.input as BashInput; + const input = toolUse.input as BashInput | undefined; return { title: input?.command ? input.command : "Terminal", kind: "execute", @@ -163,18 +163,18 @@ export function toolInfoFromToolUse( } case "Read": { - const input = toolUse.input as FileReadInput; + const input = toolUse.input as FileReadInput | undefined; let limit = ""; - if (input.limit && input.limit > 0) { + if (input?.limit && input.limit > 0) { limit = " (" + (input.offset ?? 1) + " - " + ((input.offset ?? 1) + input.limit - 1) + ")"; - } else if (input.offset) { + } else if (input?.offset) { limit = " (from line " + input.offset + ")"; } - const displayPath = input.file_path ? toDisplayPath(input.file_path, cwd) : "File"; + const displayPath = input?.file_path ? toDisplayPath(input.file_path, cwd) : "File"; return { title: "Read " + displayPath + limit, kind: "read", - locations: input.file_path + locations: input?.file_path ? [ { path: input.file_path, @@ -187,7 +187,7 @@ export function toolInfoFromToolUse( } case "Write": { - const input = toolUse.input as FileWriteInput; + const input = toolUse.input as FileWriteInput | undefined; let content: ToolCallContent[] = []; if (input && input.file_path) { content = [ @@ -216,7 +216,7 @@ export function toolInfoFromToolUse( } case "Edit": { - const input = toolUse.input as FileEditInput; + const input = toolUse.input as FileEditInput | undefined; let content: ToolCallContent[] = []; if (input && input.file_path && (input.old_string || input.new_string)) { content = [ @@ -238,44 +238,44 @@ export function toolInfoFromToolUse( } case "Glob": { - const input = toolUse.input as GlobInput; + const input = toolUse.input as GlobInput | undefined; let label = "Find"; - if (input.path) { + if (input?.path) { label += ` \`${input.path}\``; } - if (input.pattern) { + if (input?.pattern) { label += ` \`${input.pattern}\``; } return { title: label, kind: "search", content: [], - locations: input.path ? [{ path: input.path }] : [], + locations: input?.path ? [{ path: input.path }] : [], }; } case "Grep": { - const input = toolUse.input as GrepInput; + const input = toolUse.input as GrepInput | undefined; let label = "grep"; - if (input["-i"]) { + if (input?.["-i"]) { label += " -i"; } - if (input["-n"]) { + if (input?.["-n"]) { label += " -n"; } - if (input["-A"] !== undefined) { + if (input?.["-A"] !== undefined) { label += ` -A ${input["-A"]}`; } - if (input["-B"] !== undefined) { + if (input?.["-B"] !== undefined) { label += ` -B ${input["-B"]}`; } - if (input["-C"] !== undefined) { + if (input?.["-C"] !== undefined) { label += ` -C ${input["-C"]}`; } - if (input.output_mode) { + if (input?.output_mode) { switch (input.output_mode) { case "files_with_matches": label += " -l"; @@ -289,27 +289,27 @@ export function toolInfoFromToolUse( } } - if (input.head_limit !== undefined) { + if (input?.head_limit !== undefined) { label += ` | head -${input.head_limit}`; } - if (input.glob) { + if (input?.glob) { label += ` --include="${input.glob}"`; } - if (input.type) { + if (input?.type) { label += ` --type=${input.type}`; } - if (input.multiline) { + if (input?.multiline) { label += " -P"; } - if (input.pattern) { + if (input?.pattern) { label += ` "${input.pattern}"`; } - if (input.path) { + if (input?.path) { label += ` ${input.path}`; } @@ -338,14 +338,14 @@ export function toolInfoFromToolUse( } case "WebSearch": { - const input = toolUse.input as WebSearchInput; - let label = `"${input.query}"`; + const input = toolUse.input as WebSearchInput | undefined; + let label = input?.query ? `"${input.query}"` : "Web search"; - if (input.allowed_domains && input.allowed_domains.length > 0) { + if (input?.allowed_domains && input.allowed_domains.length > 0) { label += ` (allowed: ${input.allowed_domains.join(", ")})`; } - if (input.blocked_domains && input.blocked_domains.length > 0) { + if (input?.blocked_domains && input.blocked_domains.length > 0) { label += ` (blocked: ${input.blocked_domains.join(", ")})`; } @@ -357,7 +357,7 @@ export function toolInfoFromToolUse( } case "TodoWrite": { - const input = toolUse.input as TodoWriteInput; + const input = toolUse.input as TodoWriteInput | undefined; return { title: Array.isArray(input?.todos) ? `Update TODOs: ${input.todos.map((todo: any) => todo.content).join(", ")}` @@ -368,7 +368,7 @@ export function toolInfoFromToolUse( } case "ExitPlanMode": { - const planInput = toolUse.input as { plan?: string }; + const planInput = toolUse.input as { plan?: string } | undefined; return { title: "Ready to code?", kind: "switch_mode", @@ -661,10 +661,10 @@ export type ClaudePlanEntry = { activeForm: string; }; -export function planEntries(input: { todos: ClaudePlanEntry[] }): PlanEntry[] { - return input.todos.map((input) => ({ - content: input.content, - status: input.status, +export function planEntries(input: { todos: ClaudePlanEntry[] } | undefined): PlanEntry[] { + return (input?.todos ?? []).map((todo) => ({ + content: todo.content, + status: todo.status, priority: "medium", })); }