From 2085db1c83331b849910803aa6f616f277ddeefa Mon Sep 17 00:00:00 2001 From: Nick Davis Date: Sat, 31 Oct 2026 10:02:05 +0100 Subject: [PATCH 1/3] fix(responses): unwrap one extra executable freeform body wrapper --- .../content/docs/guides/codex-integration.md | 5 ++ src/responses/apply-patch-envelope.ts | 20 +++++++- src/responses/progressive-freeform-input.ts | 6 ++- structure/transports/responses.md | 9 ++-- tests/responses/apply-patch-envelope.test.ts | 49 +++++++++++++++++++ ...ses-custom-tool-stream-consistency.test.ts | 35 +++++++++++++ .../responses-freeform-wrapper-keys.test.ts | 22 +++++++++ 7 files changed, 140 insertions(+), 6 deletions(-) diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 4eb3e800e34..d5b444f3210 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -750,6 +750,11 @@ Codex. Native custom calls and converted function calls use the same completion patch previews are held while their executable form is unresolved. JavaScript that merely contains patch text and unrelated native custom payloads stay unchanged. +When a routed model double-wraps an executable freeform body, opencodex removes one extra +JSON object containing only an input string. This repair applies to local exec and apply_patch +bodies, leaves other tool grammars and ambiguous objects unchanged, and does not decode recursively. +Possible nested wrappers hold their streamed preview until the completed input is available. + A routed model can also mistakenly send a shell-argument object such as `{"cmd":"git status --short"}` to code-mode `exec`. For a verified code-mode catalog, opencodex converts an unambiguous shell object into `tools.exec_command(...)` JavaScript diff --git a/src/responses/apply-patch-envelope.ts b/src/responses/apply-patch-envelope.ts index 710b1299abe..de0275abcf8 100644 --- a/src/responses/apply-patch-envelope.ts +++ b/src/responses/apply-patch-envelope.ts @@ -39,6 +39,22 @@ function stripMarkdownCodeFence(text: string, toolName: string): string { return match ? match[1] : text; } +/** Repair one extra canonical wrapper inside an already unwrapped executable body. */ +function repairUnwrappedBody(text: string, toolName: string): string { + if (toolName !== "exec" && toolName !== "apply_patch") return text; + try { + const nested: unknown = JSON.parse(text); + if (nested && typeof nested === "object" && !Array.isArray(nested)) { + const record = nested as Record; + const keys = Object.keys(record); + if (keys.length === 1 && keys[0] === "input" && typeof record.input === "string") text = record.input; + } + } catch { + // Ordinary JavaScript and patches are not JSON wrappers. + } + return stripMarkdownCodeFence(text, toolName); +} + /** Unwrap the `{input:string}` function-call wrapper used for freeform tools. */ export function unwrapFreeformToolInput(argumentsText: unknown, toolName = ""): string { if (typeof argumentsText !== "string") return ""; @@ -48,13 +64,13 @@ export function unwrapFreeformToolInput(argumentsText: unknown, toolName = ""): const record = parsed as Record; if (Object.prototype.hasOwnProperty.call(record, "input")) { return typeof record.input === "string" - ? stripMarkdownCodeFence(record.input, toolName) + ? repairUnwrappedBody(record.input, toolName) : argumentsText; } const fallbackKeys = freeformFallbackKeys(toolName); const candidates = fallbackKeys.filter(key => typeof record[key] === "string"); if (candidates.length === 1) { - return stripMarkdownCodeFence(record[candidates[0]] as string, toolName); + return repairUnwrappedBody(record[candidates[0]] as string, toolName); } } } catch { diff --git a/src/responses/progressive-freeform-input.ts b/src/responses/progressive-freeform-input.ts index e400c780941..1fc9f3cdb0b 100644 --- a/src/responses/progressive-freeform-input.ts +++ b/src/responses/progressive-freeform-input.ts @@ -77,7 +77,8 @@ function decodeJsonStringPrefix(body: string): string | null { * * `input` is decidable from its prefix: `unwrapFreeformToolInput` returns it whenever the * key is present, whatever else the object carries, so its value can be unescaped - * progressively and never retracted. + * progressively and never retracted, except for executable bodies that may contain one + * extra canonical wrapper. Those object-leading bodies hold until authoritative completion. * * One case escapes that claim and is accepted rather than fixed: a duplicate `input` key. * `JSON.parse` keeps the last one, so `{"input":"a","input":"b"}` streams a and completes with @@ -111,6 +112,9 @@ export function progressiveFreeformInput(args: string, toolName: string): string if (scan.kind === "input") { const decoded = decodeJsonStringPrefix(args.slice(scan.valueStart)); if (decoded === null) return null; + // Completion may remove one extra {input:string} body wrapper. A later member can still + // disqualify it, so hold the whole object-shaped preview rather than decode recursively. + if ((toolName === "exec" || toolName === "apply_patch") && decoded.trimStart().startsWith("{")) return null; return mayBecomeFencedBody(decoded, toolName) ? null : decoded; } if (scan.kind === "raw") return mayBecomeFencedBody(args, toolName) ? null : args; diff --git a/structure/transports/responses.md b/structure/transports/responses.md index d67380d7ec7..576e104d2cc 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -125,6 +125,9 @@ Function-call wrappers around freeform bodies are restored by `exec` and `apply_patch`, one tool-specific alternate field or one complete outer Markdown fence is recoverable because the wrapper is otherwise unusable; two alternate fields are ambiguous and therefore remain untouched. Foreign freeform grammars never receive that compatibility rewrite. +An already unwrapped executable body may carry exactly one extra complete JSON object with only +an input string; that layer is removed once, never recursively. Extra members and non-string +values remain literal. A decoded object-leading body holds its preview until completion. #### Schema-bound flat shell repair @@ -147,9 +150,9 @@ Which wrapper applies is decided by scanning the prefix as JSON rather than matc a literal opening. `JSON.parse` decides the completed input, and it cares about neither property order nor how a name is spelled, so a canonical key arriving after other properties or written with an escape is the same wrapper and has to preview as one (#5151). The buffering policy that -follows is: an own `input` with a string value streams progressively, because completion gives -it precedence over everything else in the object whatever its position; an `input` with a -non-string value, a text that is not an object, and an object `JSON.parse` can no longer accept +follows is: an own string `input` streams progressively unless its decoded executable body +could still be a nested wrapper; completion gives that field precedence over other properties. +An `input` with a non-string value, a text that is not an object, and an object `JSON.parse` can no longer accept all publish their own bytes, because that is what completion returns for them; every other object HOLDS until it closes, because a key that has not arrived yet can still change the answer. Fallback fields decide at that close rather than from a parse: they only unwrap as the diff --git a/tests/responses/apply-patch-envelope.test.ts b/tests/responses/apply-patch-envelope.test.ts index 0228f813062..c0e9d3b047a 100644 --- a/tests/responses/apply-patch-envelope.test.ts +++ b/tests/responses/apply-patch-envelope.test.ts @@ -5,6 +5,7 @@ import { normalizeApplyPatchDelimiters, repairFreeformToolInput, stableCanonicalPatchPrefix, + unwrapFreeformToolInput, } from "../../src/responses/apply-patch-envelope"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../../src/responses/code-mode-helper-compat"; @@ -23,6 +24,48 @@ const CANONICAL_PATCH = `*** Begin Patch *** End Patch`; describe("apply_patch envelope repair", () => { + test.each([ + ["exec", 'text("nested wrapper");'], + ["apply_patch", CANONICAL_PATCH], + ])("unwraps one extra canonical string body for %s", (tool, body) => { + const nested = JSON.stringify({ input: body }); + const args = JSON.stringify({ input: nested }); + expect(repairFreeformToolInput(args, tool)).toBe(body); + expect(repairFreeformToolInput(args, tool, "functions")).toBe(body); + expect(repairFreeformToolInput(body, tool)).toBe(body); + expect(repairFreeformToolInput(nested, tool)).toBe(body); + // A third layer is deliberately left in place. + expect(repairFreeformToolInput(JSON.stringify({ input: args }), tool)).toBe(nested); + expect(repairFreeformToolInput(args, tool, "mcp")).toBe(nested); + expect(repairFreeformToolInput(args, "render_diagram")).toBe(nested); + }); + + test("the reported exec body parses only after the extra wrapper is removed", () => { + const body = 'text("nested wrapper");'; + const nested = JSON.stringify({ input: body }); + expect(() => new Function("text", nested)).toThrow(SyntaxError); + const values: string[] = []; + new Function("text", repairFreeformToolInput(JSON.stringify({ input: nested }), "exec"))( + (value: string) => values.push(value), + ); + expect(values).toEqual(["nested wrapper"]); + }); + + test.each(["exec", "apply_patch"])("preserves noncanonical nested %s bodies", tool => { + for (const nested of [ + '{"input":"text(1)","extra":true}', + '{"input":42}', '{"input":null}', '{"input":{}}', + '{"code":"text(1)"}', '["text(1)"]', '{"input":"incomplete', + ]) { + expect(repairFreeformToolInput(JSON.stringify({ input: nested }), tool)).toBe(nested); + } + for (const input of [null, 42, true, {}, []]) { + const args = JSON.stringify({ input }); + expect(repairFreeformToolInput(args, tool)).toBe(args); + } + expect(unwrapFreeformToolInput(undefined, tool)).toBe(""); + }); + test("repairs only the outer lines of a complete top-level apply_patch payload", () => { expect(repairFreeformToolInput(DECORATED_PATCH, "apply_patch")).toBe(CANONICAL_PATCH); expect(normalizeApplyPatchDelimiters(DECORATED_PATCH)).toBe(CANONICAL_PATCH); @@ -176,6 +219,12 @@ describe("raw exec patch envelope recognition", () => { expect(source).not.toContain("*** Begin Patch ***"); }); + test("recognizes a double-wrapped patch submitted as exec", () => { + const args = JSON.stringify({ input: JSON.stringify({ input: DECORATED_PATCH }) }); + expect(resolveCodeModeHelperName(undefined, "exec", args, undefined, CODE_MODE)).toBe("apply_patch"); + expect(compileCodeModeHelperInput(args, "apply_patch", "exec")).toContain(JSON.stringify(CANONICAL_PATCH)); + }); + test("refuses JavaScript that merely mentions an envelope", () => { const cases = [ "const sample = \"tools.apply_patch({ input: patchText })\";", diff --git a/tests/responses/responses-custom-tool-stream-consistency.test.ts b/tests/responses/responses-custom-tool-stream-consistency.test.ts index c3eb3b836ca..62f47581224 100644 --- a/tests/responses/responses-custom-tool-stream-consistency.test.ts +++ b/tests/responses/responses-custom-tool-stream-consistency.test.ts @@ -1,4 +1,6 @@ import { describe, expect, test } from "bun:test"; +import { bridgeToResponsesSSE } from "../../src/bridge"; +import type { AdapterEvent } from "../../src/types"; import { createRoutedCustomToolRestoreBlockRewrite, RoutedCustomToolProgressError, @@ -108,6 +110,39 @@ function restoreExecStream(argumentsText: string, fragments: readonly string[], } describe("routed Responses custom-tool stream consistency", () => { + test.each([ + ["exec", 'text("nested");'], + ["apply_patch", "*** Begin Patch\n*** Add File: a.txt\n+x\n*** End Patch"], + ])("double-wrapped %s agrees across direct and routed completion", async (tool, input) => { + const args = JSON.stringify({ input: JSON.stringify({ input }) }); + for (let split = 0; split <= args.length; split++) { + const fragments = [args.slice(0, split), args.slice(split)]; + const routed = restoreExecStream(args, fragments, tool); + expect(routed.preview, "routed split " + split).toBe(""); + expect([routed.doneInput, routed.itemInput, routed.terminalInput]).toEqual([input, input, input]); + + async function* events(): AsyncGenerator { + yield { type: "tool_call_start", id: "call_nested", name: tool }; + for (const argumentsPart of fragments) yield { type: "tool_call_delta", arguments: argumentsPart }; + yield { type: "tool_call_end", id: "call_nested" }; + yield { type: "done" }; + } + const wire = await new Response(bridgeToResponsesSSE( + events(), "routed/model", undefined, new Set([tool]), + )).text(); + const frames = wire.split("\n\n") + .filter(block => block.includes("data:") && !block.includes("data: [DONE]")) + .map(dataPayload); + const deltas = frames.filter(frame => frame.type === "response.custom_tool_call_input.delta"); + // Both transports hold the ambiguous preview; completed input is authoritative. + expect(deltas, "direct split " + split).toEqual([]); + const done = frames.find(frame => frame.type === "response.custom_tool_call_input.done"); + const item = frames.find(frame => frame.type === "response.output_item.done")?.item as { input: string }; + const terminal = frames.find(frame => frame.type === "response.completed")?.response as { output: Array<{ input: string }> }; + expect([done?.input, item.input, terminal.output[0]?.input]).toEqual([input, input, input]); + } + }); + test.each([ ["late-invalid wrapper", ['{"input":"*** Begin Patch\\n*** Add File: a\\n+x\\n*** End Patch', '\\q"}']], ["duplicate input key", ['{"input":"*** Begin Patch\\n*** Add File: a\\n+x\\n*** End Patch",', '"input":"replacement"}']], diff --git a/tests/responses/responses-freeform-wrapper-keys.test.ts b/tests/responses/responses-freeform-wrapper-keys.test.ts index 1214817dae7..d86ed962664 100644 --- a/tests/responses/responses-freeform-wrapper-keys.test.ts +++ b/tests/responses/responses-freeform-wrapper-keys.test.ts @@ -47,6 +47,28 @@ const emissions = (body: string, toolName: string) => stream(body, toolName).del const published = (body: string, toolName: string) => emissions(body, toolName).join(""); describe("freeform wrapper keys the literal matcher could not see", () => { + test.each(["exec", "apply_patch"])("holds a possible extra %s wrapper at every prefix", tool => { + const source = tool === "exec" ? 'text("nested");' : "*** Begin Patch\n*** Add File: a.txt\n+x\n*** End Patch"; + for (const body of [ + JSON.stringify({ input: JSON.stringify({ input: source }) }), + JSON.stringify({ code: JSON.stringify({ input: source }) }), + JSON.stringify({ input: ' \n { "\\u0069nput": ' + JSON.stringify(source) + " }" }), + ]) { + // The alternate field is recognized only by exec; apply_patch uses patch. + const args = tool === "apply_patch" ? body.replace('{"code":', '{"patch":') : body; + expect(unwrapFreeformToolInput(args, tool)).toBe(source); + expect(stream(args, tool)).toEqual({ deltas: [], retractions: [] }); + expect(progressiveFreeformInput(args, tool)).toBeNull(); + } + const foreign = JSON.stringify({ input: JSON.stringify({ input: source }) }); + expect(published(foreign, "")).toBe(JSON.stringify({ input: source })); + // A valid JS block can begin like an object too: only its preview is sacrificed. + const block = "{ let x = 1; return x; }"; + const wrappedBlock = JSON.stringify({ input: block }); + expect(unwrapFreeformToolInput(wrappedBlock, tool)).toBe(block); + expect(published(wrappedBlock, tool)).toBe(""); + }); + // `progressive` records whether the value can be previewed before the object closes. It is // the liveness half of the contract and it is asserted, because holding everything until // completion would satisfy the agreement half on its own while quietly ending progressive From 4e8c8b51e124930db2b838181ae107e68e4017aa Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 3 Nov 2026 16:08:54 +0900 Subject: [PATCH 2/3] fix(responses): stream object-leading executable bodies once they cannot be a nested wrapper --- .../content/docs/guides/codex-integration.md | 3 +- src/responses/progressive-freeform-input.ts | 84 ++++++++++++++++++- structure/transports/responses.md | 37 ++++---- ...ses-custom-tool-stream-consistency.test.ts | 28 +++++++ .../responses-freeform-wrapper-keys.test.ts | 53 +++++++++++- 5 files changed, 178 insertions(+), 27 deletions(-) diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 33c94904d65..7d8ad1a3dc5 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -763,7 +763,8 @@ contains patch text and unrelated native custom payloads stay unchanged. When a routed model double-wraps an executable freeform body, opencodex removes one extra JSON object containing only an input string. This repair applies to local exec and apply_patch bodies, leaves other tool grammars and ambiguous objects unchanged, and does not decode recursively. -Possible nested wrappers hold their streamed preview until the completed input is available. +A body holds its streamed preview only while it can still turn out to be such a wrapper; +JavaScript that merely starts with an object or block keeps streaming as it arrives. A routed model can also mistakenly send a shell-argument object such as `{"cmd":"git status --short"}` to code-mode `exec`. For a verified code-mode catalog, diff --git a/src/responses/progressive-freeform-input.ts b/src/responses/progressive-freeform-input.ts index 1fc9f3cdb0b..40e961b3606 100644 --- a/src/responses/progressive-freeform-input.ts +++ b/src/responses/progressive-freeform-input.ts @@ -18,6 +18,82 @@ function mayBecomeFencedBody(text: string, toolName: string): boolean { } const LOW_SURROGATE_ESCAPE = /^\\u[dD][c-fC-F][0-9a-fA-F]{2}$/; +const JSON_WHITESPACE = new Set([" ", "\t", "\n", "\r"]); +const JSON_SIMPLE_ESCAPES = new Set(['"', "\\", "/", "b", "f", "n", "r", "t"]); + +/** + * Read the JSON string whose opening quote precedes `start`: its decoded value and the index + * after the closing quote, `partial` when the text ends first, or `null` when no continuation + * can parse. Only key text needs the decoded value, so escapes other than `\\u` keep a marker. + */ +function readJsonString(text: string, start: number): { value: string; next: number; partial: boolean } | null { + let value = ""; + for (let i = start; i < text.length; i++) { + const c = text[i]!; + if (c === '"') return { value, next: i + 1, partial: false }; + if (c.charCodeAt(0) <= 0x1f) return null; + if (c !== "\\") { value += c; continue; } + const n = text[i + 1]; + if (n === undefined) break; + if (n === "u") { + const hex = text.slice(i + 2, i + 6); + if (!/^[0-9a-fA-F]*$/.test(hex)) return null; + if (hex.length < 4) break; + value += String.fromCharCode(parseInt(hex, 16)); + i += 5; + continue; + } + if (!JSON_SIMPLE_ESCAPES.has(n)) return null; + value += "\\" + n; + i += 1; + } + return { value, next: text.length, partial: true }; +} + +/** + * Whether a decoded executable body can still grow into the one extra `{"input":string}` + * wrapper that completion removes. Ordinary object-leading JavaScript such as `{ let x` or + * `({` fails within a few characters and streams from there; only a prefix that is still a + * possible wrapper holds. A false answer never turns true again as the body grows, so the + * held prefix is flushed once and streaming continues. Duplicate `input` keys are followed + * because `JSON.parse` keeps the last; a non-string value holds to completion, since a later + * duplicate could still replace it. + */ +function mayBecomeNestedWrapper(text: string): boolean { + let i = 0; + const skipWhitespace = () => { while (i < text.length && JSON_WHITESPACE.has(text[i]!)) i++; }; + skipWhitespace(); + if (i === text.length) return true; + if (text[i] !== "{") return false; + i++; + for (;;) { + skipWhitespace(); + if (i === text.length) return true; + if (text[i] !== '"') return false; + const key = readJsonString(text, i + 1); + if (key === null) return false; + if (key.partial) return "input".startsWith(key.value); + if (key.value !== "input") return false; + i = key.next; + skipWhitespace(); + if (i === text.length) return true; + if (text[i] !== ":") return false; + i++; + skipWhitespace(); + if (i === text.length || text[i] !== '"') return true; + const value = readJsonString(text, i + 1); + if (value === null) return false; + if (value.partial) return true; + i = value.next; + skipWhitespace(); + if (i === text.length) return true; + if (text[i] === ",") { i++; continue; } + if (text[i] !== "}") return false; + i++; + skipWhitespace(); + return i === text.length; + } +} /** * The decoded prefix of a JSON string body, stopping at the first byte it cannot resolve. @@ -78,7 +154,8 @@ function decodeJsonStringPrefix(body: string): string | null { * `input` is decidable from its prefix: `unwrapFreeformToolInput` returns it whenever the * key is present, whatever else the object carries, so its value can be unescaped * progressively and never retracted, except for executable bodies that may contain one - * extra canonical wrapper. Those object-leading bodies hold until authoritative completion. + * extra canonical wrapper. Such a body holds only while its prefix can still become that + * wrapper (`mayBecomeNestedWrapper`); the first byte that rules it out flushes the held text. * * One case escapes that claim and is accepted rather than fixed: a duplicate `input` key. * `JSON.parse` keeps the last one, so `{"input":"a","input":"b"}` streams a and completes with @@ -112,9 +189,8 @@ export function progressiveFreeformInput(args: string, toolName: string): string if (scan.kind === "input") { const decoded = decodeJsonStringPrefix(args.slice(scan.valueStart)); if (decoded === null) return null; - // Completion may remove one extra {input:string} body wrapper. A later member can still - // disqualify it, so hold the whole object-shaped preview rather than decode recursively. - if ((toolName === "exec" || toolName === "apply_patch") && decoded.trimStart().startsWith("{")) return null; + // Completion may remove one extra {input:string} body wrapper, so hold while it can. + if ((toolName === "exec" || toolName === "apply_patch") && mayBecomeNestedWrapper(decoded)) return null; return mayBecomeFencedBody(decoded, toolName) ? null : decoded; } if (scan.kind === "raw") return mayBecomeFencedBody(args, toolName) ? null : args; diff --git a/structure/transports/responses.md b/structure/transports/responses.md index f41e0db1f3c..e7769b996e6 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -128,9 +128,7 @@ Function-call wrappers around freeform bodies are restored by `exec` and `apply_patch`, one tool-specific alternate field or one complete outer Markdown fence is recoverable because the wrapper is otherwise unusable; two alternate fields are ambiguous and therefore remain untouched. Foreign freeform grammars never receive that compatibility rewrite. -An already unwrapped executable body may carry exactly one extra complete JSON object with only -an input string; that layer is removed once, never recursively. Extra members and non-string -values remain literal. A decoded object-leading body holds its preview until completion. +An unwrapped executable body may carry one extra `{"input":string}` object, removed once, never recursively. #### Schema-bound flat shell repair @@ -149,25 +147,24 @@ A prefix that can still become a complete outer fence stays held so completion n bytes already published in a delta; a body that is not shaped like a wrapper object remains progressive, and JSON escapes emit only complete decoded units. -Which wrapper applies is decided by scanning the prefix as JSON rather than matching it against -a literal opening. `JSON.parse` decides the completed input, and it cares about neither property +Which wrapper applies is decided by scanning the prefix as JSON rather than matching it against a +literal opening. `JSON.parse` decides the completed input, and it cares about neither property order nor how a name is spelled, so a canonical key arriving after other properties or written with an escape is the same wrapper and has to preview as one (#5151). The buffering policy that -follows is: an own string `input` streams progressively unless its decoded executable body -could still be a nested wrapper; completion gives that field precedence over other properties. -An `input` with a non-string value, a text that is not an object, and an object `JSON.parse` can no longer accept -all publish their own bytes, because that is what completion returns for them; every other -object HOLDS until it closes, because a key that has not arrived yet can still change the -answer. Fallback fields decide at that close rather than from a parse: they only unwrap as the -single string field, so the scan keeps a last-wins table of the members it already walked and -consults it once — one string fallback field releases its value as `input`, anything else -streams the raw text byte-exact through whatever trailing whitespace follows. Classification is -bounded to `MAX_FREEFORM_WRAPPER_SCAN_CHARS`, which keeps the work per delta from growing with -the arguments. Past the bound nothing is previewed at all: the authoritative parse still -unwraps the wrapper at completion, so the bound costs preview and never agreement. Because the -close resolves from the member table instead of `JSON.parse`, fragmented trailing whitespace -and deltas that happen to end on a brace can never reparse a growing provider-controlled -buffer. +follows is: an own string `input` streams progressively (completion prefers it to any other +member) unless its executable body could still be a nested wrapper. A non-string `input`, a text +that is not an object, and an object `JSON.parse` can no longer accept all publish their own +bytes, because that is what completion returns for them; every other object HOLDS until it closes, +because a key that has not arrived yet can still change the answer. Fallback fields decide at that +close rather than from a parse: they only unwrap as the single string field, so the scan keeps a +last-wins table of the members it already walked and consults it once — one string fallback field +releases its value as `input`, anything else streams the raw text byte-exact through whatever +trailing whitespace follows. Classification is bounded to `MAX_FREEFORM_WRAPPER_SCAN_CHARS`, which +keeps the work per delta from growing with the arguments. Past the bound nothing is previewed at +all: the authoritative parse still unwraps the wrapper at completion, so the bound costs preview +and never agreement. Because the close resolves from the member table instead of `JSON.parse`, +fragmented trailing whitespace and deltas that happen to end on a brace can never reparse a +growing provider-controlled buffer. What that policy costs is worth stating plainly, because it is a real narrowing. A body that IS a parseable JSON object but not a wrapper — `{"code":1}` or `{"code":"a","script":"b"}` — now diff --git a/tests/responses/responses-custom-tool-stream-consistency.test.ts b/tests/responses/responses-custom-tool-stream-consistency.test.ts index ed30a446ca8..38133d18a49 100644 --- a/tests/responses/responses-custom-tool-stream-consistency.test.ts +++ b/tests/responses/responses-custom-tool-stream-consistency.test.ts @@ -111,6 +111,34 @@ function restoreExecStream(argumentsText: string, fragments: readonly string[], } describe("routed Responses custom-tool stream consistency", () => { + test.each(["exec", "apply_patch"])("object-leading %s body streams progressively on both transports", async tool => { + const input = '{ const o = { a: 1, b: "two" };\ntext(o.b); }'; + const args = JSON.stringify({ input }); + const fragments = Array.from(args); + const routed = restoreExecStream(args, fragments, tool); + expect(routed.preview).toBe(input); + expect(routed.previews.filter(preview => preview.length > 0 && preview.length < input.length).length) + .toBeGreaterThan(input.length / 2); + expect([routed.doneInput, routed.itemInput, routed.terminalInput]).toEqual([input, input, input]); + + async function* events(): AsyncGenerator { + yield { type: "tool_call_start", id: "call_object", name: tool }; + for (const argumentsPart of fragments) yield { type: "tool_call_delta", arguments: argumentsPart }; + yield { type: "tool_call_end", id: "call_object" }; + yield { type: "done" }; + } + const wire = await new Response(bridgeToResponsesSSE(events(), "routed/model", undefined, new Set([tool]))).text(); + const frames = wire.split("\n\n") + .filter(block => block.includes("data:") && !block.includes("data: [DONE]")) + .map(dataPayload); + const deltas = frames + .filter(frame => frame.type === "response.custom_tool_call_input.delta") + .map(frame => String(frame.delta)); + expect(deltas.length).toBeGreaterThan(input.length / 2); + expect(deltas.join("")).toBe(input); + expect(frames.find(frame => frame.type === "response.custom_tool_call_input.done")?.input).toBe(input); + }); + test.each([ ["exec", 'text("nested");'], ["apply_patch", "*** Begin Patch\n*** Add File: a.txt\n+x\n*** End Patch"], diff --git a/tests/responses/responses-freeform-wrapper-keys.test.ts b/tests/responses/responses-freeform-wrapper-keys.test.ts index d86ed962664..ab1552375f5 100644 --- a/tests/responses/responses-freeform-wrapper-keys.test.ts +++ b/tests/responses/responses-freeform-wrapper-keys.test.ts @@ -62,11 +62,60 @@ describe("freeform wrapper keys the literal matcher could not see", () => { } const foreign = JSON.stringify({ input: JSON.stringify({ input: source }) }); expect(published(foreign, "")).toBe(JSON.stringify({ input: source })); - // A valid JS block can begin like an object too: only its preview is sacrificed. + // A valid JS block can begin like an object too; it streams once it cannot be a wrapper. const block = "{ let x = 1; return x; }"; const wrappedBlock = JSON.stringify({ input: block }); expect(unwrapFreeformToolInput(wrappedBlock, tool)).toBe(block); - expect(published(wrappedBlock, tool)).toBe(""); + expect(published(wrappedBlock, tool)).toBe(block); + }); + + // The extra-wrapper hold covers only prefixes that can still become {"input":}. + test.each(["exec", "apply_patch"])("object-leading %s bodies keep streaming progressively", tool => { + for (const body of [ + "{ let x = 1; text(x); }", + '({ a: 1, b: "two" }).b.length;\ntext("done");', + '{"inputs": 1}; text("not a wrapper");', + '{}; text("empty object first");', + ]) { + const args = JSON.stringify({ input: body }); + const { deltas, retractions } = stream(args, tool); + expect(retractions).toEqual([]); + expect(deltas.length, body).toBeGreaterThan(body.length / 2); + expect(deltas.join("")).toBe(body); + expect(unwrapFreeformToolInput(args, tool)).toBe(body); + } + }); + + test.each(["exec", "apply_patch"])("a real double-wrapped %s body is unwrapped once and never previewed", tool => { + const source = tool === "exec" ? "text(" + JSON.stringify("x".repeat(400)) + ");" : "*** Begin Patch\n*** Add File: a.txt\n+x\n*** End Patch"; + for (const nested of [ + JSON.stringify({ input: source }), + ' \t{ "in\\u0070ut" :\n' + JSON.stringify(source) + " } \n", + '{"input":"first","input":' + JSON.stringify(source) + "}", + ]) { + const args = JSON.stringify({ input: nested }); + expect(stream(args, tool)).toEqual({ deltas: [], retractions: [] }); + expect(unwrapFreeformToolInput(args, tool)).toBe(source); + } + }); + + test.each(["exec", "apply_patch"])("late divergence from the wrapper shape flushes then streams (%s)", tool => { + const long = "y".repeat(600); + for (const [nested, divergeAt] of [ + ['{"input":"' + long + '","extra":true}', 14 + long.length], + ['{"input":"' + long + '"} + text(2);', 14 + long.length], + ['{"input":"' + long + "\\q" + long + '"}', 12 + long.length], + ['{"inputs":"' + long + '"}', 8], + ] as const) { + const args = JSON.stringify({ input: nested }); + const { deltas, retractions } = stream(args, tool); + expect(retractions).toEqual([]); + // Everything held until the divergence arrives in one flush, then streaming resumes. + expect(deltas[0], nested.slice(0, 20)).toBe(nested.slice(0, divergeAt)); + expect(deltas.length).toBeGreaterThan(1); + expect(deltas.join("")).toBe(nested); + expect(unwrapFreeformToolInput(args, tool)).toBe(nested); + } }); // `progressive` records whether the value can be previewed before the object closes. It is From 20cf7a08b350d45f002171f1280e7142a69a20d8 Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 3 Nov 2026 16:19:56 +0900 Subject: [PATCH 3/3] perf(responses): resume progressive freeform decoding across deltas Each delta decoded the whole accumulated canonical value and walked the whole decoded body again for a nested executable wrapper, so one-character deltas did quadratic work. A per-call ProgressiveFreeformInputReader now resumes where the previous delta stopped, and the nested-wrapper check is an incremental state machine that also releases a non-string nested input once no later string duplicate can follow. --- src/bridge/sse.ts | 8 +- src/responses/freeform-wrapper-scan.ts | 8 +- src/responses/progressive-freeform-input.ts | 455 +++++++++++++----- src/server/responses-custom-tool-repair.ts | 7 +- structure/transports/byte-accounting.md | 11 +- structure/transports/responses.md | 2 +- .../responses-freeform-wrapper-keys.test.ts | 61 ++- 7 files changed, 426 insertions(+), 126 deletions(-) diff --git a/src/bridge/sse.ts b/src/bridge/sse.ts index c1390ceebb0..978dd29c3b0 100644 --- a/src/bridge/sse.ts +++ b/src/bridge/sse.ts @@ -24,7 +24,7 @@ import { mayBecomePatchEnvelope, repairFreeformToolInput, } from "../responses/apply-patch-envelope"; -import { progressiveFreeformInput } from "../responses/progressive-freeform-input"; +import { ProgressiveFreeformInputReader } from "../responses/progressive-freeform-input"; import { encodeCompactionSummary, releaseCompactionCiphertextLease } from "../responses/compaction"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../responses/code-mode-helper-compat"; import { mayBecomeCodeModeShellInput } from "../responses/code-mode-shell-input"; @@ -480,7 +480,7 @@ export function bridgeToResponsesSSE( // Full assistant text of a compaction turn (across message boundaries) — becomes the // synthetic compaction item's payload on done. let compaction = emptyChunks(); - let currentToolCall: { itemId: string; outputIndex: number; callId: string; name: string; args: string; argsBytes: number; namespace?: string; freeform?: boolean; toolSearch?: boolean; inputEmitted?: string; codeModeHelperName?: string; providerMetadata?: OcxProviderOpaqueToolCallMetadata } | null = null; + let currentToolCall: { itemId: string; outputIndex: number; callId: string; name: string; args: string; argsBytes: number; namespace?: string; freeform?: boolean; toolSearch?: boolean; inputEmitted?: string; freeformReader?: ProgressiveFreeformInputReader; codeModeHelperName?: string; providerMetadata?: OcxProviderOpaqueToolCallMetadata } | null = null; // Open native web-search cell (between begin and end). Holds the output index allocated on // begin so the matching done reuses it; closed as `failed` if the stream terminates early. let currentWebSearch: { itemId: string; eventId: string; outputIndex: number } | null = null; @@ -1157,7 +1157,9 @@ export function bridgeToResponsesSSE( // is the same disagreement in the other direction. const ownsFreeformGrammar = currentToolCall.namespace === undefined || currentToolCall.namespace === "functions"; - const full = progressiveFreeformInput( + // One reader per call resumes where the previous delta stopped. + currentToolCall.freeformReader ??= new ProgressiveFreeformInputReader(); + const full = currentToolCall.freeformReader.update( currentToolCall.args, ownsFreeformGrammar ? currentToolCall.name : "", ); diff --git a/src/responses/freeform-wrapper-scan.ts b/src/responses/freeform-wrapper-scan.ts index 581e9a5b1d6..27d7aeca471 100644 --- a/src/responses/freeform-wrapper-scan.ts +++ b/src/responses/freeform-wrapper-scan.ts @@ -56,7 +56,9 @@ const NEVER = -2; export type FreeformWrapperScan = | { kind: "hold" } | { kind: "raw" } - | { kind: "input"; valueStart: number }; + // `canonical` is false for a fallback field, which a closed object resolved and trailing text + // can still withdraw; an own `input` is decided by its prefix and stays decided. + | { kind: "input"; valueStart: number; canonical: boolean }; /** Index of the first non-whitespace character at or after `from`, or HOLD past the end. */ function skipWhitespace(text: string, from: number): number { @@ -249,7 +251,7 @@ export function scanFreeformWrapper(text: string, fallbackKeys: readonly string[ // non-string value is decidable too, in the other direction: completion hands back the // argument text unchanged rather than unwrapping a value it cannot use. return bounded[valueAt] === '"' - ? { kind: "input", valueStart: valueAt + 1 } + ? { kind: "input", valueStart: valueAt + 1, canonical: true } : { kind: "raw" }; } @@ -300,7 +302,7 @@ function afterTopLevelClose( if (!wholeText) return { kind: "hold" }; const candidates = fallbackKeys.filter(key => members.get(key)?.stringValue === true); if (candidates.length === 1) { - return { kind: "input", valueStart: members.get(candidates[0]!)!.valueStart }; + return { kind: "input", valueStart: members.get(candidates[0]!)!.valueStart, canonical: false }; } return { kind: "raw" }; } diff --git a/src/responses/progressive-freeform-input.ts b/src/responses/progressive-freeform-input.ts index 40e961b3606..5c31c422f6d 100644 --- a/src/responses/progressive-freeform-input.ts +++ b/src/responses/progressive-freeform-input.ts @@ -1,5 +1,10 @@ import { freeformFallbackKeys } from "./apply-patch-envelope"; -import { JSON_ESCAPES, scanFreeformWrapper } from "./freeform-wrapper-scan"; +import { + JSON_ESCAPES, + JSON_WHITESPACE, + MAX_FREEFORM_WRAPPER_SCAN_CHARS, + scanFreeformWrapper, +} from "./freeform-wrapper-scan"; /** * Whether a body could still grow into one complete outer Markdown fence. @@ -18,138 +23,285 @@ function mayBecomeFencedBody(text: string, toolName: string): boolean { } const LOW_SURROGATE_ESCAPE = /^\\u[dD][c-fC-F][0-9a-fA-F]{2}$/; -const JSON_WHITESPACE = new Set([" ", "\t", "\n", "\r"]); -const JSON_SIMPLE_ESCAPES = new Set(['"', "\\", "/", "b", "f", "n", "r", "t"]); +const HEX_DIGIT = /^[0-9a-fA-F]$/; +const HEX_QUAD = /^[0-9a-fA-F]{4}$/; +const JSON_NUMBER_CHAR = /^[-+0-9.eE]$/; +const JSON_NUMBER = /^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?$/; +const JSON_LITERALS = ["true", "false", "null"] as const; +const CANONICAL_KEY = "input"; +const STRING_END = Symbol("string end"); + +const ownsBodyRepair = (toolName: string): boolean => toolName === "exec" || toolName === "apply_patch"; + +type NestedState = + | "start" | "keyOpen" | "key" | "colon" | "valueOpen" | "string" + | "scalar" | "container" | "afterValue" | "trailing" | "never"; /** - * Read the JSON string whose opening quote precedes `start`: its decoded value and the index - * after the closing quote, `partial` when the text ends first, or `null` when no continuation - * can parse. Only key text needs the decoded value, so escapes other than `\\u` keep a marker. + * Whether a decoded executable body can still grow into the one extra `{"input":string}` + * wrapper that completion removes, answered one character at a time so a streamed body is walked + * once rather than once per delta. + * + * Ordinary object-leading JavaScript such as `{ let x` or `({` fails within a few characters and + * streams from there; only a prefix that is still a possible wrapper holds. A false answer never + * turns true again as the body grows, so the held prefix is flushed once and streaming continues. + * + * Every member name must be `input`, because completion unwraps only a single-key object, and + * duplicates are followed because `JSON.parse` keeps the last. A non-string value is skipped to its + * end so a later duplicate can still replace it, and an object that closes on a non-string last + * value is ruled out. Scalars are checked against the JSON grammar; containers are skipped by + * bracket and string tracking alone. That skip is looser than `JSON.parse` but never unsound: it + * finds the true end of every valid container, and any text it misreads is invalid JSON, which + * completion would not unwrap either way. */ -function readJsonString(text: string, start: number): { value: string; next: number; partial: boolean } | null { - let value = ""; - for (let i = start; i < text.length; i++) { - const c = text[i]!; - if (c === '"') return { value, next: i + 1, partial: false }; - if (c.charCodeAt(0) <= 0x1f) return null; - if (c !== "\\") { value += c; continue; } - const n = text[i + 1]; - if (n === undefined) break; - if (n === "u") { - const hex = text.slice(i + 2, i + 6); - if (!/^[0-9a-fA-F]*$/.test(hex)) return null; - if (hex.length < 4) break; - value += String.fromCharCode(parseInt(hex, 16)); - i += 5; - continue; +class NestedWrapperMatcher { + scanned = 0; + private state: NestedState = "start"; + private key = ""; + /** Pending escape inside the current string: "", a backslash, or `\\u` and the hex read so far. */ + private escape = ""; + private scalar = ""; + private readonly closers: string[] = []; + private inContainerString = false; + private containerEscape = false; + private lastValueString = false; + + get possible(): boolean { + return this.state !== "never"; + } + + feed(text: string): boolean { + for (let i = 0; i < text.length && this.state !== "never"; i++) { + this.scanned++; + this.step(text[i]!); } - if (!JSON_SIMPLE_ESCAPES.has(n)) return null; - value += "\\" + n; - i += 1; + return this.possible; } - return { value, next: text.length, partial: true }; -} -/** - * Whether a decoded executable body can still grow into the one extra `{"input":string}` - * wrapper that completion removes. Ordinary object-leading JavaScript such as `{ let x` or - * `({` fails within a few characters and streams from there; only a prefix that is still a - * possible wrapper holds. A false answer never turns true again as the body grows, so the - * held prefix is flushed once and streaming continues. Duplicate `input` keys are followed - * because `JSON.parse` keeps the last; a non-string value holds to completion, since a later - * duplicate could still replace it. - */ -function mayBecomeNestedWrapper(text: string): boolean { - let i = 0; - const skipWhitespace = () => { while (i < text.length && JSON_WHITESPACE.has(text[i]!)) i++; }; - skipWhitespace(); - if (i === text.length) return true; - if (text[i] !== "{") return false; - i++; - for (;;) { - skipWhitespace(); - if (i === text.length) return true; - if (text[i] !== '"') return false; - const key = readJsonString(text, i + 1); - if (key === null) return false; - if (key.partial) return "input".startsWith(key.value); - if (key.value !== "input") return false; - i = key.next; - skipWhitespace(); - if (i === text.length) return true; - if (text[i] !== ":") return false; - i++; - skipWhitespace(); - if (i === text.length || text[i] !== '"') return true; - const value = readJsonString(text, i + 1); - if (value === null) return false; - if (value.partial) return true; - i = value.next; - skipWhitespace(); - if (i === text.length) return true; - if (text[i] === ",") { i++; continue; } - if (text[i] !== "}") return false; - i++; - skipWhitespace(); - return i === text.length; + private step(c: string): void { + switch (this.state) { + case "start": + if (!JSON_WHITESPACE.has(c)) this.state = c === "{" ? "keyOpen" : "never"; + return; + case "keyOpen": + if (JSON_WHITESPACE.has(c)) return; + this.state = c === '"' ? "key" : "never"; + this.key = ""; + this.escape = ""; + return; + case "key": { + const part = this.stringStep(c); + if (part === null) this.state = "never"; + else if (part === STRING_END) this.state = this.key === CANONICAL_KEY ? "colon" : "never"; + else if (!CANONICAL_KEY.startsWith(this.key += part)) this.state = "never"; + return; + } + case "colon": + if (!JSON_WHITESPACE.has(c)) this.state = c === ":" ? "valueOpen" : "never"; + return; + case "valueOpen": + if (JSON_WHITESPACE.has(c)) return; + this.lastValueString = c === '"'; + this.escape = ""; + if (c === '"') this.state = "string"; + else this.startOtherValue(c); + return; + case "string": { + const part = this.stringStep(c); + if (part === null) this.state = "never"; + else if (part === STRING_END) this.state = "afterValue"; + return; + } + case "scalar": + this.stepScalar(c); + return; + case "container": + this.stepContainer(c); + return; + case "afterValue": + if (JSON_WHITESPACE.has(c)) return; + if (c === ",") this.state = "keyOpen"; + else this.state = c === "}" && this.lastValueString ? "trailing" : "never"; + return; + case "trailing": + if (!JSON_WHITESPACE.has(c)) this.state = "never"; + return; + case "never": + return; + } + } + + /** + * One character of a JSON string: the text it decodes to ("" while an escape is pending), + * `STRING_END` at the closing quote, or `null` when no continuation can parse. Only key + * text needs the decoded value, so escapes other than `\\u` keep a marker. + */ + private stringStep(c: string): string | typeof STRING_END | null { + if (this.escape === "") { + if (c === '"') return STRING_END; + if (c.charCodeAt(0) <= 0x1f) return null; + if (c !== "\\") return c; + this.escape = "\\"; + return ""; + } + if (this.escape === "\\") { + if (c === "u") { + this.escape = "\\u"; + return ""; + } + if (!JSON_ESCAPES.has(c)) return null; + this.escape = ""; + return "\\" + c; + } + if (!HEX_DIGIT.test(c)) return null; + this.escape += c; + if (this.escape.length < 6) return ""; + const code = parseInt(this.escape.slice(2), 16); + this.escape = ""; + return String.fromCharCode(code); + } + + private startOtherValue(c: string): void { + if (c === "{" || c === "[") { + this.closers.length = 0; + this.closers.push(c === "{" ? "}" : "]"); + this.inContainerString = false; + this.containerEscape = false; + this.state = "container"; + return; + } + if (JSON_NUMBER_CHAR.test(c) || c === "t" || c === "f" || c === "n") { + this.scalar = c; + this.state = "scalar"; + return; + } + this.state = "never"; + } + + private stepScalar(c: string): void { + if (JSON_WHITESPACE.has(c) || c === "," || c === "}") { + const complete = (JSON_LITERALS as readonly string[]).includes(this.scalar) || JSON_NUMBER.test(this.scalar); + if (!complete) { + this.state = "never"; + return; + } + this.state = "afterValue"; + this.step(c); + return; + } + this.scalar += c; + const literal = JSON_LITERALS.find(word => word[0] === this.scalar[0]); + const viable = literal ? literal.startsWith(this.scalar) : JSON_NUMBER_CHAR.test(c); + if (!viable) this.state = "never"; + } + + private stepContainer(c: string): void { + if (this.inContainerString) { + if (this.containerEscape) this.containerEscape = false; + else if (c === "\\") this.containerEscape = true; + else if (c === '"') this.inContainerString = false; + return; + } + if (c === '"') this.inContainerString = true; + else if (c === "{" || c === "[") this.closers.push(c === "{" ? "}" : "]"); + else if (c === "}" || c === "]") { + if (this.closers.pop() !== c) this.state = "never"; + else if (this.closers.length === 0) this.state = "afterValue"; + } } } +type DecodeStop = "open" | "closed" | "stalled" | "invalid"; + /** - * The decoded prefix of a JSON string body, stopping at the first byte it cannot resolve. + * Decode the JSON string body in `text` from `from`, stopping at the first byte it cannot + * resolve. `next` is where decoding resumes once more text arrives; `scanned` counts the characters + * read, including a split escape that will be read again. * * Every stop is a hold rather than a guess, because `JSON.parse` decides the completed value * and anything invented here would be retracted. Three of them are not obvious: * * An escape JSON does not define makes the whole wrapper unparseable no matter what arrives - * next, so completion falls back to the raw text. Returning `null` for that case stops the - * preview rather than continuing to decode a value the completed item will never carry. - * `\\b` and `\\f` are defined, and were previously decoded to the letters b and f. + * next, so completion falls back to the raw text. `invalid` stops the preview rather than + * continuing to decode a value the completed item will never carry. `\\b` and `\\f` are defined, + * and were previously decoded to the letters b and f. A literal control character is not legal + * inside a JSON string either, and stops the same way. * * A lone high surrogate is not a character. Emitting one alone puts an unpaired code unit in a - * delta the client has to decode by itself, so the pair is emitted together or not at all. + * delta the client has to decode by itself, so the pair is emitted together or not at all; a + * complete escape that is not its low half leaves the preview `stalled` for the rest of the call. */ -function decodeJsonStringPrefix(body: string): string | null { - let out = ""; - for (let i = 0; i < body.length; i++) { - const c = body[i]; - if (c === '"') break; // unescaped closing quote: value complete +function decodeJsonStringChunk( + text: string, + from: number, +): { chunk: string; next: number; stop: DecodeStop; scanned: number } { + let chunk = ""; + let scanned = 0; + let i = from; + let stop: DecodeStop = "open"; + while (i < text.length) { + const c = text[i]!; + scanned++; + if (c === '"') { + stop = "closed"; + break; + } if (c !== "\\") { - // A literal control character is not legal inside a JSON string, so `JSON.parse` will - // reject this wrapper and completion will return the raw text. Emitting the decoded value - // first is the disagreement this decoder exists to prevent, so stop instead. - if (c!.charCodeAt(0) <= 0x1f) return null; - out += c; + if (c.charCodeAt(0) <= 0x1f) { + stop = "invalid"; + break; + } + chunk += c; + i++; continue; } - const n = body[i + 1]; + const n = text[i + 1]; if (n === undefined) break; // escape split across chunks: wait for more + scanned++; if (n === "u") { - const hex = body.slice(i + 2, i + 6); + const hex = text.slice(i + 2, i + 6); + scanned += hex.length; if (hex.length < 4) break; // split across chunks: wait for more - if (!/^[0-9a-fA-F]{4}$/.test(hex)) return null; // never parses + if (!HEX_QUAD.test(hex)) { + stop = "invalid"; + break; + } const code = parseInt(hex, 16); if (code >= 0xd800 && code <= 0xdbff) { - const low = body.slice(i + 6, i + 12); - if (!LOW_SURROGATE_ESCAPE.test(low)) break; - out += String.fromCharCode(code, parseInt(low.slice(2), 16)); - i += 11; + const low = text.slice(i + 6, i + 12); + scanned += low.length; + if (!LOW_SURROGATE_ESCAPE.test(low)) { + if (low.length === 6) stop = "stalled"; + break; + } + chunk += String.fromCharCode(code, parseInt(low.slice(2), 16)); + i += 12; continue; } - out += String.fromCharCode(code); - i += 5; + chunk += String.fromCharCode(code); + i += 6; continue; } const escaped = JSON_ESCAPES.get(n); - if (escaped === undefined) return null; // never parses - out += escaped; - i += 1; + if (escaped === undefined) { + stop = "invalid"; + break; + } + chunk += escaped; + i += 2; } - return out; + return { chunk, next: i, stop, scanned }; } /** - * The value to stream so far, or `null` to HOLD because nothing can be decided yet. + * The value to stream so far for one call's growing arguments, or `null` to HOLD because nothing + * can be decided yet. + * + * A reader belongs to one call and is fed that call's whole accumulated arguments on each delta. + * They only grow, so it resumes where the previous delta stopped: the canonical value is decoded + * and checked for a nested wrapper once overall, which keeps a long body streamed in small deltas + * linear instead of quadratic. A shorter buffer or a different tool name starts over. + * * * `input` is decidable from its prefix: `unwrapFreeformToolInput` returns it whenever the * key is present, whatever else the object carries, so its value can be unescaped @@ -184,21 +336,98 @@ function decodeJsonStringPrefix(body: string): string | null { * cost of holding is preview on an object body that turns out not to be a wrapper; the cost of * not holding was publishing bytes the completed item removes. */ -export function progressiveFreeformInput(args: string, toolName: string): string | null { - const scan = scanFreeformWrapper(args, freeformFallbackKeys(toolName)); - if (scan.kind === "input") { - const decoded = decodeJsonStringPrefix(args.slice(scan.valueStart)); - if (decoded === null) return null; +export class ProgressiveFreeformInputReader { + private toolName: string | undefined; + private seen = 0; + private mode: "scan" | "raw" | "hold" | "input" = "scan"; + private next = 0; + private decoded = ""; + private stop: DecodeStop = "open"; + private nested: NestedWrapperMatcher | null = null; + private fenceRuledOut = false; + private decodeScanned = 0; + + /** Characters this reader has walked so far; a test seam for the linear-work bound. */ + get scannedChars(): number { + return this.decodeScanned + (this.nested?.scanned ?? 0); + } + + update(args: string, toolName: string): string | null { + if (toolName !== this.toolName || args.length < this.seen) this.reset(toolName); + this.seen = args.length; + if (this.mode === "scan") { + const scan = scanFreeformWrapper(args, freeformFallbackKeys(toolName)); + if (scan.kind === "raw") { + // Prefix-decided: no extension of this text becomes a wrapper again. + this.mode = "raw"; + } else if (scan.kind === "hold") { + // Past the bound the scan only ever sees the same clamped prefix, so it holds for good. + if (args.length > MAX_FREEFORM_WRAPPER_SCAN_CHARS) this.mode = "hold"; + return null; + } else if (scan.canonical) { + this.mode = "input"; + this.next = scan.valueStart; + this.nested = ownsBodyRepair(toolName) ? new NestedWrapperMatcher() : null; + } else { + // A closed fallback wrapper can still be withdrawn by trailing text, so it is decided + // afresh on each delta. Its object fits inside the scan bound, which bounds this too. + return this.closedFallbackValue(args, scan.valueStart, toolName); + } + } + if (this.mode === "hold") return null; + if (this.mode === "raw") return this.mayBecomeFenced(args) ? null : args; + return this.canonicalValue(args); + } + + private reset(toolName: string): void { + this.toolName = toolName; + this.seen = 0; + this.mode = "scan"; + this.next = 0; + this.decoded = ""; + this.stop = "open"; + this.nested = null; + this.fenceRuledOut = false; + } + + private canonicalValue(args: string): string | null { + if (this.stop === "open") { + const step = decodeJsonStringChunk(args, this.next); + this.decodeScanned += step.scanned; + this.next = step.next; + this.stop = step.stop; + this.decoded += step.chunk; + if (this.nested?.possible) this.nested.feed(step.chunk); + } + if (this.stop === "invalid") return null; // Completion may remove one extra {input:string} body wrapper, so hold while it can. - if ((toolName === "exec" || toolName === "apply_patch") && mayBecomeNestedWrapper(decoded)) return null; - return mayBecomeFencedBody(decoded, toolName) ? null : decoded; + if (this.nested?.possible) return null; + return this.mayBecomeFenced(this.decoded) ? null : this.decoded; } - if (scan.kind === "raw") return mayBecomeFencedBody(args, toolName) ? null : args; - - // Undecided: an incomplete object, or a close whose tail is clamped out of view. Both stay - // held — the first is provably incomplete, and re-walking a budget-exhausted buffer on every - // delta is work for a preview that arrives in the same instant as the authoritative - // completion behind it. A closed object the scan could see whole never reaches here: its - // member table already resolved it to `input` or `raw`. - return null; + + private closedFallbackValue(args: string, valueStart: number, toolName: string): string | null { + const step = decodeJsonStringChunk(args, valueStart); + this.decodeScanned += step.scanned; + if (step.stop === "invalid") return null; + if (ownsBodyRepair(toolName)) { + const nested = new NestedWrapperMatcher(); + const possible = nested.feed(step.chunk); + this.decodeScanned += nested.scanned; + if (possible) return null; + } + return mayBecomeFencedBody(step.chunk, toolName) ? null : step.chunk; + } + + /** `mayBecomeFencedBody`, remembering a false answer: a ruled-out fence never returns. */ + private mayBecomeFenced(text: string): boolean { + if (this.fenceRuledOut) return false; + if (mayBecomeFencedBody(text, this.toolName ?? "")) return true; + this.fenceRuledOut = true; + return false; + } +} + +/** One-shot form of `ProgressiveFreeformInputReader` for a buffer read once. */ +export function progressiveFreeformInput(args: string, toolName: string): string | null { + return new ProgressiveFreeformInputReader().update(args, toolName); } diff --git a/src/server/responses-custom-tool-repair.ts b/src/server/responses-custom-tool-repair.ts index b2cf37f561a..7b4acc38bc8 100644 --- a/src/server/responses-custom-tool-repair.ts +++ b/src/server/responses-custom-tool-repair.ts @@ -6,7 +6,7 @@ import { } from "../responses/apply-patch-envelope"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../responses/code-mode-helper-compat"; import { mayBecomeCodeModeShellInput } from "../responses/code-mode-shell-input"; -import { progressiveFreeformInput } from "../responses/progressive-freeform-input"; +import { ProgressiveFreeformInputReader } from "../responses/progressive-freeform-input"; import { declaresCodeModeExec } from "../types/tools"; import { customToolItemId, @@ -45,6 +45,8 @@ type OpenCustomCall = { emittedInput: string; canonicalPatchProgress: boolean; retainedBytes: number; + // Resumes across deltas so the growing arguments are decoded once rather than once per delta. + freeformReader?: ProgressiveFreeformInputReader; }; export class RoutedCustomToolProgressError extends Error { @@ -352,7 +354,8 @@ export function createRoutedCustomToolRestoreBlockRewrite( const itemName = itemNames.get(upstreamItemId); const ownsFreeformGrammar = itemName?.namespace === undefined || itemName?.namespace === "functions"; - const fullInput = progressiveFreeformInput( + open.freeformReader ??= new ProgressiveFreeformInputReader(); + const fullInput = open.freeformReader.update( open.argumentsText, ownsFreeformGrammar ? itemName?.name ?? "" : "", ); diff --git a/structure/transports/byte-accounting.md b/structure/transports/byte-accounting.md index 3eefc7360a8..0238949a3f3 100644 --- a/structure/transports/byte-accounting.md +++ b/structure/transports/byte-accounting.md @@ -221,10 +221,15 @@ inspection consumes it without creating an event. The text rewrite relay keeps t outside rewrite/drop callbacks and settles injected nonfinal CR delimiters as CRLF. `src/server/responses-custom-tool-repair.ts` continues to own retained routed argument bytes and -their charge/release lifecycle while it asks the pure progressive decoder in -`src/responses/progressive-freeform-input.ts` which prefix is safe to publish. The decoder neither -charges nor releases translator budget; extracting it from `src/bridge/sse.ts` does not create a +their charge/release lifecycle while it asks a per-call progressive reader from +`src/responses/progressive-freeform-input.ts` which prefix is safe to publish. The reader holds only +parse position and decoded text, and it neither charges nor releases translator budget; extracting +it from `src/bridge/sse.ts` does not create a second retention owner or change terminal, failure, incomplete, or disposal release behavior. +Both callers keep one reader per open call, in their own call state, and feed it the growing +arguments. It resumes where the previous delta stopped, so the canonical value is decoded and +checked for a nested executable wrapper once per call; rescanning both on every delta made +one-character deltas quadratic in the body length. Canonical routed `apply_patch` progress is also tracked by that owner. If authoritative completion does not extend the emitted stable prefix, the owner releases every retained call and pending block before raising the rewrite failure; the relay emits a failed terminal and never exposes a successful diff --git a/structure/transports/responses.md b/structure/transports/responses.md index e7769b996e6..fbf26244e4a 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -152,7 +152,7 @@ literal opening. `JSON.parse` decides the completed input, and it cares about ne order nor how a name is spelled, so a canonical key arriving after other properties or written with an escape is the same wrapper and has to preview as one (#5151). The buffering policy that follows is: an own string `input` streams progressively (completion prefers it to any other -member) unless its executable body could still be a nested wrapper. A non-string `input`, a text +member) unless its executable body could still be a nested wrapper (a nested non-string `input` holds only while a later string duplicate can follow). A non-string `input`, a text that is not an object, and an object `JSON.parse` can no longer accept all publish their own bytes, because that is what completion returns for them; every other object HOLDS until it closes, because a key that has not arrived yet can still change the answer. Fallback fields decide at that diff --git a/tests/responses/responses-freeform-wrapper-keys.test.ts b/tests/responses/responses-freeform-wrapper-keys.test.ts index ab1552375f5..bc5550f1ec8 100644 --- a/tests/responses/responses-freeform-wrapper-keys.test.ts +++ b/tests/responses/responses-freeform-wrapper-keys.test.ts @@ -1,7 +1,7 @@ import { describe, expect, spyOn, test } from "bun:test"; import { unwrapFreeformToolInput } from "../../src/responses/apply-patch-envelope"; import { MAX_FREEFORM_WRAPPER_SCAN_CHARS } from "../../src/responses/freeform-wrapper-scan"; -import { progressiveFreeformInput } from "../../src/responses/progressive-freeform-input"; +import { ProgressiveFreeformInputReader, progressiveFreeformInput } from "../../src/responses/progressive-freeform-input"; /** * #5151. The progressive decoder matched the buffer against the literal wrapper opening @@ -285,4 +285,63 @@ describe("freeform wrapper keys the literal matcher could not see", () => { expect(published(lateInvalid, "exec")).toBe("safe"); expect(unwrapFreeformToolInput(lateInvalid, "exec")).toBe(lateInvalid); }); + + test("streaming a long nested wrapper one character at a time scans linearly", () => { + // Each delta used to decode the whole accumulated value and walk the whole decoded body + // again for a possible nested wrapper, so one-character deltas cost quadratic work. The + // reader resumes where the previous delta stopped; count what it scans instead of timing it. + const scanned = (length: number): { chars: number; args: number } => { + const source = Array.from({ length }, (_, i) => + i % 17 === 0 ? "\n" : i % 13 === 0 ? '"' : i % 11 === 0 ? "\\" : i % 7 === 0 ? "\u00e9" : "a").join(""); + const args = JSON.stringify({ input: JSON.stringify({ input: source }) }); + expect(unwrapFreeformToolInput(args, "exec")).toBe(source); + const reader = new ProgressiveFreeformInputReader(); + for (let end = 1; end <= args.length; end++) { + // The wrapper stays hidden at every prefix, including the complete one. + expect(reader.update(args.slice(0, end), "exec")).toBeNull(); + } + return { chars: reader.scannedChars, args: args.length }; + }; + for (const length of [4_000, 16_000]) { + const { chars, args } = scanned(length); + expect({ length, linear: chars <= 3 * args }).toEqual({ length, linear: true }); + } + }); + + test("a non-string nested input holds only while a later duplicate can still unwrap it", () => { + const wrap = (body: string) => JSON.stringify({ input: body }); + // Once text after the object rules a wrapper out, the held prefix flushes and streams on. + for (const body of ['{"input":1}.input', '{"input":[1,"]"]} + 1', '{"input":"a","input":2}']) { + expect({ body, completed: unwrapFreeformToolInput(wrap(body), "exec") }).toEqual({ body, completed: body }); + const { deltas, retractions } = stream(wrap(body), "exec"); + expect({ body, streamed: deltas.join(""), retractions }).toEqual({ body, streamed: body, retractions: [] }); + } + expect(emissions(wrap('{"input":1}.input + more()'), "exec").length).toBeGreaterThan(1); + // A later duplicate replaces the non-string value, so this one stays hidden throughout. + const replaced = wrap('{"input":1,"input":{"a":"}"},"input":"x"}'); + expect(unwrapFreeformToolInput(replaced, "exec")).toBe("x"); + expect(stream(replaced, "exec")).toEqual({ deltas: [], retractions: [] }); + }); + + test("a reader resumed across deltas answers like a fresh read of each prefix", () => { + const bodies = [ + JSON.stringify({ input: JSON.stringify({ input: 'text("\\u00e9\\n")' }) }), + JSON.stringify({ input: '{"input":1}.input' }), + JSON.stringify({ input: "{ let x = 1; }\uD83D\uDE00" }).replace("\u{1F600}", "\\ud83d\\ude00"), + '{"input":"a\\ud83dX"}', + '{"metadata":1,"input":"cmd\\q"}', + '{"code":"cmd"} ', + "plain body", + ]; + for (const tool of ["exec", "apply_patch", ""]) { + for (const body of bodies) { + const reader = new ProgressiveFreeformInputReader(); + for (let end = 1; end <= body.length; end++) { + const prefix = body.slice(0, end); + expect({ prefix, tool, resumed: reader.update(prefix, tool) }) + .toEqual({ prefix, tool, resumed: progressiveFreeformInput(prefix, tool) }); + } + } + } + }); });