Merge remote-tracking branch 'origin/farm/d5671b86/strip-todo-op-newline'

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

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<string, unknown>)[token];
+	}
+	return current;
+}
+
+function normalizeEnumStringWhitespace(
+	schema: unknown,
+	value: unknown,
+	root: unknown = schema,
+	refs: ReadonlySet<string> = 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<string, unknown>;
+	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<string, unknown>).$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<string, unknown>;
+	const valueObject = value as Record<string, unknown>;
+	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<string> = new Set([
+	"path",
+	"paths",
+	"file",
+	"file_path",
+	"filePath",
+	"filepath",
+	"url",
+	"uri",
+	"title",
+	"label",
+]);
+
+const CONTENT_CARRYING_KEYS: ReadonlySet<string> = 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<string, unknown>;
+	let changed = false;
+	let out: Record<string, unknown> = 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.

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");
+	});
+});

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",
+		});
+	});
+});