From 43b1de6d596b7ec892a9fe29b02485d65fe2aceb Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:15:16 +0100 Subject: [PATCH] fix: signal malformed tool arguments in toolcall_end event (#2647) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the API stream is truncated mid-tool-call, PartialMessageBuilder emits a toolcall_end event with { _raw: "" } in the arguments — but the event looks identical to a healthy tool completion. Downstream consumers (error classifiers, tool handlers, activity log) have no way to distinguish a truncated call from a completed one. Add a malformedArguments: boolean flag to the toolcall_end event variant in AssistantMessageEvent. The flag is set to true only in the JSON parse catch path, so existing consumers (which do not check for it) are unaffected. New consumers like classifyProviderError can use it to handle truncated tool calls appropriately. Closes #2574 --- packages/pi-ai/src/types.ts | 2 +- .../claude-code-cli/partial-builder.ts | 5 + .../tests/partial-builder.test.ts | 105 ++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 src/resources/extensions/claude-code-cli/tests/partial-builder.test.ts diff --git a/packages/pi-ai/src/types.ts b/packages/pi-ai/src/types.ts index af3afc5c8..51db40313 100644 --- a/packages/pi-ai/src/types.ts +++ b/packages/pi-ai/src/types.ts @@ -250,7 +250,7 @@ export type AssistantMessageEvent = | { type: "thinking_end"; contentIndex: number; content: string; partial: AssistantMessage } | { type: "toolcall_start"; contentIndex: number; partial: AssistantMessage } | { type: "toolcall_delta"; contentIndex: number; delta: string; partial: AssistantMessage } - | { type: "toolcall_end"; contentIndex: number; toolCall: ToolCall; partial: AssistantMessage } + | { type: "toolcall_end"; contentIndex: number; toolCall: ToolCall; partial: AssistantMessage; malformedArguments?: boolean } | { type: "server_tool_use"; contentIndex: number; partial: AssistantMessage } | { type: "web_search_result"; contentIndex: number; partial: AssistantMessage } | { type: "done"; reason: Extract; message: AssistantMessage } diff --git a/src/resources/extensions/claude-code-cli/partial-builder.ts b/src/resources/extensions/claude-code-cli/partial-builder.ts index 6886cccee..99bd7ca0f 100644 --- a/src/resources/extensions/claude-code-cli/partial-builder.ts +++ b/src/resources/extensions/claude-code-cli/partial-builder.ts @@ -244,7 +244,12 @@ export class PartialMessageBuilder { try { block.arguments = JSON.parse(jsonStr); } catch { + // Stream was truncated mid-tool-call — JSON is garbage. + // Preserve the raw string for diagnostics but signal the + // malformation explicitly so downstream consumers can + // distinguish this from a healthy tool completion (#2574). block.arguments = { _raw: jsonStr }; + return { type: "toolcall_end", contentIndex, toolCall: block, partial: this.partial, malformedArguments: true }; } return { type: "toolcall_end", contentIndex, toolCall: block, partial: this.partial }; } diff --git a/src/resources/extensions/claude-code-cli/tests/partial-builder.test.ts b/src/resources/extensions/claude-code-cli/tests/partial-builder.test.ts new file mode 100644 index 000000000..2a9612986 --- /dev/null +++ b/src/resources/extensions/claude-code-cli/tests/partial-builder.test.ts @@ -0,0 +1,105 @@ +import { describe, test } from "node:test"; +import assert from "node:assert/strict"; +import { PartialMessageBuilder } from "../partial-builder.ts"; +import type { BetaRawMessageStreamEvent } from "../sdk-types.ts"; + +describe("PartialMessageBuilder — malformed tool arguments (#2574)", () => { + /** + * Helper: feed a tool_use block through the builder lifecycle and return + * the toolcall_end event. Simulates: content_block_start → N deltas → content_block_stop. + */ + function feedToolCall( + builder: PartialMessageBuilder, + jsonFragments: string[], + ) { + // Start the tool_use block at stream index 0 + builder.handleEvent({ + type: "content_block_start", + index: 0, + content_block: { type: "tool_use", id: "tool_1", name: "gsd_plan_slice", input: {} }, + } as BetaRawMessageStreamEvent); + + // Feed JSON fragments as input_json_delta + for (const fragment of jsonFragments) { + builder.handleEvent({ + type: "content_block_delta", + index: 0, + delta: { type: "input_json_delta", partial_json: fragment }, + } as BetaRawMessageStreamEvent); + } + + // Stop the block — this is where JSON parse happens + return builder.handleEvent({ + type: "content_block_stop", + index: 0, + } as BetaRawMessageStreamEvent); + } + + test("valid JSON → toolcall_end without malformedArguments", () => { + const builder = new PartialMessageBuilder("claude-sonnet-4-20250514"); + const event = feedToolCall(builder, ['{"milestone', 'Id": "M001"}']); + + assert.ok(event, "event should not be null"); + assert.equal(event!.type, "toolcall_end"); + // Valid JSON should NOT have the malformedArguments flag + assert.equal( + (event as any).malformedArguments, + undefined, + "valid JSON should not set malformedArguments", + ); + // Arguments should be parsed correctly + if (event!.type === "toolcall_end") { + assert.deepEqual(event!.toolCall.arguments, { milestoneId: "M001" }); + } + }); + + test("truncated JSON → toolcall_end WITH malformedArguments: true", () => { + const builder = new PartialMessageBuilder("claude-sonnet-4-20250514"); + // Simulate a stream truncation: JSON is cut off mid-value + const event = feedToolCall(builder, ['{"milestone', 'Id": "M00']); + + assert.ok(event, "event should not be null"); + assert.equal(event!.type, "toolcall_end"); + assert.equal( + (event as any).malformedArguments, + true, + "truncated JSON should set malformedArguments: true", + ); + // The _raw field should contain the original broken JSON + if (event!.type === "toolcall_end") { + assert.equal( + event!.toolCall.arguments._raw, + '{"milestoneId": "M00', + "_raw should contain the truncated JSON string", + ); + } + }); + + test("no JSON deltas → malformedArguments: true (empty accumulator is not valid JSON)", () => { + const builder = new PartialMessageBuilder("claude-sonnet-4-20250514"); + // No deltas — the accumulator is initialized to "" by content_block_start, + // and "" is not valid JSON, so this correctly signals malformed. + const event = feedToolCall(builder, []); + + assert.ok(event, "event should not be null"); + assert.equal(event!.type, "toolcall_end"); + assert.equal( + (event as any).malformedArguments, + true, + "empty accumulator (no JSON deltas) is not valid JSON → malformed", + ); + }); + + test("garbage input (non-JSON) → malformedArguments: true", () => { + const builder = new PartialMessageBuilder("claude-sonnet-4-20250514"); + const event = feedToolCall(builder, ["not json at all "]); + + assert.ok(event, "event should not be null"); + assert.equal(event!.type, "toolcall_end"); + assert.equal( + (event as any).malformedArguments, + true, + "non-JSON content should set malformedArguments: true", + ); + }); +});