Skip to content

Commit 9fc3978

Browse files
committed
fix(core): stop reconnecting a session stream after the consumer cancels
Cancelling the returned stream aborted the internal connection, but the retry path only bailed on the caller-provided abort signal, so a consumer cancel was treated as a transient drop and kept reconnecting (up to maxRetries), issuing SSE requests after the consumer was gone. Track consumer cancellation separately, wake any pending backoff, and short-circuit the retry. Adds a regression test. Also from review: drop the unused s2 networkEndpoint field, order the webapp test-server env so the worker-disable vars win over extraEnv as documented, abort the cross-origin browser read on a timer instead of a between-reads deadline, and correct the client-protocol docs (the caught-up check counts the highest raw seq_num including skipped command records; the chat transport closes on caught-up, SSEStreamSubscription only exposes the signal).
1 parent 51ff968 commit 9fc3978

6 files changed

Lines changed: 54 additions & 11 deletions

File tree

apps/webapp/test/session-stream.browser.e2e.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ describe("session stream browser e2e", () => {
9191
const result = await page.evaluate(
9292
async ({ url, token }) => {
9393
const ac = new AbortController();
94+
const deadlineTimer = setTimeout(() => ac.abort(), 8000);
9495
try {
9596
const res = await fetch(url, {
9697
headers: { Authorization: `Bearer ${token}`, Accept: "text/event-stream" },
@@ -99,8 +100,7 @@ describe("session stream browser e2e", () => {
99100
const reader = (res.body as ReadableStream<Uint8Array>).getReader();
100101
const decoder = new TextDecoder();
101102
let text = "";
102-
const deadline = Date.now() + 8000;
103-
while (Date.now() < deadline) {
103+
while (true) {
104104
const { done, value } = await reader.read();
105105
if (done) break;
106106
text += decoder.decode(value, { stream: true });
@@ -115,6 +115,8 @@ describe("session stream browser e2e", () => {
115115
};
116116
} catch (e) {
117117
return { error: String(e) };
118+
} finally {
119+
clearTimeout(deadlineTimer);
118120
}
119121
},
120122
{ url: sseUrl, token }

docs/ai-chat/client-protocol.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ Decoded `data` payload:
359359
| `records[].timestamp` | Unix ms when the record was written to S2. |
360360
| `records[].body` | For data records: a JSON-encoded **string** wrapping `{ data: UIMessageChunk, id: string }`. For control records: an empty string (semantics live in `headers`). For S2 command records: opaque bytes. See [Records on session.out](#records-on-session-out). |
361361
| `records[].headers` | Optional `[name, value]` pairs. Empty for data records; a `trigger-control` entry for control records; a single empty-name `["", "<op>"]` entry for S2 command records. |
362-
| `tail.seq_num` | Latest known tail of the S2 stream, useful for detecting how far behind the live edge you are. When `last delivered seq_num + 1 === tail.seq_num` you have drained the backlog and are caught up to the live edge. The same `tail` also rides on `ping` events. Skip if you don't need it. |
362+
| `tail.seq_num` | Latest known tail of the S2 stream, useful for detecting how far behind the live edge you are. Track the **highest `seq_num` you have received**, counting every record in the batch (including the command records you skip, see below), not just the last application-visible one. When `highest received seq_num + 1 === tail.seq_num` you have drained the backlog and are caught up to the live edge. The same `tail` also rides on `ping` events. Skip if you don't need it. |
363363
| `tail.timestamp` | Timestamp of `tail.seq_num`. |
364364
365365
### Records on `session.out`
@@ -652,7 +652,7 @@ On **reconnect-on-reload** paths (resuming a chat where nothing may be streaming
652652
653653
**Do not send `X-Peek-Settled` on the active-send response-stream path.** The peek would race the newly-triggered turn's first chunk — if the agent hasn't written the new turn's first record yet, the peek sees the prior turn's `turn-complete` and closes the SSE before the response lands on S2. The built-in `TriggerChatTransport.reconnectToStream` sets the header; `sendMessages → subscribeToStream` does not.
654654
655-
If you use the TypeScript `SSEStreamSubscription` (or `useChat`, which builds on it), the client settles on its own too: on the reconnect path it watches the `tail` on batch and ping events and closes a resumed stream as soon as it reaches the live edge, so a settled idle reconnect closes promptly without waiting out the long poll. Hand-rolled clients can do the same by comparing their last processed `seq_num` to the ping/batch `tail`. On older self-hosted backends whose `ping` carries no `tail`, this falls back to the `X-Peek-Settled` behavior above.
655+
If you use `TriggerChatTransport` (or `useChat`, which builds on it), the transport settles the stream for you: on the reconnect path it watches the `tail` on batch and ping events via `SSEStreamSubscription.caughtUp()` and closes the resumed stream as soon as it reaches the live edge, so a settled idle reconnect closes promptly without waiting out the long poll. `SSEStreamSubscription` on its own only exposes the caught-up signal, it does not close itself: consuming it directly, call `caughtUp()` (or compare your last received `seq_num` to the ping/batch `tail`) and close your own stream. On older self-hosted backends whose `ping` carries no `tail`, this falls back to the `X-Peek-Settled` behavior above.
656656
657657
```ts
658658
// Reconnect path (page reload)

internal-packages/testcontainers/src/s2.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ export interface StartedS2Container {
2828
container: StartedTestContainer;
2929
/** Base URL of the s2-lite HTTP API, e.g. `http://localhost:49xxx`. */
3030
endpoint: string;
31-
/** In-network URL (for the spawned webapp on the same docker network). */
32-
networkEndpoint: string;
3331
/** The single basin s2-lite is initialised with. */
3432
basin: string;
3533
}
@@ -64,7 +62,6 @@ export async function createS2Container(
6462
return {
6563
container,
6664
endpoint: `http://${container.getHost()}:${mappedPort}`,
67-
networkEndpoint: "http://s2:80",
6865
basin,
6966
};
7067
}

internal-packages/testcontainers/src/webapp.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export async function startWebapp(
124124
REDIS_HOST: redis.host,
125125
REDIS_PORT: String(redis.port),
126126
REDIS_TLS_DISABLED: "true", // all *_REDIS_TLS_DISABLED vars default to this; test Redis has no TLS
127+
...(options.extraEnv ?? {}),
127128
// Disable all background workers. Each worker has its own env var and its own
128129
// check idiom ("0" vs "false" vs boolean), so we set all of them explicitly.
129130
WORKER_ENABLED: "false", // disables workerQueue.initialize() (checked === "true")
@@ -143,7 +144,6 @@ export async function startWebapp(
143144
// to "0" so a local apps/webapp/.env that sets it to "1" doesn't
144145
// short-circuit the loader past the REQUIRE_PLUGINS check.
145146
...(requirePlugins ? { REQUIRE_PLUGINS: "1", RBAC_FORCE_FALLBACK: "0" } : {}),
146-
...(options.extraEnv ?? {}),
147147
NODE_PATH: nodePath,
148148
},
149149
stdio: ["ignore", "pipe", "pipe"],

packages/core/src/v3/apiClient/runStream.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,22 @@ describe("SSEStreamSubscription retry behavior", () => {
2727
});
2828
}
2929

30+
/**
31+
* A response.body that emits one SSE event then stays open (never closes),
32+
* so the connection sits mid-read until the fetch signal aborts.
33+
*/
34+
function makeOpenSSEResponse() {
35+
const body = new ReadableStream<Uint8Array>({
36+
start(controller) {
37+
controller.enqueue(new TextEncoder().encode(`id: 1\ndata: {"hello":1}\n\n`));
38+
},
39+
});
40+
return new Response(body, {
41+
status: 200,
42+
headers: { "Content-Type": "text/event-stream", "X-Stream-Version": "v1" },
43+
});
44+
}
45+
3046
// Drain a ReadableStream<SSEStreamPart> until it closes or errors.
3147
// Returns received chunks plus terminal state.
3248
async function drain(stream: ReadableStream<{ id: string; chunk: unknown }>) {
@@ -74,6 +90,30 @@ describe("SSEStreamSubscription retry behavior", () => {
7490
expect(result.chunks).toHaveLength(1);
7591
});
7692

93+
it("does not reconnect after the consumer cancels the returned stream", async () => {
94+
let attempts = 0;
95+
globalThis.fetch = vi.fn().mockImplementation(async () => {
96+
attempts++;
97+
return makeOpenSSEResponse();
98+
});
99+
100+
const sub = new SSEStreamSubscription("http://example.test/sse", {
101+
retryDelayMs: 1,
102+
maxRetryDelayMs: 5,
103+
maxRetries: 10,
104+
});
105+
106+
const stream = await sub.subscribe();
107+
const reader = stream.getReader();
108+
const first = await reader.read();
109+
expect(first.done).toBe(false);
110+
111+
await reader.cancel();
112+
await new Promise((r) => setTimeout(r, 50));
113+
114+
expect(attempts).toBe(1);
115+
});
116+
77117
it("caps the exponential backoff at maxRetryDelayMs", async () => {
78118
let attempts = 0;
79119
const callTimes: number[] = [];

packages/core/src/v3/apiClient/runStream.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ export class SSEStreamSubscription implements StreamSubscription {
209209
private nonRetryableStatuses: ReadonlySet<number>;
210210
private retryNowController: AbortController | null = null;
211211
private internalAbort: AbortController | null = null;
212+
private cancelledByConsumer = false;
212213
private caughtUpTracker = new CaughtUpTracker();
213214

214215
constructor(
@@ -329,7 +330,9 @@ export class SSEStreamSubscription implements StreamSubscription {
329330
await self.connectStream(controller);
330331
},
331332
cancel() {
333+
self.cancelledByConsumer = true;
332334
self.internalAbort?.abort();
335+
self.retryNowController?.abort();
333336
},
334337
});
335338
const internalReader = internal.getReader();
@@ -362,6 +365,7 @@ export class SSEStreamSubscription implements StreamSubscription {
362365
}
363366
},
364367
cancel(reason) {
368+
self.cancelledByConsumer = true;
365369
self.caughtUpTracker.end();
366370
self.options.onComplete?.();
367371
internalReader.cancel(reason).catch(() => {});
@@ -591,7 +595,7 @@ export class SSEStreamSubscription implements StreamSubscription {
591595
throw error;
592596
}
593597
} catch (error) {
594-
if (this.options.signal?.aborted) {
598+
if (this.options.signal?.aborted || this.cancelledByConsumer) {
595599
// User cancel — exit cleanly, don't retry.
596600
controller.close();
597601
return;
@@ -616,7 +620,7 @@ export class SSEStreamSubscription implements StreamSubscription {
616620
controller: ReadableStreamDefaultController,
617621
error?: Error
618622
): Promise<void> {
619-
if (this.options.signal?.aborted) {
623+
if (this.options.signal?.aborted || this.cancelledByConsumer) {
620624
controller.close();
621625
return;
622626
}
@@ -657,7 +661,7 @@ export class SSEStreamSubscription implements StreamSubscription {
657661
});
658662
this.retryNowController = null;
659663

660-
if (this.options.signal?.aborted) {
664+
if (this.options.signal?.aborted || this.cancelledByConsumer) {
661665
controller.close();
662666
return;
663667
}

0 commit comments

Comments
 (0)