diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 20ef357ef..c42b7eca3 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- 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 ### Added diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index 9c78c8485..04e782e59 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -835,6 +835,263 @@ function normalizeOptionalNullsForSchema( return { value: changed ? nextValue : value, changed }; } +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 => branchMatches(branch, value))) return { value, changed: false }; + + for (const branch of branches) { + const normalized = normalizeEnumStringWhitespace(branch, value, root, refs); + if (!normalized.changed) continue; + if (branchMatches(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, root, refs); + 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)) { + let changed = false; + let nextValue = value; + 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; + } + } + 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], root, refs); + if (!normalized.changed) continue; + if (!changed) { + nextValue = { ...nextValue }; + changed = true; + } + nextValue[key] = normalized.value; + } + return { value: changed ? nextValue : valueObject, changed }; +} + +// ============================================================================ +// Identifier-string trailing-whitespace normalization (LLM quirk). +// ============================================================================ +// +// 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 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 `path: "docs/report "` still targets the file + * whose name ends in a space. + */ +const IDENTIFIER_STRING_KEYS: ReadonlySet = new Set([ + "path", + "paths", + "file", + "file_path", + "filePath", + "filepath", + "url", + "uri", + "title", + "label", +]); + +const CONTENT_CARRYING_KEYS: ReadonlySet = new Set(["content", "input", "body", "text", "command", "code"]); + +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 = trimTrailingLineTerminators(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 = trimTrailingLineTerminators(item); + if (trimmed === item) continue; + if (!changed) { + next = input.slice(); + changed = true; + } + next[i] = trimmed; + } + return changed ? next : input; + } + return input; +} + +/** + * 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 while preserving nested payloads under content-carrying keys. + */ +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 = normalizeIdentifierStringWhitespace(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 (CONTENT_CARRYING_KEYS.has(key)) continue; + if (IDENTIFIER_STRING_KEYS.has(key)) { + const trimmed = trimIdentifierStringLeaf(entry); + if (trimmed !== entry) nextEntry = trimmed; + } + const nested = normalizeIdentifierStringWhitespace(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). // ============================================================================ @@ -1485,6 +1742,23 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ changed = true; } + const enumStringNormalization = normalizeEnumStringWhitespace(json, normalizedArgs); + if (enumStringNormalization.changed) { + normalizedArgs = enumStringNormalization.value; + changed = true; + } + + // 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; + } + // 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 @@ -1495,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); @@ -1527,6 +1807,16 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[ normalizedArgs = nullNormalization.value; } + const enumStringNormalizationPass = normalizeEnumStringWhitespace(json, normalizedArgs); + if (enumStringNormalizationPass.changed) { + normalizedArgs = enumStringNormalizationPass.value; + } + + const identifierStringNormalizationPass = normalizeIdentifierStringWhitespace(normalizedArgs); + if (identifierStringNormalizationPass.changed) { + normalizedArgs = identifierStringNormalizationPass.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 @@ -1536,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/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 new file mode 100644 index 000000000..49a24ffd5 --- /dev/null +++ b/packages/ai/test/todo-op-whitespace.test.ts @@ -0,0 +1,213 @@ +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 argument whitespace normalization", () => { + 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"] }); + }); + + 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("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", + 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 line terminators but preserves ordinary spaces in path arrays", () => { + 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("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", + 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" }); + }); + + 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", + 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", + }); + }); +});