Repository navigation
fix(responses): restore plaintext V2 calls from verified unlabeled SSE #5683
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -127,6 +127,80 @@ import { restoreRoutedCustomCallsInJson } from "../../responses/custom-tool-comp | |
| import { restoreRoutedToolSearchCallsInJson } from "../../responses/tool-search-compat"; | ||
| import { responsesJsonToSseStream } from "../responses-json-events"; | ||
|
|
||
| const PLAINTEXT_V2_SSE_PREFIX_LIMIT = 4096; | ||
|
|
||
| function classifyPlaintextV2SsePrefix(prefix: string): "sse" | "unknown" | "more" { | ||
| const lastLineEnd = prefix.lastIndexOf("\n"); | ||
| if (lastLineEnd < 0) return "more"; | ||
| for (const rawLine of prefix.slice(0, lastLineEnd + 1).split("\n")) { | ||
| const line = rawLine.replace(/\r$/, "").trim(); | ||
| if (!line || line.startsWith(":")) continue; | ||
| if (/^(id|retry):/.test(line)) continue; | ||
| if (line.startsWith("event:")) { | ||
| return /^event:\s*(?:response\.[\w.-]+|error)$/.test(line) ? "sse" : "unknown"; | ||
| } | ||
| if (line.startsWith("data:")) { | ||
| try { | ||
| const value = JSON.parse(line.slice(5).trim()) as { type?: unknown }; | ||
| return typeof value.type === "string" && /^(?:response\.[\w.-]+|error)$/.test(value.type) | ||
| ? "sse" : "unknown"; | ||
| } catch { | ||
| return "unknown"; | ||
| } | ||
| } | ||
| return "unknown"; | ||
| } | ||
| return "more"; | ||
| } | ||
|
|
||
| /** Confirm an unlabeled successful body is Responses SSE before alias restoration. */ | ||
| async function classifyPlaintextV2SseResponse(response: Response): Promise<Response> { | ||
| if (!response.body) return response; | ||
| const reader = response.body.getReader(); | ||
| const decoder = new TextDecoder(); | ||
| const buffered: Uint8Array[] = []; | ||
| let prefix = ""; | ||
| let inspectedBytes = 0; | ||
| while (inspectedBytes < PLAINTEXT_V2_SSE_PREFIX_LIMIT) { | ||
| const next = await reader.read(); | ||
|
Contributor
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- classifier and delivery sections ---'
sed -n '1,230p' src/server/responses/passthrough-delivery.ts
printf '%s\n' '--- delivery integration ---'
sed -n '250,330p' src/server/responses/passthrough-delivery.ts
printf '%s\n' '--- relevant symbols and timeout/abort references ---'
rg -n -C 3 'classifyPlaintextV2SseResponse|reader\.read|AbortSignal|abort|timeout|deadline|deliverPassthroughResponse' src/server/responses src | head -n 240
printf '%s\n' '--- changed-file diff against supplied base ---'
git diff --no-ext-diff --unified=30 aed3bb8f420ff75205b04ab83235c20a78c3ba93 3c08c9cfca3f1bc18cc8d0815b1001be8fc20f70 -- src/server/responses/passthrough-delivery.tsRepository: lidge-jun/opencodex Length of output: 45037 🏁 Script executed: #!/bin/bash
set -eu
sed -n '1,230p' src/server/responses/passthrough-delivery.ts
sed -n '250,330p' src/server/responses/passthrough-delivery.ts
rg -n -C 3 'classifyPlaintextV2SseResponse|reader\.read|AbortSignal|abort|timeout|deadline|deliverPassthroughResponse' src/server/responses src
git diff --no-ext-diff --unified=30 aed3bb8f420ff75205b04ab83235c20a78c3ba93 3c08c9cfca3f1bc18cc8d0815b1001be8fc20f70 -- src/server/responses/passthrough-delivery.tsRepository: lidge-jun/opencodex Length of output: 45535 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- passthrough delivery signal/lifetime references ---'
rg -n -C 5 'deliverPassthroughResponse|linkAbortSignal|UPSTREAM_JSON_BODY_READ_OPTIONS|options\.abortSignal|req\.signal|responseCompletionCancelled|trackStreamLifetime|registerTurn|unregisterTurn' src/server/responses/passthrough-delivery.ts src/server/responses/*.ts
printf '%s\n' '--- core lifetime definitions ---'
rg -n -C 8 'export .*linkAbortSignal|function linkAbortSignal|UPSTREAM_JSON_BODY_READ_OPTIONS|inactivityTimeout|firstByteTimeout|totalTimeout' src/server/responses/core-lifetime.ts src/lib/bounded-body.ts
printf '%s\n' '--- callers and upstream response creation ---'
rg -n -C 8 'deliverPassthroughResponse\(|upstreamResponse\s*=|upstreamResponse:' src/server/responses --glob '*.ts'Repository: lidge-jun/opencodex Length of output: 42113 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- passthrough execution ---'
sed -n '1,180p' src/server/responses/passthrough-execution.ts
printf '%s\n' '--- inactivity guard implementation and usages ---'
rg -n -C 12 'guardDirectPassthroughBodyInactivity|function guardDirectPassthroughBodyInactivity|response-body-inactivity' src/lib src/serverRepository: lidge-jun/opencodex Length of output: 13813 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- upstream controller and fetch setup ---'
rg -n -C 12 'new AbortController|upstream\.abort|linkAbortSignal|fetchWithHeaderTimeout|providerFetch|upstream.signal' src/server/responses/passthrough-dispatch.ts src/server/responses/fetch-helpers.ts
printf '%s\n' '--- delivery error response branches ---'
rg -n -C 10 'formatPassthroughUpstreamError|upstream_error|formatErrorResponse|ResponseBodyInactivityError|readResponse' src/server/responses/passthrough-delivery.tsRepository: lidge-jun/opencodex Length of output: 42261 Bound the first prefix read.
If the upstream sends successful headers and then no body chunk, the request can remain pending until the client disconnects. Pass the upstream abort signal and a body deadline into the classifier. On timeout, cancel the reader and return the existing 502 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| if (next.done) break; | ||
| buffered.push(next.value); | ||
| const inspected = next.value.subarray(0, PLAINTEXT_V2_SSE_PREFIX_LIMIT - inspectedBytes); | ||
| inspectedBytes += inspected.byteLength; | ||
| prefix += decoder.decode(inspected, { stream: true }); | ||
| const kind = classifyPlaintextV2SsePrefix(prefix); | ||
| if (kind === "sse") { | ||
| let bufferedIndex = 0; | ||
| const body = new ReadableStream<Uint8Array>({ | ||
| async pull(controller) { | ||
| if (bufferedIndex < buffered.length) { | ||
| controller.enqueue(buffered[bufferedIndex++]!); | ||
| return; | ||
| } | ||
| try { | ||
| const result = await reader.read(); | ||
| if (result.done) controller.close(); | ||
| else controller.enqueue(result.value); | ||
| } catch (error) { | ||
| controller.error(error); | ||
| } | ||
| }, | ||
| cancel(reason) { return reader.cancel(reason); }, | ||
| }); | ||
| const headers = new Headers(response.headers); | ||
| headers.set("content-type", "text/event-stream"); | ||
| return new Response(body, { status: response.status, statusText: response.statusText, headers }); | ||
| } | ||
| if (kind === "unknown") break; | ||
| } | ||
| void reader.cancel().catch(() => {}); | ||
| return new Response(null, { | ||
| status: response.status, | ||
| statusText: response.statusText, | ||
| headers: response.headers, | ||
| }); | ||
| } | ||
|
|
||
| /** One responsibility of the Responses request pipeline; state owners are explicit. */ | ||
| export async function deliverPassthroughResponse( | ||
| requestContext: Pick<ResponsesRequestContext, "logCtx" | "config" | "options" | "req">, | ||
|
|
@@ -180,7 +254,6 @@ export async function deliverPassthroughResponse( | |
| ): Promise<Response> { | ||
| const { logCtx, config, options, req } = requestContext; | ||
| const { | ||
| upstreamResponse, | ||
| codexSafetyBufferingOptions, | ||
| upstream, | ||
| connectMs, | ||
|
|
@@ -205,18 +278,26 @@ export async function deliverPassthroughResponse( | |
| const { openAiSidecar } = sidecarState; | ||
| const { requestBindings } = transportState; | ||
|
|
||
| let upstreamResponse = nativeExchange.upstreamResponse; | ||
| const originalContentType = upstreamResponse.headers.get("content-type"); | ||
| if (isUsageDebugEnabled() && originalContentType) logCtx.usageDebugContentType = originalContentType; | ||
| if (responseEffects.plaintextV2AgentMessageToolNames.size > 0 | ||
| && upstreamResponse.ok && upstreamResponse.body && parsed.stream | ||
| && !originalContentType?.toLowerCase().includes("text/event-stream") | ||
| && !originalContentType?.toLowerCase().includes("application/json") | ||
| && !isCodexWsUpstreamResponse(upstreamResponse) | ||
| && !(options.nativeControl && isNativeControlResponse(upstreamResponse))) { | ||
| upstreamResponse = await classifyPlaintextV2SseResponse(upstreamResponse); | ||
| } | ||
|
|
||
| const headers = sanitizePassthroughHeaders(upstreamResponse.headers, codexSafetyBufferingOptions); | ||
| const resolvedModel = headers.get("openai-model")?.trim(); | ||
| if (resolvedModel) { | ||
| logCtx.servedModel = resolvedModel; | ||
| if (!logCtx.preserveResolvedModelFromRoute) logCtx.resolvedModel = resolvedModel; | ||
| } | ||
| if (isUsageDebugEnabled()) { | ||
| const upstreamContentType = upstreamResponse.headers.get("content-type"); | ||
| if (upstreamContentType) logCtx.usageDebugContentType = upstreamContentType; | ||
| } | ||
| // The chatgpt backend may omit Content-Type on SSE responses. Fall back to | ||
| // treating a successful body as SSE when the caller requested streaming. | ||
| // ChatGPT may omit Content-Type on SSE responses. Plaintext V2 responses | ||
| // reach this fallback only after their first Responses event is confirmed. | ||
| const passthroughCt = headers.get("content-type")?.toLowerCase(); | ||
| const isEventStream = passthroughCt?.includes("text/event-stream") | ||
| || (responseEffects.plaintextV2AgentMessageToolNames.size === 0 && upstreamResponse.ok && !!upstreamResponse.body && !passthroughCt && parsed.stream); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Limit the documented recovery to unrecognized content types.
src/server/responses/passthrough-delivery.tsLines 286-287 excludeapplication/json, so an SSE body mislabeled as JSON does not receive this prefix check.docs-site/src/content/docs/guides/sub-agent-surface.md#L347-L349: replace the unqualified “mislabels” claim with missing or otherwise unrecognized content types.structure/subagents.md#L35-L37: make the same distinction in the architecture description.As per coding guidelines, “Document current shipped or intentionally pending behavior.” As per path instructions, “Check that user-facing docs stay in sync with actual CLI/API behavior.”
📍 Affects 2 files
docs-site/src/content/docs/guides/sub-agent-surface.md#L347-L349(this comment)structure/subagents.md#L35-L37🤖 Prompt for AI Agents
Sources: Coding guidelines, Path instructions