Skip to content

Commit be1694d

Browse files
fix(client): fire onRequestStreamEnd on the GET-resume 405/null-body terminal; skip auto-open when handler registration throws; await transport close on connect-abort during the auto-open ack wait; document the protected-override contract
1 parent b142b80 commit be1694d

6 files changed

Lines changed: 268 additions & 8 deletions

File tree

.changeset/subscriptions-listen-client.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
'@modelcontextprotocol/client': minor
44
---
55

6-
`Client.listen(filter)` opens a `subscriptions/listen` stream on a 2026-07-28-era connection, resolving once the server's acknowledged notification arrives with an `McpSubscription { honoredFilter, close(), closed }`. `closed` is a `Promise<'local' | 'remote'>` that resolves exactly once when the subscription terminates (`'local'` = you called `close()`; `'remote'` = the server cancelled, the stream ended, or the transport dropped — re-listen if you still want events) and never rejects. Change notifications delivered on the stream dispatch to the existing `setNotificationHandler` registrations — the same handlers the 2025-era unsolicited notifications fire on a legacy connection — so `listen()` is era-transparent for consumers that already register those. `close()` aborts the listen request's stream (where the transport supports it) and sends `notifications/cancelled` referencing the listen id — both, on every transport; no automatic re-listen. On a 2025-era connection `listen()` throws a typed `MethodNotSupportedByProtocolVersion` steering to `resources/subscribe` and `ClientOptions.listChanged`. `ClientOptions.listChanged` now auto-opens a listen stream on a modern connection — the filter is the intersection of the configured sub-options and the server-advertised `listChanged` capabilities; auto-open is skipped (`client.autoOpenedSubscription` stays `undefined`) when that intersection is empty; otherwise the auto-opened subscription is exposed at `client.autoOpenedSubscription`. `TransportSendOptions` gains `requestSignal` for per-request abort on the Streamable HTTP transport.
6+
`Client.listen(filter)` opens a `subscriptions/listen` stream on a 2026-07-28-era connection, resolving once the server's acknowledged notification arrives with an `McpSubscription { honoredFilter, close(), closed }`. `closed` is a `Promise<'local' | 'remote'>` that resolves exactly once when the subscription terminates (`'local'` = you called `close()`; `'remote'` = the server cancelled, the stream ended, or the transport dropped — re-listen if you still want events) and never rejects. Change notifications delivered on the stream dispatch to the existing `setNotificationHandler` registrations — the same handlers the 2025-era unsolicited notifications fire on a legacy connection — so `listen()` is era-transparent for consumers that already register those. `close()` aborts the listen request's stream (where the transport supports it) and sends `notifications/cancelled` referencing the listen id — both, on every transport; no automatic re-listen. On a 2025-era connection `listen()` throws a typed `MethodNotSupportedByProtocolVersion` steering to `resources/subscribe` and `ClientOptions.listChanged`. `ClientOptions.listChanged` now auto-opens a listen stream on a modern connection — the filter is the intersection of the configured sub-options and the server-advertised `listChanged` capabilities; auto-open is skipped (`client.autoOpenedSubscription` stays `undefined`) when that intersection is empty; otherwise the auto-opened subscription is exposed at `client.autoOpenedSubscription`. `TransportSendOptions` gains `requestSignal` (per-request abort) and `onRequestStreamEnd` (fires when a per-request response stream ends or errors for any non-deliberate reason) on the Streamable HTTP transport.

