fix(sdk): accept flattened array argument paths from providers

Some providers (notably Gemini) serialize array tool arguments with flattened property paths — questions[0].id, questions[0].options[0].label — instead of a nested questions array. The schema sees only unrecognized extra keys and rejects the call (e.g. the ask tool). Add a pre-validation normalization pass (alongside the existing LLM-quirk passes) that rebuilds the nested structure. Conservative: fires only when a key is a well-formed array-index path, preserves non-flattened siblings, and aborts wholesale on any shape conflict so genuine schema mistakes still surface as validation errors. Fixes #8886
2026-08-19 02:33:47 +08:00
parent 8500092296
commit adf2595931
2 changed files with 328 additions and 0 deletions
@@ -1360,6 +1360,184 @@ function normalizeSingleStringField(schema: unknown, value: unknown): { value: u
 	return { value, changed: false };
 }

+// ============================================================================
+// Flattened array-property normalization (LLM quirk).
+// ============================================================================
+//
+// Some providers (notably Gemini) serialize array arguments using flattened
+// property paths — `questions[0].id`, `questions[0].options[0].label`, ... —
+// instead of a nested `questions` array of objects. The schema sees only
+// unrecognized extra keys and rejects the call. This pass rebuilds the nested
+// structure before the schema ever runs.
+//
+// Conservative by design:
+//   - fires only when at least one key is a well-formed array-index path
+//     (`name[i]`, `name[i].prop`, `name[i][j]`, ...); plain keys and
+//     non-array dotted keys (`a.b`) never match;
+//   - aborts wholesale (returns unchanged) on any shape conflict so genuine
+//     schema mistakes still surface as validation errors;
+//   - array indices are capped so a runaway/hostile payload cannot allocate
+//     oversized arrays.
+// ============================================================================
+
+/** Cap on array indices accepted by the flattened-path parser. */
+const MAX_FLATTENED_INDEX = 100_000;
+
+interface FlattenedPathStep {
+	kind: "prop" | "index";
+	/** For `kind: "prop"` — the property name. */
+	name?: string;
+	/** For `kind: "index"` — the resolved array index. */
+	index: number;
+}
+
+interface ParsedFlattenedPath {
+	steps: FlattenedPathStep[];
+}
+
+const FLATTENED_IDENT_RE = /^[A-Za-z_$][A-Za-z0-9_$]*/;
+const FLATTENED_INDEX_RE = /^\[(\d+)\]/;
+
+/**
+ * Parse a single flattened array-path key into build steps. Returns `null` for
+ * keys that are not flattened array paths:
+ *   - no `[<digits>]` index anywhere (`questions`, `a.b`),
+ *   - a malformed or non-numeric index (`foo[bar]`),
+ *   - an index-first path (`[0].x`),
+ *   - an index outside the safety cap.
+ */
+function parseFlattenedPath(key: string): ParsedFlattenedPath | null {
+	if (key.length === 0) return null;
+	const steps: FlattenedPathStep[] = [];
+	// The path must start with a property name so `[0].x` / `[0]` are left alone.
+	const first = FLATTENED_IDENT_RE.exec(key);
+	if (!first) return null;
+	steps.push({ kind: "prop", name: first[0], index: 0 });
+	let pos = first[0].length;
+	let sawIndex = false;
+	while (pos < key.length) {
+		if (key[pos] === ".") {
+			pos++;
+			const m = FLATTENED_IDENT_RE.exec(key.slice(pos));
+			if (!m || m[0].length === 0) return null;
+			steps.push({ kind: "prop", name: m[0], index: 0 });
+			pos += m[0].length;
+			continue;
+		}
+		if (key[pos] === "[") {
+			const m = FLATTENED_INDEX_RE.exec(key.slice(pos));
+			if (!m) return null;
+			const index = Number(m[1]);
+			if (!Number.isSafeInteger(index) || index < 0 || index > MAX_FLATTENED_INDEX) return null;
+			steps.push({ kind: "index", index });
+			sawIndex = true;
+			pos += m[0].length;
+			continue;
+		}
+		// Any other character (lone `[foo]`, whitespace, invalid ident chars) is
+		// not a flattened array path.
+		return null;
+	}
+	if (!sawIndex) return null;
+	return { steps };
+}
+
+/**
+ * Write a leaf value into `root` along `steps`, creating intermediate objects
+ * and arrays as needed. Returns `false` (and leaves `root` in an undefined
+ * partial state — the caller aborts the whole normalization on that) when an
+ * existing node has a shape that contradicts the path.
+ */
+function buildFlattenedPath(root: Record<string, unknown>, steps: FlattenedPathStep[], value: unknown): boolean {
+	let node: unknown = root;
+	for (let i = 0; i < steps.length - 1; i++) {
+		const step = steps[i];
+		const nextIsArray = steps[i + 1].kind === "index";
+		if (step.kind === "prop") {
+			const obj = node as Record<string, unknown>;
+			if (obj === null || typeof obj !== "object" || Array.isArray(obj)) return false;
+			const existing = Object.hasOwn(obj, step.name!) ? obj[step.name!] : undefined;
+			let child: unknown;
+			if (existing === undefined && !Object.hasOwn(obj, step.name!)) {
+				child = nextIsArray ? [] : {};
+			} else {
+				if (Array.isArray(existing) !== nextIsArray) return false;
+				child = existing;
+			}
+			// `defineProperty` so a decoded `__proto__` step becomes an own property.
+			Object.defineProperty(obj, step.name!, {
+				value: child,
+				writable: true,
+				enumerable: true,
+				configurable: true,
+			});
+			node = child;
+			continue;
+		}
+		const arr = node;
+		if (!Array.isArray(arr)) return false;
+		while (arr.length <= step.index) arr.push(undefined);
+		let child = arr[step.index];
+		if (child === undefined) {
+			child = nextIsArray ? [] : {};
+			arr[step.index] = child;
+		} else {
+			if (Array.isArray(child)) {
+				if (!nextIsArray) return false;
+			} else if (typeof child !== "object" || child === null) {
+				return false;
+			} else if (nextIsArray) {
+				return false;
+			}
+		}
+		node = child;
+	}
+	const last = steps[steps.length - 1];
+	if (last.kind === "prop") {
+		const obj = node as Record<string, unknown>;
+		if (obj === null || typeof obj !== "object" || Array.isArray(obj)) return false;
+		Object.defineProperty(obj, last.name!, {
+			value,
+			writable: true,
+			enumerable: true,
+			configurable: true,
+		});
+	} else {
+		const arr = node;
+		if (!Array.isArray(arr)) return false;
+		while (arr.length <= last.index) arr.push(undefined);
+		arr[last.index] = value;
+	}
+	return true;
+}
+
+/**
+ * Rebuild nested arrays/objects from LLM-emitted flattened property paths.
+ * See https://github.com/can1357/oh-my-pi/issues/8886.
+ */
+function normalizeFlattenedArrayProperties(value: unknown): { value: unknown; changed: boolean } {
+	if (!isPlainRecord(value)) return { value, changed: false };
+	const source = value as Record<string, unknown>;
+	const out: Record<string, unknown> = {};
+	let changed = false;
+	for (const [key, entry] of Object.entries(source)) {
+		const parsed = parseFlattenedPath(key);
+		if (!parsed) {
+			// Preserve non-flattened sibling keys. A plain key colliding with an
+			// already-built path is ambiguous — bail to the safer failure path so
+			// genuine schema mistakes still surface.
+			if (Object.hasOwn(out, key)) return { value, changed: false };
+			Object.defineProperty(out, key, { value: entry, writable: true, enumerable: true, configurable: true });
+			continue;
+		}
+		if (entry === undefined) continue;
+		if (!buildFlattenedPath(out, parsed.steps, entry)) return { value, changed: false };
+		changed = true;
+	}
+	if (!changed) return { value, changed: false };
+	return { value: out, changed: true };
+}
+
 // Validation issue → coercion bridge

 interface FlatIssue {
@@ -1765,6 +1943,16 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[
 		changed = true;
 	}

+	// Rebuild nested arrays/objects from flattened property paths some
+	// providers emit instead of real arrays (`questions[0].id`, ...). Runs
+	// after key unwrapping but before any schema pass so the validator sees the
+	// structurally correct payload.
+	const flattenedArgs = normalizeFlattenedArrayProperties(normalizedArgs);
+	if (flattenedArgs.changed) {
+		normalizedArgs = flattenedArgs.value;
+		changed = true;
+	}
+
 	const initialNormalization = normalizeOptionalNullsForSchema(json, normalizedArgs);
 	if (initialNormalization.changed) {
 		normalizedArgs = initialNormalization.value;
@@ -0,0 +1,140 @@
+import { describe, expect, it } from "bun:test";
+import { type } from "@oh-my-pi/omptype";
+import type { Tool } from "@oh-my-pi/pi-ai/types";
+import { validateToolArguments } from "@oh-my-pi/pi-ai/utils/validation";
+
+// Issue #8886 — some providers (notably Gemini) serialize array arguments as
+// flattened property paths (`questions[0].id`) instead of a nested array.
+
+// Mirrors the shape of OMP's `ask` tool (`packages/coding-agent/src/tools/ask.ts`).
+const questionItem = type({
+	id: type("string"),
+	question: type("string"),
+	options: type({ label: type("string") }).array(),
+	"recommended?": type("number"),
+});
+
+const askTool: Tool = {
+	name: "ask",
+	description: "Ask the user a question",
+	parameters: type({ questions: questionItem.array().atLeastLength(1) }),
+};
+
+function callWith(
+	parameters: Record<string, unknown>,
+	tool: Tool = askTool,
+): { success: boolean; args: unknown; error?: unknown } {
+	try {
+		return {
+			success: true,
+			args: validateToolArguments(tool, {
+				type: "toolCall",
+				id: "call-1",
+				name: tool.name,
+				arguments: parameters,
+			}),
+		};
+	} catch (error) {
+		return { success: false, args: parameters, error };
+	}
+}
+
+describe("Flattened array-property normalization (issue #8886)", () => {
+	it("rebuilds a nested questions array from flattened property paths", () => {
+		const result = callWith({
+			"questions[0].id": "doc_structure",
+			"questions[0].question": "Which format should we adopt?",
+			"questions[0].options[0].label": "Structured Markdown",
+			"questions[0].options[1].label": "Plain text",
+			"questions[0].recommended": 0,
+		});
+
+		expect(result.success).toBe(true);
+		expect(result.args).toEqual({
+			questions: [
+				{
+					id: "doc_structure",
+					question: "Which format should we adopt?",
+					options: [{ label: "Structured Markdown" }, { label: "Plain text" }],
+					recommended: 0,
+				},
+			],
+		});
+	});
+
+	it("handles multiple array elements across the same property", () => {
+		const result = callWith({
+			"questions[0].id": "q1",
+			"questions[0].question": "First",
+			"questions[0].options[0].label": "A",
+			"questions[1].id": "q2",
+			"questions[1].question": "Second",
+			"questions[1].options[0].label": "B",
+		});
+
+		expect(result.success).toBe(true);
+		expect(result.args).toEqual({
+			questions: [
+				{ id: "q1", question: "First", options: [{ label: "A" }] },
+				{ id: "q2", question: "Second", options: [{ label: "B" }] },
+			],
+		});
+	});
+
+	it("supports bare leaf array elements", () => {
+		const tool: Tool = {
+			name: "t",
+			description: "",
+			parameters: type({ tags: type("string").array().atLeastLength(2) }),
+		};
+		const result = callWith({ "tags[0]": "alpha", "tags[1]": "beta" }, tool);
+		expect(result.success).toBe(true);
+		expect(result.args).toEqual({ tags: ["alpha", "beta"] });
+	});
+
+	it("preserves non-flattened sibling keys", () => {
+		const result = callWith({
+			title: "Session",
+			"questions[0].id": "q",
+			"questions[0].question": "Go?",
+			"questions[0].options[0].label": "Yes",
+		});
+		expect(result.success).toBe(true);
+		expect(result.args).toEqual({
+			title: "Session",
+			questions: [{ id: "q", question: "Go?", options: [{ label: "Yes" }] }],
+		});
+	});
+
+	it("leaves plain nested objects untouched", () => {
+		const args = { questions: [{ id: "q", question: "Go?", options: [{ label: "Yes" }] }] };
+		const result = callWith(args);
+		expect(result.success).toBe(true);
+		expect(result.args).toEqual(args);
+	});
+
+	it("leaves non-array dotted keys untouched", () => {
+		const tool: Tool = { name: "t", description: "", parameters: type({ "a.b": type("number"), c: type("number") }) };
+		const args = { "a.b": 1, c: 2 };
+		const result = callWith(args, tool);
+		expect(result.success).toBe(true);
+		expect(result.args).toEqual(args);
+	});
+
+	it("leaves malformed indexed keys untouched and surfaces the validation error", () => {
+		const result = callWith({ "questions[foo]": "nope" });
+		expect(result.success).toBe(false);
+	});
+
+	it("leaves non-indexed keys untouched on schema mismatch too", () => {
+		const result = callWith({ label: "300" });
+		expect(result.success).toBe(false);
+	});
+
+	it("bails (does not silently drop data) when a flattened path collides with a plain key", () => {
+		const result = callWith({ questions: [5], "questions[0].id": "x" });
+		// Ambiguous input must not lose the plain key — fall through to a genuine
+		// validation error instead of a partial rebuild.
+		expect(result.success).toBe(false);
+	});
+});