fix(responses): bound trailing wrapper parsing to prevent quadratic JSON.parse

.github/workflows/ci.yml

@@ -611,12 +611,55 @@ jobs:
           source scripts/ci/bun-crash-signatures.sh
 
           run_macos_suite() {
-            local suite_log suite_status
+            local suite_log suite_status watchdog_pid
             suite_log="$(mktemp -t ocx-macos-suite.XXXXXX)" || return $?
+
+            # --timeout 60000 bounds a test that is awaiting work; it cannot bound a
+            # wedged runner. A Bun isolate on this lane has been observed wedged
+            # inside spawnSync: the spawned child is already dead but the worker's
+            # private wait loop never polls its exit again, so the suite writes
+            # nothing further, the per-test ceiling fires inside the spin without
+            # breaking it, and only the job's timeout-minutes ends it -- twenty
+            # silent minutes later, on whichever file happened to be running.
+            # A suite making progress emits a result line inside every per-test
+            # window, so silence several times that window is a wedge, not slow
+            # work: kill Bun so the leg fails inside minutes, at the file the log
+            # last grouped. macOS has no timeout(1), so the bound is a growth poll
+            # on the log the suite already tees into.
+            (
+              last_size=-1
+              silent_for=0
+              # A suite that finishes kills this subshell mid-`sleep`; the orphaned
+              # sleep would keep the step's output pipes open for its remaining run,
+              # so its descriptors are detached up front.
+              while sleep 15 </dev/null >/dev/null 2>&1; do
+                size="$(wc -c < "$suite_log" 2>/dev/null || echo 0)"
+                if [ "$size" -le "$last_size" ]; then
+                  silent_for=$((silent_for + 15))
+                else
+                  silent_for=0
+                  last_size=$size
+                fi
+                if [ "$silent_for" -ge "${OCX_MACOS_SUITE_SILENCE_SECONDS:-300}" ]; then
+                  echo "::error::macOS suite produced no output for ${silent_for}s -- a wedged isolate, not slow work (several times the 60s per-test ceiling). Killing Bun; the last ##[group] in this log names the file it stalled inside."
+                  pkill -KILL -x bun 2>/dev/null || true
+                  # tee's read ends only when every descendant that inherited the
+                  # merged stream is dead; a lingering grandchild must not hang the
+                  # leg a second time, so give EOF a moment, then reap tee itself.
+                  sleep 5 </dev/null >/dev/null 2>&1
+                  pkill -KILL -f "tee $suite_log" 2>/dev/null || true
+                  exit 0
+                fi
+              done
+            ) &
+            watchdog_pid=$!
+
             # The per-test ceiling applies to every invocation, including each isolated
             # serial file. One attempt, whatever the outcome.
             bun test --isolate --timeout 60000 "$@" 2>&1 | tee "$suite_log"
             suite_status="${PIPESTATUS[0]}"
+            kill "$watchdog_pid" 2>/dev/null
+            wait "$watchdog_pid" 2>/dev/null
             if [ "$suite_status" -eq 0 ]; then
               rm -f "$suite_log"
               return 0

src/responses/apply-patch-envelope.ts

@@ -28,6 +28,11 @@ const FREEFORM_FALLBACK_KEYS: Readonly<Record<string, readonly string[]>> = {
   apply_patch: ["patch", "content"],
 };
 
+/** The field names a tool's freeform wrapper may unwrap through, besides canonical `input`. */
+export function freeformFallbackKeys(toolName: string): readonly string[] {
+  return FREEFORM_FALLBACK_KEYS[toolName] ?? [];
+}
+
 function stripMarkdownCodeFence(text: string, toolName: string): string {
   if (toolName !== "exec" && toolName !== "apply_patch") return text;
   const match = OUTER_MARKDOWN_CODE_FENCE.exec(text.trim());
@@ -46,7 +51,7 @@ export function unwrapFreeformToolInput(argumentsText: unknown, toolName = ""):
           ? stripMarkdownCodeFence(record.input, toolName)
           : argumentsText;
       }
-      const fallbackKeys = FREEFORM_FALLBACK_KEYS[toolName] ?? [];
+      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);

src/responses/freeform-wrapper-scan.ts

