From adf259593130c89072a5364279db7bccedc67b83 Mon Sep 17 00:00:00 2001 From: re2zero Date: Wed, 19 Aug 2026 02:33:47 +0800 Subject: [PATCH] fix(sdk): accept flattened array argument paths from providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/ai/src/utils/validation.ts | 188 ++++++++++++++++++ .../test/flattened-array-properties.test.ts | 140 +++++++++++++ 2 files changed, 328 insertions(+) create mode 100644 packages/ai/test/flattened-array-properties.test.ts diff --git a/packages/ai/src/utils/validation.ts b/packages/ai/src/utils/validation.ts index a91a1ca3c..16839c2b8 100644 --- a/packages/ai/src/utils/validation.ts +++ b/packages/ai/src/utils/validation.ts @@ -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 `[]` 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, 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; + 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; + 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; + const out: Record = {}; + 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; diff --git a/packages/ai/test/flattened-array-properties.test.ts b/packages/ai/test/flattened-array-properties.test.ts new file mode 100644 index 000000000..6453628b3 --- /dev/null +++ b/packages/ai/test/flattened-array-properties.test.ts @@ -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, + 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); + }); +});