review: count long-lived idle streams as progress, keep resumption token, dedupe scheduling

- A stream that stays open at least maxReconnectionDelay resets the
  attempt count even without messages, so healthy-but-quiet sessions
  whose standby stream is periodically idle-closed (e.g. behind an ALB)
  keep reconnecting indefinitely; only rapid connect-then-close cycles
  are bounded by maxRetries.
- Hoist a single scheduleNext() closure used by both the graceful-close
  and mid-stream-error branches (the duplicated block is how the
  original bug happened).
- Fall back to the stream's own resumptionToken when it ended before
  any event arrived, so empty reconnects no longer drop Last-Event-ID.
- Update the migration guide bullet that claimed exhaustion accounting
  was unchanged from v1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eqc26ABfbhTimyUUszsUxL

Author: claude

.changeset/bound-standby-sse-reconnects.md

@@ -2,6 +2,8 @@
 '@modelcontextprotocol/client': patch
 ---
 
-Bound standby SSE reconnects when the server keeps idle-closing the stream. `StreamableHTTPClientTransport` reset its reconnection attempt count to `0` every time a stream ended, so a server that gracefully idle-closes the standby GET/SSE stream (spec-compliant behavior) kept the client reconnecting forever at `initialReconnectionDelay` — `maxRetries` never tripped, and every cycle re-ran the authenticated fetch path.
+Bound standby SSE reconnects when the server rapidly idle-closes the stream. `StreamableHTTPClientTransport` reset its reconnection attempt count to `0` every time a stream ended, so a server that gracefully idle-closes the standby GET/SSE stream immediately after every reconnect (spec-compliant behavior) kept the client reconnecting forever at `initialReconnectionDelay` — `maxRetries` never tripped, and every cycle re-ran the authenticated fetch path.
 
-The attempt count now persists across connect-then-close cycles that deliver no messages: after `maxRetries` consecutive fruitless reconnects the transport stops and surfaces `onerror` ("Maximum reconnection attempts exceeded"), exactly as it already did for reconnects that fail outright. A stream that delivers a message still resets the count, so healthy long-lived streams reconnect indefinitely as before.
+The attempt count now persists across connect-then-close cycles that make no progress: after `maxRetries` consecutive fruitless reconnects the transport stops and surfaces `onerror` ("Maximum reconnection attempts exceeded"), exactly as it already did for reconnects that fail outright. A stream counts as having made progress — and resets the count — when it delivers a message or stays open for at least `maxReconnectionDelay`, so healthy sessions whose idle standby stream is periodically closed by the server or an intermediary keep reconnecting indefinitely as before.
+
+Also fixed in the same path: a reconnected stream that ended before any event arrived no longer drops the `Last-Event-ID` resumption token it was opened with — the next attempt resumes from the same token instead of silently starting a fresh stream.

docs/migration/upgrade-to-v2.md

@@ -1511,11 +1511,18 @@ rewrite required unless noted.
   no `notifications/cancelled` goes on the wire — the spec forbids cancelling
   `initialize`, and v1 sent one anyway. v1 tests asserting that notification need
   re-baselining.
-- **Also unchanged: SSE reconnection exhaustion.** `StreamableHTTPClientTransport`'s
-  standalone GET-stream reconnection behavior and its exhaustion signal carry over from
-  v1: when retries run out, the transport emits `onerror` with a plain `Error` whose
-  message is `Maximum reconnection attempts (N) exceeded.` — there is no typed error
-  class for this condition, so monitors that match the message text keep working.
+- **Changed: SSE reconnection exhaustion accounting.** The exhaustion _signal_ carries
+  over from v1: when retries run out, `StreamableHTTPClientTransport` emits `onerror`
+  with a plain `Error` whose message is `Maximum reconnection attempts (N) exceeded.` —
+  there is no typed error class for this condition, so monitors that match the message
+  text keep working. The _accounting_ changed: v1 reset the retry counter every time a
+  stream closed, so a server that gracefully idle-closed the standby GET stream
+  immediately after each reconnect kept the client reconnecting forever and
+  `maxRetries` never fired. v2 counts consecutive reconnects that make no progress —
+  no message delivered and the connection lasted less than `maxReconnectionDelay` — so
+  such loops now stop with the error above after `maxRetries` attempts. Streams that
+  deliver a message or stay open at least `maxReconnectionDelay` reset the counter and
+  reconnect indefinitely, exactly as in v1.
 - **Also unchanged: elicitation response validation.** `elicitInput`'s local validation
   of elicitation responses against `requestedSchema`, the resulting `-32602` error
   message wording (`Elicitation response content does not match requested schema: …`),