packages/client/src/client/client.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -932,17 +932,24 @@ export class Client extends Protocol<ClientContext> {
932932
// throw on misconfiguration; the modern connection IS established
933933
// at this point and is fully usable without listChanged handlers,
934934
// so a misconfiguration surfaces via onerror and connect resolves
935-
// (matching the auto-open soft-fail posture).
935+
// (matching the auto-open soft-fail posture). When registration
936+
// fails the auto-open is SKIPPED — opening a listen stream for
937+
// types whose handler never registered would consume a server
938+
// slot to deliver notifications nothing handles.
939+
let handlersRegistered = true;
936940
try {
937941
this._setupListChangedHandlers(effective);
938942
} catch (error) {
943+
handlersRegistered = false;
939944
this.onerror?.(error instanceof Error ? error : new Error(String(error)));
940945
}
941-
const filter: SubscriptionFilter = {
942-
...(effective.tools && { toolsListChanged: true as const }),
943-
...(effective.prompts && { promptsListChanged: true as const }),
944-
...(effective.resources && { resourcesListChanged: true as const })
945-
};
946+
const filter: SubscriptionFilter = handlersRegistered
947+
? {
948+
...(effective.tools && { toolsListChanged: true as const }),
949+
...(effective.prompts && { promptsListChanged: true as const }),
950+
...(effective.resources && { resourcesListChanged: true as const })
951+
}
952+
: {};
946953
if (Object.keys(filter).length > 0) {
947954
// A failed auto-open MUST NOT fail connect: the modern
948955
// connection is fully usable without a listen stream (the
@@ -980,7 +987,7 @@ export class Client extends Protocol<ClientContext> {
980987
// half-open connection. A server-side refusal stays a
981988
// soft onerror (connect succeeds, no listen stream).
982989
if (options?.signal?.aborted) {
983-
void this.close();
990+
await this.close().catch(() => {});
984991
throw error;
985992
}
986993
this.onerror?.(error instanceof Error ? error : new Error(String(error)));

packages/client/src/client/streamableHttp.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,15 @@ export class StreamableHTTPClientTransport implements Transport {
375375
// 405 indicates that the server does not offer an SSE stream at GET endpoint
376376
// This is an expected case that should not trigger an error
377377
if (response.status === 405) {
378+
// A 405 on the standalone-GET path is benign (the caller
379+
// never had a per-request stream). On the POST→GET resume
380+
// path it is a TERMINAL non-resumable outcome for a
381+
// per-request stream the caller is observing — fire the
382+
// stream-end callback so the caller can settle (otherwise
383+
// a resumed listen subscription dead-ends silently). The
384+
// standalone-GET callers never pass `onRequestStreamEnd`,
385+
// so this is a no-op for them.
386+
options.onRequestStreamEnd?.();
378387
return;
379388
}
380389

@@ -463,6 +472,11 @@ export class StreamableHTTPClientTransport implements Transport {
463472

464473
private _handleSseStream(stream: ReadableStream<Uint8Array> | null, options: StartSSEOptions, isReconnectable: boolean): void {
465474
if (!stream) {
475+
// A null body on a per-request stream (or its GET resume) is the
476+
// same terminal non-resumable outcome as a 405 — fire the
477+
// stream-end callback so the caller can settle. No-op for
478+
// standalone-GET callers (they never pass `onRequestStreamEnd`).
479+
options.onRequestStreamEnd?.();
466480
return;
467481
}
468482
const { onresumptiontoken, replayMessageId, requestSignal, onRequestStreamEnd } = options;

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,31 @@ describe('Client.listen()', () => {
553553
await client.close();
554554
});
555555

556+
it('a misconfigured listChanged handler surfaces via onerror and SKIPS auto-open (no wire write)', async () => {
557+
// Regression: when handler registration threw (the soft-fail catch),
558+
// the auto-open filter was still built from the same `effective`,
559+
// opening a listen stream for types whose handler never registered —
560+
// delivered notifications dropped on the floor while consuming a
561+
// server slot. Now a registration failure skips auto-open entirely.
562+
const { clientTx, written } = await scriptedModernNoAck();
563+
const onChanged = () => {};
564+
const client = new Client(
565+
{ name: 'c', version: '1' },
566+
{ versionNegotiation: { mode: 'auto' }, listChanged: { tools: { onChanged, debounceMs: -1 } } }
567+
);
568+
const errors: Error[] = [];
569+
client.onerror = e => errors.push(e);
570+
// connect MUST resolve: the modern connection is usable without listen.
571+
await client.connect(clientTx);
572+
expect(errors).toHaveLength(1);
573+
expect(errors[0]!.message).toContain('Invalid tools listChanged options');
574+
// Auto-open SKIPPED: no listen request hit the wire, no subscription.
575+
expect(client.autoOpenedSubscription).toBeUndefined();
576+
expect(written.some(m => (m as { method?: string }).method === 'subscriptions/listen')).toBe(false);
577+
expect((client as unknown as { _listenState: Map<unknown, unknown> })._listenState.size).toBe(0);
578+
await client.close();
579+
});
580+
556581
it('connect-scoped signal does NOT bind to the auto-opened subscription lifetime', async () => {
557582
// Regression: forwarding connect()'s full RequestOptions into the
558583
// auto-open listen() call meant a connect-scoped signal — typically
@@ -591,6 +616,7 @@ describe('Client.listen()', () => {
591616
// meant connect()'s signal could not cancel the in-connect ack wait —
592617
// an aborted connect blocked here for the full ack timeout.
593618
const { clientTx } = await scriptedModernNoAck();
619+
const closeSpy = vi.spyOn(clientTx, 'close');
594620
const onChanged = () => {};
595621
const client = new Client(
596622
{ name: 'c', version: '1' },
@@ -607,6 +633,11 @@ describe('Client.listen()', () => {
607633
expect(Date.now() - t0).toBeLessThan(1000);
608634
// No leaked per-listen state on the aborted connect.
609635
expect((client as unknown as { _listenState: Map<unknown, unknown> })._listenState.size).toBe(0);
636+
// A connect() rejection MUST NOT leave a half-open connection: the
637+
// transport was closed before rethrowing (b142b80ea regression assertion).
638+
await flush();
639+
expect(closeSpy).toHaveBeenCalled();
640+
expect(client.transport).toBeUndefined();
610641
await client.close();
611642
});
612643

0 commit comments

Comments
 (0)