From c13769a35c2bfa31e00c3662a7f902115a547323 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Thu, 10 Sep 2026 11:32:51 +0900 Subject: [PATCH] fix(openai): avoid eager tool schema cloning --- src/adapters/responses-tool-schema.ts | 122 +++++++++++------- .../openai/openai-chat-hardening.test.ts | 18 +++ 2 files changed, 94 insertions(+), 46 deletions(-) diff --git a/src/adapters/responses-tool-schema.ts b/src/adapters/responses-tool-schema.ts index 7d10b9beec..6b28cca6ea 100644 --- a/src/adapters/responses-tool-schema.ts +++ b/src/adapters/responses-tool-schema.ts @@ -107,66 +107,96 @@ function usesUnicodePropertyEscape(pattern: string): boolean { * preserved: loosening a nested constraint can instead reject an input in those contexts. * Unsupported patterns there remain the destination's validation responsibility. * - * Returns `node` itself when nothing was dropped. Uses an explicit stack for caller-controlled - * nesting depth; the separate Responses-only encrypted-marker normalization is unchanged. + * Returns `node` itself when nothing was dropped. Traversal keeps only the active path and clones + * only ancestors of a removed constraint, so a broad no-op schema does not create an output tree + * or one pending closure per sibling. The explicit stack still handles caller-controlled nesting + * depth; the separate Responses-only encrypted-marker normalization is unchanged. */ export function stripUnicodePropertyPatterns(node: unknown, inNameBag = false): unknown { - type Assign = (value: unknown) => void; - interface Frame { node: unknown; inNameBag: boolean; assign: Assign } - - let result: unknown; - let dropped = 0; - const stack: Frame[] = [{ node, inNameBag, assign: value => { result = value; } }]; + interface Frame { + node: unknown[] | Record; + inNameBag: boolean; + parent?: Frame; + parentKey?: string | number; + output?: unknown[] | Record; + index?: number; + entries?: IterableIterator<[string, unknown]>; + } - while (stack.length > 0) { - const frame = stack.pop()!; - const current = frame.node; + function * ownEntries(value: Record): IterableIterator<[string, unknown]> { + // Unlike Object.entries(), this does not materialize every key/value pair before traversal. + for (const key in value) { + if (Object.prototype.hasOwnProperty.call(value, key)) yield [key, value[key]]; + } + } - if (Array.isArray(current)) { - const out: unknown[] = new Array(current.length); - frame.assign(out); - // Array items are schemas in their own right, never a name bag. - for (let i = current.length - 1; i >= 0; i--) { - stack.push({ node: current[i], inNameBag: false, assign: value => { out[i] = value; } }); - } - continue; + function cloneContainer(frame: Frame): unknown[] | Record { + if (frame.output) return frame.output; + if (Array.isArray(frame.node)) { + frame.output = frame.node.slice(); + return frame.output; } - if (!current || typeof current !== "object") { - frame.assign(current); - continue; + const output: Record = Object.create(null) as Record; + for (const key in frame.node) { + if (Object.prototype.hasOwnProperty.call(frame.node, key)) output[key] = frame.node[key]; } + frame.output = output; + return output; + } - // A schema name may be `__proto__`; a null-prototype record keeps it as data. - const out: Record = Object.create(null) as Record; - frame.assign(out); + function finish(frame: Frame): void { + if (!frame.output || !frame.parent) return; + const parent = cloneContainer(frame.parent); + if (Array.isArray(parent)) parent[frame.parentKey as number] = frame.output; + else parent[frame.parentKey as string] = frame.output; + } - for (const [key, value] of Object.entries(current as Record)) { - if (frame.inNameBag) { - // Inside a name bag every key is a caller-chosen name, so `pattern` here is a property - // name; its value is still a schema and is walked as one. - stack.push({ node: value, inNameBag: false, assign: v => { out[key] = v; } }); - continue; - } - if (PRESERVED_PATTERN_SUBTREES.has(key)) { - out[key] = value; - continue; - } - if (key === "pattern" && typeof value === "string" && usesUnicodePropertyEscape(value)) { - dropped++; + if (!node || typeof node !== "object") return node; + const root: Frame = { node: node as unknown[] | Record, inNameBag }; + const stack: Frame[] = [root]; + + while (stack.length > 0) { + const frame = stack[stack.length - 1]!; + + if (Array.isArray(frame.node)) { + const index = frame.index ?? 0; + if (index >= frame.node.length) { + stack.pop(); + finish(frame); continue; } - if (SCHEMA_LITERAL_VALUE_KEYS.has(key)) { - // Literal payloads are values, not schemas: a `pattern` key inside them is data. - out[key] = value; - continue; + frame.index = index + 1; + const child = frame.node[index]; + if (child && typeof child === "object") { + stack.push({ node: child as unknown[] | Record, inNameBag: false, parent: frame, parentKey: index }); } + continue; + } + + frame.entries ??= ownEntries(frame.node); + const next = frame.entries.next(); + if (next.done) { + stack.pop(); + finish(frame); + continue; + } + const [key, value] = next.value; + if (!frame.inNameBag && key === "pattern" && typeof value === "string" && usesUnicodePropertyEscape(value)) { + delete (cloneContainer(frame) as Record)[key]; + continue; + } + if (!frame.inNameBag && (PRESERVED_PATTERN_SUBTREES.has(key) || SCHEMA_LITERAL_VALUE_KEYS.has(key))) { + continue; + } + if (value && typeof value === "object") { stack.push({ - node: value, - inNameBag: SCHEMA_NAME_BAG_KEYS.has(key), - assign: v => { out[key] = v; }, + node: value as unknown[] | Record, + inNameBag: !frame.inNameBag && SCHEMA_NAME_BAG_KEYS.has(key), + parent: frame, + parentKey: key, }); } } - return dropped === 0 ? node : result; + return root.output ?? node; } diff --git a/tests/adapters/openai/openai-chat-hardening.test.ts b/tests/adapters/openai/openai-chat-hardening.test.ts index 2279e4937e..15a912fb55 100644 --- a/tests/adapters/openai/openai-chat-hardening.test.ts +++ b/tests/adapters/openai/openai-chat-hardening.test.ts @@ -308,6 +308,24 @@ describe("unicode property-escape pattern stripping", () => { expect(stripped.properties.plain.pattern).toBe("^[a-z0-9_-]{1,64}$"); }); + test("clones only paths that contain a removed pattern in a broad schema", () => { + const properties: Record> = {}; + for (let i = 0; i < 25_000; i++) properties[`field_${i}`] = { type: "string" }; + properties.affected = { type: "string", pattern: artifactFieldPattern }; + const before = { type: "object", properties }; + + const stripped = stripUnicodePropertyPatterns(before) as typeof before; + + expect(stripped).not.toBe(before); + expect(stripped.properties).not.toBe(properties); + expect(stripped.properties.affected).not.toBe(properties.affected); + expect(stripped.properties.affected.pattern).toBeUndefined(); + // Unchanged siblings retain identity instead of being cloned while thousands of traversal + // frames and temporary output objects are live at once. + expect(stripped.properties.field_0).toBe(properties.field_0); + expect(stripped.properties.field_24999).toBe(properties.field_24999); + }); + test("an escaped backslash before `p{` is a literal, not a property escape", () => { // `\\p{2}` is a literal backslash followed by a quantified `p`; Python compiles it, so a // substring scan for `\p{` would throw away a working pattern.