From c7e33187c1679a40c004dd036ecd6ed87fc71e66 Mon Sep 17 00:00:00 2001 From: Subhadip Hazra Date: Mon, 20 Jul 2026 20:22:16 +0530 Subject: [PATCH] fix(server): tag list_changed notifications with the in-flight request id Server.sendToolListChanged()/sendResourceListChanged()/sendPromptListChanged() send with no relatedRequestId, so the transport always routes them at the standalone GET SSE stream. On a stateless Streamable HTTP transport that stream never exists, so a notification fired from inside a request handler (e.g. enabling a tool via RegisteredTool.enable()) is silently dropped, even though the handler's own request has a response stream sitting right there. Protocol now runs each request handler inside a tracked context (_runHandlerInContext) and, when a notification is sent without an explicit relatedRequestId, defaults it to whatever request is currently in flight (_currentInflightRequestId). Server backs this with an AsyncLocalStorage sourced from the existing per-runtime _shims subpath (a single-slot synchronous-scope fallback on workerd/browser, where node:async_hooks isn't available). Fixes #2232. --- ...ss-tool-list-changed-related-request-id.md | 5 + packages/core-internal/src/shared/protocol.ts | 46 +++++++- packages/server/src/server/server.ts | 21 +++- packages/server/src/shimsBrowser.ts | 21 ++++ packages/server/src/shimsNode.ts | 1 + packages/server/src/shimsWorkerd.ts | 25 +++++ ...atelessListChangedRelatedRequestId.test.ts | 104 ++++++++++++++++++ 7 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 .changeset/stateless-tool-list-changed-related-request-id.md create mode 100644 packages/server/test/server/statelessListChangedRelatedRequestId.test.ts diff --git a/.changeset/stateless-tool-list-changed-related-request-id.md b/.changeset/stateless-tool-list-changed-related-request-id.md new file mode 100644 index 0000000000..1b81ed743f --- /dev/null +++ b/.changeset/stateless-tool-list-changed-related-request-id.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/server': patch +--- + +Fix `sendToolListChanged()`/`sendResourceListChanged()`/`sendPromptListChanged()` silently dropping their notification on a stateless Streamable HTTP transport when called from inside a request handler. These methods send with no `relatedRequestId`, so the transport routed them at the standalone GET SSE stream — which stateless transports never open, so the message had nowhere to go. Notifications sent while a handler is in flight are now automatically tagged with that handler's request id (unless the caller supplied its own `relatedRequestId`), so they ride the request's own response stream instead. diff --git a/packages/core-internal/src/shared/protocol.ts b/packages/core-internal/src/shared/protocol.ts index ffab6e8df8..9989cbb92d 100644 --- a/packages/core-internal/src/shared/protocol.ts +++ b/packages/core-internal/src/shared/protocol.ts @@ -664,6 +664,33 @@ export abstract class Protocol { return undefined; } + /** + * Runs `fn` as the currently in-flight handler for `requestId`, so that a + * notification sent from within it (directly via `this.notification()`, + * not through the per-request `ctx.mcpReq.notify` closure, which already + * stamps `relatedRequestId` unconditionally) can be tagged with the + * request it was triggered by instead of defaulting to no related request + * at all. The base implementation is a plain pass-through: `Protocol` has + * no notion of concurrent in-flight handlers and stays runtime-neutral + * (no `node:async_hooks` import here). `Server` overrides this with an + * `AsyncLocalStorage`-backed implementation. + */ + protected _runHandlerInContext(_requestId: RequestId, fn: () => T | Promise): T | Promise { + return fn(); + } + + /** + * The request id `_runHandlerInContext` is currently running a handler + * for, or `undefined` outside of any such context. Consulted by + * {@linkcode _notificationViaCodec} to default `relatedRequestId` when the + * caller didn't supply one. The base implementation always returns + * `undefined`, matching the pass-through default of + * {@linkcode _runHandlerInContext}. + */ + protected _currentInflightRequestId(): RequestId | undefined { + return undefined; + } + /** * Attach this instance's outbound `_meta` envelope (when one is configured) * to a request or notification. A no-op when the seam returns `undefined` @@ -1095,7 +1122,7 @@ export abstract class Protocol { // Starting with Promise.resolve() puts any synchronous errors into the monad as well. Promise.resolve() - .then(() => handler(request, ctx)) + .then(() => this._runHandlerInContext(request.id, () => handler(request, ctx))) .then( async result => { if (abortController.signal.aborted) { @@ -1607,10 +1634,21 @@ export abstract class Protocol { const jsonrpcNotification = this._envelopeOutbound({ jsonrpc: '2.0' as const, ...notification }); + // A caller-supplied `relatedRequestId` always wins; otherwise, if this + // notification is being sent while a handler is in flight (tracked via + // `_runHandlerInContext`/`_currentInflightRequestId` — see `Server`'s + // `AsyncLocalStorage`-backed override), default to that request's id. + // This is what lets `sendToolListChanged()` and friends — which pass no + // options at all — still land on the current request's own response + // stream instead of unconditionally targeting the standalone stream, + // which may not exist (e.g. a stateless transport). + const relatedRequestId = options?.relatedRequestId ?? this._currentInflightRequestId(); + const effectiveOptions = relatedRequestId === undefined ? options : { ...options, relatedRequestId }; + const debouncedMethods = this._options?.debouncedNotificationMethods ?? []; // A notification can only be debounced if it's in the list AND it's "simple" // (i.e., has no parameters and no related request ID that could be lost). - const canDebounce = debouncedMethods.includes(notification.method) && !notification.params && !options?.relatedRequestId; + const canDebounce = debouncedMethods.includes(notification.method) && !notification.params && !effectiveOptions?.relatedRequestId; if (canDebounce) { // If a notification of this type is already scheduled, do nothing. @@ -1634,14 +1672,14 @@ export abstract class Protocol { // Send the notification, but don't await it here to avoid blocking. // Handle potential errors with a .catch(). - this._transport?.send(jsonrpcNotification, options).catch(error => this._onerror(error)); + this._transport?.send(jsonrpcNotification, effectiveOptions).catch(error => this._onerror(error)); }); // Return immediately. return; } - await this._transport.send(jsonrpcNotification, options); + await this._transport.send(jsonrpcNotification, effectiveOptions); } /** diff --git a/packages/server/src/server/server.ts b/packages/server/src/server/server.ts index d69e01c1b2..21d357fa87 100644 --- a/packages/server/src/server/server.ts +++ b/packages/server/src/server/server.ts @@ -28,6 +28,7 @@ import type { NotificationMethod, NotificationOptions, ProtocolOptions, + RequestId, RequestMethod, RequestOptions, ResourceUpdatedNotification, @@ -61,7 +62,7 @@ import { SdkErrorCode, withRequestStateValue } from '@modelcontextprotocol/core-internal'; -import { DefaultJsonSchemaValidator } from '@modelcontextprotocol/server/_shims'; +import { AsyncLocalStorage, DefaultJsonSchemaValidator } from '@modelcontextprotocol/server/_shims'; import { coerceEmbeddedInputRequest, LegacyInputRequiredShim, resolveLegacyShimOptions } from './legacyInputRequiredShim'; @@ -276,6 +277,24 @@ export class Server extends Protocol { private _requestStateVerify?: (state: string, ctx: ServerContext) => unknown | Promise; private _inputRequiredServing: { maxRounds: number; roundTimeoutMs: number; legacyShim: boolean }; private _legacyShim?: LegacyInputRequiredShim; + /** Tracks the request id a handler is currently running for — see `_runHandlerInContext`. */ + private _inflightRequest = new AsyncLocalStorage<{ requestId: RequestId }>(); + + /** + * Overrides {@linkcode Protocol._runHandlerInContext}: runs the handler + * inside an `AsyncLocalStorage` context carrying `requestId`, so a + * notification sent from within it (e.g. `sendToolListChanged()`, which + * passes no `relatedRequestId` of its own) can still be tagged with the + * request that triggered it — see `_currentInflightRequestId` below. + */ + protected override _runHandlerInContext(requestId: RequestId, fn: () => T | Promise): T | Promise { + return this._inflightRequest.run({ requestId }, fn); + } + + /** Overrides {@linkcode Protocol._currentInflightRequestId}. */ + protected override _currentInflightRequestId(): RequestId | undefined { + return this._inflightRequest.getStore()?.requestId; + } /** Lazily-built legacy shim; the loop lives in legacyInputRequiredShim.ts behind a narrow host contract. */ private _legacyInputRequiredShim(): LegacyInputRequiredShim { diff --git a/packages/server/src/shimsBrowser.ts b/packages/server/src/shimsBrowser.ts index 7a9c7154f5..4ad0549a2d 100644 --- a/packages/server/src/shimsBrowser.ts +++ b/packages/server/src/shimsBrowser.ts @@ -25,3 +25,24 @@ export const process = { return notSupported(); } }; + +/** + * Single-slot fallback — browsers have no `node:async_hooks`. See the + * identical implementation in `shimsWorkerd.ts` for the scope/limitations of + * this fallback (synchronous-scope only, unlike the real `AsyncLocalStorage`). + */ +export class AsyncLocalStorage { + private _store: T | undefined; + getStore(): T | undefined { + return this._store; + } + run(store: T, cb: (...args: A) => R, ...args: A): R { + const prev = this._store; + this._store = store; + try { + return cb(...args); + } finally { + this._store = prev; + } + } +} diff --git a/packages/server/src/shimsNode.ts b/packages/server/src/shimsNode.ts index 6ec703659a..2c72916331 100644 --- a/packages/server/src/shimsNode.ts +++ b/packages/server/src/shimsNode.ts @@ -4,4 +4,5 @@ * This file is selected via package.json export conditions when running in Node.js. */ export { AjvJsonSchemaValidator as DefaultJsonSchemaValidator } from '@modelcontextprotocol/core-internal/validators/ajv'; +export { AsyncLocalStorage } from 'node:async_hooks'; export { default as process } from 'node:process'; diff --git a/packages/server/src/shimsWorkerd.ts b/packages/server/src/shimsWorkerd.ts index bf0ddfc7d4..8923eee30f 100644 --- a/packages/server/src/shimsWorkerd.ts +++ b/packages/server/src/shimsWorkerd.ts @@ -30,3 +30,28 @@ export const process = { return notSupported(); } }; + +/** + * Single-slot fallback for environments without `node:async_hooks` (workerd + * without the `nodejs_compat` flag). Unlike the real `AsyncLocalStorage`, the + * store is only guaranteed live for the SYNCHRONOUS portion of `run()`'s + * callback — it is cleared as soon as that call returns, not after any + * promise it returns settles. This still covers the common case (a handler + * that triggers a related notification before its first `await`); a handler + * that awaits first and only then triggers one loses the association here. + */ +export class AsyncLocalStorage { + private _store: T | undefined; + getStore(): T | undefined { + return this._store; + } + run(store: T, cb: (...args: A) => R, ...args: A): R { + const prev = this._store; + this._store = store; + try { + return cb(...args); + } finally { + this._store = prev; + } + } +} diff --git a/packages/server/test/server/statelessListChangedRelatedRequestId.test.ts b/packages/server/test/server/statelessListChangedRelatedRequestId.test.ts new file mode 100644 index 0000000000..1575dc98e0 --- /dev/null +++ b/packages/server/test/server/statelessListChangedRelatedRequestId.test.ts @@ -0,0 +1,104 @@ +/** + * Regression coverage for #2232: `sendToolListChanged()` (and the equivalent + * resource/prompt notifications) carry no `relatedRequestId`, so on a + * stateless Streamable HTTP transport — which has no standalone GET SSE + * stream for the transport to fall back to — a list-changed notification + * fired from inside a request handler has nowhere to go and is silently + * dropped, even though the response stream for the *current* request is + * right there and able to carry it. + */ +import type { CallToolResult } from '@modelcontextprotocol/core-internal'; +import { describe, expect, it } from 'vitest'; +import * as z from 'zod/v4'; + +import { McpServer } from '../../src/server/mcp'; +import { WebStandardStreamableHTTPServerTransport } from '../../src/server/streamableHttp'; + +async function readFullSSEBody(response: Response): Promise { + const reader = response.body?.getReader(); + if (!reader) return ''; + const decoder = new TextDecoder(); + let body = ''; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + body += decoder.decode(value, { stream: true }); + } + return body; +} + +function buildServer(): McpServer { + const mcpServer = new McpServer( + { name: 'stateless-list-changed-test', version: '1.0.0' }, + { capabilities: { tools: { listChanged: true } } } + ); + + const hidden = mcpServer.registerTool( + 'hidden-tool', + { description: 'Starts disabled', inputSchema: z.object({}) }, + async (): Promise => ({ content: [{ type: 'text', text: 'hidden' }] }) + ); + hidden.disable(); + + mcpServer.registerTool( + 'enable-hidden', + { description: 'Enables hidden-tool from inside its own handler', inputSchema: z.object({}) }, + async (): Promise => { + hidden.enable(); + return { content: [{ type: 'text', text: 'enabled hidden-tool' }] }; + } + ); + + return mcpServer; +} + +function postRequest(body: unknown): Request { + return new Request('http://localhost/mcp', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream' + }, + body: JSON.stringify(body) + }); +} + +describe('stateless Streamable HTTP — tools/list_changed emitted from inside a tools/call handler', () => { + it('delivers notifications/tools/list_changed on the same response stream as the tools/call result', async () => { + const mcpServer = buildServer(); + const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: undefined }); + await mcpServer.connect(transport); + + await transport.handleRequest( + postRequest({ + jsonrpc: '2.0', + id: 'init-1', + method: 'initialize', + params: { protocolVersion: '2025-11-25', capabilities: {}, clientInfo: { name: 'test-client', version: '1.0' } } + }) + ); + + const response = await transport.handleRequest( + postRequest({ + jsonrpc: '2.0', + id: 'call-1', + method: 'tools/call', + params: { name: 'enable-hidden', arguments: {} } + }) + ); + + expect(response.status).toBe(200); + const body = await readFullSSEBody(response); + + // The tool call itself always succeeds — this is not in question. + expect(body).toContain('enabled hidden-tool'); + + // This is the bug: on a stateless transport there is no standalone SSE + // stream for a `relatedRequestId`-less notification to land on, so it + // is silently dropped instead of riding the current request's stream. + expect(body).toContain('notifications/tools/list_changed'); + + await transport.close(); + await mcpServer.close(); + }); +});