jerry/claude-agent-acp
1
0
Fork 0
mirror of https://github.com/agentclientprotocol/claude-agent-acp.git synced 2026-10-27 04:30:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: guard tool info rendering when tool_use input is undefined (#536)

Browse Source 
## 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 <noreply@anthropic.com>
Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
This commit is contained in:
zhangyc
2026-04-13 23:03:01 +08:00
committed by GitHub
parent 403a668078
commit d627b8c5e9
3 changed files with 153 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/acp-agent.ts
View File

@@ -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[] }),

116
src/tests/tools.test.ts
View File

@@ -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);
});
});

72
src/tools.ts
View File

@@ -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",
}));
}