Repository navigation
fix(transport): match freeform streaming to its completion grammar and decode SOCKS5 content-codings #5070
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(transport): match freeform streaming to its completion grammar and decode SOCKS5 content-codings #5070
Changes from 1 commit
0d60e1b
3e848f9
558d008
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,40 +157,52 @@ export function bridgeToResponsesSSE( | |
| // (`response.custom_tool_call_input.delta` — codex-rs uses it for UI preview only; | ||
| // 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":"'; | ||
| /** `{"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}":"`), | ||
| ]; | ||
| const JSON_WHITESPACE = new Set([" ", "\t", "\n", "\r"]); | ||
| type WrapperOpening = | ||
| | { state: "none" } | ||
| | { state: "prefix" } | ||
| | { state: "open"; valueStart: number }; | ||
| /** | ||
| * 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. | ||
| * Where the string value of `{"<key>":"` begins, tolerating the insignificant whitespace | ||
| * `JSON.parse` accepts. | ||
| * | ||
| * 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). | ||
| * The earlier form of this compared the buffer against the compact literal `{"key":"`, so a | ||
| * wrapper written with spaces or newlines matched no prefix at all, streamed as raw JSON | ||
| * deltas and then completed as the unwrapped body. That is the same delta/completion | ||
| * disagreement #5047 closed for compact wrappers, reached through a different spelling: | ||
| * `unwrapFreeformToolInput` reads the completed text with `JSON.parse`, which does not care | ||
| * how the object is laid out, so neither can the streaming side. | ||
| */ | ||
| 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; | ||
| const wrapperOpening = (args: string, key: string): WrapperOpening => { | ||
| let index = 0; | ||
| for (const token of ["{", `"${key}"`, ":", '"']) { | ||
| while (index < args.length && JSON_WHITESPACE.has(args[index]!)) index++; | ||
| if (index >= args.length) return { state: "prefix" }; | ||
| for (const expected of token) { | ||
| if (index >= args.length) return { state: "prefix" }; | ||
| if (args[index] !== expected) return { state: "none" }; | ||
| index++; | ||
| } | ||
| return unwrapFreeformToolInput(args, toolName); | ||
| } | ||
| const body = args.slice(FREEFORM_WRAP_PREFIX.length); | ||
| return { state: "open", valueStart: index }; | ||
| }; | ||
| /** | ||
| * Whether a body could still grow into one complete outer Markdown fence. | ||
| * | ||
| * `stripMarkdownCodeFence` removes such a fence at completion for exactly the two tools that | ||
| * own the grammar, so a fenced body's streamed bytes and its completed input disagree unless | ||
| * the stream holds. A buffer that does not open with a fence can never acquire one, so | ||
| * ordinary bodies are unaffected; a buffer that does keeps its preview suppressed for the | ||
| * whole call, because a closing fence can still be followed by more text that withdraws it. | ||
| */ | ||
| const mayBecomeFencedBody = (text: string, toolName: string): boolean => { | ||
| if (toolName !== "exec" && toolName !== "apply_patch") return false; | ||
| const head = text.trimStart(); | ||
| if (head === "") return true; | ||
| return head.startsWith("```") || "```".startsWith(head); | ||
| }; | ||
| /** The decoded prefix of a JSON string body, stopping at the first byte it cannot resolve. */ | ||
| const decodeJsonStringPrefix = (body: string): string => { | ||
| let out = ""; | ||
| for (let i = 0; i < body.length; i++) { | ||
| const c = body[i]; | ||
|
|
@@ -211,6 +223,40 @@ export function bridgeToResponsesSSE( | |
| } | ||
| return out; | ||
| }; | ||
| /** | ||
| * 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 openings = ["input", ...freeformFallbackKeys(toolName)] | ||
| .map(key => ({ key, opening: wrapperOpening(args, key) })); | ||
| const canonical = openings[0]!.opening; | ||
| if (canonical.state === "open") { | ||
| const decoded = decodeJsonStringPrefix(args.slice(canonical.valueStart)); | ||
| return mayBecomeFencedBody(decoded, toolName) ? null : decoded; | ||
|
Comment on lines
+285
to
+288
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a provider emits a malformed JSON-looking freeform body such as AGENTS.md reference: src/AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| } | ||
| if (openings.some(entry => entry.opening.state === "open")) { | ||
| // Committed to a fallback wrapper. Undecidable until the object is complete. | ||
| try { | ||
| JSON.parse(args); | ||
| } catch { | ||
| return null; | ||
| } | ||
| return unwrapFreeformToolInput(args, toolName); | ||
| } | ||
| // Still an ambiguous prefix of some wrapper: which wrapper, if any, is not known yet. | ||
| if (openings.some(entry => entry.opening.state === "prefix")) return null; | ||
| return mayBecomeFencedBody(args, toolName) ? null : args; | ||
| }; | ||
| // tool_search_call carries arguments as a JSON object ({query, limit}); parse the model's arg string. | ||
| const parseArgsObj = (args: string): Record<string, unknown> => { | ||
| try { const o = JSON.parse(args); return o && typeof o === "object" ? o : {}; } catch { return {}; } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -321,6 +321,11 @@ function requestHeaders(request: Request, target: URL): { text: string; chunked: | |
| const headers = new Headers(request.headers); | ||
| if (!headers.has("host")) headers.set("host", target.host); | ||
| if (!headers.has("connection")) headers.set("connection", "close"); | ||
| // This transport assembles the body itself, so a content-coding it did not ask for becomes | ||
| // its own problem to undo. Ask for none by default and leave an explicit caller choice | ||
| // alone: `fetchWithHeaderTimeout` selects identity for some streaming calls and preserves a | ||
| // caller's selection elsewhere, and silently overriding that would make the two disagree. | ||
| if (!headers.has("accept-encoding")) headers.set("accept-encoding", "identity"); | ||
| const chunked = request.body !== null | ||
| && !headers.has("content-length") | ||
| && !headers.has("transfer-encoding"); | ||
|
|
@@ -504,6 +509,28 @@ function responseBody( | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Undo the content-coding this transport has to undo itself. | ||
| * | ||
| * `fetch` decodes content-codings below the Response constructor. This one assembles the body | ||
| * from a raw socket, so `new Response(body, { headers })` hands the compressed bytes straight | ||
| * to whatever reads them: `response.json()` throws SyntaxError on the gzip magic number and an | ||
| * SSE reader sees noise instead of frames. The request asks for `identity`, so a coded | ||
| * response means an upstream ignored that; gzip and deflate are undone here, and any other | ||
| * coding fails closed rather than surfacing bytes no caller can parse. | ||
| * | ||
| * No decompressed-size ceiling is imposed. The identity path has no total-size bound either — | ||
| * it cannot, because a long-lived SSE stream is legitimately unbounded — and a ceiling on only | ||
| * the coded path would fail responses that succeed uncompressed. | ||
| */ | ||
| function contentCodingFormat(headers: Headers): "gzip" | "deflate" | undefined { | ||
| const coding = (headers.get("content-encoding") ?? "").trim().toLowerCase(); | ||
| if (coding === "" || coding === "identity") return undefined; | ||
| if (coding === "gzip" || coding === "x-gzip") return "gzip"; | ||
| if (coding === "deflate") return "deflate"; | ||
| throw new Socks5FetchError("SOCKS5 upstream returned an unsupported content-encoding: " + coding); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| } | ||
|
|
||
| export async function socks5Fetch( | ||
| input: RequestInfo | URL, | ||
| init: RequestInit | undefined, | ||
|
|
@@ -559,12 +586,24 @@ export async function socks5Fetch( | |
| while (responseHead.status >= 100 && responseHead.status < 200 && responseHead.status !== 101) { | ||
| responseHead = parseResponseHead(await reader.readUntil(HEADER_END, MAX_RESPONSE_HEADER_BYTES, request.signal)); | ||
| } | ||
| // Reject a coding this transport cannot undo before the body stream takes the socket, so | ||
| // the failure path is the ordinary one below rather than an orphaned reader. | ||
| const codingFormat = contentCodingFormat(responseHead.headers); | ||
| const body = responseBody(reader, socket, request.signal, responseHead.headers, responseHead.status, request.method); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a Useful? React with 👍 / 👎. |
||
| const responseHeaders = new Headers(responseHead.headers); | ||
| if (codingFormat !== undefined) { | ||
| responseHeaders.delete("content-encoding"); | ||
| // The declared length describes the coded bytes, not what the caller now reads. | ||
| responseHeaders.delete("content-length"); | ||
| } | ||
| const decodedBody = body !== null && codingFormat !== undefined | ||
| ? body.pipeThrough(new DecompressionStream(codingFormat)) | ||
| : body; | ||
| request.signal.removeEventListener("abort", onAbort); | ||
| return new Response(body, { | ||
| return new Response(decodedBody, { | ||
| status: responseHead.status, | ||
| statusText: responseHead.statusText, | ||
| headers: responseHead.headers, | ||
| headers: responseHeaders, | ||
| }); | ||
| } catch (error) { | ||
| request.signal.removeEventListener("abort", onAbort); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -240,4 +240,11 @@ fragmented framing, header limits and explicit-route snapshot preservation. | |
| Explicit `http2` / `h2` pins reject before network I/O: this HTTP/1.1 tunnel cannot | ||
| honor them and must not silently downgrade the provider contract. | ||
|
|
||
| Content-coding is this transport's own obligation. `fetch` decodes a coded body below the | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This changes the AGENTS.md reference: structure/AGENTS.md:L49-L50 Useful? React with 👍 / 👎. |
||
| Response constructor; this tunnel assembles the body from a socket, so a response wrapped with | ||
| its upstream headers hands the coded bytes to whatever parses them. The request therefore asks | ||
| for `identity` unless the caller chose an `accept-encoding` itself, a `gzip` or `deflate` | ||
| response is decoded and stops advertising the coding and the coded length, and any other coding | ||
| is refused by name rather than surfaced as bytes no caller can read. | ||
|
|
||
| Dashboard Fast-row persistence and client refresh follow the [Fast selector rows setting contract](../gui-and-management-api.md#fast-selector-rows-setting). | ||
Uh oh!
There was an error while loading. Please reload this page.