Skip to content

Commit bd5b1ba

Browse files
fix(client): McpSubscription.close() is transport-agnostic — always abort + send notifications/cancelled
The close path branched on detectProbeTransportKind(), which detects child- process transports, not 'honors requestSignal'. On InMemoryTransport, SSE, or any custom transport, close() resolved without ever telling the server, leaving the subscription open server-side. close() now unconditionally aborts the listen request's requestSignal AND sends notifications/cancelled for the listen id. The abort closes the SSE stream where the transport honors it; the cancelled notification reaches the stdio listen router and any spec-compliant server. Idempotent over HTTP, correct on every other transport. detectProbeTransportKind is no longer in the close path.
1 parent a087d8c commit bd5b1ba

2 files changed

Lines changed: 24 additions & 16 deletions

File tree

packages/client/src/client/client.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,10 +1228,12 @@ export class Client extends Protocol<ClientContext> {
12281228
* 2025-era unsolicited notifications fire on a legacy connection — so
12291229
* `listen()` is era-transparent for consumers that already register those.
12301230
*
1231-
* `close()` tears the subscription down: on Streamable HTTP it closes the
1232-
* listen request's SSE stream; on stdio it sends `notifications/cancelled`
1233-
* referencing the listen request id. No automatic re-listen — call
1234-
* `listen()` again to re-establish.
1231+
* `close()` tears the subscription down by aborting the listen request's
1232+
* `requestSignal` (closes the SSE stream where the transport honors it)
1233+
* AND sending `notifications/cancelled` referencing the listen request id
1234+
* — both, unconditionally, so any spec-compliant server on any transport
1235+
* sees the cancel. No automatic re-listen — call `listen()` again to
1236+
* re-establish.
12351237
*
12361238
* On a 2025-era connection this throws a typed
12371239
* {@linkcode SdkErrorCode.MethodNotSupportedByProtocolVersion} steering to
@@ -1263,7 +1265,6 @@ export class Client extends Protocol<ClientContext> {
12631265
}
12641266

12651267
const requestAbort = new AbortController();
1266-
const transportKind = detectProbeTransportKind(this.transport);
12671268

12681269
// Explicit `opening → open → closed` state machine. Every termination
12691270
// path — ack-arrives, ack-timeout, server-cancelled, user-close,
@@ -1310,17 +1311,21 @@ export class Client extends Protocol<ClientContext> {
13101311
};
13111312

13121313
// Wire-level teardown for a locally-initiated close (user close or ack
1313-
// timeout): HTTP closes the request's SSE stream; stdio sends
1314-
// notifications/cancelled referencing the listen id. Not called when
1315-
// the server already terminated (error / server-cancelled).
1314+
// timeout). Transport-agnostic: ALWAYS abort the request signal (closes
1315+
// the SSE stream where the transport honors `requestSignal` — HTTP does,
1316+
// stdio does not) AND send `notifications/cancelled` referencing the
1317+
// listen id (which the stdio listen router and any spec-compliant
1318+
// server honor). Idempotent over HTTP — the cancelled notification is
1319+
// a no-op once the stream is gone; correct on every other transport.
1320+
// Not called when the server already terminated (error / server-
1321+
// cancelled).
13161322
const wireTeardown = async (): Promise<void> => {
1323+
requestAbort.abort();
13171324
const id = parked?.messageId;
1318-
if (transportKind === 'stdio' && id !== undefined) {
1325+
if (id !== undefined) {
13191326
await this.transport
13201327
?.send({ jsonrpc: '2.0', method: 'notifications/cancelled', params: { requestId: id } })
13211328
.catch(() => {});
1322-
} else {
1323-
requestAbort.abort();
13241329
}
13251330
};
13261331

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
* `Client.listen()` — the `subscriptions/listen` driver (protocol revision
33
* 2026-07-28). Covers ack-resolved-promise, change-notification dispatch to
44
* existing setNotificationHandler registrations, the F-12 legacy-era steer,
5-
* stdio-style close (sends notifications/cancelled), inbound server-side
6-
* cancel, and ClientOptions.listChanged auto-open on a modern connection.
5+
* transport-agnostic close (always sends notifications/cancelled), inbound
6+
* server-side cancel, and ClientOptions.listChanged auto-open on a modern
7+
* connection.
78
*/
89
import type { JSONRPCMessage, JSONRPCNotification } from '@modelcontextprotocol/core';
910
import { InMemoryTransport, LATEST_PROTOCOL_VERSION, SdkError, SdkErrorCode, SUBSCRIPTION_ID_META_KEY } from '@modelcontextprotocol/core';
@@ -103,10 +104,12 @@ describe('Client.listen()', () => {
103104
await client.close();
104105
});
105106

106-
it('close() on a stdio-style transport sends notifications/cancelled referencing the listen id', async () => {
107+
it('close() sends notifications/cancelled referencing the listen id on any transport', async () => {
108+
// Plain InMemoryTransport (neither child-process nor SSE-stream
109+
// semantics): close() must NOT depend on transport-kind detection —
110+
// it always sends notifications/cancelled, so a spec-compliant server
111+
// on InMemory / SSE / a custom transport tears the subscription down.
107112
const { clientTx, written } = await scriptedModern();
108-
// Make the in-memory transport quack like a stdio child-process transport.
109-
Object.assign(clientTx, { stderr: null, pid: 1 });
110113
const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } });
111114
await client.connect(clientTx);
112115
const sub = await client.listen({ toolsListChanged: true });

0 commit comments

Comments
 (0)