@@ -19,7 +19,8 @@
 //   `raw`    no wrapper can apply, because the text is not an object or because it is one that
 //            `JSON.parse` will reject. Completion returns the buffer, so streaming it agrees.
 //   `hold`   undecided. A key that has not arrived yet can still change the answer, so nothing
-//            is published until the object parses and completion's own rule decides.
+//            is published until the object closes — where the scan itself already holds the
+//            member table completion's own rule consults, so no parse of the buffer is needed.
 //
 // The bound matters as much as the classification. A scan that walks the whole buffer on every
 // delta is quadratic in the argument size, so classification gives up after
@@ -53,7 +54,7 @@ const HOLD = -1;
 const NEVER = -2;
 
 export type FreeformWrapperScan =
-  | { kind: "hold"; parse: boolean }
+  | { kind: "hold" }
   | { kind: "raw" }
   | { kind: "input"; valueStart: number };
 
@@ -192,38 +193,36 @@ function scanValue(text: string, from: number): number {
 /**
  * Which wrapper the completed text will unwrap to, as far as this prefix can say.
  *
- * Fallback keys are deliberately not recognized here. They only unwrap when exactly one of them
- * carries a string, and a second one can still arrive, so no prefix decides them — which makes
- * them indistinguishable from any other undecided object and lets one HOLD cover both.
+ * `fallbackKeys` is the tool's alternate-field vocabulary — `freeformFallbackKeys`, the same
+ * list `unwrapFreeformToolInput` filters against. No open prefix can decide a fallback key,
+ * because it unwraps only as the SINGLE string field and a second one can still arrive; so the
+ * scan keeps a table of the members it has already walked and consults it once, at the close.
  */
-export function scanFreeformWrapper(text: string): FreeformWrapperScan {
+export function scanFreeformWrapper(text: string, fallbackKeys: readonly string[]): FreeformWrapperScan {
   // One clamp rather than a budget threaded through every helper. Every helper already holds
   // when it runs off the end of what it can see, so a buffer whose classification needs more
   // than this holds for exactly the right reason, and no scan can cost more than this many
   // characters however large the arguments grow. Indices into the clamp are indices into the
   // full text, because the clamp is a prefix of it.
-  const bounded = text.length > MAX_FREEFORM_WRAPPER_SCAN_CHARS
-    ? text.slice(0, MAX_FREEFORM_WRAPPER_SCAN_CHARS)
-    : text;
-  // `parse` is true only where this scan actually SAW the object close. Every other hold ran
-  // out of buffer or out of budget, and in both cases asking `JSON.parse` is work with no
-  // possible payoff: the first is provably incomplete, and the second would re-read a growing
-  // buffer on every delta that happens to end in a brace — repeated braces inside a long
-  // unterminated string are enough to make that quadratic. Holding a budget-exhausted prefix
-  // costs nothing that matters, because the value it would release arrives in the same instant
-  // as the authoritative completion that follows it.
-  const hold = (): FreeformWrapperScan => ({ kind: "hold", parse: false });
+  const wholeText = text.length <= MAX_FREEFORM_WRAPPER_SCAN_CHARS;
+  const bounded = wholeText ? text : text.slice(0, MAX_FREEFORM_WRAPPER_SCAN_CHARS);
+  const hold = (): FreeformWrapperScan => ({ kind: "hold" });
   const open = skipWhitespace(bounded, 0);
   if (open === HOLD) return hold();
   // Not an object, so no wrapper rule reaches it: arrays, scalars and ordinary bodies all
   // complete as themselves.
   if (bounded[open] !== "{") return { kind: "raw" };
 
+  // Every top-level member is recorded as the scan passes it, last occurrence winning — the
+  // keep-last rule `JSON.parse` applies to duplicate keys. When the object closes, this table
+  // answers the only question completion asks of it — which members carried strings — so the
+  // closed object classifies directly instead of reparsing a buffer the scan just walked.
+  const members = new Map<string, { stringValue: boolean; valueStart: number }>();
   let i = open + 1;
   for (;;) {
     const at = skipWhitespace(bounded, i);
     if (at === HOLD) return hold();
-    if (bounded[at] === "}") return afterTopLevelClose(bounded, at + 1);
+    if (bounded[at] === "}") return afterTopLevelClose(bounded, at + 1, members, fallbackKeys, wholeText);
     if (bounded[at] !== '"') return { kind: "raw" };
 
     const nameEnd = scanString(bounded, at);
@@ -254,6 +253,7 @@ export function scanFreeformWrapper(text: string): FreeformWrapperScan {
         : { kind: "raw" };
     }
 
+    members.set(name, { stringValue: bounded[valueAt] === '"', valueStart: valueAt + 1 });
     const valueEnd = scanValue(bounded, valueAt);
     if (valueEnd === HOLD) return hold();
     if (valueEnd === NEVER) return { kind: "raw" };
@@ -264,16 +264,35 @@ export function scanFreeformWrapper(text: string): FreeformWrapperScan {
       i = next + 1;
       continue;
     }
-    if (bounded[next] === "}") return afterTopLevelClose(bounded, next + 1);
+    if (bounded[next] === "}") return afterTopLevelClose(bounded, next + 1, members, fallbackKeys, wholeText);
     return { kind: "raw" };
   }
 }
 
 /**
  * The object closed without a canonical key. Only whitespace may follow one that parses, so
- * anything else makes the text raw; otherwise the completed parse decides between a fallback
- * wrapper and no wrapper at all.
+ * anything else makes the text raw. Whitespace to the end IS legal, but the member table
+ * already says which way completion goes on it: exactly one string-valued fallback field is
+ * the wrapper's value, and any other member shape streams the raw text — byte-exact through
+ * whatever trailing whitespace arrived, where a reparsed growing buffer was quadratic work on
+ * every whitespace delta and a held one swallowed the whitespace bytes entirely (#566).
  */
-function afterTopLevelClose(text: string, from: number): FreeformWrapperScan {
-  return skipWhitespace(text, from) === HOLD ? { kind: "hold", parse: true } : { kind: "raw" };
+function afterTopLevelClose(
+  text: string,
+  from: number,
+  members: ReadonlyMap<string, { stringValue: boolean; valueStart: number }>,
+  fallbackKeys: readonly string[],
+  wholeText: boolean,
+): FreeformWrapperScan {
+  const trailing = skipWhitespace(text, from);
+  if (trailing !== HOLD) return { kind: "raw" };
+  // A clamped prefix ends inside this whitespace, so what follows is unseen: hold rather than
+  // guess at a tail that could still invalidate the close. When the whole text fit, the object
+  // plus its trailing whitespace is complete — and its members decide it without a parse.
+  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: "raw" };
 }

src/responses/progressive-freeform-input.ts

@@ -1,4 +1,4 @@
-import { unwrapFreeformToolInput } from "./apply-patch-envelope";
+import { freeformFallbackKeys } from "./apply-patch-envelope";
 import { JSON_ESCAPES, scanFreeformWrapper } from "./freeform-wrapper-scan";
 
 /**
@@ -92,11 +92,12 @@ function decodeJsonStringPrefix(body: string): string | null {
  * damage is what is available without giving up progressive streaming, and the args are
  * unusable in that case whichever representation wins.
  *
- * A fallback key is not decidable. It only unwraps when it is the SINGLE string field, and a
- * second key can still arrive — so a value emitted early would have to be taken back. That is
- * the rewind this holds instead: stream nothing until the object closes, then publish the one
- * repaired body. The routed passthrough in `responses-custom-tool-repair.ts` already holds
- * any object prefix for the same reason (#5047).
+ * A fallback key decides only at the object's close. It unwraps as the SINGLE string field,
+ * and a second key can still arrive while the object is open — so a value emitted early would
+ * have to be taken back. The scan therefore keeps a member table and consults it at the
+ * close, publishing the wrapped value (or the raw text) without ever reparsing the buffer.
+ * The routed passthrough in `responses-custom-tool-repair.ts` already holds any object prefix
+ * for the same reason (#5047).
  *
  * An object that has not reached a canonical key YET is in exactly that position, and used to
  * be treated as raw because it did not match the literal `{"input":"`. It holds now: `input`
@@ -106,25 +107,18 @@ function decodeJsonStringPrefix(body: string): string | null {
  * not holding was publishing bytes the completed item removes.
  */
 export function progressiveFreeformInput(args: string, toolName: string): string | null {
-  const scan = scanFreeformWrapper(args);
+  const scan = scanFreeformWrapper(args, freeformFallbackKeys(toolName));
   if (scan.kind === "input") {
     const decoded = decodeJsonStringPrefix(args.slice(scan.valueStart));
     if (decoded === null) return null;
     return mayBecomeFencedBody(decoded, toolName) ? null : decoded;
   }
   if (scan.kind === "raw") return mayBecomeFencedBody(args, toolName) ? null : args;
 
-  // Undecided: some wrapper may still apply, and only the completed object says which one.
-  // The parse runs exactly where the scan SAW the object close, so it reads a buffer the scan
-  // already walked and its cost is bounded by the same clamp. A hold from either limit stays
-  // held: an incomplete object has nothing to parse, and re-reading a budget-exhausted buffer
-  // on every delta is quadratic work for a delta that would arrive in the same instant as the
-  // authoritative completion behind it.
-  if (!scan.parse) return null;
-  try {
-    JSON.parse(args);
-  } catch {
-    return null;
-  }
-  return unwrapFreeformToolInput(args, toolName);
+  // 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;
 }

structure/transports/responses.md

@@ -140,15 +140,17 @@ follows is: an own `input` with a string value streams progressively, because co
 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
 all publish their own bytes, because that is what completion returns for them; every other
-object HOLDS until it parses, because a key that has not arrived yet can still change the
-answer. Fallback fields fall out of that last rule rather than being recognized separately:
-they only unwrap as the single string field, so no prefix decides them. Classification is
+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. The parse
-that releases a held object therefore runs only where the scan SAW the object close, which is
-what keeps a buffer whose deltas happen to end on a brace from being re-read on every one of
-them.
+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

tests/responses/responses-freeform-wrapper-keys.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, test } from "bun:test";
+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";
@@ -111,6 +111,45 @@ describe("freeform wrapper keys the literal matcher could not see", () => {
     expect(emissions(program, "exec").length).toBeGreaterThan(1);
   });
 
+  test("does not reparse a closed object on trailing whitespace deltas", () => {
+    const parse = spyOn(JSON, "parse");
+    try {
+      const body = "{}" + " ".repeat(MAX_FREEFORM_WRAPPER_SCAN_CHARS + 1);
+      // Inside the bound every whitespace prefix publishes byte-exact; past it the scan holds
+      // and completion stays authoritative, exactly like every other budget-exhausted buffer.
+      expect(published(body, "exec")).toBe(body.slice(0, MAX_FREEFORM_WRAPPER_SCAN_CHARS));
+      // Nothing reparses the accumulated buffer at all anymore: a closed object resolves from
+      // the scan's own member table, and `{}` carries no member names to decode. Provider-
+      // controlled whitespace deltas therefore cannot turn into repeated parse work.
+      expect(parse).not.toHaveBeenCalled();
+    } finally {
+      parse.mockRestore();
+    }
+  });
+
+  test("a closed object's trailing whitespace streams byte-exact", () => {
+    // The review gap in the first fix: once the object closed, later whitespace prefixes held
+    // forever, so the concatenated deltas lost the whitespace the completed item keeps. Every
+    // raw-object spelling must publish all of its bytes, fragmented or in one chunk.
+    for (const body of ["{} ", '{"a":1}   ', '{"code":"a","script":"b"}\t\n', '{ "x" : [ 1 , 2 ] } ']) {
+      expect({ body, completed: unwrapFreeformToolInput(body, "exec") })
+        .toEqual({ body, completed: body });
+      expect({ body, streamed: published(body, "exec") }).toEqual({ body, streamed: body });
+      expect({ body, whole: progressiveFreeformInput(body, "exec") })
+        .toEqual({ body, whole: body });
+      expect({ body, retractions: stream(body, "exec").retractions })
+        .toEqual({ body, retractions: [] });
+    }
+
+    // A closed fallback wrapper publishes its value once; the trailing whitespace is wrapper
+    // syntax rather than input bytes, so it is correctly absent whichever way it arrives.
+    for (const body of ['{"code":"cmd"}', '{"code":"cmd"}  ', '{"code":"cmd"}\n']) {
+      expect({ body, completed: unwrapFreeformToolInput(body, "exec") })
+        .toEqual({ body, completed: "cmd" });
+      expect({ body, streamed: published(body, "exec") }).toEqual({ body, streamed: "cmd" });
+    }
+  });
+
   test("canonical input and plain bodies stay progressive", () => {
     // Holding an undecided object must not cost the progressive streaming these paths provide.
     const canonical = '{"input":"line one\\nline two"}';