From c3458a2eb5b16e1b428462011828606081e2447a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 8 Oct 2026 13:47:00 -0400 Subject: [PATCH] Q81: unreadable tool arguments are a named protocol error, never an empty call Co-Authored-By: Claude Opus 5.5 --- src/main/services/agent-goal-tools.ts | 8 +-- src/main/services/agent-run-loop.ts | 7 ++- src/main/services/provider-tool-turn.ts | 42 ++++++++++++---- .../provider-tool-turns/anthropic-format.ts | 4 +- .../provider-tool-turns/bedrock-format.ts | 4 +- .../provider-tool-turns/chat-format.ts | 20 ++++++-- .../provider-tool-turns/gemini-format.ts | 6 +-- .../provider-tool-turns/responses-format.ts | 4 +- tests/unit/agent-run-loop.test.ts | 12 +++++ tests/unit/tool-call-arguments.test.ts | 50 +++++++++++++++++++ 10 files changed, 127 insertions(+), 30 deletions(-) create mode 100644 tests/unit/tool-call-arguments.test.ts diff --git a/src/main/services/agent-goal-tools.ts b/src/main/services/agent-goal-tools.ts index 80d7a69d..a398ada9 100644 --- a/src/main/services/agent-goal-tools.ts +++ b/src/main/services/agent-goal-tools.ts @@ -5,7 +5,7 @@ import { optionalString, requireString } from "./agent-tool-args"; import type { AgentToolContext, AgentToolDeps } from "./agent-tools"; import type { JsonRecord } from "./provider-http"; import type { ProviderService } from "./provider-service"; -import type { AgentTurnMessage, ToolCall, ToolDefinition } from "./provider-tool-turn"; +import { unreadableArguments, type AgentTurnMessage, type ToolCall, type ToolDefinition } from "./provider-tool-turn"; import { MAX_TOOL_CALLS_PER_TURN, throwIfRunStopped, untilStopped } from "./run-control"; const AUDIT_MAX_ITERATIONS = 6; @@ -148,9 +148,9 @@ async function auditGoalCompletion( const results: Array<{ toolCallId: string; toolName: string; output: string }> = []; for (const [index, toolCall] of turn.toolCalls.entries()) { throwIfRunStopped(); - const output = index < MAX_TOOL_CALLS_PER_TURN - ? await untilStopped(executeWorkspaceTool(auditCtx, toolCall)) - : `ERROR: not run: this turn asked for ${turn.toolCalls.length} tool calls; the limit is ${MAX_TOOL_CALLS_PER_TURN}.`; + const output = index >= MAX_TOOL_CALLS_PER_TURN + ? `ERROR: not run: this turn asked for ${turn.toolCalls.length} tool calls; the limit is ${MAX_TOOL_CALLS_PER_TURN}.` + : toolCall.argumentsError ? unreadableArguments(toolCall) : await untilStopped(executeWorkspaceTool(auditCtx, toolCall)); results.push({ toolCallId: toolCall.id, toolName: toolCall.name, output }); } messages.push({ role: "tool", results }); diff --git a/src/main/services/agent-run-loop.ts b/src/main/services/agent-run-loop.ts index ebf0e749..95d1c63f 100644 --- a/src/main/services/agent-run-loop.ts +++ b/src/main/services/agent-run-loop.ts @@ -4,7 +4,7 @@ import { completeGoalTool } from "./agent-goal-tools"; import { executeWebTool, isWebTool, WEB_TOOLS } from "./agent-web-tools"; import { executeWorkspaceTool, isWorkspaceTool, WORKSPACE_TOOLS } from "./agent-workspace-tools"; import { canonicalMentionHandle } from "../../shared/mention-handles"; -import type { AgentTurnMessage, AgentTurnResult, ToolCall, ToolDefinition } from "./provider-tool-turn"; +import { unreadableArguments, type AgentTurnMessage, type AgentTurnResult, type ToolCall, type ToolDefinition } from "./provider-tool-turn"; import { isComposioTool, executeComposioTool } from "./composio/composio-tool-bridge"; import { capToolOutput, compactToolHistory } from "./run-context-compaction"; import { fitContextBudget } from "./context-budget"; @@ -110,6 +110,11 @@ export async function runAgentLoop(deps: AgentRunLoopDeps, input: AgentRunLoopIn results.push({ toolCallId: call.id, toolName: call.name, output: `ERROR: not run: this turn asked for ${turn.toolCalls.length} tool calls; the limit is ${MAX_TOOL_CALLS_PER_TURN}. Repeat the ones you still need.` }); continue; } + if (call.argumentsError) { + console.warn(`[run-loop] ${call.name} not run: arguments ${call.argumentsError}`); + results.push({ toolCallId: call.id, toolName: call.name, output: unreadableArguments(call) }); + continue; + } if (call.name === "complete_goal" && goalAudits >= MAX_GOAL_AUDITS_PER_SESSION) { results.push({ toolCallId: call.id, toolName: call.name, output: `ERROR: this session already ran ${MAX_GOAL_AUDITS_PER_SESSION} goal audits. Close the gaps they named, then try again next session.` }); continue; diff --git a/src/main/services/provider-tool-turn.ts b/src/main/services/provider-tool-turn.ts index 2580749f..4fcffef3 100644 --- a/src/main/services/provider-tool-turn.ts +++ b/src/main/services/provider-tool-turn.ts @@ -20,6 +20,8 @@ export interface ToolCall { id: string; name: string; arguments: JsonRecord; + /** Why the provider's arguments were unusable; such a call is answered with this and never run. */ + argumentsError?: string; } /** The provider's own assistant output, sent back unchanged so reasoning, encrypted reasoning and signatures survive tool turns. */ @@ -97,16 +99,34 @@ export function requireFinished(reason: unknown, finished: readonly string[]): v if (typeof reason !== "string" || !finished.includes(reason)) throw incompleteTurn(reason); } -/** Coerce a tool-call argument payload (string or object) into a JSON object. */ -export function parseArguments(value: unknown): JsonRecord { - if (isRecord(value)) return value; - if (typeof value === "string" && value.trim()) { - try { - const parsed = JSON.parse(value) as unknown; - if (isRecord(parsed)) return parsed; - } catch { - return {}; - } +const RAW_ARGUMENTS_SHOWN = 200; + +/** A call whose arguments arrive as JSON text (chat, Responses): empty means none, the text must parse to an object. */ +export function toolCallFromJson(id: string, name: string, raw: unknown): ToolCall { + if (typeof raw !== "string" || !raw.trim()) return toolCallFromObject(id, name, raw === "" ? undefined : raw); + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (error) { + return { id, name, arguments: {}, argumentsError: `not valid JSON (${error instanceof Error ? error.message : String(error)}): ${raw.slice(0, RAW_ARGUMENTS_SHOWN)}` }; } - return {}; + return toolCallFromObject(id, name, parsed); +} + +/** A call whose arguments arrive as an object (Anthropic, Bedrock, Gemini): absent means none, anything else must be an object. */ +export function toolCallFromObject(id: string, name: string, raw: unknown): ToolCall { + if (isRecord(raw)) return { id, name, arguments: raw }; + if (raw === undefined) return { id, name, arguments: {} }; + return { id, name, arguments: {}, argumentsError: `expected a JSON object, got ${jsonKind(raw)}` }; +} + +/** The answer to a call whose arguments could not be read, naming why so the model can resend them. */ +export function unreadableArguments(call: ToolCall): string { + return `ERROR: not run: the arguments of ${call.name} were ${call.argumentsError}. Call ${call.name} again with a complete JSON object.`; +} + +function jsonKind(value: unknown): string { + if (value === null) return "null"; + if (Array.isArray(value)) return "an array"; + return typeof value === "object" ? "an object" : `a ${typeof value}`; } diff --git a/src/main/services/provider-tool-turns/anthropic-format.ts b/src/main/services/provider-tool-turns/anthropic-format.ts index 2b3b1e17..7d4104d6 100644 --- a/src/main/services/provider-tool-turns/anthropic-format.ts +++ b/src/main/services/provider-tool-turns/anthropic-format.ts @@ -1,6 +1,6 @@ /** Anthropic Messages API encoding/decoding, shared by anthropic + claude-pro. */ import { isRecord, type JsonRecord } from "../provider-http"; -import { FINISHED_STOP_REASONS, replayIn, requireFinished, type AgentTurnMessage, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; +import { FINISHED_STOP_REASONS, replayIn, requireFinished, toolCallFromObject, type AgentTurnMessage, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; export function anthropicToolDefs(tools: ToolDefinition[]): unknown[] { return tools.map((tool) => ({ name: tool.name, description: tool.description, input_schema: tool.parameters })); @@ -35,7 +35,7 @@ export function parseAnthropicContent(data: JsonRecord): { text: string; toolCal if (!isRecord(block)) continue; if (block.type === "text" && typeof block.text === "string") textParts.push(block.text); else if (block.type === "tool_use" && typeof block.id === "string" && typeof block.name === "string") { - toolCalls.push({ id: block.id, name: block.name, arguments: isRecord(block.input) ? block.input : {} }); + toolCalls.push(toolCallFromObject(block.id, block.name, block.input)); } } return { text: textParts.join("\n").trim(), toolCalls, replay: { format: "anthropic", content } }; diff --git a/src/main/services/provider-tool-turns/bedrock-format.ts b/src/main/services/provider-tool-turns/bedrock-format.ts index 1b828f35..a6db8c1a 100644 --- a/src/main/services/provider-tool-turns/bedrock-format.ts +++ b/src/main/services/provider-tool-turns/bedrock-format.ts @@ -1,6 +1,6 @@ /** AWS Bedrock Converse API encoding/decoding (ported from Pi's bedrock handler). */ import { isRecord, type JsonRecord } from "../provider-http"; -import { FINISHED_STOP_REASONS, replayIn, requireFinished, type AgentTurnMessage, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; +import { FINISHED_STOP_REASONS, replayIn, requireFinished, toolCallFromObject, type AgentTurnMessage, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; export function bedrockToolConfig(tools: ToolDefinition[]): JsonRecord { return { @@ -43,7 +43,7 @@ export function parseBedrockOutput(data: JsonRecord): { text: string; toolCalls: const toolUse = block.toolUse; const id = typeof toolUse.toolUseId === "string" ? toolUse.toolUseId : ""; const name = typeof toolUse.name === "string" ? toolUse.name : ""; - if (id && name) toolCalls.push({ id, name, arguments: isRecord(toolUse.input) ? toolUse.input : {} }); + if (id && name) toolCalls.push(toolCallFromObject(id, name, toolUse.input)); } } return { text: textParts.join("\n").trim(), toolCalls, replay: { format: "bedrock", content } }; diff --git a/src/main/services/provider-tool-turns/chat-format.ts b/src/main/services/provider-tool-turns/chat-format.ts index 7af27db2..dd5f1e90 100644 --- a/src/main/services/provider-tool-turns/chat-format.ts +++ b/src/main/services/provider-tool-turns/chat-format.ts @@ -1,6 +1,6 @@ /** OpenAI Chat Completions encoding/decoding (xAI, Groq, Mistral, Cerebras, OpenRouter, Kimi family). */ import { isRecord, ProviderHttpError, type JsonRecord, type ProviderEventStreamMessage } from "../provider-http"; -import { FINISHED_STOP_REASONS, parseArguments, replayIn, requireFinished, type AgentTurnMessage, type AgentTurnResult, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; +import { FINISHED_STOP_REASONS, replayIn, requireFinished, type AgentTurnMessage, type AgentTurnResult, type AssistantReplay, toolCallFromJson, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; /** Fields providers use for reasoning text; the first non-empty one is the provider's, as in Pi. */ const REASONING_FIELDS = ["reasoning_content", "reasoning", "reasoning_text"] as const; @@ -46,7 +46,7 @@ export function parseChatChoice(data: JsonRecord): { text: string; toolCalls: To const fn = isRecord(call.function) ? call.function : null; const id = typeof call.id === "string" ? call.id : ""; const name = fn && typeof fn.name === "string" ? fn.name : ""; - if (id && name) toolCalls.push({ id, name, arguments: parseArguments(fn?.arguments) }); + if (id && name) toolCalls.push(toolCallFromJson(id, name, fn?.arguments)); } const field = REASONING_FIELDS.find((name) => typeof message[name] === "string" && message[name]); const details = Array.isArray(message.reasoning_details) ? message.reasoning_details.filter(isRecord) : []; @@ -76,7 +76,7 @@ function appendReasoningDetail(details: JsonRecord[], detail: JsonRecord): void export async function accumulateChatStream(messages: AsyncIterable): Promise { const raw: string[] = []; const textParts: string[] = []; - const fragments = new Map(); + const fragments = new Map(); const reasoning = { field: "", text: "" }; const details: JsonRecord[] = []; let finishReason: unknown = null; @@ -96,11 +96,12 @@ export async function accumulateChatStream(messages: AsyncIterable a - b)) { - if (fragment.id && fragment.name) toolCalls.push({ id: fragment.id, name: fragment.name, arguments: parseArguments(fragment.args) }); + if (fragment.id && fragment.name) toolCalls.push(streamedToolCall(fragment)); } return { text: textParts.join("").trim(), toolCalls, rawResponseJson: JSON.stringify(raw), ...chatReplay(reasoning.field ? reasoning : null, details) }; } +/** Streamed arguments are text fragments or one whole value; a mix of the two cannot be read as one object. */ +function streamedToolCall(fragment: { id: string; name: string; args: string; nonText: unknown[] }): ToolCall { + const { id, name, args, nonText } = fragment; + if (nonText.length === 0) return toolCallFromJson(id, name, args); + if (nonText.length === 1 && !args) return toolCallFromJson(id, name, nonText[0]); + const parts = args ? `text and ${nonText.length} non-text fragment${nonText.length > 1 ? "s" : ""}` : `${nonText.length} non-text fragments`; + return { id, name, arguments: {}, argumentsError: `expected one JSON object, got ${parts}` }; +} + /** An in-stream error object's HTTP-style code (OpenRouter sends e.g. { code: 500 }), or null. */ function streamErrorStatus(error: unknown): number | null { return isRecord(error) && typeof error.code === "number" ? error.code : null; diff --git a/src/main/services/provider-tool-turns/gemini-format.ts b/src/main/services/provider-tool-turns/gemini-format.ts index 203cef3c..71da8146 100644 --- a/src/main/services/provider-tool-turns/gemini-format.ts +++ b/src/main/services/provider-tool-turns/gemini-format.ts @@ -4,7 +4,7 @@ * Antigravity request wrapper in google-gemini-cli. */ import { isRecord, ProviderHttpError, type JsonRecord, type ProviderEventStreamMessage } from "../provider-http"; -import { FINISHED_STOP_REASONS, replayIn, requireFinished, type AgentTurnMessage, type AgentTurnResult, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; +import { FINISHED_STOP_REASONS, replayIn, requireFinished, toolCallFromObject, type AgentTurnMessage, type AgentTurnResult, type AssistantReplay, type ToolCall, type ToolDefinition } from "../provider-tool-turn"; const ANTIGRAVITY_SYSTEM_INSTRUCTION = "You are Antigravity, a powerful agentic AI coding assistant designed by the Google Deepmind team working on Advanced Agentic Coding." + @@ -118,7 +118,7 @@ function partsToResult(parts: unknown[]): { text: string[]; toolCalls: ToolCall[ const name = typeof fc.name === "string" ? fc.name : ""; if (!name) continue; const id = typeof fc.id === "string" && fc.id ? fc.id : `${name}_${(counter += 1)}`; - toolCalls.push({ id, name, arguments: isRecord(fc.args) ? fc.args : {} }); + toolCalls.push(toolCallFromObject(id, name, fc.args)); } } return { text, toolCalls }; @@ -163,7 +163,7 @@ export async function accumulateGeminiStream(messages: AsyncIterable { const results = (lastToolMessage as { results: Array<{ output: string }> }).results; expect(results.filter((result) => result.output.startsWith("ERROR: not run: this turn asked for 20 tool calls; the limit is 16"))).toHaveLength(4); + // A call whose arguments could not be read is answered with the reason and never run. + const broken = [{ text: "Writing.", toolCalls: [{ id: "c_bad", name: "send_message", arguments: {}, argumentsError: "not valid JSON (Unexpected end of JSON input): {\"body\":\"hi" }] }]; + let brokenResults: Array<{ output: string }> = []; + const breaking = fakeProvider(broken, connection.db); + const brokenRun = await runAgentLoop({ providers: { ...breaking, runToolTurn: async (input: { messages: Array<{ role: string; results?: Array<{ output: string }> }> }) => { + const last = input.messages.at(-1); + if (last?.role === "tool") brokenResults = last.results ?? []; + return breaking.runToolTurn(); + } }, tools } as never, loopInput); + expect(brokenRun.toolCallCount).toBe(0); + expect(brokenResults.map((result) => result.output)).toEqual(['ERROR: not run: the arguments of send_message were not valid JSON (Unexpected end of JSON input): {"body":"hi. Call send_message again with a complete JSON object.']); + // A third goal audit in one session is refused before it runs. const audits = [{ text: "Closing.", toolCalls: [call("complete_goal", { evidence: "a" }), call("complete_goal", { evidence: "b" }), call("complete_goal", { evidence: "c" })] }]; let auditResults: Array<{ output: string }> = []; diff --git a/tests/unit/tool-call-arguments.test.ts b/tests/unit/tool-call-arguments.test.ts new file mode 100644 index 00000000..73f74fa2 --- /dev/null +++ b/tests/unit/tool-call-arguments.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, test } from "vitest"; +import { parseChatChoice, accumulateChatStream } from "../../src/main/services/provider-tool-turns/chat-format"; +import { parseResponsesOutput } from "../../src/main/services/provider-tool-turns/responses-format"; +import { parseAnthropicContent } from "../../src/main/services/provider-tool-turns/anthropic-format"; +import { parseBedrockOutput } from "../../src/main/services/provider-tool-turns/bedrock-format"; +import { parseGeminiResponse } from "../../src/main/services/provider-tool-turns/gemini-format"; +import type { JsonRecord, ProviderEventStreamMessage } from "../../src/main/services/provider-http"; + +async function* stream(items: JsonRecord[]): AsyncIterable { + for (const data of items) yield { eventName: null, data, rawData: JSON.stringify(data) }; +} + +const chat = (args: unknown) => parseChatChoice({ choices: [{ message: { content: "", tool_calls: [{ id: "c1", function: { name: "write_file", arguments: args } }] }, finish_reason: "tool_calls" }] }).toolCalls[0]; + +describe("tool-call arguments", () => { + test("half-written JSON is a named protocol error that keeps the parser's reason and the raw text", () => { + expect(chat('{"path":"a.txt","content":"he')).toMatchObject({ arguments: {}, argumentsError: expect.stringMatching(/^not valid JSON \(.+\): \{"path":"a\.txt","content":"he$/) }); + }); + + test("JSON that is not an object is an error naming what it was", () => { + expect(chat("[1,2]").argumentsError).toBe("expected a JSON object, got an array"); + expect(chat('"a.txt"').argumentsError).toBe("expected a JSON object, got a string"); + expect(parseAnthropicContent({ stop_reason: "tool_use", content: [{ type: "tool_use", id: "t", name: "f", input: "x" }] }).toolCalls[0].argumentsError).toBe("expected a JSON object, got a string"); + expect(parseBedrockOutput({ stopReason: "tool_use", output: { message: { content: [{ toolUse: { toolUseId: "t", name: "f", input: [] } }] } } }).toolCalls[0].argumentsError).toBe("expected a JSON object, got an array"); + expect(parseGeminiResponse({ candidates: [{ content: { parts: [{ functionCall: { name: "f", args: 3 } }] }, finishReason: "STOP" }] }).toolCalls[0].argumentsError).toBe("expected a JSON object, got a number"); + }); + + test("absent or empty arguments are a call with none; objects pass through", async () => { + expect(chat("")).toEqual({ id: "c1", name: "write_file", arguments: {} }); + expect(chat('{"path":"a"}')).toEqual({ id: "c1", name: "write_file", arguments: { path: "a" } }); + expect(parseResponsesOutput({ status: "completed", output: [{ type: "function_call", call_id: "c2", name: "f", arguments: "{}" }] }).toolCalls[0]).toEqual({ id: "c2", name: "f", arguments: {} }); + const streamed = await accumulateChatStream(stream([ + { choices: [{ delta: { tool_calls: [{ index: 0, id: "c3", function: { name: "f", arguments: '{"a":' } }] }, finish_reason: "tool_calls" }] }, + ])); + expect(streamed.toolCalls[0].argumentsError).toMatch(/^not valid JSON/); + const typed = await accumulateChatStream(stream([ + { choices: [{ delta: { tool_calls: [{ index: 0, id: "c4", function: { name: "f", arguments: 42 } }, { index: 1, id: "c5", function: { name: "g", arguments: { a: 1 } } }] }, finish_reason: "tool_calls" }] }, + ])); + const mixed = await accumulateChatStream(stream([ + { choices: [{ delta: { tool_calls: [{ index: 0, id: "c6", function: { name: "f", arguments: 42 } }] } }] }, + { choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: {} } }] } }] }, + { choices: [{ delta: { tool_calls: [{ index: 1, id: "c7", function: { name: "g", arguments: '{"a":' } }] } }] }, + { choices: [{ delta: { tool_calls: [{ index: 1, function: { arguments: { a: 1 } } }] }, finish_reason: "tool_calls" }] }, + ])); + expect(mixed.toolCalls.map((call) => call.argumentsError)).toEqual([ + "expected one JSON object, got 2 non-text fragments", "expected one JSON object, got text and 1 non-text fragment", + ]); + expect(typed.toolCalls).toEqual([{ id: "c4", name: "f", arguments: {}, argumentsError: "expected a JSON object, got a number" }, { id: "c5", name: "g", arguments: { a: 1 } }]); + }); +});