From 87f9a36419e8164bba77397d15c54ff39fac5e8a Mon Sep 17 00:00:00 2001 From: David Brailovsky Date: Wed, 14 Oct 2026 21:41:42 +0200 Subject: [PATCH] fix(ai): report malformed final tool-call arguments instead of keeping the parsed prefix (#10944) When a finished tool call's arguments text was not valid JSON, the streaming parser kept only the fields before the error and the tool ran with them. Finished calls are now parsed strictly (with the existing string repair); on failure arguments are {} and argumentsError holds the reason and raw text, which validateToolArguments reports to the model. fixes #10897 --- packages/ai/CHANGELOG.md | 1 + packages/ai/src/api/anthropic-messages.ts | 4 +- .../ai/src/api/bedrock-converse-stream.ts | 4 +- packages/ai/src/api/mistral-conversations.ts | 4 +- packages/ai/src/api/openai-completions.ts | 4 +- .../ai/src/api/openai-responses-shared.ts | 4 +- packages/ai/src/types.ts | 5 +++ packages/ai/src/utils/json-parse.ts | 25 +++++++++++++ packages/ai/src/utils/validation.ts | 3 ++ packages/ai/test/json-parse.test.ts | 37 ++++++++++++++++++- ...nai-responses-partial-json-cleanup.test.ts | 26 +++++++++++++ packages/ai/test/validation.test.ts | 19 ++++++++++ 12 files changed, 125 insertions(+), 11 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index bf6a2ab24..8d0599ab5 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -21,6 +21,7 @@ - Fixed Gemini 4 Argon requests on Google Vertex failing with "Thinking budget is not supported", and its thinking level being ignored on the Gemini API, by sending `thinkingLevel` for Gemini 4 and later models ([#10890](https://github.com/earendil-works/pi/issues/10890)) - Fixed Xiaomi MiMo models failing with a 400 at the `minimal` thinking level by sending only the documented thinking on/off toggle and exposing only the `off` and `high` levels ([#10818](https://github.com/earendil-works/pi/issues/10818)) - Fixed `openai-codex/gpt-6.1-sol` using a 272K context window instead of the 500K window Codex now provides ([#10840](https://github.com/earendil-works/pi/issues/10840)) +- Fixed tool calls whose final arguments are malformed JSON silently running with only the fields parsed before the error; such calls now carry `argumentsError` and fail validation with the raw arguments text, so the model sees what it sent ([#10897](https://github.com/earendil-works/pi/issues/10897)) ## [1.2.0] - 2026-10-12 diff --git a/packages/ai/src/api/anthropic-messages.ts b/packages/ai/src/api/anthropic-messages.ts index 3081c7246..dee42ee65 100644 --- a/packages/ai/src/api/anthropic-messages.ts +++ b/packages/ai/src/api/anthropic-messages.ts @@ -40,7 +40,7 @@ import type { import { appendAssistantMessageDiagnostic } from "../utils/diagnostics.ts"; import { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { headersToRecord } from "../utils/headers.ts"; -import { parseJsonWithRepair, parseStreamingJson } from "../utils/json-parse.ts"; +import { parseJsonWithRepair, parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; import { getProviderEnvValue } from "../utils/provider-env.ts"; import { retryProviderRequest } from "../utils/provider-retry.ts"; @@ -833,7 +833,7 @@ export const stream: StreamFunction<"anthropic-messages", AnthropicOptions> = ( partial: output, }); } else if (block.type === "toolCall") { - block.arguments = parseStreamingJson(block.partialJson); + Object.assign(block, parseToolCallArguments(block.partialJson)); // Finalize in-place and strip the scratch buffer so replay only // carries parsed arguments. delete (block as { partialJson?: string }).partialJson; diff --git a/packages/ai/src/api/bedrock-converse-stream.ts b/packages/ai/src/api/bedrock-converse-stream.ts index 1bada0746..04cd40cf2 100644 --- a/packages/ai/src/api/bedrock-converse-stream.ts +++ b/packages/ai/src/api/bedrock-converse-stream.ts @@ -53,7 +53,7 @@ import { appendAssistantMessageDiagnostic } from "../utils/diagnostics.ts"; import { normalizeProviderError } from "../utils/error-body.ts"; import { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { providerHeadersToRecord } from "../utils/headers.ts"; -import { parseStreamingJson } from "../utils/json-parse.ts"; +import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts"; import { resolveHttpProxyUrlForTarget } from "../utils/node-http-proxy.ts"; import { getProviderEnvValue } from "../utils/provider-env.ts"; import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts"; @@ -741,7 +741,7 @@ function handleContentBlockStop( stream.push({ type: "thinking_end", contentIndex: index, content: block.thinking, partial: output }); break; case "toolCall": - block.arguments = parseStreamingJson(block.partialJson); + Object.assign(block, parseToolCallArguments(block.partialJson)); // Finalize in-place and strip the scratch buffer so replay only // carries parsed arguments. delete (block as Block).partialJson; diff --git a/packages/ai/src/api/mistral-conversations.ts b/packages/ai/src/api/mistral-conversations.ts index b4a54dd06..11788eb39 100644 --- a/packages/ai/src/api/mistral-conversations.ts +++ b/packages/ai/src/api/mistral-conversations.ts @@ -17,7 +17,7 @@ import { combineAbortSignals } from "../utils/abort-signals.ts"; import { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { shortHash } from "../utils/hash.ts"; import { headersToRecord } from "../utils/headers.ts"; -import { parseStreamingJson } from "../utils/json-parse.ts"; +import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts"; import { getSystemMessageText, renderSystemMessageUpdate } from "../utils/text.ts"; @@ -761,7 +761,7 @@ async function consumeChatStream( const block = output.content[index]; if (block.type !== "toolCall") continue; const toolBlock = block as ToolCall & { partialArgs?: string }; - toolBlock.arguments = parseStreamingJson(toolBlock.partialArgs); + Object.assign(toolBlock, parseToolCallArguments(toolBlock.partialArgs)); // Finalize in-place and strip the scratch buffer so replay only // carries parsed arguments. delete toolBlock.partialArgs; diff --git a/packages/ai/src/api/openai-completions.ts b/packages/ai/src/api/openai-completions.ts index 5c20a4aa0..32cf42b45 100644 --- a/packages/ai/src/api/openai-completions.ts +++ b/packages/ai/src/api/openai-completions.ts @@ -39,7 +39,7 @@ import { formatProviderError, normalizeProviderError } from "../utils/error-body import { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { shortHash } from "../utils/hash.ts"; import { headersToRecord } from "../utils/headers.ts"; -import { parseStreamingJson } from "../utils/json-parse.ts"; +import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; import { getProviderEnvValue } from "../utils/provider-env.ts"; import { retryProviderRequest } from "../utils/provider-retry.ts"; @@ -461,7 +461,7 @@ export const stream: StreamFunction<"openai-completions", OpenAICompletionsOptio }); } } else { - block.arguments = parseStreamingJson(block.partialArgs); + Object.assign(block, parseToolCallArguments(block.partialArgs)); } // Finalize in-place and strip the scratch buffers so replay only // carries parsed arguments. diff --git a/packages/ai/src/api/openai-responses-shared.ts b/packages/ai/src/api/openai-responses-shared.ts index 31abd39c8..001864d58 100644 --- a/packages/ai/src/api/openai-responses-shared.ts +++ b/packages/ai/src/api/openai-responses-shared.ts @@ -33,7 +33,7 @@ import type { } from "../types.ts"; import type { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { shortHash } from "../utils/hash.ts"; -import { parseStreamingJson } from "../utils/json-parse.ts"; +import { parseStreamingJson, parseToolCallArguments } from "../utils/json-parse.ts"; import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts"; import { getSystemMessageText, renderSystemMessageUpdate } from "../utils/text.ts"; import { resolveTranscript, resolveTranscriptTools } from "../utils/transcript.ts"; @@ -738,7 +738,7 @@ export async function processResponsesStream( slot?.type === "toolCall" && slot.block.partialJson !== undefined ) { - slot.block.arguments = parseStreamingJson(item.arguments || slot.block.partialJson || "{}"); + Object.assign(slot.block, parseToolCallArguments(item.arguments || slot.block.partialJson)); if (item.namespace !== undefined) slot.block.namespace = item.namespace; // Finalize in-place and strip the scratch buffer so replay only // carries parsed arguments. diff --git a/packages/ai/src/types.ts b/packages/ai/src/types.ts index 6a9a1e57a..302f95581 100644 --- a/packages/ai/src/types.ts +++ b/packages/ai/src/types.ts @@ -441,6 +441,11 @@ export interface ToolCall { thoughtSignature?: string; // Google-specific: opaque signature for reusing thought context /** OpenAI Responses namespace for calls to dynamically loaded or namespaced tools. */ namespace?: string; + /** + * Set when the model's complete arguments text was not a valid JSON object. `arguments` is then `{}`. + * Describes the problem and includes the text the model sent; tool argument validation fails with it. + */ + argumentsError?: string; } export interface Usage { diff --git a/packages/ai/src/utils/json-parse.ts b/packages/ai/src/utils/json-parse.ts index 7c909045e..d6fdce639 100644 --- a/packages/ai/src/utils/json-parse.ts +++ b/packages/ai/src/utils/json-parse.ts @@ -1,4 +1,5 @@ import { parse as partialParse } from "partial-json"; +import type { ToolCall } from "../types.ts"; const VALID_JSON_ESCAPES = new Set(['"', "\\", "/", "b", "f", "n", "r", "t"]); @@ -94,6 +95,30 @@ export function parseJsonWithRepair(json: string): T { } } +/** + * Parses the model's complete arguments text of a finished tool call. + * + * Unlike {@link parseStreamingJson}, this never keeps a partially parsed prefix: once the call is + * finished, text that is not a valid JSON object means the model sent malformed arguments. In that + * case `arguments` is `{}` and `argumentsError` describes the problem, including the text the + * model sent, so tool validation can report it back to the model. + */ +export function parseToolCallArguments(json: string | undefined): Pick { + if (!json?.trim()) return { arguments: {} }; + try { + const parsed = parseJsonWithRepair(json); + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw new Error("Tool arguments must be a JSON object"); + } + return { arguments: parsed }; + } catch (error) { + return { + arguments: {}, + argumentsError: `${error instanceof Error ? error.message : String(error)}\n\nReceived arguments:\n${json}`, + }; + } +} + /** * Attempts to parse potentially incomplete JSON during streaming. * Always returns a valid object, even if the JSON is incomplete. diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index cf6902df2..c9e818834 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -315,6 +315,9 @@ export function validateToolCall(tools: Tool[], toolCall: ToolCall): any { * @throws Error with formatted message if validation fails */ export function validateToolArguments(tool: Tool, toolCall: ToolCall): any { + if (toolCall.argumentsError) { + throw new Error(`Invalid arguments for tool "${toolCall.name}": ${toolCall.argumentsError}`); + } const args = structuredClone(toolCall.arguments); normalizeOptionalNulls(args, tool.parameters as JsonSchemaObject); Value.Convert(tool.parameters, args); diff --git a/packages/ai/test/json-parse.test.ts b/packages/ai/test/json-parse.test.ts index 04526a6b4..5783d9568 100644 --- a/packages/ai/test/json-parse.test.ts +++ b/packages/ai/test/json-parse.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { parseStreamingJson } from "../src/utils/json-parse.ts"; +import { parseStreamingJson, parseToolCallArguments } from "../src/utils/json-parse.ts"; describe("parseStreamingJson", () => { // #10868 @@ -20,3 +20,38 @@ describe("parseStreamingJson", () => { expect(parseStreamingJson(String.raw`{"text":"ab\u00`)).toEqual({ text: "ab" }); }); }); + +// #10897 +describe("parseToolCallArguments", () => { + it("parses complete JSON and repairs invalid escapes", () => { + expect(parseToolCallArguments(String.raw`{"kind":"seek","note":"\url"}`)).toEqual({ + arguments: { kind: "seek", note: "\\url" }, + }); + }); + + it("uses empty arguments for empty text", () => { + expect(parseToolCallArguments("")).toEqual({ arguments: {} }); + }); + + it("does not keep the valid prefix of malformed JSON", () => { + const raw = '{"action":"apply","actions":[{"kind":"seek","time":.5}]}'; + const result = parseToolCallArguments(raw); + expect(result.arguments).toEqual({}); + expect(result.argumentsError).toMatch(/\n\nReceived arguments:\n/); + expect(result.argumentsError?.endsWith(raw)).toBe(true); + }); + + it("does not complete cut-off JSON", () => { + const raw = '{"kind":"seek","clipId":"clip-fl'; + const result = parseToolCallArguments(raw); + expect(result.arguments).toEqual({}); + expect(result.argumentsError?.endsWith(raw)).toBe(true); + }); + + it("rejects JSON that is not an object", () => { + expect(parseToolCallArguments("[1,2]")).toEqual({ + arguments: {}, + argumentsError: "Tool arguments must be a JSON object\n\nReceived arguments:\n[1,2]", + }); + }); +}); diff --git a/packages/ai/test/openai-responses-partial-json-cleanup.test.ts b/packages/ai/test/openai-responses-partial-json-cleanup.test.ts index d01800c48..e0b83283c 100644 --- a/packages/ai/test/openai-responses-partial-json-cleanup.test.ts +++ b/packages/ai/test/openai-responses-partial-json-cleanup.test.ts @@ -103,4 +103,30 @@ describe("openai responses partialJson cleanup", () => { expect(toolCallEnd.toolCall).toBe(persistedToolCall); expect("partialJson" in toolCallEnd.toolCall).toBe(false); }); + + // #10897 + it("marks malformed final arguments instead of keeping the parsed prefix", async () => { + const model: Model<"openai-responses"> = { + id: "gpt-5-mini", + name: "GPT-5 Mini", + api: "openai-responses", + provider: "openai", + baseUrl: "https://api.openai.com/v1", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 400000, + maxTokens: 128000, + }; + const output = createOutput(model); + const stream = new AssistantMessageEventStream(); + const argumentsJson = '{"path":"README.md","content":"updated","mode":.5}'; + + await processResponsesStream(createFunctionCallEvents(argumentsJson), output, stream, model); + + const toolCall = output.content[0]; + if (!toolCall || toolCall.type !== "toolCall") throw new Error("Expected toolCall block"); + expect(toolCall.arguments).toEqual({}); + expect(toolCall.argumentsError?.endsWith(`Received arguments:\n${argumentsJson}`)).toBe(true); + }); }); diff --git a/packages/ai/test/validation.test.ts b/packages/ai/test/validation.test.ts index 816f99a64..7cebbe541 100644 --- a/packages/ai/test/validation.test.ts +++ b/packages/ai/test/validation.test.ts @@ -207,4 +207,23 @@ describe("validateToolArguments", () => { expect(() => validateToolArguments(tool, toolCall)).toThrow("Validation failed"); } }); + // #10897 + it("reports malformed arguments JSON instead of validating empty arguments", () => { + const tool: Tool = { + name: "echo", + description: "Echo tool", + parameters: Type.Object({ value: Type.Optional(Type.String()) }), + }; + const toolCall: ToolCall = { + type: "toolCall", + id: "tool-1", + name: "echo", + arguments: {}, + argumentsError: "Unexpected token", + }; + + expect(() => validateToolArguments(tool, toolCall)).toThrow( + 'Invalid arguments for tool "echo": Unexpected token', + ); + }); });