From 8fae50bc1d3929e82eb32c9854a2f357419a7991 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 3 Jul 2026 15:49:56 +0000 Subject: [PATCH 1/5] fix(ai): trimmed whitespace around enum tool args Normalized schema-matching enum and const strings before tool validation so provider-emitted trailing newlines do not reject valid tool calls. Fixes #4461 --- packages/ai/CHANGELOG.md | 4 + packages/ai/src/utils/validation.ts | 103 ++++++++++++++++++++ packages/ai/test/todo-op-whitespace.test.ts | 26 +++++ 3 files changed, 133 insertions(+) create mode 100644 packages/ai/test/todo-op-whitespace.test.ts diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 20ef357ef..b234b2b6f 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed tool-call validation to trim schema-matching enum values with stray surrounding whitespace before dispatch ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). + ## [16.3.4] - 2026-07-03 ### Added diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 9c78c8485..58fb710f3 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -835,6 +835,98 @@ function normalizeOptionalNullsForSchema( return { value: changed ? nextValue : value, changed }; } +function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value: unknown; changed: boolean } { + if (value === null || value === undefined) return { value, changed: false }; + if (schema === null || typeof schema !== "object") return { value, changed: false }; + + const schemaObject = schema as Record; + + const normalizeAnyOfLike = (keyword: "anyOf" | "oneOf"): { value: unknown; changed: boolean } => { + const branches = schemaObject[keyword]; + if (!Array.isArray(branches)) return { value, changed: false }; + if (branches.some(branch => branchMatchesSchema(branch, value))) return { value, changed: false }; + + for (const branch of branches) { + const normalized = normalizeEnumStringWhitespace(branch, value); + if (!normalized.changed) continue; + if (branchMatchesSchema(branch, normalized.value)) return normalized; + } + return { value, changed: false }; + }; + + const anyOfNormalization = normalizeAnyOfLike("anyOf"); + if (anyOfNormalization.changed) return anyOfNormalization; + + const oneOfNormalization = normalizeAnyOfLike("oneOf"); + if (oneOfNormalization.changed) return oneOfNormalization; + + if (Array.isArray(schemaObject.allOf)) { + let changed = false; + let nextValue: unknown = value; + for (const branch of schemaObject.allOf) { + const normalized = normalizeEnumStringWhitespace(branch, nextValue); + if (!normalized.changed) continue; + nextValue = normalized.value; + changed = true; + } + if (changed) return { value: nextValue, changed: true }; + } + + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed !== value) { + const enumValues = schemaObject.enum; + if (Array.isArray(enumValues) && !enumValues.includes(value) && enumValues.includes(trimmed)) { + return { value: trimmed, changed: true }; + } + const constValue = schemaObject.const; + if (typeof constValue === "string" && trimmed === constValue) { + return { value: trimmed, changed: true }; + } + } + return { value, changed: false }; + } + + if (Array.isArray(value)) { + const itemSchema = schemaObject.items; + if (itemSchema === null || typeof itemSchema !== "object" || Array.isArray(itemSchema)) { + return { value, changed: false }; + } + let changed = false; + let nextValue = value; + for (let i = 0; i < value.length; i += 1) { + const normalized = normalizeEnumStringWhitespace(itemSchema, value[i]); + if (!normalized.changed) continue; + if (!changed) { + nextValue = [...value]; + changed = true; + } + nextValue[i] = normalized.value; + } + return { value: changed ? nextValue : value, changed }; + } + + if (typeof value !== "object") return { value, changed: false }; + const properties = schemaObject.properties; + if (!properties || typeof properties !== "object") return { value, changed: false }; + + const propsObject = properties as Record; + const valueObject = value as Record; + let changed = false; + let nextValue = valueObject; + for (const [key, propertySchema] of Object.entries(propsObject)) { + if (!(key in nextValue)) continue; + const normalized = normalizeEnumStringWhitespace(propertySchema, nextValue[key]); + if (!normalized.changed) continue; + if (!changed) { + nextValue = { ...nextValue }; + changed = true; + } + nextValue[key] = normalized.value; + } + return { value: changed ? nextValue : valueObject, changed }; +} + // ============================================================================ // Double-encoded object-key normalization (LLM quirk). // ============================================================================ @@ -1485,6 +1577,12 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ changed = true; } + const enumStringNormalization = normalizeEnumStringWhitespace(json, normalizedArgs); + if (enumStringNormalization.changed) { + normalizedArgs = enumStringNormalization.value; + changed = true; + } + // Then re-shape JSON-stringified arrays whose schema accepts both string // and array (e.g. `paths: string | string[]`). Without this, zod accepts // the literal `'["a","b"]'` as a string and downstream tools treat it as @@ -1527,6 +1625,11 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ normalizedArgs = nullNormalization.value; } + const enumStringNormalizationPass = normalizeEnumStringWhitespace(json, normalizedArgs); + if (enumStringNormalizationPass.changed) { + normalizedArgs = enumStringNormalizationPass.value; + } + // Re-run the union-string coercion because `coerceArgsFromIssues` may // have just unwrapped a JSON-stringified object at the root or inside a // nested field — exposing `string | string[]` descendants the initial diff --git a/packages/ai/test/todo-op-whitespace.test.ts b/packages/ai/test/todo-op-whitespace.test.ts new file mode 100644 index 000000000..f40bd8fa6 --- /dev/null +++ b/packages/ai/test/todo-op-whitespace.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "bun:test"; +import type { Tool } from "@oh-my-pi/pi-ai/types"; +import { validateToolArguments } from "@oh-my-pi/pi-ai/utils/validation"; +import { z } from "zod/v4"; + +describe("Tool enum argument whitespace", () => { + it("trims trailing whitespace from enum strings before validation", () => { + const tool: Tool = { + name: "todo", + description: "", + parameters: z.object({ + op: z.enum(["append", "done", "drop", "init", "rm", "start", "view"]), + items: z.array(z.string()).optional(), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-todo-op-newline", + name: "todo", + arguments: { op: "init\n", items: ["Fix RNG divergence"] }, + }); + + expect(result).toEqual({ op: "init", items: ["Fix RNG divergence"] }); + }); +}); From f7af05bc70225614206f4fa29b82a62c108cf2e9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 3 Jul 2026 15:58:02 +0000 Subject: [PATCH 2/5] fix(ai): trimmed trailing whitespace on path-like tool args Extended the whitespace normalizer to strip trailing whitespace from string values on well-known path/URL property names (path, paths, file, file_path, url, uri) so read-like tools no longer see stray trailing newlines emitted by the model. Content-carrying fields (content, input, etc.) are excluded to preserve intentional trailing newlines. Fixes #4461 --- packages/ai/CHANGELOG.md | 2 +- packages/ai/src/utils/validation.ts | 119 ++++++++++++++++++++ packages/ai/test/todo-op-whitespace.test.ts | 68 ++++++++++- 3 files changed, 187 insertions(+), 2 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index b234b2b6f..e51376ff6 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed tool-call validation to trim schema-matching enum values with stray surrounding whitespace before dispatch ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). +- Fixed tool-call validation to strip stray trailing whitespace on schema-matching enum values and on well-known path/URL fields (`path`, `paths`, `file`, `file_path`, `url`, `uri`) before dispatch, keeping content-carrying fields (`content`, `input`, etc.) intact ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). ## [16.3.4] - 2026-07-03 diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 58fb710f3..e9110c7da 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -927,6 +927,110 @@ function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value return { value: changed ? nextValue : valueObject, changed }; } +// ============================================================================ +// Path-like string trailing-whitespace normalization (LLM quirk). +// ============================================================================ +// +// LLMs sometimes emit tool arguments with a trailing newline dangling off a +// path/URL — the artifact of a token boundary or a JSON stream heuristic. Path +// values are never legitimately terminated by whitespace, so we strip trailing +// whitespace from string values on well-known path/URL properties before the +// tool ever sees them. Content-carrying properties (`content`, `input`, `body`, +// `text`, `command`) are excluded so genuine trailing newlines survive. +// ============================================================================ + +/** + * Property names whose values are treated as filesystem paths, URLs, or URIs. + * The trim only fires on strings sitting under one of these keys, so + * `content: "line1\n"` on `write` and similar content-carrying fields keep + * their trailing whitespace intact. + */ +const PATH_LIKE_KEYS: ReadonlySet = new Set([ + "path", + "paths", + "file", + "file_path", + "filePath", + "filepath", + "url", + "uri", +]); + +const TRAILING_WHITESPACE_RE = /\s+$/; + +function trimTrailingWhitespaceString(input: string): string { + if (!TRAILING_WHITESPACE_RE.test(input)) return input; + return input.replace(TRAILING_WHITESPACE_RE, ""); +} + +function trimPathLikeStringLeaf(input: unknown): unknown { + if (typeof input === "string") { + const trimmed = trimTrailingWhitespaceString(input); + return trimmed === input ? input : trimmed; + } + if (Array.isArray(input)) { + let changed = false; + let next = input; + for (let i = 0; i < input.length; i += 1) { + const item = input[i]; + if (typeof item !== "string") continue; + const trimmed = trimTrailingWhitespaceString(item); + if (trimmed === item) continue; + if (!changed) { + next = input.slice(); + changed = true; + } + next[i] = trimmed; + } + return changed ? next : input; + } + return input; +} + +/** + * Recursively strip trailing whitespace from string values whose property key + * matches {@link PATH_LIKE_KEYS}. Runs by property name only (schema-agnostic) + * so it fires uniformly across Zod, ArkType, and plain JSON Schema tools. + */ +function normalizePathLikeFieldWhitespace(value: unknown): { value: unknown; changed: boolean } { + if (Array.isArray(value)) { + let changed = false; + let next = value; + for (let i = 0; i < value.length; i += 1) { + const normalized = normalizePathLikeFieldWhitespace(value[i]); + if (!normalized.changed) continue; + if (!changed) { + next = [...value]; + changed = true; + } + next[i] = normalized.value; + } + return { value: changed ? next : value, changed }; + } + + if (value === null || typeof value !== "object") return { value, changed: false }; + + const source = value as Record; + let changed = false; + let out: Record = source; + for (const [key, entry] of Object.entries(source)) { + let nextEntry = entry; + if (PATH_LIKE_KEYS.has(key)) { + const trimmed = trimPathLikeStringLeaf(entry); + if (trimmed !== entry) nextEntry = trimmed; + } + const nested = normalizePathLikeFieldWhitespace(nextEntry); + if (nested.changed) nextEntry = nested.value; + if (nextEntry === entry) continue; + if (!changed) { + out = { ...source }; + changed = true; + } + out[key] = nextEntry; + } + return { value: changed ? out : value, changed }; +} + // ============================================================================ // Double-encoded object-key normalization (LLM quirk). // ============================================================================ @@ -1583,6 +1687,16 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ changed = true; } + // Strip trailing whitespace from string values on well-known path/URL + // property names. Some models tack a newline onto a path arg from stream + // artifacts; downstream tools then either fail to stat the target or + // annotate a "corrected from" hint the model misreads as tool corruption. + const pathLikeNormalization = normalizePathLikeFieldWhitespace(normalizedArgs); + if (pathLikeNormalization.changed) { + normalizedArgs = pathLikeNormalization.value; + changed = true; + } + // Then re-shape JSON-stringified arrays whose schema accepts both string // and array (e.g. `paths: string | string[]`). Without this, zod accepts // the literal `'["a","b"]'` as a string and downstream tools treat it as @@ -1630,6 +1744,11 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ normalizedArgs = enumStringNormalizationPass.value; } + const pathLikeNormalizationPass = normalizePathLikeFieldWhitespace(normalizedArgs); + if (pathLikeNormalizationPass.changed) { + normalizedArgs = pathLikeNormalizationPass.value; + } + // Re-run the union-string coercion because `coerceArgsFromIssues` may // have just unwrapped a JSON-stringified object at the root or inside a // nested field — exposing `string | string[]` descendants the initial diff --git a/packages/ai/test/todo-op-whitespace.test.ts b/packages/ai/test/todo-op-whitespace.test.ts index f40bd8fa6..63c09a392 100644 --- a/packages/ai/test/todo-op-whitespace.test.ts +++ b/packages/ai/test/todo-op-whitespace.test.ts @@ -3,7 +3,7 @@ import type { Tool } from "@oh-my-pi/pi-ai/types"; import { validateToolArguments } from "@oh-my-pi/pi-ai/utils/validation"; import { z } from "zod/v4"; -describe("Tool enum argument whitespace", () => { +describe("Tool argument whitespace normalization", () => { it("trims trailing whitespace from enum strings before validation", () => { const tool: Tool = { name: "todo", @@ -23,4 +23,70 @@ describe("Tool enum argument whitespace", () => { expect(result).toEqual({ op: "init", items: ["Fix RNG divergence"] }); }); + + it("strips trailing newlines from path fields on read-like tools", () => { + const tool: Tool = { + name: "read", + description: "", + parameters: z.object({ + path: z.string(), + offset: z.number().optional(), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-read-path-newline", + name: "read", + arguments: { path: "examples/multi_observation.py:36-55\n" }, + }); + + expect(result).toEqual({ path: "examples/multi_observation.py:36-55" }); + }); + + it("strips trailing whitespace from every entry in a path array", () => { + const tool: Tool = { + name: "search", + description: "", + parameters: z.object({ + pattern: z.string(), + paths: z.array(z.string()), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-search-paths-newline", + name: "search", + arguments: { + pattern: "TODO", + paths: ["src/foo.ts\n", "src/bar.ts "], + }, + }); + + expect(result).toEqual({ + pattern: "TODO", + paths: ["src/foo.ts", "src/bar.ts"], + }); + }); + + it("leaves trailing newlines on content-carrying fields intact", () => { + const tool: Tool = { + name: "write", + description: "", + parameters: z.object({ + path: z.string(), + content: z.string(), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-write-content-newline", + name: "write", + arguments: { path: "docs/foo.md\n", content: "hello\n" }, + }); + + expect(result).toEqual({ path: "docs/foo.md", content: "hello\n" }); + }); }); From 46265ca2ba0b58af7cf400302974f65ad5d54342 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 3 Jul 2026 16:03:31 +0000 Subject: [PATCH 3/5] fix(ai): widened identifier trim to cover title/label fields Extended the pre-dispatch whitespace strip from path/URL keys to also cover the display-label keys 'title' and 'label', so tools like eval no longer surface stray trailing newlines the model emits on the title field. Content-carrying fields (content, input, code, command, body, text) still keep their trailing whitespace. Added an eval-tool regression using the actual ArkType enum shape to prove the language field trim survives ArkType's wire schema. Fixes #4461 --- packages/ai/CHANGELOG.md | 2 +- packages/ai/src/utils/validation.ts | 61 ++++++++++--------- .../ai/test/eval-language-whitespace.test.ts | 29 +++++++++ packages/ai/test/todo-op-whitespace.test.ts | 29 +++++++++ 4 files changed, 92 insertions(+), 29 deletions(-) create mode 100644 packages/ai/test/eval-language-whitespace.test.ts diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index e51376ff6..5f8b7678c 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed tool-call validation to strip stray trailing whitespace on schema-matching enum values and on well-known path/URL fields (`path`, `paths`, `file`, `file_path`, `url`, `uri`) before dispatch, keeping content-carrying fields (`content`, `input`, etc.) intact ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). +- Fixed tool-call validation to strip stray trailing whitespace on schema-matching enum values and on well-known identifier fields (`path`, `paths`, `file`, `file_path`, `url`, `uri`, `title`, `label`) before dispatch, keeping content-carrying fields (`content`, `input`, `code`, `command`, etc.) intact ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). ## [16.3.4] - 2026-07-03 diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index e9110c7da..33e871199 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -928,24 +928,25 @@ function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value } // ============================================================================ -// Path-like string trailing-whitespace normalization (LLM quirk). +// Identifier-string trailing-whitespace normalization (LLM quirk). // ============================================================================ // // LLMs sometimes emit tool arguments with a trailing newline dangling off a -// path/URL — the artifact of a token boundary or a JSON stream heuristic. Path +// short identifier — a path, URL, or a display label like `title`. These // values are never legitimately terminated by whitespace, so we strip trailing -// whitespace from string values on well-known path/URL properties before the -// tool ever sees them. Content-carrying properties (`content`, `input`, `body`, -// `text`, `command`) are excluded so genuine trailing newlines survive. +// whitespace from string values on the well-known keys below before the tool +// ever sees them. Content-carrying properties (`content`, `input`, `body`, +// `text`, `command`, `code`) are intentionally NOT trimmed so genuine trailing +// newlines survive on writes, patches, shell commands, and eval snippets. // ============================================================================ /** - * Property names whose values are treated as filesystem paths, URLs, or URIs. - * The trim only fires on strings sitting under one of these keys, so - * `content: "line1\n"` on `write` and similar content-carrying fields keep - * their trailing whitespace intact. + * Property names whose values are treated as short identifiers — filesystem + * paths, URLs, URIs, or display labels. The trim only fires on strings sitting + * under one of these keys, so `content: "line1\n"` on `write` and `code: + * "console.log('hi')\n"` on `eval` keep their trailing whitespace intact. */ -const PATH_LIKE_KEYS: ReadonlySet = new Set([ +const IDENTIFIER_STRING_KEYS: ReadonlySet = new Set([ "path", "paths", "file", @@ -954,6 +955,8 @@ const PATH_LIKE_KEYS: ReadonlySet = new Set([ "filepath", "url", "uri", + "title", + "label", ]); const TRAILING_WHITESPACE_RE = /\s+$/; @@ -963,7 +966,7 @@ function trimTrailingWhitespaceString(input: string): string { return input.replace(TRAILING_WHITESPACE_RE, ""); } -function trimPathLikeStringLeaf(input: unknown): unknown { +function trimIdentifierStringLeaf(input: unknown): unknown { if (typeof input === "string") { const trimmed = trimTrailingWhitespaceString(input); return trimmed === input ? input : trimmed; @@ -989,15 +992,16 @@ function trimPathLikeStringLeaf(input: unknown): unknown { /** * Recursively strip trailing whitespace from string values whose property key - * matches {@link PATH_LIKE_KEYS}. Runs by property name only (schema-agnostic) - * so it fires uniformly across Zod, ArkType, and plain JSON Schema tools. + * matches {@link IDENTIFIER_STRING_KEYS}. Runs by property name only + * (schema-agnostic) so it fires uniformly across Zod, ArkType, and plain JSON + * Schema tools. */ -function normalizePathLikeFieldWhitespace(value: unknown): { value: unknown; changed: boolean } { +function normalizeIdentifierStringWhitespace(value: unknown): { value: unknown; changed: boolean } { if (Array.isArray(value)) { let changed = false; let next = value; for (let i = 0; i < value.length; i += 1) { - const normalized = normalizePathLikeFieldWhitespace(value[i]); + const normalized = normalizeIdentifierStringWhitespace(value[i]); if (!normalized.changed) continue; if (!changed) { next = [...value]; @@ -1015,11 +1019,11 @@ function normalizePathLikeFieldWhitespace(value: unknown): { value: unknown; cha let out: Record = source; for (const [key, entry] of Object.entries(source)) { let nextEntry = entry; - if (PATH_LIKE_KEYS.has(key)) { - const trimmed = trimPathLikeStringLeaf(entry); + if (IDENTIFIER_STRING_KEYS.has(key)) { + const trimmed = trimIdentifierStringLeaf(entry); if (trimmed !== entry) nextEntry = trimmed; } - const nested = normalizePathLikeFieldWhitespace(nextEntry); + const nested = normalizeIdentifierStringWhitespace(nextEntry); if (nested.changed) nextEntry = nested.value; if (nextEntry === entry) continue; if (!changed) { @@ -1687,13 +1691,14 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ changed = true; } - // Strip trailing whitespace from string values on well-known path/URL - // property names. Some models tack a newline onto a path arg from stream - // artifacts; downstream tools then either fail to stat the target or - // annotate a "corrected from" hint the model misreads as tool corruption. - const pathLikeNormalization = normalizePathLikeFieldWhitespace(normalizedArgs); - if (pathLikeNormalization.changed) { - normalizedArgs = pathLikeNormalization.value; + // Strip trailing whitespace from string values on well-known + // identifier-like property names (paths, URLs, titles). Some models tack + // a newline onto a short-identifier arg from stream artifacts; downstream + // tools then either fail to stat the target or annotate a "corrected + // from" hint the model misreads as tool corruption. + const identifierStringNormalization = normalizeIdentifierStringWhitespace(normalizedArgs); + if (identifierStringNormalization.changed) { + normalizedArgs = identifierStringNormalization.value; changed = true; } @@ -1744,9 +1749,9 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ normalizedArgs = enumStringNormalizationPass.value; } - const pathLikeNormalizationPass = normalizePathLikeFieldWhitespace(normalizedArgs); - if (pathLikeNormalizationPass.changed) { - normalizedArgs = pathLikeNormalizationPass.value; + const identifierStringNormalizationPass = normalizeIdentifierStringWhitespace(normalizedArgs); + if (identifierStringNormalizationPass.changed) { + normalizedArgs = identifierStringNormalizationPass.value; } // Re-run the union-string coercion because `coerceArgsFromIssues` may diff --git a/packages/ai/test/eval-language-whitespace.test.ts b/packages/ai/test/eval-language-whitespace.test.ts new file mode 100644 index 000000000..4d0b47f2e --- /dev/null +++ b/packages/ai/test/eval-language-whitespace.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "bun:test"; +import type { Tool } from "@oh-my-pi/pi-ai/types"; +import { validateToolArguments } from "@oh-my-pi/pi-ai/utils/validation"; +import { type } from "arktype"; + +describe("Eval-tool language whitespace normalization", () => { + it("trims a trailing newline on the ArkType-emitted language enum", () => { + const tool: Tool = { + name: "eval", + description: "", + parameters: type({ + language: type("'py' | 'js' | 'rb' | 'jl'").describe(""), + code: type("string").describe(""), + "title?": type("string").describe(""), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-eval-language-newline", + name: "eval", + arguments: { language: "js\n", code: "console.log('hi')", title: "smoke" }, + }) as { language: string; code: string; title?: string }; + + expect(result.language).toBe("js"); + expect(result.code).toBe("console.log('hi')"); + expect(result.title).toBe("smoke"); + }); +}); diff --git a/packages/ai/test/todo-op-whitespace.test.ts b/packages/ai/test/todo-op-whitespace.test.ts index 63c09a392..cf46a581c 100644 --- a/packages/ai/test/todo-op-whitespace.test.ts +++ b/packages/ai/test/todo-op-whitespace.test.ts @@ -89,4 +89,33 @@ describe("Tool argument whitespace normalization", () => { expect(result).toEqual({ path: "docs/foo.md", content: "hello\n" }); }); + + it("trims trailing whitespace from title fields while keeping code content", () => { + const tool: Tool = { + name: "eval", + description: "", + parameters: z.object({ + language: z.enum(["py", "js", "rb", "jl"]), + code: z.string(), + title: z.string().optional(), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-eval-title-newline", + name: "eval", + arguments: { + language: "js\n", + title: "read multi_observation lines 36-100\n", + code: "console.log('hi')\n", + }, + }); + + expect(result).toEqual({ + language: "js", + title: "read multi_observation lines 36-100", + code: "console.log('hi')\n", + }); + }); }); From 9826e20b393b20ce6c0670cd0c23ef13bd6ac34d Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 3 Jul 2026 16:08:44 +0000 Subject: [PATCH 4/5] fix(ai): resolved ref schemas during enum whitespace trim Resolved local JSON Schema refs before trimming enum/const string values so plain JSON Schema tools using or definitions get the same normalization path as inline enum schemas. Added regression coverage for a referenced enum and a legacy definitions const. Fixes #4461 --- packages/ai/src/utils/validation.ts | 53 ++++++++++++++++++--- packages/ai/test/todo-op-whitespace.test.ts | 31 ++++++++++++ 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 33e871199..61eb1556d 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -835,21 +835,60 @@ function normalizeOptionalNullsForSchema( return { value: changed ? nextValue : value, changed }; } -function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value: unknown; changed: boolean } { +function decodeJsonPointerToken(token: string): string { + return token.replace(/~1/g, "/").replace(/~0/g, "~"); +} + +function resolveLocalJsonSchemaRef(root: unknown, ref: string): unknown | undefined { + if (ref === "#") return root; + if (!ref.startsWith("#/")) return undefined; + let current: unknown = root; + for (const rawToken of ref.slice(2).split("/")) { + const token = decodeJsonPointerToken(rawToken); + if (current === null || typeof current !== "object") return undefined; + current = (current as Record)[token]; + } + return current; +} + +function normalizeEnumStringWhitespace( + schema: unknown, + value: unknown, + root: unknown = schema, + refs: ReadonlySet = new Set(), +): { value: unknown; changed: boolean } { if (value === null || value === undefined) return { value, changed: false }; if (schema === null || typeof schema !== "object") return { value, changed: false }; const schemaObject = schema as Record; + const ref = schemaObject.$ref; + if (typeof ref === "string") { + if (refs.has(ref)) return { value, changed: false }; + const resolved = resolveLocalJsonSchemaRef(root, ref); + if (resolved === undefined) return { value, changed: false }; + return normalizeEnumStringWhitespace(resolved, value, root, new Set([...refs, ref])); + } + + const branchMatches = (branch: unknown, candidate: unknown): boolean => { + if (branch !== null && typeof branch === "object") { + const branchRef = (branch as Record).$ref; + if (typeof branchRef === "string" && !refs.has(branchRef)) { + const resolved = resolveLocalJsonSchemaRef(root, branchRef); + if (resolved !== undefined) return branchMatchesSchema(resolved, candidate); + } + } + return branchMatchesSchema(branch, candidate); + }; const normalizeAnyOfLike = (keyword: "anyOf" | "oneOf"): { value: unknown; changed: boolean } => { const branches = schemaObject[keyword]; if (!Array.isArray(branches)) return { value, changed: false }; - if (branches.some(branch => branchMatchesSchema(branch, value))) return { value, changed: false }; + if (branches.some(branch => branchMatches(branch, value))) return { value, changed: false }; for (const branch of branches) { - const normalized = normalizeEnumStringWhitespace(branch, value); + const normalized = normalizeEnumStringWhitespace(branch, value, root, refs); if (!normalized.changed) continue; - if (branchMatchesSchema(branch, normalized.value)) return normalized; + if (branchMatches(branch, normalized.value)) return normalized; } return { value, changed: false }; }; @@ -864,7 +903,7 @@ function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value let changed = false; let nextValue: unknown = value; for (const branch of schemaObject.allOf) { - const normalized = normalizeEnumStringWhitespace(branch, nextValue); + const normalized = normalizeEnumStringWhitespace(branch, nextValue, root, refs); if (!normalized.changed) continue; nextValue = normalized.value; changed = true; @@ -895,7 +934,7 @@ function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value let changed = false; let nextValue = value; for (let i = 0; i < value.length; i += 1) { - const normalized = normalizeEnumStringWhitespace(itemSchema, value[i]); + const normalized = normalizeEnumStringWhitespace(itemSchema, value[i], root, refs); if (!normalized.changed) continue; if (!changed) { nextValue = [...value]; @@ -916,7 +955,7 @@ function normalizeEnumStringWhitespace(schema: unknown, value: unknown): { value let nextValue = valueObject; for (const [key, propertySchema] of Object.entries(propsObject)) { if (!(key in nextValue)) continue; - const normalized = normalizeEnumStringWhitespace(propertySchema, nextValue[key]); + const normalized = normalizeEnumStringWhitespace(propertySchema, nextValue[key], root, refs); if (!normalized.changed) continue; if (!changed) { nextValue = { ...nextValue }; diff --git a/packages/ai/test/todo-op-whitespace.test.ts b/packages/ai/test/todo-op-whitespace.test.ts index cf46a581c..736458da0 100644 --- a/packages/ai/test/todo-op-whitespace.test.ts +++ b/packages/ai/test/todo-op-whitespace.test.ts @@ -24,6 +24,37 @@ describe("Tool argument whitespace normalization", () => { expect(result).toEqual({ op: "init", items: ["Fix RNG divergence"] }); }); + it("trims trailing whitespace from enum and const strings behind local JSON Schema refs", () => { + const tool: Tool = { + name: "todo", + description: "", + parameters: { + type: "object", + properties: { + op: { $ref: "#/$defs/Op" }, + view: { $ref: "#/definitions/View" }, + }, + required: ["op", "view"], + additionalProperties: false, + $defs: { + Op: { enum: ["init", "done"] }, + }, + definitions: { + View: { const: "summary" }, + }, + }, + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-json-schema-ref-enum-newline", + name: "todo", + arguments: { op: "init\n", view: "summary\n" }, + }); + + expect(result).toEqual({ op: "init", view: "summary" }); + }); + it("strips trailing newlines from path fields on read-like tools", () => { const tool: Tool = { name: "read", From a979713d08cd537cc8428c8000048a4871fe72b4 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 3 Jul 2026 23:28:46 +0000 Subject: [PATCH 5/5] fix(ai): tightened tool-argument whitespace normalization Resolved Codex review feedback by narrowing identifier normalization to line terminators, preserving POSIX-significant trailing spaces, skipping content payload subtrees, re-running identifier normalization after stringified array coercion, and normalizing tuple prefixItems. Added regressions for ordinary trailing spaces in paths, stringified path arrays, tuple enum arguments, and nested content payloads. Fixes #4461 --- packages/ai/CHANGELOG.md | 2 +- packages/ai/src/utils/validation.ts | 83 ++++++++++++++------- packages/ai/test/todo-op-whitespace.test.ts | 65 +++++++++++++++- 3 files changed, 120 insertions(+), 30 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 5f8b7678c..c42b7eca3 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed tool-call validation to strip stray trailing whitespace on schema-matching enum values and on well-known identifier fields (`path`, `paths`, `file`, `file_path`, `url`, `uri`, `title`, `label`) before dispatch, keeping content-carrying fields (`content`, `input`, `code`, `command`, etc.) intact ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). +- Fixed tool-call validation to strip stray trailing line terminators on schema-matching enum values and on well-known identifier fields (`path`, `paths`, `file`, `file_path`, `url`, `uri`, `title`, `label`) before dispatch, keeping ordinary trailing spaces and content-carrying fields (`content`, `input`, `code`, `command`, etc.) intact ([#4461](https://github.com/can1357/oh-my-pi/issues/4461)). ## [16.3.4] - 2026-07-03 diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 61eb1556d..04e782e59 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -927,20 +927,34 @@ function normalizeEnumStringWhitespace( } if (Array.isArray(value)) { - const itemSchema = schemaObject.items; - if (itemSchema === null || typeof itemSchema !== "object" || Array.isArray(itemSchema)) { - return { value, changed: false }; - } let changed = false; let nextValue = value; - for (let i = 0; i < value.length; i += 1) { - const normalized = normalizeEnumStringWhitespace(itemSchema, value[i], root, refs); - if (!normalized.changed) continue; - if (!changed) { - nextValue = [...value]; - changed = true; + const prefixItems = schemaObject.prefixItems; + if (Array.isArray(prefixItems)) { + for (let i = 0; i < value.length && i < prefixItems.length; i += 1) { + const itemSchema = prefixItems[i]; + const normalized = normalizeEnumStringWhitespace(itemSchema, value[i], root, refs); + if (!normalized.changed) continue; + if (!changed) { + nextValue = [...value]; + changed = true; + } + nextValue[i] = normalized.value; + } + } + + const itemSchema = schemaObject.items; + if (itemSchema !== null && typeof itemSchema === "object" && !Array.isArray(itemSchema)) { + for (let i = 0; i < value.length; i += 1) { + if (Array.isArray(prefixItems) && i < prefixItems.length) continue; + const normalized = normalizeEnumStringWhitespace(itemSchema, nextValue[i], root, refs); + if (!normalized.changed) continue; + if (!changed) { + nextValue = [...value]; + changed = true; + } + nextValue[i] = normalized.value; } - nextValue[i] = normalized.value; } return { value: changed ? nextValue : value, changed }; } @@ -972,18 +986,19 @@ function normalizeEnumStringWhitespace( // // LLMs sometimes emit tool arguments with a trailing newline dangling off a // short identifier — a path, URL, or a display label like `title`. These -// values are never legitimately terminated by whitespace, so we strip trailing -// whitespace from string values on the well-known keys below before the tool -// ever sees them. Content-carrying properties (`content`, `input`, `body`, -// `text`, `command`, `code`) are intentionally NOT trimmed so genuine trailing -// newlines survive on writes, patches, shell commands, and eval snippets. +// values are never legitimately terminated by line breaks, so we strip trailing +// line terminators from string values on the well-known keys below before the +// tool ever sees them. Content-carrying properties (`content`, `input`, `body`, +// `text`, `command`, `code`) are intentionally not traversed or trimmed so +// genuine trailing whitespace survives on writes, patches, shell commands, and +// eval snippets. // ============================================================================ /** * Property names whose values are treated as short identifiers — filesystem * paths, URLs, URIs, or display labels. The trim only fires on strings sitting - * under one of these keys, so `content: "line1\n"` on `write` and `code: - * "console.log('hi')\n"` on `eval` keep their trailing whitespace intact. + * under one of these keys, so `path: "docs/report "` still targets the file + * whose name ends in a space. */ const IDENTIFIER_STRING_KEYS: ReadonlySet = new Set([ "path", @@ -998,16 +1013,18 @@ const IDENTIFIER_STRING_KEYS: ReadonlySet = new Set([ "label", ]); -const TRAILING_WHITESPACE_RE = /\s+$/; +const CONTENT_CARRYING_KEYS: ReadonlySet = new Set(["content", "input", "body", "text", "command", "code"]); -function trimTrailingWhitespaceString(input: string): string { - if (!TRAILING_WHITESPACE_RE.test(input)) return input; - return input.replace(TRAILING_WHITESPACE_RE, ""); +const TRAILING_LINE_TERMINATOR_RE = /[\r\n]+$/; + +function trimTrailingLineTerminators(input: string): string { + if (!TRAILING_LINE_TERMINATOR_RE.test(input)) return input; + return input.replace(TRAILING_LINE_TERMINATOR_RE, ""); } function trimIdentifierStringLeaf(input: unknown): unknown { if (typeof input === "string") { - const trimmed = trimTrailingWhitespaceString(input); + const trimmed = trimTrailingLineTerminators(input); return trimmed === input ? input : trimmed; } if (Array.isArray(input)) { @@ -1016,7 +1033,7 @@ function trimIdentifierStringLeaf(input: unknown): unknown { for (let i = 0; i < input.length; i += 1) { const item = input[i]; if (typeof item !== "string") continue; - const trimmed = trimTrailingWhitespaceString(item); + const trimmed = trimTrailingLineTerminators(item); if (trimmed === item) continue; if (!changed) { next = input.slice(); @@ -1030,10 +1047,10 @@ function trimIdentifierStringLeaf(input: unknown): unknown { } /** - * Recursively strip trailing whitespace from string values whose property key - * matches {@link IDENTIFIER_STRING_KEYS}. Runs by property name only + * Recursively strip trailing line terminators from string values whose property + * key matches {@link IDENTIFIER_STRING_KEYS}. Runs by property name only * (schema-agnostic) so it fires uniformly across Zod, ArkType, and plain JSON - * Schema tools. + * Schema tools while preserving nested payloads under content-carrying keys. */ function normalizeIdentifierStringWhitespace(value: unknown): { value: unknown; changed: boolean } { if (Array.isArray(value)) { @@ -1058,6 +1075,7 @@ function normalizeIdentifierStringWhitespace(value: unknown): { value: unknown; let out: Record = source; for (const [key, entry] of Object.entries(source)) { let nextEntry = entry; + if (CONTENT_CARRYING_KEYS.has(key)) continue; if (IDENTIFIER_STRING_KEYS.has(key)) { const trimmed = trimIdentifierStringLeaf(entry); if (trimmed !== entry) nextEntry = trimmed; @@ -1751,6 +1769,12 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ changed = true; } + const identifierStringNormalizationAfterArray = normalizeIdentifierStringWhitespace(normalizedArgs); + if (identifierStringNormalizationAfterArray.changed) { + normalizedArgs = identifierStringNormalizationAfterArray.value; + changed = true; + } + // Single-argument tools (e.g. `edit`): if the model put the lone required // string under a different key, adopt the first string field as that key. const singleStringNorm = normalizeSingleStringField(json, normalizedArgs); @@ -1802,6 +1826,11 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ normalizedArgs = stringEncodedArrayNormPass.value; } + const identifierStringNormalizationAfterArrayPass = normalizeIdentifierStringWhitespace(normalizedArgs); + if (identifierStringNormalizationAfterArrayPass.changed) { + normalizedArgs = identifierStringNormalizationAfterArrayPass.value; + } + // Re-run single-string remap: `coerceArgsFromIssues` may have just // unwrapped a JSON-stringified root object, exposing a mislabelled lone // string field the initial pre-pass could not see. diff --git a/packages/ai/test/todo-op-whitespace.test.ts b/packages/ai/test/todo-op-whitespace.test.ts index 736458da0..49a24ffd5 100644 --- a/packages/ai/test/todo-op-whitespace.test.ts +++ b/packages/ai/test/todo-op-whitespace.test.ts @@ -55,6 +55,25 @@ describe("Tool argument whitespace normalization", () => { expect(result).toEqual({ op: "init", view: "summary" }); }); + it("trims enum strings inside tuple prefix items", () => { + const tool: Tool = { + name: "tuple-op", + description: "", + parameters: z.object({ + args: z.tuple([z.enum(["init"])]), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-tuple-enum-newline", + name: "tuple-op", + arguments: { args: ["init\n"] }, + }); + + expect(result).toEqual({ args: ["init"] }); + }); + it("strips trailing newlines from path fields on read-like tools", () => { const tool: Tool = { name: "read", @@ -75,7 +94,7 @@ describe("Tool argument whitespace normalization", () => { expect(result).toEqual({ path: "examples/multi_observation.py:36-55" }); }); - it("strips trailing whitespace from every entry in a path array", () => { + it("strips trailing line terminators but preserves ordinary spaces in path arrays", () => { const tool: Tool = { name: "search", description: "", @@ -97,10 +116,31 @@ describe("Tool argument whitespace normalization", () => { expect(result).toEqual({ pattern: "TODO", - paths: ["src/foo.ts", "src/bar.ts"], + paths: ["src/foo.ts", "src/bar.ts "], }); }); + it("trims path line terminators after stringified array coercion", () => { + const tool: Tool = { + name: "search", + description: "", + parameters: z.object({ + paths: z.union([z.string(), z.array(z.string())]), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-search-stringified-paths-newline", + name: "search", + arguments: { + paths: JSON.stringify(["src/foo.ts\n", "src/bar.ts "]), + }, + }); + + expect(result).toEqual({ paths: ["src/foo.ts", "src/bar.ts "] }); + }); + it("leaves trailing newlines on content-carrying fields intact", () => { const tool: Tool = { name: "write", @@ -121,6 +161,27 @@ describe("Tool argument whitespace normalization", () => { expect(result).toEqual({ path: "docs/foo.md", content: "hello\n" }); }); + it("does not trim identifier-looking fields nested under content payloads", () => { + const tool: Tool = { + name: "http", + description: "", + parameters: z.object({ + body: z.object({ + title: z.string(), + }), + }), + }; + + const result = validateToolArguments(tool, { + type: "toolCall", + id: "call-body-title-space", + name: "http", + arguments: { body: { title: "Draft \n" } }, + }); + + expect(result).toEqual({ body: { title: "Draft \n" } }); + }); + it("trims trailing whitespace from title fields while keeping code content", () => { const tool: Tool = { name: "eval",