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
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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: "",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user