diff --git a/src/agents/pi-tool-definition-adapter.test.ts b/src/agents/pi-tool-definition-adapter.test.ts index 6def07167cb..1bc8ad94db4 100644 --- a/src/agents/pi-tool-definition-adapter.test.ts +++ b/src/agents/pi-tool-definition-adapter.test.ts @@ -1,7 +1,8 @@ import type { AgentTool } from "@mariozechner/pi-agent-core"; import { Type } from "@sinclair/typebox"; import { describe, expect, it } from "vitest"; -import { toToolDefinitions } from "./pi-tool-definition-adapter.js"; +import type { ClientToolDefinition } from "./pi-embedded-runner/run/params.js"; +import { toClientToolDefinitions, toToolDefinitions } from "./pi-tool-definition-adapter.js"; type ToolExecute = ReturnType[number]["execute"]; const extensionContext = {} as Parameters[4]; @@ -98,3 +99,81 @@ describe("pi tool definition adapter", () => { expect((result.content[0] as { text?: string }).text).toContain('"count"'); }); }); + +// --------------------------------------------------------------------------- +// toClientToolDefinitions – streaming tool-call argument coercion (#57009) +// --------------------------------------------------------------------------- + +function makeClientTool(name: string): ClientToolDefinition { + return { + type: "function", + function: { + name, + description: `${name} tool`, + parameters: { type: "object", properties: { query: { type: "string" } } }, + }, + }; +} + +async function executeClientTool( + params: unknown, +): Promise<{ calledWith: Record | undefined }> { + let captured: Record | undefined; + const [def] = toClientToolDefinitions([makeClientTool("search")], (_name, p) => { + captured = p; + }); + if (!def) { + throw new Error("missing client tool definition"); + } + await def.execute("call-c1", params, undefined, undefined, extensionContext); + return { calledWith: captured }; +} + +describe("toClientToolDefinitions – param coercion", () => { + it("passes plain object params through unchanged", async () => { + const { calledWith } = await executeClientTool({ query: "hello" }); + expect(calledWith).toEqual({ query: "hello" }); + }); + + it("parses a JSON string into an object (streaming delta accumulation)", async () => { + const { calledWith } = await executeClientTool('{"query":"hello","limit":10}'); + expect(calledWith).toEqual({ query: "hello", limit: 10 }); + }); + + it("parses a JSON string with surrounding whitespace", async () => { + const { calledWith } = await executeClientTool(' {"query":"hello"} '); + expect(calledWith).toEqual({ query: "hello" }); + }); + + it("falls back to empty object for invalid JSON string", async () => { + const { calledWith } = await executeClientTool("not-json"); + expect(calledWith).toEqual({}); + }); + + it("falls back to empty object for empty string", async () => { + const { calledWith } = await executeClientTool(""); + expect(calledWith).toEqual({}); + }); + + it("falls back to empty object for null", async () => { + const { calledWith } = await executeClientTool(null); + expect(calledWith).toEqual({}); + }); + + it("falls back to empty object for undefined", async () => { + const { calledWith } = await executeClientTool(undefined); + expect(calledWith).toEqual({}); + }); + + it("falls back to empty object for a JSON array string", async () => { + const { calledWith } = await executeClientTool("[1,2,3]"); + expect(calledWith).toEqual({}); + }); + + it("handles nested JSON string correctly", async () => { + const { calledWith } = await executeClientTool( + '{"action":"search","params":{"q":"test","page":1}}', + ); + expect(calledWith).toEqual({ action: "search", params: { q: "test", page: 1 } }); + }); +}); diff --git a/src/agents/pi-tool-definition-adapter.ts b/src/agents/pi-tool-definition-adapter.ts index d2673ff46b1..da345a74ea8 100644 --- a/src/agents/pi-tool-definition-adapter.ts +++ b/src/agents/pi-tool-definition-adapter.ts @@ -172,6 +172,38 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] { }); } +/** + * Coerce tool-call params into a plain object. + * + * Some providers (e.g. Gemini) stream tool-call arguments as incremental + * string deltas. By the time the framework invokes the tool's `execute` + * callback the accumulated value may still be a JSON **string** rather than + * a parsed object. `isPlainObject()` returns `false` for strings, which + * caused the params to be silently replaced with `{}`. + * + * This helper tries `JSON.parse` when the value is a string and falls back + * to an empty object only when parsing genuinely fails. + */ +function coerceParamsRecord(value: unknown): Record { + if (isPlainObject(value)) { + return value; + } + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed.length > 0) { + try { + const parsed: unknown = JSON.parse(trimmed); + if (isPlainObject(parsed)) { + return parsed; + } + } catch { + // not valid JSON – fall through to empty object + } + } + } + return {}; +} + // Convert client tools (OpenResponses hosted tools) to ToolDefinition format // These tools are intercepted to return a "pending" result instead of executing export function toClientToolDefinitions( @@ -198,7 +230,7 @@ export function toClientToolDefinitions( throw new Error(outcome.reason); } const adjustedParams = outcome.params; - const paramsRecord = isPlainObject(adjustedParams) ? adjustedParams : {}; + const paramsRecord = coerceParamsRecord(adjustedParams); // Notify handler that a client tool was called if (onClientToolCall) { onClientToolCall(func.name, paramsRecord);