diff --git a/.changeset/listen-close-when-nothing-honored.md b/.changeset/listen-close-when-nothing-honored.md new file mode 100644 index 0000000000..69992f948e --- /dev/null +++ b/.changeset/listen-close-when-nothing-honored.md @@ -0,0 +1,22 @@ +--- +'@modelcontextprotocol/server': patch +--- + +Close a `subscriptions/listen` stream that has honored nothing. + +`listenRouter.serve()` computed `honoredSubset(filter, capabilities)` and then +opened the stream, acknowledged it, subscribed to the event bus and armed a +keep-alive without consulting the result. A server declaring no `listChanged` +capabilities and no `resources.subscribe` produced an empty set, so +`listenFilterAccepts({}, event)` was false for every event kind and the +subscription could never deliver anything — yet nothing closed it, since +`teardown` ran only on client disconnect or abort. + +It now writes the acknowledgement and then takes the same graceful +`teardown(true)` path `closeAll()` uses, so the client still learns exactly what +was honored and still receives the `resultType: "complete"` result. Streams that +honor at least one notification type are unaffected. + +This is most visible on request-scoped runtimes, where an invocation held open +for a subscription that can never deliver runs until the platform kills it and +the client immediately reconnects. diff --git a/packages/server/src/server/listenRouter.ts b/packages/server/src/server/listenRouter.ts index 40c4a38cf2..408aa04c09 100644 --- a/packages/server/src/server/listenRouter.ts +++ b/packages/server/src/server/listenRouter.ts @@ -213,6 +213,24 @@ export function createListenRouter(options: ListenRouterOptions): ListenRouter { ); writeNotification(ack.method, ack.params); + // Nothing honored: the ack has already told the client this + // stream carries nothing, and `listenFilterAccepts({}, event)` + // is false for every event kind, so the subscription below is + // provably a no-op. Close gracefully rather than hold an idle + // connection and a keepalive timer for a set that is empty — + // the spec's transport binding ends the listen stream "until + // the client or server closes" it, and this is the server + // closing with the same result frame `closeAll()` emits. + // + // Matters most where a connection is request-scoped: a + // serverless invocation held for a subscription that can never + // deliver runs until the platform kills it, and the client + // reopens, indefinitely. + if (Object.keys(honored).length === 0) { + teardown(true); + return; + } + // Only after the ack frame is enqueued does delivery activate. unsubscribe = bus.subscribe(event => { if (closed || !listenFilterAccepts(honored, event)) return; diff --git a/packages/server/test/server/createMcpHandlerListen.test.ts b/packages/server/test/server/createMcpHandlerListen.test.ts index fe17aa7536..0d9bb8a74d 100644 --- a/packages/server/test/server/createMcpHandlerListen.test.ts +++ b/packages/server/test/server/createMcpHandlerListen.test.ts @@ -264,6 +264,57 @@ describe('createMcpHandler — subscriptions/listen', () => { }); }); + it('acks and closes when capabilities honor nothing, instead of holding the stream', async () => { + // A server declaring no listChanged and no resources.subscribe honors + // nothing, so the stream can never carry a notification: every + // listenFilterAccepts({}, event) is false. It should end, not idle. + const bareFactory = () => new McpServer({ name: 'listen-test-server', version: '1.0.0' }, { capabilities: {} }); + const handler = createMcpHandler(bareFactory, { keepAliveMs: 0 }); + const response = await handler.fetch( + listenRequest(7, { toolsListChanged: true, resourcesListChanged: true, resourceSubscriptions: ['file:///a'] }) + ); + + // The stream ends on its own: draining it terminates without anything + // cancelling the reader. + const messages = await readMessages(response, 2); + + expect(messages).toEqual([ + { + jsonrpc: '2.0', + method: 'notifications/subscriptions/acknowledged', + params: { notifications: {}, _meta: { [SUBSCRIPTION_ID_META_KEY]: 7 } } + }, + { + jsonrpc: '2.0', + id: 7, + result: { + resultType: 'complete', + _meta: { + [SUBSCRIPTION_ID_META_KEY]: 7, + 'io.modelcontextprotocol/serverInfo': { name: 'listen-test-server', version: '1.0.0' } + } + } + } + ]); + }); + + it('still holds the stream when at least one type is honored', async () => { + // The narrowing must not close a stream that can still deliver: tools + // is honored here, resources is not. + const partialFactory = () => + new McpServer({ name: 'listen-test-server', version: '1.0.0' }, { capabilities: { tools: { listChanged: true } } }); + const handler = createMcpHandler(partialFactory, { keepAliveMs: 0 }); + const response = await handler.fetch(listenRequest(8, { toolsListChanged: true, resourcesListChanged: true })); + + const [ack] = await readMessages(response, 1); + expect(ack).toEqual({ + jsonrpc: '2.0', + method: 'notifications/subscriptions/acknowledged', + params: { notifications: { toolsListChanged: true }, _meta: { [SUBSCRIPTION_ID_META_KEY]: 8 } } + }); + await handler.close(); + }); + it('legacy-classified listen never reaches the entry listen router (no ack delivered)', async () => { const handler = createMcpHandler(trivialFactory(), { keepAliveMs: 0 }); // No envelope claim → classified legacy → dispatched through the diff --git a/test/e2e/scenarios/subscriptions.test.ts b/test/e2e/scenarios/subscriptions.test.ts index ce644fa430..ef3a42e444 100644 --- a/test/e2e/scenarios/subscriptions.test.ts +++ b/test/e2e/scenarios/subscriptions.test.ts @@ -276,7 +276,7 @@ verifies('subscriptions:listen:capacity-guard', async () => { jsonrpc: '2.0', id, method: 'subscriptions/listen', - params: { _meta: modernEnvelopeMeta(), notifications: {} } + params: { _meta: modernEnvelopeMeta(), notifications: { toolsListChanged: true } } }) }) );