packages/client/src/client/streamableHttp.ts

@@ -118,9 +118,14 @@ export interface StreamableHTTPReconnectionOptions {
     /**
      * Maximum number of consecutive reconnection attempts before giving up.
      * An attempt counts against this limit when it fails outright or when the
-     * reconnected stream ends again without having delivered a message (for
-     * example, a server that gracefully idle-closes its standby SSE stream);
-     * a stream that delivers a message resets the count.
+     * reconnected stream ends again without making progress — no message
+     * delivered and the connection lasted less than
+     * {@linkcode maxReconnectionDelay} (for example, a server that gracefully
+     * idle-closes its standby SSE stream immediately after every reconnect).
+     * A stream that delivers a message or stays open at least
+     * {@linkcode maxReconnectionDelay} resets the count, so healthy sessions
+     * whose standby stream is periodically idle-closed keep reconnecting
+     * indefinitely.
      * Default is 2.
      */
     maxRetries: number;
@@ -717,9 +722,10 @@ export class StreamableHTTPClientTransport implements Transport {
     /**
      * @param reconnectAttempt - Which reconnection attempt produced this
      * stream (0 for an original stream). Carried into the next
-     * `_scheduleReconnection` call when the stream ends without having
-     * delivered a message, so consecutive fruitless reconnects are bounded by
-     * `maxRetries`; a stream that delivered a message resets the count.
+     * `_scheduleReconnection` call when the stream ends without having made
+     * progress (no message delivered, connection shorter than
+     * `maxReconnectionDelay`), so consecutive fruitless reconnects are bounded
+     * by `maxRetries`; a stream that made progress resets the count.
      */
     private _handleSseStream(
         stream: ReadableStream<Uint8Array> | null,
@@ -756,6 +762,40 @@ export class StreamableHTTPClientTransport implements Transport {
         // keeping repeated connect/idle-close cycles bounded by `maxRetries`
         // (#2682).
         let receivedMessage = false;
+        const streamOpenedAt = Date.now();
+
+        // Single scheduling site for both the graceful-close and the
+        // mid-stream-error paths below — the #2682 bug existed precisely
+        // because the two branches carried separate copies of this block.
+        const scheduleNext = (): void => {
+            // A stream counts as having made progress when it delivered a
+            // message, or when it stayed open for at least
+            // `maxReconnectionDelay` before ending — an idle standby stream
+            // that a server (or intermediary) periodically closes is healthy,
+            // and resetting the count for it can never produce a reconnect
+            // rate faster than the maximum backoff already permits. Without
+            // progress, a connect-then-close cycle is retry-equivalent to a
+            // failed attempt: the count continues so a server that idle-closes
+            // every standby stream right away cannot keep the transport
+            // looping forever (#2682). Priming events alone deliberately do
+            // not reset the count — a server can send one and still close
+            // immediately, which would re-arm exactly that loop.
+            const madeProgress = receivedMessage || Date.now() - streamOpenedAt >= this._reconnectionOptions.maxReconnectionDelay;
+            this._scheduleReconnection(
+                {
+                    // A reconnected stream that ended before any event arrived
+                    // must not drop the token the stream was opened with —
+                    // fall back to it so the next attempt still resumes.
+                    resumptionToken: lastEventId ?? options.resumptionToken,
+                    onresumptiontoken,
+                    replayMessageId,
+                    requestSignal,
+                    onRequestStreamEnd
+                },
+                madeProgress ? 0 : reconnectAttempt
+            );
+        };
+
         const processStream = async () => {
             // this is the closest we can get to trying to catch network errors
             // if something happens reader will throw
@@ -818,21 +858,7 @@ export class StreamableHTTPClientTransport implements Transport {
                 const canResume = isReconnectable || hasPrimingEvent;
                 const needsReconnect = canResume && !receivedResponse;
                 if (needsReconnect && this._abortController && !isIntentionalAbort()) {
-                    // A stream that delivered a message was genuinely working —
-                    // restart the attempt count. A graceful close without one is
-                    // retry-equivalent to a failed attempt: continue the count so
-                    // a server that idle-closes every standby stream cannot keep
-                    // the transport reconnecting forever (#2682).
-                    this._scheduleReconnection(
-                        {
-                            resumptionToken: lastEventId,
-                            onresumptiontoken,
-                            replayMessageId,
-                            requestSignal,
-                            onRequestStreamEnd
-                        },
-                        receivedMessage ? 0 : reconnectAttempt
-                    );
+                    scheduleNext();
                 } else if (!isIntentionalAbort()) {
                     // The per-request stream ended without reconnecting (no
                     // priming event for a POST stream, or response already
@@ -855,19 +881,9 @@ export class StreamableHTTPClientTransport implements Transport {
                 const needsReconnect = canResume && !receivedResponse;
                 if (needsReconnect && this._abortController && !isIntentionalAbort()) {
                     // Use the exponential backoff reconnection strategy. Same
-                    // accounting as the graceful-close path: only a stream that
-                    // delivered a message restarts the attempt count.
+                    // accounting as the graceful-close path.
                     try {
-                        this._scheduleReconnection(
-                            {
-                                resumptionToken: lastEventId,
-                                onresumptiontoken,
-                                replayMessageId,
-                                requestSignal,
-                                onRequestStreamEnd
-                            },
-                            receivedMessage ? 0 : reconnectAttempt
-                        );
+                        scheduleNext();
                     } catch (error) {
                         this.onerror?.(new Error(`Failed to reconnect: ${error instanceof Error ? error.message : String(error)}`));
                         onRequestStreamEnd?.();

packages/client/test/client/streamableHttp.test.ts

@@ -2652,6 +2652,81 @@ describe('StreamableHTTPClientTransport', () => {
             await vi.advanceTimersByTimeAsync(5000);
             expect(fetchMock).toHaveBeenCalledTimes(4);
         });
+
+        it('treats a long-lived idle stream as progress, so periodic idle-closes reconnect indefinitely', async () => {
+            // A healthy-but-quiet session behind e.g. a load balancer with an
+            // idle timeout: the standby stream delivers nothing but stays open
+            // well past maxReconnectionDelay before each close. That must NOT
+            // count against maxRetries — the notification channel stays alive.
+            transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
+                reconnectionOptions: {
+                    initialReconnectionDelay: 10,
+                    maxReconnectionDelay: 1000,
+                    reconnectionDelayGrowFactor: 1,
+                    maxRetries: 2
+                }
+            });
+            const errorSpy = vi.fn();
+            transport.onerror = errorSpy;
+
+            const controllers: ReadableStreamDefaultController<Uint8Array>[] = [];
+            const fetchMock = globalThis.fetch as Mock;
+            fetchMock.mockImplementation(async () => ({
+                ok: true,
+                status: 200,
+                headers: new Headers({ 'content-type': 'text/event-stream' }),
+                body: new ReadableStream<Uint8Array>({
+                    start(controller) {
+                        controllers.push(controller);
+                    }
+                })
+            }));
+
+            await transport.start();
+            await transport['_startOrAuthSse']({});
+
+            // Five cycles: each stream lives longer than maxReconnectionDelay
+            // (fake timers also drive Date.now), then idle-closes empty.
+            for (let cycle = 0; cycle < 5; cycle++) {
+                await vi.advanceTimersByTimeAsync(1500);
+                controllers[cycle]!.close();
+                await vi.advanceTimersByTimeAsync(50);
+            }
+
+            // Well past 1 + maxRetries fetches, and no exhaustion error.
+            expect(fetchMock.mock.calls.length).toBeGreaterThan(3);
+            expect(errorSpy).not.toHaveBeenCalled();
+        });
+
+        it('carries the resumption token through reconnected streams that ended before any event', async () => {
+            // Stream 1 delivers a priming event (id only), so the first
+            // reconnect resumes from it. Stream 2 ends empty — the next
+            // attempt must still send the same Last-Event-ID instead of
+            // dropping it and starting a fresh stream.
+            transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), {
+                reconnectionOptions: {
+                    initialReconnectionDelay: 10,
+                    maxReconnectionDelay: 1000,
+                    reconnectionDelayGrowFactor: 1,
+                    maxRetries: 3
+                }
+            });
+
+            const fetchMock = globalThis.fetch as Mock;
+            const bodies: string[][] = [['id: event-1\ndata: \n\n']];
+            fetchMock.mockImplementation(async () => sseResponse(bodies.shift() ?? []));
+
+            await transport.start();
+            await transport['_startOrAuthSse']({});
+            await vi.advanceTimersByTimeAsync(100);
+
+            expect(fetchMock.mock.calls.length).toBeGreaterThanOrEqual(3);
+            const secondCallHeaders = fetchMock.mock.calls[1]![1]?.headers as Headers;
+            const thirdCallHeaders = fetchMock.mock.calls[2]![1]?.headers as Headers;
+            expect(secondCallHeaders.get('last-event-id')).toBe('event-1');
+            // Before the fix this was null: the empty second stream dropped the token.
+            expect(thirdCallHeaders.get('last-event-id')).toBe('event-1');
+        });
     });
 
     describe('prevent infinite recursion when server returns 401 after successful auth', () => {