Skip to content

Commit d0ab275

Browse files
committed
fix(responses): close passthrough streams at terminal events
1 parent 6a7351b commit d0ab275

9 files changed

Lines changed: 285 additions & 59 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+
* follows this runtime/config decision, preserving the explicit legacy-tee
10+
* safety pin. 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: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
* up to the drain window.
2525
*/
2626

27-
import { buildFailedTailPayload } from "./relay";
27+
import { buildFailedTailPayload, createSseTerminalOutputBoundary } from "./relay";
2828
import {
2929
nextSseBlock,
3030
replaceSseDataPayload,
@@ -92,6 +92,7 @@ export function relaySseEagerBounded(
9292
const now = opts?.now ?? Date.now;
9393

9494
const reader = body.getReader();
95+
const terminalBoundary = createSseTerminalOutputBoundary();
9596
const rewrite = hooks.rewritePayload;
9697
const rewriteDecoder = rewrite ? new TextDecoder() : null;
9798
const rewriteEncoder = rewrite ? new TextEncoder() : null;
@@ -161,6 +162,7 @@ export function relaySseEagerBounded(
161162
let queuedBytes = 0;
162163
let cancelled = false;
163164
let done = false;
165+
const terminalSentinel = new TextEncoder().encode("data: [DONE]\n\n");
164166
// Pause gate: resolved by client pull, client cancel, or upstream abort so a
165167
// paused producer ALWAYS resumes (audit blocker 2 — no deadlock; onDone and
166168
// turn unregistration stay reachable, drainAndShutdown never hangs).
@@ -216,12 +218,17 @@ export function relaySseEagerBounded(
216218
if (upstream.signal.aborted) break;
217219
if (upstreamDone) {
218220
hooks.finishInspection();
221+
const boundedTail = terminalBoundary.finish();
219222
if (rewrite) {
220-
const tail = flushRewriteTail();
223+
const rewritten = rewriteOutbound(boundedTail);
224+
const tail = joinUint8Arrays(rewritten, flushRewriteTail());
221225
if (tail.byteLength > 0 && !cancelled) {
222226
queuedBytes += tail.byteLength;
223227
try { controllerRef?.enqueue(tail); } catch { /* client already gone */ }
224228
}
229+
} else if (boundedTail.byteLength > 0 && !cancelled) {
230+
queuedBytes += boundedTail.byteLength;
231+
try { controllerRef?.enqueue(boundedTail); } catch { /* client already gone */ }
225232
}
226233
if (!hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) {
227234
syntheticKind = "incomplete";
@@ -237,17 +244,30 @@ export function relaySseEagerBounded(
237244
}
238245
continue;
239246
}
240-
const outbound = rewrite ? rewriteOutbound(value) : value;
241-
if (outbound.byteLength === 0) continue;
242-
queuedBytes += outbound.byteLength;
243-
try {
244-
controllerRef?.enqueue(outbound);
245-
} catch {
246-
// Controller already torn down (client went away without cancel()).
247-
cancelled = true;
248-
drainDeadline = now() + drainMs;
249-
armDrainTimer();
250-
continue;
247+
const terminalBounded = terminalBoundary.feed(value);
248+
const outbound = rewrite ? rewriteOutbound(terminalBounded) : terminalBounded;
249+
if (outbound.byteLength > 0) {
250+
queuedBytes += outbound.byteLength;
251+
try {
252+
controllerRef?.enqueue(outbound);
253+
} catch {
254+
// Controller already torn down (client went away without cancel()).
255+
cancelled = true;
256+
drainDeadline = now() + drainMs;
257+
armDrainTimer();
258+
continue;
259+
}
260+
}
261+
if (terminalBoundary.terminalSeen()) {
262+
// The Responses terminal event ends the turn even when a compatible
263+
// gateway keeps its HTTP connection alive. Add the conventional
264+
// sentinel and stop the single-reader relay at that protocol boundary.
265+
if (!terminalBoundary.doneSeen()) {
266+
queuedBytes += terminalSentinel.byteLength;
267+
try { controllerRef?.enqueue(terminalSentinel); } catch { /* client already gone */ }
268+
}
269+
reader.cancel("Responses terminal event received").catch(() => {});
270+
break;
251271
}
252272
while (queuedBytes > maxQueueBytes && !cancelled && !upstream.signal.aborted) {
253273
await paused();
@@ -279,6 +299,7 @@ export function relaySseEagerBounded(
279299
try { rewriteBudget.releaseRetained(frameBufferBytes, { kind: "live_transient" }); } catch { /* teardown must not throw */ }
280300
frameBufferBytes = 0;
281301
}
302+
terminalBoundary.dispose();
282303
if (syntheticKind) hooks.onSynthetic(syntheticKind);
283304
if (cancelled && !hooks.sawTerminal()) {
284305
hooks.onClientCancel();
@@ -318,3 +339,12 @@ export function relaySseEagerBounded(
318339
},
319340
});
320341
}
342+
343+
function joinUint8Arrays(first: Uint8Array, second: Uint8Array): Uint8Array {
344+
if (first.byteLength === 0) return second;
345+
if (second.byteLength === 0) return first;
346+
const joined = new Uint8Array(first.byteLength + second.byteLength);
347+
joined.set(first);
348+
joined.set(second, first.byteLength);
349+
return joined;
350+
}

src/server/relay.ts

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,79 @@ export function buildFailedTailPayload(err: unknown): string {
9595
});
9696
}
9797

98+
export type SseTerminalOutputBoundary = {
99+
feed(chunk: Uint8Array): Uint8Array;
100+
finish(): Uint8Array;
101+
terminalSeen(): boolean;
102+
doneSeen(): boolean;
103+
dispose(): void;
104+
};
105+
106+
/**
107+
* Frame-aware client output boundary shared by both native Responses relays.
108+
* It buffers only the current incomplete SSE block, forwards complete blocks
109+
* through the first Responses terminal, and drops every later block/byte.
110+
*/
111+
export function createSseTerminalOutputBoundary(): SseTerminalOutputBoundary {
112+
let decoder: TextDecoder | null = new TextDecoder();
113+
const encoder = new TextEncoder();
114+
let buffer = "";
115+
let terminal = false;
116+
let done = false;
117+
let disposed = false;
118+
119+
const process = (flush: boolean): Uint8Array => {
120+
if (disposed || terminal) return new Uint8Array(0);
121+
let output = "";
122+
let responsesTerminal = false;
123+
for (;;) {
124+
const next = nextSseBlock(buffer);
125+
if (!next) break;
126+
buffer = next.rest;
127+
const payload = sseDataPayload(next.block);
128+
if (!responsesTerminal) output += next.block + next.delimiter;
129+
if (payload === "[DONE]") {
130+
done = true;
131+
if (responsesTerminal) output += next.block + next.delimiter;
132+
continue;
133+
}
134+
if (!responsesTerminal && payload && terminalStatusFromSsePayload(payload)) {
135+
responsesTerminal = true;
136+
}
137+
}
138+
if (responsesTerminal) {
139+
terminal = true;
140+
buffer = "";
141+
}
142+
if (flush && !terminal && buffer.length > 0) {
143+
output += buffer;
144+
buffer = "";
145+
}
146+
return encoder.encode(output);
147+
};
148+
149+
return {
150+
feed(chunk) {
151+
if (disposed || terminal) return new Uint8Array(0);
152+
buffer += decoder!.decode(chunk, { stream: true });
153+
return process(false);
154+
},
155+
finish() {
156+
if (disposed || terminal) return new Uint8Array(0);
157+
buffer += decoder!.decode();
158+
return process(true);
159+
},
160+
terminalSeen: () => terminal,
161+
doneSeen: () => done,
162+
dispose() {
163+
if (disposed) return;
164+
disposed = true;
165+
decoder = null;
166+
buffer = "";
167+
},
168+
};
169+
}
170+
98171
/**
99172
* Relay a passthrough SSE body like relayWithAbort, but convert a MID-STREAM failure (upstream
100173
* reset after headers) into a clean terminal: any partial block is closed off, then a synthetic
@@ -110,18 +183,57 @@ export function relaySseWithFailedTail(
110183
): ReadableStream<Uint8Array> {
111184
const reader = body.getReader();
112185
const encoder = new TextEncoder();
186+
const terminalBoundary = createSseTerminalOutputBoundary();
187+
let closed = false;
188+
const relayChunk = (
189+
controller: ReadableStreamDefaultController<Uint8Array>,
190+
value: Uint8Array,
191+
): "terminal" | "output" | "buffered" => {
192+
const outbound = terminalBoundary.feed(value);
193+
if (outbound.byteLength > 0) controller.enqueue(outbound);
194+
if (!terminalBoundary.terminalSeen()) return outbound.byteLength > 0 ? "output" : "buffered";
195+
196+
// A Responses terminal frame is the protocol boundary. Some compatible
197+
// gateways leave the HTTP connection open after response.completed, which
198+
// otherwise leaves Codex waiting forever even though the model turn is done.
199+
// Preserve through the terminal block only, add the conventional sentinel
200+
// when there was no real [DONE] data event, then stop reading upstream.
201+
if (!terminalBoundary.doneSeen()) {
202+
controller.enqueue(encoder.encode("data: [DONE]\n\n"));
203+
}
204+
closed = true;
205+
controller.close();
206+
const reason = "Responses terminal event received";
207+
// Notify the tee inspection branch as well. It has already received the
208+
// same terminal-bearing upstream chunk, so its bounded drain records the
209+
// real terminal and then releases the turn/upstream keep-alive connection.
210+
onClientGone?.(reason);
211+
reader.cancel(reason).catch(() => {});
212+
terminalBoundary.dispose();
213+
return "terminal";
214+
};
113215
return new ReadableStream<Uint8Array>({
114216
async pull(controller) {
115217
try {
116-
const { done, value } = await reader.read();
117-
if (done) {
118-
controller.close();
119-
return;
218+
for (;;) {
219+
const { done, value } = await reader.read();
220+
if (done) {
221+
const tail = terminalBoundary.finish();
222+
if (tail.byteLength > 0) controller.enqueue(tail);
223+
terminalBoundary.dispose();
224+
controller.close();
225+
return;
226+
}
227+
const result = relayChunk(controller, value);
228+
if (result !== "buffered") return;
120229
}
121-
controller.enqueue(value);
122230
} catch (err) {
231+
const partial = terminalBoundary.finish();
232+
terminalBoundary.dispose();
233+
if (closed) return;
123234
const payload = buildFailedTailPayload(err);
124235
try {
236+
if (partial.byteLength > 0) controller.enqueue(partial);
125237
// Leading blank line terminates a partial SSE block so the failed frame parses cleanly.
126238
controller.enqueue(encoder.encode(`\n\nevent: response.failed\ndata: ${payload}\n\ndata: [DONE]\n\n`));
127239
controller.close();
@@ -130,18 +242,20 @@ export function relaySseWithFailedTail(
130242
}
131243
},
132244
cancel(reason) {
245+
terminalBoundary.dispose();
133246
if (onClientGone) onClientGone(reason);
134247
else upstream.abort(reason);
135248
reader.cancel(reason).catch(() => {});
136249
},
137250
});
138251
}
139252

140-
export function nextSseBlock(buffer: string): { block: string; rest: string } | null {
253+
export function nextSseBlock(buffer: string): { block: string; delimiter: string; rest: string } | null {
141254
const match = buffer.match(/\r?\n\r?\n/);
142255
if (!match || match.index === undefined) return null;
143256
return {
144257
block: buffer.slice(0, match.index),
258+
delimiter: match[0],
145259
rest: buffer.slice(match.index + match[0].length),
146260
};
147261
}

src/server/responses/core.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,7 +1870,9 @@ async function handleResponsesInner(
18701870
inspectChunk: chunk => inspector.feed(chunk),
18711871
finishInspection: () => inspector.finish(),
18721872
disposeInspection: () => inspector.dispose(),
1873-
sawTerminal: () => inspector.reported(),
1873+
// Stream lifetime follows the protocol terminal even when this request
1874+
// has no outcome callback configured (reported() would stay false).
1875+
sawTerminal: () => inspector.terminalSeen(),
18741876
...(win32EagerRewrite
18751877
? { rewritePayload: composeSsePayloadRewrites(...payloadRewrites) }
18761878
: {}),
@@ -1888,9 +1890,9 @@ async function handleResponsesInner(
18881890
onClientCancel: () => options.onNativePassthroughCancel?.(),
18891891
onDone: () => unregisterTurn(turnAc),
18901892
}, win32EagerRewrite ? { rewriteBudget: translatorBudget } : undefined);
1891-
// selectEagerPath admits only no-rewrite traffic on both eligible platforms;
1892-
// win32 rewrite traffic reaches this relay too, but with the payload rewrite
1893-
// applied inline — never via an image/item-id JS pull wrapper (#32111, #864).
1893+
// Windows always reaches this relay so response.completed can close a
1894+
// keep-alive upstream. Rewrite traffic applies its payload transform
1895+
// inline — never via the Bun#32111-unsafe tee()+JS-pull chain (#864).
18941896
if (!headers.has("content-type")) headers.set("content-type", "text/event-stream");
18951897
return markEagerRelaySseResponse(
18961898
markNativePassthroughSseResponse(new Response(eagerBody, {
@@ -1957,15 +1959,13 @@ async function handleResponsesInner(
19571959
);
19581960
}
19591961
if (!headers.has("content-type")) headers.set("content-type", "text/event-stream");
1960-
// win32 must keep the pure native relay (Bun#32111 JS-sink segfault); elsewhere a JS pull
1961-
// relay is established practice (relayWithAbort, relaySseWithHeartbeat) and lets a
1962-
// mid-stream reset end with a clean response.failed terminal instead of a raw socket error.
1962+
// Windows was handled by the eager terminal-aware branch above. Remaining
1963+
// tee traffic can use the JS relay to close on a protocol terminal and to
1964+
// convert a mid-stream reset into a clean response.failed event.
19631965
const rewrittenBody = payloadRewrites.length > 0
19641966
? relaySseWithPayloadRewrite(nativeBody, composeSsePayloadRewrites(...payloadRewrites), translatorBudget)
19651967
: nativeBody;
1966-
const clientBody = process.platform === "win32" && !needsClientRewrite
1967-
? nativeBody
1968-
: relaySseWithFailedTail(rewrittenBody, upstream, reason => clientGone.abort(reason));
1968+
const clientBody = relaySseWithFailedTail(rewrittenBody, upstream, reason => clientGone.abort(reason));
19691969
return markNativePassthroughSseResponse(new Response(clientBody, {
19701970
status: upstreamResponse.status,
19711971
headers,

structure/04_transports-and-sidecars.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,19 @@ 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
48-
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
46+
- **Terminal-aware eager bounded relay** (`src/server/relay-eager.ts`). Windows
47+
uses this single-reader shape for rewrite traffic and for no-rewrite traffic
48+
selected by `selectEagerPath` in `src/lib/bun-stream-caps.ts`; the latter keeps
49+
`legacy-tee` and known-bad-runtime `auto` on tee as documented. When selected,
50+
`response.completed` closes the client stream even if upstream keeps HTTP/SSE
51+
alive. Darwin uses it for no-client-rewrite traffic only (neither image-gen
52+
aliases nor item-id repair) and is explicit-only: `auto` stays tee even after
53+
a future threshold bump. One eager reader + byte-bounded
5454
client queue + post-cancel bounded discard-drain replaces the tee and goes
5555
directly to the response without a JS rewrite wrapper, preserving the full
5656
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 no-rewrite traffic must honor the stream-mode/runtime gate so
51+
// legacy-tee remains a safety escape hatch for Bun#32111.
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("eagerPath?.useEagerRelay || win32EagerRewrite");
57+
expect(sseBranch).not.toContain("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).

0 commit comments

Comments
 (0)