From 4e19bc80c9e43cd7b06c3fba0679b4fad08e60a9 Mon Sep 17 00:00:00 2001 From: Alex Knight Date: Mon, 27 Apr 2026 17:45:59 +1000 Subject: [PATCH] Fix null params for parameterless tools (#72673) * fix tool null params for parameterless schemas * guard composite required tool schemas --- src/agents/pi-tools.schema.test.ts | 190 +++++++++++++++++++++++++++++ src/agents/pi-tools.schema.ts | 55 ++++++++- 2 files changed, 244 insertions(+), 1 deletion(-) diff --git a/src/agents/pi-tools.schema.test.ts b/src/agents/pi-tools.schema.test.ts index ce10f2719b8..c6f12b67389 100644 --- a/src/agents/pi-tools.schema.test.ts +++ b/src/agents/pi-tools.schema.test.ts @@ -1,5 +1,8 @@ +import { runAgentLoop, type AgentEvent, type StreamFn } from "@mariozechner/pi-agent-core"; +import { createAssistantMessageEventStream, validateToolArguments } from "@mariozechner/pi-ai"; import { Type, type TSchema } from "typebox"; import { describe, expect, it, vi } from "vitest"; +import { wrapToolWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js"; import { cleanToolSchemaForGemini, normalizeToolParameterSchema, @@ -7,6 +10,15 @@ import { } from "./pi-tools.schema.js"; import type { AnyAgentTool } from "./pi-tools.types.js"; +const TEST_USAGE = { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, +}; + describe("normalizeToolParameterSchema", () => { it("normalizes truly empty schemas to type:object with properties:{}", () => { expect(normalizeToolParameterSchema({})).toEqual({ @@ -197,6 +209,184 @@ describe("normalizeToolParameters", () => { expect(parameters.additionalProperties).toBe(true); }); + it("prepares null arguments as empty objects for object schemas without required params", () => { + const tool: AnyAgentTool = { + name: "wiki_lint", + label: "wiki_lint", + description: "Lint wiki vault", + parameters: { type: "object", properties: {}, required: [] }, + execute: vi.fn(), + }; + + const normalized = normalizeToolParameters(tool); + const prepared = normalized.prepareArguments?.(null) as Record; + + expect(prepared).toEqual({}); + expect( + validateToolArguments(normalized, { + type: "toolCall", + id: "call-1", + name: "wiki_lint", + arguments: prepared, + }), + ).toEqual({}); + }); + + it("leaves null arguments invalid when the object schema has required params", () => { + const tool: AnyAgentTool = { + name: "query", + label: "query", + description: "Run query", + parameters: { type: "object", properties: { q: { type: "string" } }, required: ["q"] }, + execute: vi.fn(), + }; + + const normalized = normalizeToolParameters(tool); + + expect(normalized.prepareArguments).toBeUndefined(); + expect(() => + validateToolArguments(normalized, { + type: "toolCall", + id: "call-1", + name: "query", + arguments: null as never, + }), + ).toThrow('Validation failed for tool "query"'); + }); + + it("leaves null arguments invalid when required params are nested in composite schemas", () => { + const tool: AnyAgentTool = { + name: "query", + label: "query", + description: "Run query", + parameters: { + type: "object", + allOf: [ + { + type: "object", + properties: { q: { type: "string" } }, + required: ["q"], + }, + ], + }, + execute: vi.fn(), + }; + + const normalized = normalizeToolParameters(tool); + + expect(normalized.prepareArguments).toBeUndefined(); + expect(() => + validateToolArguments(normalized, { + type: "toolCall", + id: "call-1", + name: "query", + arguments: null as never, + }), + ).toThrow('Validation failed for tool "query"'); + }); + + it("runs null arguments for parameterless tools through the agent loop without validation failure", async () => { + const execute = vi.fn().mockResolvedValue({ + content: [{ type: "text", text: "wiki ok" }], + details: { ok: true }, + }); + const normalized = normalizeToolParameters({ + name: "wiki_lint", + label: "wiki_lint", + description: "Lint wiki vault", + parameters: { type: "object", properties: {}, required: [] }, + execute, + }); + const tool = wrapToolWithBeforeToolCallHook(normalized, { + agentId: "main", + sessionKey: "e2e-null-args", + loopDetection: { enabled: true }, + }); + const events: AgentEvent[] = []; + let streamCalls = 0; + const streamFn: StreamFn = () => { + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + streamCalls += 1; + const message = + streamCalls === 1 + ? { + role: "assistant" as const, + content: [ + { + type: "toolCall" as const, + id: "call-null-args", + name: "wiki_lint", + arguments: null as never, + }, + ], + api: "faux", + provider: "faux", + model: "faux-1", + usage: TEST_USAGE, + stopReason: "toolUse" as const, + timestamp: Date.now(), + } + : { + role: "assistant" as const, + content: [{ type: "text" as const, text: "done" }], + api: "faux", + provider: "faux", + model: "faux-1", + usage: TEST_USAGE, + stopReason: "stop" as const, + timestamp: Date.now(), + }; + stream.push({ type: "done", reason: message.stopReason, message }); + }); + return stream; + }; + + const messages = await runAgentLoop( + [{ role: "user", content: "lint the wiki", timestamp: Date.now() }], + { systemPrompt: "test", messages: [], tools: [tool] }, + { + model: { + id: "faux-1", + name: "Faux", + provider: "faux", + api: "faux", + baseUrl: "http://localhost:0", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128000, + maxTokens: 1024, + }, + convertToLlm: (agentMessages) => agentMessages as never, + }, + (event) => { + events.push(event); + }, + undefined, + streamFn, + ); + + expect(streamCalls).toBe(2); + expect(execute).toHaveBeenCalledWith("call-null-args", {}, undefined, expect.any(Function)); + const toolResult = messages.find((message) => message.role === "toolResult"); + expect(toolResult).toMatchObject({ + role: "toolResult", + toolCallId: "call-null-args", + toolName: "wiki_lint", + isError: false, + content: [{ type: "text", text: "wiki ok" }], + }); + const endedToolCall = events.find((event) => event.type === "tool_execution_end"); + expect(endedToolCall).toMatchObject({ + type: "tool_execution_end", + toolCallId: "call-null-args", + toolName: "wiki_lint", + isError: false, + }); + expect(JSON.stringify(messages)).not.toContain("Validation failed for tool"); + }); + it("strips compat-declared unsupported schema keywords without provider-specific branching", () => { const tool: AnyAgentTool = { name: "demo", diff --git a/src/agents/pi-tools.schema.ts b/src/agents/pi-tools.schema.ts index bfac93f39f7..48bb3cd5549 100644 --- a/src/agents/pi-tools.schema.ts +++ b/src/agents/pi-tools.schema.ts @@ -8,6 +8,57 @@ import type { AnyAgentTool } from "./pi-tools.types.js"; export { normalizeToolParameterSchema }; +function isObjectSchemaWithNoRequiredParams(schema: unknown): boolean { + if (!schema || typeof schema !== "object" || Array.isArray(schema)) { + return false; + } + const record = schema as Record; + const type = record.type; + const hasObjectType = + type === "object" || (Array.isArray(type) && type.some((entry) => entry === "object")); + if (!hasObjectType) { + return false; + } + return !schemaHasRequiredParams(record); +} + +function schemaHasRequiredParams(schema: Record): boolean { + if (Array.isArray(schema.required) && schema.required.length > 0) { + return true; + } + for (const key of ["allOf", "anyOf", "oneOf"]) { + const variants = schema[key]; + if (!Array.isArray(variants)) { + continue; + } + if ( + variants.some( + (variant) => + variant !== null && + typeof variant === "object" && + !Array.isArray(variant) && + schemaHasRequiredParams(variant as Record), + ) + ) { + return true; + } + } + return false; +} + +function addEmptyObjectArgumentPreparation(tool: AnyAgentTool, parameters: unknown): AnyAgentTool { + if (!isObjectSchemaWithNoRequiredParams(parameters)) { + return tool; + } + return { + ...tool, + prepareArguments: (args: unknown) => { + const prepared = tool.prepareArguments ? tool.prepareArguments(args) : args; + return prepared === null || prepared === undefined ? {} : prepared; + }, + }; +} + export function normalizeToolParameters( tool: AnyAgentTool, options?: ToolParameterSchemaOptions, @@ -24,9 +75,11 @@ export function normalizeToolParameters( if (!schema) { return tool; } + const parameters = normalizeToolParameterSchema(schema, options); return preserveToolMeta({ ...tool, - parameters: normalizeToolParameterSchema(schema, options), + ...addEmptyObjectArgumentPreparation(tool, parameters), + parameters, }); }