From bdfa8c08040658e4efc0fb1b0445a7d8f843a3b9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 18 Jun 2026 14:38:18 +0000 Subject: [PATCH] fix(ai): pruned optional empty tool params Treat empty strings on optional tool arguments as omitted before schema validation so MCP calls do not fail pattern or type checks for model-filled placeholders. Fixes #2981 --- packages/ai/CHANGELOG.md | 1 + packages/ai/src/utils/validation.ts | 16 +++++----- .../ai/test/tool-argument-coercion.test.ts | 29 +++++++++++++++++-- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 7f34ea30e..cc3075e1b 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -42,6 +42,7 @@ - Fixed OpenRouter Responses requests tagging the streamed assistant message with a hardcoded `openai-responses` API instead of the runtime `model.api`, which silently disabled native-history replay (`buildResponsesInput`) and cross-model tool-call item-id stripping on subsequent OpenRouter turns. The message now carries `model.api` (matching the Chat Completions path). - Fixed OpenAI-family streaming leaking a pre-retry `errorMessage` onto a successful turn: the OpenRouter Anthropic compiled-grammar strict-tool fallback set `errorMessage` before retrying with strict tools disabled and never cleared it on success, and the Chat Completions success path could carry an `errorMessage` from an internally-retried attempt — both made a successful turn read as errored in agent state and telemetry. The Responses fallback no longer assigns `errorMessage`, and the Completions success path clears it before emitting the terminal `done` event. - Fixed Codex stream-error `.code` resolution to use the same nested-first precedence (`error.code` → `error.type` → top-level `code`) as `isRetryableCodexFailureEvent` and the formatted message. Previously the error factory resolved top-level-first, so a failure event carrying both a top-level and a differing nested error code surfaced a `.code` that could disagree with its own `retryable` flag and message text. +- Fixed MCP tool argument validation to drop optional empty-string parameters before schema validation, matching the existing optional null handling and avoiding pattern/type failures for omitted model-filled fields. ([#2981](https://github.com/can1357/oh-my-pi/issues/2981)) ## [16.0.5] - 2026-06-17 diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 382941bb0..d751d0fcb 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -764,10 +764,11 @@ function normalizeOptionalNullsForSchema( if (!(key in nextValue)) continue; const currentValue = nextValue[key]; const isNullish = currentValue === null || currentValue === "null"; + const isEmptyString = currentValue === ""; - // Strip null and the string "null" from optional fields. - // The LLM sometimes outputs string "null" to mean "no value". - if (isNullish && !required.has(key)) { + // Strip null, string "null", and empty strings from optional fields. + // LLMs sometimes output these placeholders to mean "no value". + if ((isNullish || isEmptyString) && !required.has(key)) { if (!changed) { nextValue = { ...nextValue }; changed = true; @@ -1281,7 +1282,8 @@ function truncateArgsForError(value: unknown): unknown { /** * Validates tool call arguments against the tool's schema (Zod or plain JSON * Schema). Applies LLM-quirk coercions (numeric strings, JSON-string - * containers, null-for-optional, null-for-default) before declaring failure. + * containers, null/empty-string-for-optional, null-for-default) before + * declaring failure. * * @throws Error with a formatted message when validation cannot be reconciled. */ @@ -1290,9 +1292,9 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ const ctx = getValidationContext(tool); const { json } = ctx; - // Always normalize first — strip null and string "null" from optional - // fields and substitute defaults. Handles LLM outputting string "null" - // to mean "no value" even when validation would otherwise pass. + // Always normalize first — strip null, string "null", and empty strings + // from optional fields and substitute defaults. Handles LLM outputting + // placeholders for "no value" even when validation would otherwise pass. let normalizedArgs: unknown = originalArgs; let changed = false; const initialNormalization = normalizeOptionalNullsForSchema(json, normalizedArgs); diff --git a/packages/ai/test/tool-argument-coercion.test.ts b/packages/ai/test/tool-argument-coercion.test.ts index 2a64fb3c7..7dead03d3 100644 --- a/packages/ai/test/tool-argument-coercion.test.ts +++ b/packages/ai/test/tool-argument-coercion.test.ts @@ -898,6 +898,32 @@ describe("Tool argument coercion", () => { expect(result).toEqual({ requiredText: "ok" }); }); + it("strips empty strings from optional properties before schema validation", () => { + const tool: Tool = { + name: "mcp-like", + description: "", + parameters: { + type: "object", + properties: { + namespace: { type: "string" }, + fieldSelector: { type: "string", pattern: "^.+$" }, + limit: { type: "number" }, + }, + required: ["namespace"], + additionalProperties: false, + }, + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-empty-optionals", + name: "mcp-like", + arguments: { namespace: "kube-system", fieldSelector: "", limit: "" }, + }); + + expect(result).toEqual({ namespace: "kube-system" }); + }); + it("drops null optional properties nested in array objects", () => { const tool: Tool = { name: "t12", @@ -924,7 +950,7 @@ describe("Tool argument coercion", () => { expect(result).toEqual({ edits: [{ target: "a", end: "e" }] }); }); - it("drops null optional properties in anyOf object branches", () => { + it("drops null and empty-string optional properties in anyOf object branches", () => { const opSchema = z.union([ z.object({ op: z.literal("add_task"), @@ -972,7 +998,6 @@ describe("Tool argument coercion", () => { op: "update", id: "task-1", status: "completed", - notes: "", }, ], });