From 5ce0b8af14f44e448b11576196d16087d121758a Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 18 Sep 2026 19:21:15 +0900 Subject: [PATCH] fix(bridge): hold a freeform wrapper the completed item will unwrap #4983 widened what counts as a freeform wrapper at completion. Partial input streaming still knew only the compact `{"input":"` form, so a wrapper such as `{"code":"const x = 1"}` streamed as raw JSON through `response.custom_tool_call_input.delta` and then finished with the unwrapped body. Concatenated deltas no longer equalled the authoritative input, and a client that renders or accumulates tool input mid-stream had to rewind. `input` stays progressive: `unwrapFreeformToolInput` returns it whenever the key is present, whatever else the object carries, so its value is decidable from the prefix and can never be retracted. A fallback key is not decidable that way. It unwraps only 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. Those buffers are held until the object closes and then published once. The routed passthrough in `responses-custom-tool-repair.ts` already holds any object prefix for the same reason. The streaming side also drops the tool name for a namespaced tool that does not own the apply-patch grammar, because `repairFreeformToolInput` drops it at completion; streaming under a vocabulary the completed item does not use is the same disagreement in the other direction. Closes #5047. --- src/bridge/sse.ts | 60 +++++++++++++++++--- src/responses/apply-patch-envelope.ts | 12 ++++ tests/adapters/bridge.test.ts | 82 +++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 7 deletions(-) diff --git a/src/bridge/sse.ts b/src/bridge/sse.ts index eda83f01f78..a3eea6c9b42 100644 --- a/src/bridge/sse.ts +++ b/src/bridge/sse.ts @@ -16,7 +16,12 @@ import { type OcxErrorPayload, } from "../lib/errors"; import { redactSecretString } from "../lib/redact"; -import { mayBecomePatchEnvelope, repairFreeformToolInput } from "../responses/apply-patch-envelope"; +import { + freeformFallbackKeys, + mayBecomePatchEnvelope, + repairFreeformToolInput, + unwrapFreeformToolInput, +} from "../responses/apply-patch-envelope"; import { encodeCompactionSummary } from "../responses/compaction"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../responses/code-mode-helper-compat"; import { isTruncatedStopReason, truncationReasonFor } from "../responses/truncated-stop-reason"; @@ -153,8 +158,38 @@ export function bridgeToResponsesSSE( // the completed custom_tool_call item stays authoritative). Compact `{"input":"...` // buffers get their string value progressively unescaped; anything else streams raw. const FREEFORM_WRAP_PREFIX = '{"input":"'; - const freeformPartialInput = (args: string): string => { - if (!args.startsWith(FREEFORM_WRAP_PREFIX)) return args; + /** `{"key":"` for every wrapper this tool name accepts. Keys are distinct, so order is free. */ + const freeformWrapPrefixes = (toolName: string): string[] => [ + FREEFORM_WRAP_PREFIX, + ...freeformFallbackKeys(toolName).map(key => `{"${key}":"`), + ]; + /** + * The value to stream so far, or `null` to HOLD because nothing can be decided yet. + * + * `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. + * + * A fallback key is not. 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). + */ + const freeformPartialInput = (args: string, toolName: string): string | null => { + const prefixes = freeformWrapPrefixes(toolName); + // Still an ambiguous prefix of some wrapper: which wrapper, if any, is not known yet. + if (prefixes.some(prefix => prefix.startsWith(args))) return null; + if (!args.startsWith(FREEFORM_WRAP_PREFIX)) { + if (!prefixes.some(prefix => args.startsWith(prefix))) return args; + // Committed to a fallback wrapper. Undecidable until the object is complete. + try { + JSON.parse(args); + } catch { + return null; + } + return unwrapFreeformToolInput(args, toolName); + } const body = args.slice(FREEFORM_WRAP_PREFIX.length); let out = ""; for (let i = 0; i < body.length; i++) { @@ -1075,10 +1110,21 @@ export function bridgeToResponsesSSE( }); } if (currentToolCall.freeform && !currentToolCall.codeModeHelperName) { - // Hold while the buffer is still an ambiguous prefix of the JSON wrapper, - // then stream only the unwrapped input suffix (never rewind on mode flips). - if (!FREEFORM_WRAP_PREFIX.startsWith(currentToolCall.args)) { - const full = freeformPartialInput(currentToolCall.args); + // `freeformPartialInput` holds while the buffer is still an ambiguous prefix + // of a JSON wrapper; otherwise stream only the unwrapped input suffix, never + // rewinding on a mode flip. + // + // The name is dropped for a namespaced tool that does not own the apply-patch + // grammar, because `repairFreeformToolInput` drops it at completion for the + // same reason. Streaming under a vocabulary the completed item does not use + // is the same disagreement in the other direction. + const ownsFreeformGrammar = currentToolCall.namespace === undefined + || currentToolCall.namespace === "functions"; + const full = freeformPartialInput( + currentToolCall.args, + ownsFreeformGrammar ? currentToolCall.name : "", + ); + if (full !== null) { const emitted = currentToolCall.inputEmitted ?? ""; // Also hold a buffer that could still become a complete patch envelope: // at completion such a body is recompiled into an apply_patch helper call, diff --git a/src/responses/apply-patch-envelope.ts b/src/responses/apply-patch-envelope.ts index fdca20259bc..7d3d885a9a0 100644 --- a/src/responses/apply-patch-envelope.ts +++ b/src/responses/apply-patch-envelope.ts @@ -34,6 +34,18 @@ function stripMarkdownCodeFence(text: string, toolName: string): string { return match ? match[1] : text; } +/** + * The single-field wrappers `unwrapFreeformToolInput` accepts for one tool name, besides the + * canonical `input`. + * + * Exported so the streaming side can hold a buffer that is still turning into one of these. + * A second list of key names beside this one is how the streamed bytes and the completed item + * come to disagree, which is the defect it exists to prevent (#5047). + */ +export function freeformFallbackKeys(toolName: string): readonly string[] { + return FREEFORM_FALLBACK_KEYS[toolName] ?? []; +} + /** Unwrap the `{input:string}` function-call wrapper used for freeform tools. */ export function unwrapFreeformToolInput(argumentsText: unknown, toolName = ""): string { if (typeof argumentsText !== "string") return ""; diff --git a/tests/adapters/bridge.test.ts b/tests/adapters/bridge.test.ts index c8026cc1b29..8703d52fb5a 100644 --- a/tests/adapters/bridge.test.ts +++ b/tests/adapters/bridge.test.ts @@ -1735,3 +1735,85 @@ describe("declared tool enforcement is separate from declared tool normalization expect((json.error as Record).message).toContain("undeclared client tool"); }); }); + +// #5047: #4983 widened what counts as a freeform wrapper at COMPLETION, but partial input +// streaming still knew only the compact `{"input":"` form. A wrapper such as `{"code":"..."}` +// therefore streamed as raw JSON deltas and then finished with the unwrapped body, so +// concatenated deltas no longer equalled the authoritative input and a client that renders +// tool input mid-stream had to rewind. +describe("fallback freeform wrappers stream one stable representation (#5047)", () => { + const FALLBACK_KEYS = ["code", "script", "js", "javascript", "command", "cmd", "content"]; + + async function streamExec(chunks: string[]) { + return collectSse(bridgeToResponsesSSE(replay([ + { type: "tool_call_start", id: "c1", name: "exec" } as AdapterEvent, + ...chunks.map(chunk => ({ type: "tool_call_delta", arguments: chunk }) as AdapterEvent), + { type: "tool_call_end" } as AdapterEvent, + { type: "done" } as AdapterEvent, + ]), "model", undefined, new Set(["exec"]))); + } + + function inputView(frames: { event?: string; data: Record }[]) { + const deltas = frames + .filter(f => f.event === "response.custom_tool_call_input.delta") + .map(f => String(f.data.delta)); + const done = frames.find(f => f.event === "response.custom_tool_call_input.done")?.data.input; + const item = frames.find(f => f.event === "response.output_item.done")?.data.item as Record | undefined; + return { concatenated: deltas.join(""), done, itemInput: item?.input }; + } + + test("delta concatenation equals the completed input for every accepted fallback key", async () => { + for (const key of FALLBACK_KEYS) { + const body = "const x = 1;"; + const view = inputView(await streamExec([JSON.stringify({ [key]: body })])); + expect({ key, ...view }).toEqual({ key, concatenated: body, done: body, itemInput: body }); + } + }); + + test("a wrapper split at every byte boundary never leaks raw JSON and never rewinds", async () => { + const wrapper = JSON.stringify({ code: "a\nb" }); + for (let cut = 1; cut < wrapper.length; cut++) { + const view = inputView(await streamExec([wrapper.slice(0, cut), wrapper.slice(cut)])); + expect({ cut, ...view }).toEqual({ cut, concatenated: "a\nb", done: "a\nb", itemInput: "a\nb" }); + } + }); + + test("an ambiguous multi-field object stays unrepaired and byte-exact", async () => { + // Two string fallback fields: `unwrapFreeformToolInput` declines to guess and returns the + // object unchanged. The stream must reach the same answer, which is the whole point of + // holding until the object closes rather than unwrapping the first key that appears. + const wrapper = JSON.stringify({ code: "a", script: "b" }); + expect(inputView(await streamExec([wrapper]))) + .toEqual({ concatenated: wrapper, done: wrapper, itemInput: wrapper }); + + // A non-string value is not a wrapper either, and it never matched `{"code":"`, so it was + // never held: this pins that ordinary bodies keep streaming immediately. + const numeric = JSON.stringify({ code: 1 }); + expect(inputView(await streamExec([numeric]))) + .toEqual({ concatenated: numeric, done: numeric, itemInput: numeric }); + }); + + test("the canonical input wrapper still streams progressively", async () => { + // `input` wins by precedence in `unwrapFreeformToolInput` whatever else the object carries, + // so it stays decidable from its prefix and must not regress into holding. + const frames = await streamExec(['{"input":"line1\\', 'nline2"}']); + expect(frames.filter(f => f.event === "response.custom_tool_call_input.delta").length) + .toBeGreaterThan(1); + expect(inputView(frames)) + .toEqual({ concatenated: "line1\nline2", done: "line1\nline2", itemInput: "line1\nline2" }); + }); + + test("a stream that dies inside a held wrapper manufactures no tool call", async () => { + // The held buffer is suppressed output, never content. An aborted turn must not turn it + // into a completed call, and must not release it as raw JSON either. + const frames = await collectSse(bridgeToResponsesSSE(replay([ + { type: "tool_call_start", id: "c1", name: "exec" } as AdapterEvent, + { type: "tool_call_delta", arguments: '{"code":"const x = 1' } as AdapterEvent, + ]), "model", undefined, new Set(["exec"]))); + expect(frames.filter(f => f.event === "response.custom_tool_call_input.delta")).toEqual([]); + const completed = frames + .map(f => f.data.item as Record | undefined) + .filter(item => item?.type === "custom_tool_call" && item?.status === "completed"); + expect(completed).toEqual([]); + }); +});