fix(client): bound McpSubscription.close()'s wait on the cancelled-notification send

.changeset/listen-close-bounded-teardown.md

@@ -0,0 +1,5 @@
+---
+'@modelcontextprotocol/client': patch
+---
+
+Bound the wait in `McpSubscription.close()`. It serially awaited the `notifications/cancelled` teardown send, so a transport send that never settles (e.g. a stdio write parked on `'drain'`, which ignores `requestSignal`) left `await sub.close()` hanging forever. `close()` still waits for the notification so it is on the wire when it resolves on healthy transports, but the wait is now capped (5s); the subscription's state machine settles immediately either way.

packages/client/src/client/client.ts

@@ -435,6 +435,15 @@ const LIST_CHANGED_EVICTIONS: Readonly<Record<string, readonly string[]>> = {
 
 const DEFAULT_LIST_MAX_PAGES = 64;
 
+/**
+ * Upper bound on how long `McpSubscription.close()` waits for the
+ * `notifications/cancelled` teardown send before resolving anyway. A healthy
+ * transport settles the send in well under this; the bound exists so a send
+ * that never settles (e.g. a stdio write parked on `'drain'`, which ignores
+ * `requestSignal`) cannot park `close()` forever. See #2641/#2643.
+ */
+const LISTEN_CLOSE_TEARDOWN_WAIT_MSEC = 5000;
+
 /**
  * A handle to an open `subscriptions/listen` stream (protocol revision
  * 2026-07-28). Change notifications delivered on the stream dispatch to the
@@ -450,7 +459,10 @@ export interface McpSubscription {
      * Tears the subscription down. Idempotent. Aborts the listen request's
      * stream (where the transport supports it) AND sends
      * `notifications/cancelled` referencing the listen request id — both,
-     * always, so close works on any transport.
+     * always, so close works on any transport. The wait for the
+     * cancelled-notification send is capped (~5s); on a transport whose send
+     * never settles, `close()` resolves with the notification still in
+     * flight.
      */
     close(): Promise<void>;
     /**
@@ -2060,7 +2072,25 @@ export class Client extends Protocol<ClientContext> {
         const close = async (): Promise<void> => {
             if (state === 'closed') return;
             settle({ cause: 'local' });
-            await wireTeardown();
+            // Bounded wait: close() waits for the cancelled notification so
+            // it is on the wire when close() resolves (transports settle a
+            // healthy send quickly), but a send that never settles — a stdio
+            // write parked on 'drain' ignores `requestSignal`, so the
+            // `requestAbort.abort()` above cannot reach it — must not park
+            // close() forever. The bound cuts only the WAIT; the notification
+            // stays in flight, and the state machine settled above regardless.
+            await new Promise<void>(resolve => {
+                const timer = setTimeout(resolve, LISTEN_CLOSE_TEARDOWN_WAIT_MSEC);
+                // In the parked-send case clearTimeout below is unreachable,
+                // and the armed timer would hold an idle Node process open
+                // for the full bound. Guarded: this module is runtime-neutral
+                // and browser timers have no unref.
+                (timer as { unref?: () => void }).unref?.();
+                void wireTeardown().finally(() => {
+                    clearTimeout(timer);
+                    resolve();
+                });
+            });
         };
 
         // The per-subscription state is registered BEFORE the request is sent

packages/client/test/client/listen.test.ts

@@ -953,6 +953,74 @@ describe('_resetConnectionState() clears connection-scoped debounce timers (fake
     });
 });
 
+describe('McpSubscription.close() — bounded teardown wait (fake timers)', () => {
+    beforeEach(() => vi.useFakeTimers());
+    afterEach(() => vi.useRealTimers());
+
+    it('close() resolves within the bound when the cancelled-notification send never settles', async () => {
+        const { clientTx } = await scriptedModern();
+        const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } });
+        const connecting = client.connect(clientTx);
+        await vi.runAllTimersAsync();
+        await connecting;
+
+        const subPromise = client.listen({ toolsListChanged: true });
+        await vi.advanceTimersByTimeAsync(0);
+        const sub = await subPromise;
+
+        // Every subsequent send parks forever — models a stdio pipe backed up
+        // on 'drain' (stdio send() ignores requestSignal, so the teardown's
+        // requestAbort cannot reach it).
+        clientTx.send = () => new Promise<void>(() => {});
+
+        let closeSettled = false;
+        const closing = sub.close().then(() => {
+            closeSettled = true;
+        });
+        // The state machine settles immediately even though the wire teardown
+        // is parked.
+        await expect(sub.closed).resolves.toBe('local');
+        await vi.advanceTimersByTimeAsync(0);
+        expect(closeSettled).toBe(false); // still waiting on the parked send
+        // …but the wait is bounded: close() resolves once the bound elapses.
+        await vi.advanceTimersByTimeAsync(5000);
+        await closing;
+        expect(closeSettled).toBe(true);
+
+        vi.useRealTimers();
+        await client.close();
+    });
+
+    it('a healthy close() still resolves with the cancelled notification already on the wire (no added latency)', async () => {
+        const { clientTx, written } = await scriptedModern();
+        const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } });
+        const connecting = client.connect(clientTx);
+        await vi.runAllTimersAsync();
+        await connecting;
+
+        const subPromise = client.listen({ toolsListChanged: true });
+        await vi.advanceTimersByTimeAsync(0);
+        const sub = await subPromise;
+        written.length = 0;
+
+        let closeSettled = false;
+        const closing = sub.close().then(() => {
+            closeSettled = true;
+        });
+        // No timer advance beyond microtask draining: a settling send must
+        // resolve close() without waiting out any part of the bound.
+        await vi.advanceTimersByTimeAsync(0);
+        await closing;
+        expect(closeSettled).toBe(true);
+        expect(written.some(m => (m as { method?: string }).method === 'notifications/cancelled')).toBe(true);
+        // The bound's timer was cleared — nothing left armed.
+        expect(vi.getTimerCount()).toBe(0);
+
+        vi.useRealTimers();
+        await client.close();
+    });
+});
+
 describe('Client.listen() — ack timeout (fake timers)', () => {
     beforeEach(() => vi.useFakeTimers());
     afterEach(() => vi.useRealTimers());