Skip to content

Commit 4f34411

Browse files
committed
fix(responses): close passthrough streams at terminal events
1 parent f9b9440 commit 4f34411

9 files changed

Lines changed: 131 additions & 48 deletions

File tree

src/lib/bun-stream-caps.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
* PR #32120, merged 2026-06-21). No RELEASED Bun version is proven to carry
77
* that fix yet, so `MIN_FIXED_BUN_VERSION` is null: every runtime is
88
* "known-bad" until a bundle-bump commit sets it. Windows no-rewrite traffic
9-
* follows this runtime/config decision. Darwin no-rewrite traffic stays on tee
9+
* reports this runtime/config decision; the final Responses dispatch may still
10+
* require the eager relay to enforce a terminal boundary. Darwin no-rewrite traffic stays on tee
1011
* for `auto` regardless of runtime capability and reaches eager relay only via
1112
* explicit `streamMode: "eager-relay"` opt-in (see
1213
* devlog/_plan/260731_macos_rss_retention/100_darwin_eager_optin.md).

src/server/index.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,18 +250,15 @@ function attachLiveSidebandUpstream(ws: ServerWebSocket<WsData>): void {
250250
// if (isEventStream && upstreamResponse.body) {
251251
// const repairConfig = route.provider.responsesItemIdRepair;
252252
// const needsClientRewrite = imageGenCallAliases.size > 0
253-
// #314 gated shape: win32 no-rewrite traffic follows runtime/config policy; darwin no-rewrite
254-
// traffic requires explicit config-eager opt-in (`auto` always stays tee on darwin). Default OFF
255-
// on the bundled known-bad runtime; policy lives in 260731_macos_rss_retention phase 100.
253+
// #314 gated shape: win32 always uses the terminal-aware eager relay so a keep-alive
254+
// upstream cannot hold Codex open after response.completed; darwin no-rewrite traffic
255+
// requires explicit config-eager opt-in (`auto` always stays tee on darwin).
256256
// selectEagerPath(process.platform, needsClientRewrite, config.streamMode ?? "auto")
257257
// relaySseEagerBounded(upstreamResponse.body, turnAc,
258258
// new Response(eagerBody,
259259
// Default shape (tee + background inspection):
260260
// upstreamResponse.body.tee()
261261
// const repairedBody = hasResponsesItemIdRepair(repairConfig)
262-
// process.platform === "win32"
263-
// && !needsClientRewrite
264-
// ? nativeBody
265262
// relaySseWithFailedTail(repairedBody, upstream)
266263
// new Response(clientBody
267264
// markNativePassthroughSseResponse

src/server/relay-eager.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ export function relaySseEagerBounded(
161161
let queuedBytes = 0;
162162
let cancelled = false;
163163
let done = false;
164+
const terminalSentinel = new TextEncoder().encode("data: [DONE]\n\n");
164165
// Pause gate: resolved by client pull, client cancel, or upstream abort so a
165166
// paused producer ALWAYS resumes (audit blocker 2 — no deadlock; onDone and
166167
// turn unregistration stay reachable, drainAndShutdown never hangs).
@@ -231,16 +232,28 @@ export function relaySseEagerBounded(
231232
continue;
232233
}
233234
const outbound = rewrite ? rewriteOutbound(value) : value;
234-
if (outbound.byteLength === 0) continue;
235-
queuedBytes += outbound.byteLength;
236-
try {
237-
controllerRef?.enqueue(outbound);
238-
} catch {
239-
// Controller already torn down (client went away without cancel()).
240-
cancelled = true;
241-
drainDeadline = now() + drainMs;
242-
armDrainTimer();
243-
continue;
235+
if (outbound.byteLength > 0) {
236+
queuedBytes += outbound.byteLength;
237+
try {
238+
controllerRef?.enqueue(outbound);
239+
} catch {
240+
// Controller already torn down (client went away without cancel()).
241+
cancelled = true;
242+
drainDeadline = now() + drainMs;
243+
armDrainTimer();
244+
continue;
245+
}
246+
}
247+
if (hooks.sawTerminal()) {
248+
// The Responses terminal event ends the turn even when a compatible
249+
// gateway keeps its HTTP connection alive. Add the conventional
250+
// sentinel and stop the single-reader relay at that protocol boundary.
251+
if (!new TextDecoder().decode(outbound).includes("data: [DONE]")) {
252+
queuedBytes += terminalSentinel.byteLength;
253+
try { controllerRef?.enqueue(terminalSentinel); } catch { /* client already gone */ }
254+
}
255+
reader.cancel("Responses terminal event received").catch(() => {});
256+
break;
244257
}
245258
while (queuedBytes > maxQueueBytes && !cancelled && !upstream.signal.aborted) {
246259
await paused();

src/server/relay.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,49 @@ export function relaySseWithFailedTail(
110110
): ReadableStream<Uint8Array> {
111111
const reader = body.getReader();
112112
const encoder = new TextEncoder();
113+
const terminalInspector = createSseInspector({
114+
onTerminal: () => {},
115+
});
116+
let closed = false;
117+
const closeAtTerminal = (controller: ReadableStreamDefaultController<Uint8Array>, value: Uint8Array): boolean => {
118+
terminalInspector.feed(value);
119+
if (!terminalInspector.reported()) return false;
120+
121+
// A Responses terminal frame is the protocol boundary. Some compatible
122+
// gateways leave the HTTP connection open after response.completed, which
123+
// otherwise leaves Codex waiting forever even though the model turn is done.
124+
// Preserve the terminal-bearing chunk, add the conventional sentinel when
125+
// it was not coalesced into that same chunk, then stop reading upstream.
126+
controller.enqueue(value);
127+
if (!new TextDecoder().decode(value).includes("data: [DONE]")) {
128+
controller.enqueue(encoder.encode("data: [DONE]\n\n"));
129+
}
130+
closed = true;
131+
controller.close();
132+
const reason = "Responses terminal event received";
133+
// Notify the tee inspection branch as well. It has already received the
134+
// same terminal-bearing upstream chunk, so its bounded drain records the
135+
// real terminal and then releases the turn/upstream keep-alive connection.
136+
onClientGone?.(reason);
137+
reader.cancel(reason).catch(() => {});
138+
terminalInspector.dispose();
139+
return true;
140+
};
113141
return new ReadableStream<Uint8Array>({
114142
async pull(controller) {
115143
try {
116144
const { done, value } = await reader.read();
117145
if (done) {
146+
terminalInspector.finish();
147+
terminalInspector.dispose();
118148
controller.close();
119149
return;
120150
}
151+
if (closeAtTerminal(controller, value)) return;
121152
controller.enqueue(value);
122153
} catch (err) {
154+
terminalInspector.dispose();
155+
if (closed) return;
123156
const payload = buildFailedTailPayload(err);
124157
try {
125158
// Leading blank line terminates a partial SSE block so the failed frame parses cleanly.
@@ -130,6 +163,7 @@ export function relaySseWithFailedTail(
130163
}
131164
},
132165
cancel(reason) {
166+
terminalInspector.dispose();
133167
if (onClientGone) onClientGone(reason);
134168
else upstream.abort(reason);
135169
reader.cancel(reason).catch(() => {});

src/server/responses/core.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1770,12 +1770,17 @@ async function handleResponsesInner(
17701770
// (Bun#32111 JS-sink segfault — text frames pass, the terminal block is
17711771
// lost). The eager single reader applies the same rewrites inline.
17721772
const win32EagerRewrite = isWin32EagerRewrite(process.platform, needsClientRewrite);
1773+
// A native tee branch cannot be closed at a Responses protocol terminal
1774+
// while its upstream HTTP connection remains open. Windows therefore
1775+
// needs the existing single-reader relay even without payload rewrites;
1776+
// otherwise Codex keeps thinking after response.completed.
1777+
const win32TerminalRelay = process.platform === "win32";
17731778
const eagerPath = selectEagerPath(
17741779
process.platform,
17751780
needsClientRewrite,
17761781
config.streamMode ?? "auto",
17771782
);
1778-
if (eagerPath?.useEagerRelay || win32EagerRewrite) {
1783+
if (eagerPath?.useEagerRelay || win32EagerRewrite || win32TerminalRelay) {
17791784
const turnAc = new AbortController();
17801785
linkAbortSignal(upstream, turnAc.signal);
17811786
registerTurn(turnAc, options.turnAdmissionLease);
@@ -1811,7 +1816,9 @@ async function handleResponsesInner(
18111816
inspectChunk: chunk => inspector.feed(chunk),
18121817
finishInspection: () => inspector.finish(),
18131818
disposeInspection: () => inspector.dispose(),
1814-
sawTerminal: () => inspector.reported(),
1819+
// Stream lifetime follows the protocol terminal even when this request
1820+
// has no outcome callback configured (reported() would stay false).
1821+
sawTerminal: () => inspector.terminalSeen(),
18151822
...(win32EagerRewrite
18161823
? { rewritePayload: composeSsePayloadRewrites(...payloadRewrites) }
18171824
: {}),
@@ -1829,9 +1836,9 @@ async function handleResponsesInner(
18291836
onClientCancel: () => options.onNativePassthroughCancel?.(),
18301837
onDone: () => unregisterTurn(turnAc),
18311838
}, win32EagerRewrite ? { rewriteBudget: translatorBudget } : undefined);
1832-
// selectEagerPath admits only no-rewrite traffic on both eligible platforms;
1833-
// win32 rewrite traffic reaches this relay too, but with the payload rewrite
1834-
// applied inline — never via an image/item-id JS pull wrapper (#32111, #864).
1839+
// Windows always reaches this relay so response.completed can close a
1840+
// keep-alive upstream. Rewrite traffic applies its payload transform
1841+
// inline — never via the Bun#32111-unsafe tee()+JS-pull chain (#864).
18351842
if (!headers.has("content-type")) headers.set("content-type", "text/event-stream");
18361843
return markEagerRelaySseResponse(
18371844
markNativePassthroughSseResponse(new Response(eagerBody, {
@@ -1898,15 +1905,13 @@ async function handleResponsesInner(
18981905
);
18991906
}
19001907
if (!headers.has("content-type")) headers.set("content-type", "text/event-stream");
1901-
// win32 must keep the pure native relay (Bun#32111 JS-sink segfault); elsewhere a JS pull
1902-
// relay is established practice (relayWithAbort, relaySseWithHeartbeat) and lets a
1903-
// mid-stream reset end with a clean response.failed terminal instead of a raw socket error.
1908+
// Windows was handled by the eager terminal-aware branch above. Remaining
1909+
// tee traffic can use the JS relay to close on a protocol terminal and to
1910+
// convert a mid-stream reset into a clean response.failed event.
19041911
const rewrittenBody = payloadRewrites.length > 0
19051912
? relaySseWithPayloadRewrite(nativeBody, composeSsePayloadRewrites(...payloadRewrites), translatorBudget)
19061913
: nativeBody;
1907-
const clientBody = process.platform === "win32" && !needsClientRewrite
1908-
? nativeBody
1909-
: relaySseWithFailedTail(rewrittenBody, upstream, reason => clientGone.abort(reason));
1914+
const clientBody = relaySseWithFailedTail(rewrittenBody, upstream, reason => clientGone.abort(reason));
19101915
return markNativePassthroughSseResponse(new Response(clientBody, {
19111916
status: upstreamResponse.status,
19121917
headers,

structure/04_transports-and-sidecars.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,20 @@ to GUI static serving.
3838
Native passthrough SSE has TWO shapes, selected per request in
3939
`src/server/responses/core.ts`:
4040

41-
- **Default: tee + background inspection.** `upstreamResponse.body.tee()` sends
42-
branch[0] to the client (pure native relay on win32 without any client-facing
43-
rewrite — the Bun#32111 crash workaround; a JS relay elsewhere) while branch[1] is
41+
- **Default outside Windows: tee + background inspection.** `upstreamResponse.body.tee()` sends
42+
branch[0] through a terminal-aware client relay while branch[1] is
4443
drained eagerly by `consumeForInspection`/`consumeForResponseLogMetadata`
4544
for terminal-outcome recording, quota, the passthrough continuation cache,
4645
and request logs. This remains the default shape on bundled Bun 1.3.14.
47-
- **Gated: eager bounded relay** (`src/server/relay-eager.ts`). win32 and darwin
46+
- **Terminal-aware eager bounded relay** (`src/server/relay-eager.ts`). Windows
47+
always uses this single-reader shape so `response.completed` closes the client
48+
stream even when an upstream keeps HTTP/SSE alive. Darwin uses it for
4849
no-client-rewrite traffic only (neither image-gen aliases nor item-id repair),
49-
selected by `selectEagerPath` in `src/lib/bun-stream-caps.ts`. Windows `auto`
50-
becomes eager only on runtimes proven to carry the Bun#32111 fix
51-
(`MIN_FIXED_BUN_VERSION`, null until a bundle bump), while explicit
52-
`streamMode: "eager-relay"` opts in today. Darwin is explicit-only: `auto`
53-
stays tee even after a future threshold bump. One eager reader + byte-bounded
50+
selected by `selectEagerPath` in `src/lib/bun-stream-caps.ts`. Windows ignores
51+
the no-rewrite gate at the final server dispatch because protocol termination
52+
cannot be implemented by the native tee branch; rewrite traffic continues to
53+
apply transforms inline. Darwin is explicit-only: `auto` stays tee even after
54+
a future threshold bump. One eager reader + byte-bounded
5455
client queue + post-cancel bounded discard-drain replaces the tee and goes
5556
directly to the response without a JS rewrite wrapper, preserving the full
5657
inspection side-effect set (shared `createSseInspector` factory in `relay.ts`)

tests/passthrough-abort.test.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,14 @@ describe("passthrough relayWithAbort (RC2, passthrough path)", () => {
4747
);
4848

4949
expect(sseBranch).toContain("upstreamResponse.body.tee()");
50-
// win32 must receive the tee'd body untouched when no client rewrite is required — no JS pull
51-
// wrapper on the default path (Bun#32111 segfault).
50+
// Windows must use the terminal-aware single-reader relay even without a
51+
// payload rewrite, so response.completed can close a keep-alive upstream.
5252
expect(sseBranch).toContain("const repairConfig = route.provider.responsesItemIdRepair;");
5353
expect(sseBranch).toContain("const needsClientRewrite = imageGenCallAliases.size > 0");
5454
expect(sseBranch).toContain("new Response(eagerBody");
5555
expect(sseBranch).toContain("const rewrittenBody = payloadRewrites.length > 0");
56-
expect(sseBranch).toContain('process.platform === "win32"');
57-
expect(sseBranch).toContain("&& !needsClientRewrite");
58-
expect(sseBranch).toContain("? nativeBody");
56+
expect(sseBranch).toContain('const win32TerminalRelay = process.platform === "win32"');
57+
expect(sseBranch).toContain("eagerPath?.useEagerRelay || win32EagerRewrite || win32TerminalRelay");
5958
// #864: win32 traffic that DOES need a client rewrite takes the eager single
6059
// reader with the payload rewrite applied inline — never the tee()+JS-pull
6160
// chain that loses the terminal block on Windows (Bun#32111).

tests/relay-eager.test.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,10 @@ describe("relaySseEagerBounded — inline payload rewrite (#864)", () => {
131131
expect(text).toContain("RESTORED");
132132
expect(text).not.toContain("image_gen__gen");
133133
expect(text).toContain("response.completed");
134-
// A partial trailing block reaches the client verbatim at EOF.
135-
expect(text).toContain("trailing-partial");
134+
// The protocol terminal ends the client stream; bytes produced after it
135+
// belong to the gateway's retained connection and must not hold Codex open.
136+
expect(text).not.toContain("trailing-partial");
137+
expect(text.endsWith("data: [DONE]\n\n")).toBe(true);
136138
});
137139

138140
test("identity rewrite preserves framing byte-for-byte", async () => {
@@ -153,7 +155,9 @@ describe("relaySseEagerBounded — inline payload rewrite (#864)", () => {
153155
up.close();
154156

155157
const text = await reading;
156-
expect(text).toBe(new TextDecoder().decode(joinBytes([first, enc.encode(second)])));
158+
expect(text).toBe(
159+
new TextDecoder().decode(joinBytes([first, enc.encode(second)])) + "data: [DONE]\n\n",
160+
);
157161
// The rewrite actually ran — this is what makes the test red pre-fix.
158162
expect(rewriteCalls).toBeGreaterThan(0);
159163
});
@@ -209,7 +213,7 @@ describe("relaySseEagerBounded — inline payload rewrite (#864)", () => {
209213
expect(budget.snapshot().currentBytes).toBe(0);
210214
});
211215

212-
test("blocks without a data field pass through untouched", async () => {
216+
test("blocks without a data field pass through untouched before the terminal", async () => {
213217
const up = controlledUpstream();
214218
const { hooks } = makeHooks();
215219
let rewriteCalls = 0;
@@ -220,8 +224,8 @@ describe("relaySseEagerBounded — inline payload rewrite (#864)", () => {
220224
const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks);
221225
const reading = readAll(relayed);
222226

223-
up.push(enc.encode(`event: response.completed\ndata: ${COMPLETED}\n\n`));
224227
up.push(enc.encode(`: keepalive comment\n\n`));
228+
up.push(enc.encode(`event: response.completed\ndata: ${COMPLETED}\n\n`));
225229
up.close();
226230

227231
const text = await reading;
@@ -317,7 +321,7 @@ describe("relaySseEagerBounded — side-effect parity", () => {
317321

318322
const clientBytes = await readAllBytes(relayed);
319323
await settle();
320-
expect(clientBytes).toEqual(joinBytes(frames));
324+
expect(clientBytes).toEqual(joinBytes([...frames, enc.encode("data: [DONE]\n\n")]));
321325
const wireText = new TextDecoder().decode(clientBytes);
322326
expect(wireText).not.toContain('"output":');
323327
expect(rec.completed).toHaveLength(1);

tests/sse-failed-tail.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,35 @@ describe("relaySseWithFailedTail", () => {
5959
expect(upstream.signal.aborted).toBe(false);
6060
});
6161

62+
test("closes at response.completed when the upstream keeps its SSE connection open", async () => {
63+
const upstream = new AbortController();
64+
let sourceCancelled = false;
65+
let sentTerminal = false;
66+
const src = new ReadableStream<Uint8Array>({
67+
pull(controller) {
68+
if (!sentTerminal) {
69+
sentTerminal = true;
70+
controller.enqueue(encoder.encode(
71+
'event: response.completed\ndata: {"type":"response.completed","response":{"status":"completed"}}\n\n',
72+
));
73+
}
74+
// Deliberately never close: several Responses-compatible gateways keep
75+
// this connection alive after the protocol terminal event.
76+
},
77+
cancel() { sourceCancelled = true; },
78+
});
79+
80+
const out = await Promise.race([
81+
drain(relaySseWithFailedTail(src, upstream)),
82+
new Promise<never>((_, reject) => setTimeout(() => reject(new Error("relay did not close at terminal")), 200)),
83+
]);
84+
85+
expect(out).toContain("response.completed");
86+
expect(out.endsWith("data: [DONE]\n\n")).toBe(true);
87+
expect(sourceCancelled).toBe(true);
88+
expect(upstream.signal.aborted).toBe(false);
89+
});
90+
6291
test("mid-stream error keeps prior bytes and appends a clean failed terminal", async () => {
6392
const upstream = new AbortController();
6493
const src = sourceStream(['data: {"type":"response.output_text.delta","delta":"hel', ""], { failAfter: true });

0 commit comments

Comments
 (0)