Skip to content

2026-06-30 routing exp 1#2094

Draft
KKonstantinov wants to merge 9 commits into
mainfrom
feature/2026-06-30-routing-exp-1
Draft

2026-06-30 routing exp 1#2094
KKonstantinov wants to merge 9 commits into
mainfrom
feature/2026-06-30-routing-exp-1

Conversation

@KKonstantinov
Copy link
Copy Markdown
Contributor

@KKonstantinov KKonstantinov commented May 15, 2026

Dual-protocol routing: serve MCP 2025-11 and 2026-06 clients from a single server

A single HTTP endpoint (or stdio process) transparently serves both legacy (2025-11) and modern (2026-06) MCP protocol clients. Server authors register tools/resources/prompts once; the SDK handles version detection, routing, and per-protocol dispatch.

Motivation and Context

The MCP 2026-06 spec (SEPs 2575, 2567, 2322, 2243) fundamentally changes the connection model: no initialize handshake, no session state, per-request _meta with protocol version / client capabilities / client info, server/discover for capability advertisement, and Mcp-Method header-based routing.

Servers need to support both protocol generations during the transition period. The spec explicitly documents this dual-support as a MAY (see lifecycle.mdx at 85ec3771). This PR implements the routing infrastructure so that a single McpServer instance can serve both client generations without any code changes from server authors.

What's implemented (scope: routing + basic operations)

Server-side:

  • WebStandardStreamableHTTPServerTransport — dual-protocol HTTP transport that detects modern clients via the Mcp-Method header and routes them to a stateless ModernProtocolHandler. Legacy clients are routed to per-session LegacyServer instances with full initialize handshake, Mcp-Session-Id tracking, and SSE push support.
  • StdioServerTransport — dual-protocol stdio transport that detects the client's protocol version from the first message (initialize → legacy, server/discover or _meta.protocolVersion present → modern) and locks for the connection lifetime.
  • ModernProtocolHandler — stateless request handler for 2026-06 clients. Implements server/discover, validates _meta.protocolVersion, builds per-request ServerContext, adds result_type: "complete" to responses.
  • LegacyServer (renamed from Server) — the existing Protocol-based server, now used internally by the routing transports for legacy client sessions.

Client-side:

  • StreamableHTTPClientTransport — probes server/discover during start() via a direct HTTP fetch (bypassing the transport's onmessage). Sets mode: 'modern' on success and automatically injects Mcp-Method header on every outgoing request.
  • StdioClientTransport — sends a server/discover probe over stdio during start(), with configurable timeout. Queues any non-probe messages received during probing for later delivery.
  • ModernClientImpl — lightweight client for 2026-06 servers. Does NOT extend Protocol. Manages its own request/response correlation, injects _meta with protocolVersion, clientCapabilities, and clientInfo into every request, and dispatches incoming server-to-client requests/notifications via the shared HandlerRegistry.
  • Client facade — detects VersionProbingTransport and delegates to ModernClientImpl (modern) or LegacyClient (legacy). All setRequestHandler / setNotificationHandler calls go through a shared HandlerRegistry so handlers registered before connect() work regardless of which protocol path is taken.

Core refactoring:

  • HandlerRegistry extracted from Protocol — a standalone request/notification handler registry with capability checking and handler wrapping. Shared between Client facade, LegacyClient, and ModernClientImpl.
  • Server decoupled from Protocol via composition — the Server class now delegates to Protocol rather than extending it, with a fallbackRequestHandler hook that routing transports use to forward requests to McpServer's tool/resource/prompt handlers.
  • ProtocolConfig interface on Transport — transports can receive handler maps, server info, and capabilities at connect() time via setProtocolConfig(), enabling routing transports to create inner server instances on demand.

Removals:

  • All task-related code removed: TaskManager, InMemoryTaskStore, InMemoryTaskMessageQueue, experimental.tasks APIs, task schemas, task capability types, and all task tests. Tasks will return as an extension (ext-tasks) per SEP 2663 in a future PR.

What's NOT yet implemented (future PRs)

These 2026-06 spec features are not part of this routing MVP:

Feature SEP Status
subscriptions/listen (push notification channel) 2575 Not started
IncompleteResult / MRTR (input_required return path for elicitation/sampling) 2322 Not started
MCP-Protocol-Version header enforcement (required header matching _meta.protocolVersion) 2575 Not started
UnsupportedProtocolVersionError with supported[] 2575, 2716 Not started
Per-request _meta.logLevel and in-band logging 2575 Not started
ttl on list results (cache hints) 2549 Not started
Extensions framework (registrar for ext-tasks, ext-roots, etc.) 2133 Not started
Tasks as extension (ext-tasks) 2663 Removed; will return as extension

How Has This Been Tested?

All 1,294 tests pass across all packages (pnpm test:all).

New test suites:

  • httpVersionRouting.test.ts (618 lines) — tests HTTP transport routing: modern client detection via Mcp-Method header, server/discover responses, modern tools/call and tools/list, legacy client initialize flow through the routing transport, concurrent modern and legacy sessions, error handling for missing _meta.protocolVersion, and batch request rejection on the modern path.
  • stdioVersionRouting.test.ts (480 lines) — tests stdio transport routing: version detection from first message, server/discover probing, modern tools/call via stdio, legacy initialize flow, version lock behavior (once detected, mode is locked for the connection).
  • versionProbing.test.ts (693 lines) — tests client-side version probing: HTTP server/discover probing, fallback to legacy on probe failure, forceLegacy option, ModernClientImpl _meta injection, Client facade delegation between modern and legacy paths.
  • stdioVersionProbing.test.ts (128 lines) — tests stdio client probing: legacy server detection, modern server detection via fixture servers, probe timeout handling, forceLegacy behavior.
  • handlerRegistry.test.ts (93 lines) — tests the extracted HandlerRegistry: handler registration/removal, capability checking, fallback handlers.

Modified test suites:

  • streamableHttp.test.ts (middleware/node) — adapted to work with routing transport: tools registered before connect(), notifications sent via tool handler context (not transport.send()), second initialize creates a new session instead of being rejected.
  • Integration tests — task-related tests removed, remaining tests pass unchanged.

Breaking Changes

  • Server is now a facade — the public Server export from @modelcontextprotocol/server is still named Server, but it is no longer a Protocol subclass. It is a lightweight facade that delegates to an internal LegacyServer (the old Protocol-based class, not publicly exported) or passes a ProtocolConfig to routing transports. Code that only uses Server via McpServer is unaffected. Code that accessed Protocol internals on Server (e.g., server.transport, server.request()) may break.
  • WebStandardStreamableHTTPServerTransport replaced — the previous class is now LegacyWebStandardStreamableHTTPServerTransport. The new WebStandardStreamableHTTPServerTransport is the dual-protocol routing transport.
  • StdioServerTransport replaced — same pattern. Previous is LegacyStdioServerTransport.
  • Client transports replacedStreamableHTTPClientTransportLegacyStreamableHTTPClientTransport, StdioClientTransportLegacyStdioClientTransport. New classes with the same names add version probing.
  • All task APIs removedInMemoryTaskStore, InMemoryTaskMessageQueue, experimental.tasks, task capability types, task schemas. Will return as ext-tasks extension.
  • Second initialize on same HTTP endpoint now creates a new session instead of returning 400 "Server already initialized". This is an architectural consequence of per-session server stacks (each LegacyServer instance only sees one initialize), not a deliberate spec change. Session creation rate-limiting may be needed to prevent abuse.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Architecture: how routing works

flowchart TD
    A["McpServer (unchanged)<br/>.tool() / .resource() / .prompt()"] -->|registers handlers| B["Server (facade)<br/>connect(routingTransport)<br/>→ setProtocolConfig() on transport"]
    B --> C{"Routing Transport<br/>(HTTP or stdio)"}
    C -->|"Mcp-Method header present<br/>(or server/discover,<br/>_meta.protocolVersion)"| D["ModernProtocolHandler<br/>(stateless)"]
    C -->|No Mcp-Method header| E["Per-session LegacyServer<br/>+ LegacyTransport<br/>(initialize handshake,<br/>Mcp-Session-Id, SSE push)"]
Loading

Design decisions

  1. Routing at the transport layer, not the server layer. The routing transport owns the version detection logic and creates inner servers/handlers as needed. McpServer and tool handlers are completely unaware of which protocol version they're serving.

  2. Shared HandlerRegistry. The Client facade, LegacyClient, and ModernClientImpl all share a single HandlerRegistry. Handlers registered before connect() (e.g., sampling/createMessage) work regardless of which path is taken. This avoids the need to register handlers twice.

  3. Per-session legacy stacks on HTTP. Each legacy initialize creates a fresh LegacyServer + LegacyTransport pair. This is necessary because legacy clients expect per-session state (Mcp-Session-Id, SSE stream, logging level). The routing transport manages a Map<sessionId, {server, transport}>.

  4. Version lock on stdio. Once the first message determines the protocol generation (legacy or modern), it's locked for the connection lifetime. This matches the spec's assumption that a stdio connection serves a single client.

  5. forceLegacy escape hatch. Both client and server transports accept forceLegacy: true to skip version probing and always use the legacy path. Useful for testing and for environments where the probe causes issues.

  Tasks in their current form (TaskManager, InMemoryTaskStore,
  InMemoryTaskMessageQueue, experimental task server/client) are being
  removed from the MCP spec entirely. This simplifies Protocol._onrequest()
  and the handler dispatch path in preparation for the routing transport.

  - Delete packages/*/src/experimental/tasks/ directories
  - Delete packages/core/src/shared/taskManager.ts
  - Remove task processing from Protocol._onrequest, _onresponse,
    _requestWithSchema, and notification methods
  - Remove assertTaskCapability/assertTaskHandlerCapability from Protocol
  - Simplify Server._wrapHandler and Client._wrapHandler (remove task branches)
  - Remove handleAutomaticTaskPolling from McpServer
  - Remove 16 task schemas and capability definitions from schemas.ts
  - Remove task type exports from all index files
  - Delete task-only test and example files (~8,500 test lines removed)
  - Add @ts-expect-error for intentional SDK-vs-spec divergence in
    spec.types.test.ts (spec still has task types)

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-06)

  MCP protocol clients. Version detection uses the Mcp-Method header
  (required in 2026-06, absent in 2025-11).

  - Add ProtocolConfig interface and setProtocolConfig() to Transport
  - Add protected requestHandlers getter on Protocol
  - Override Server.connect() to pass config to routing transports
  - Add ModernProtocolHandler for stateless 2026-06 dispatch
  - Add HTTPVersionRoutingTransport with Mcp-Method header routing
  - Add integration tests proving both paths return identical results
  - Add versionRoutingExample.ts with Express
  - Clean up empty experimental dirs and unused imports from task removal

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  EOF
  )
  Rename the Protocol-based Server to LegacyServer and introduce a new
  Server class that composes it. Server no longer extends Protocol — it
  wraps a LegacyServer internally and delegates all public methods.

  - LegacyServer: the current Protocol-based implementation, used
    internally for legacy transport connections and per-session stacks
  - Server: composition wrapper with same constructor and public API,
    branches connect() between routing and regular transports
  - Server.connect(routingTransport) passes config via getProtocolConfig()
    without wiring Protocol's message loop
  - Server.connect(regularTransport) delegates to LegacyServer.connect()
  - LegacyServer exported from @modelcontextprotocol/server for advanced use
  - Zero public API breaks — all 1,256 tests pass unchanged

  Also includes post-MVP changes 1-4:
  - Skip Protocol.connect() for routing transports (protected setTransport)
  - createServer factory in ProtocolConfig (preserves Server subclasses)
  - Test coverage for resources/prompts on modern path (4 new tests)
  - Test coverage for GET/DELETE legacy routing (3 new tests)

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  )
…odernClientImpl

  Handlers registered via Client.setRequestHandler() were only stored in
  LegacyClient's internal registry. ModernClientImpl had its own empty
  handler maps, so server-to-client requests (sampling, elicitation)
  always got 'Method not found' on the modern path.

  Fix: Client facade owns the HandlerRegistry and passes it to both
  LegacyClient (via options.registry) and ModernClientImpl (via
  constructor). Extract assertClientHandlerCapability() and
  clientWrapHandler() as standalone functions (mirrors server.ts pattern)
  so they work without a LegacyClient instance.
  A single StdioServerTransport detects the protocol version from the
  first message and locks for the connection lifetime. A single
  StdioClientTransport probes server/discover after spawning the process.

  Server side: modern clients → ModernProtocolHandler, legacy clients →
  LegacyServer via VirtualStdioTransport adapter. Client side: probe
  success → ModernClientImpl, probe failure/timeout → LegacyClient.

  Client.connect() generalized from instanceof StreamableHTTPClientTransport
  to isVersionProbingTransport() interface check, supporting both HTTP and
  stdio routing transports.
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 15, 2026

⚠️ No Changeset found

Latest commit: be9b371

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 15, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2094

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2094

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2094

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2094

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2094

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2094

commit: be9b371

@KKonstantinov
Copy link
Copy Markdown
Contributor Author

@claude review

Comment on lines +1379 to +1381
async sendRootsListChanged() {
return this._legacyImpl.sendRootsListChanged();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Client.sendRootsListChanged() unconditionally delegates to _legacyImpl, but on the modern path _legacyImpl is never connected, so Protocol.notification() throws SdkError(NotConnected, 'Not connected'). Every other facade method guards with if (this._modernImpl) — this one needs the same guard (no-op like Client.notification() does, or route through the modern impl), otherwise any client that declares roots.listChanged and calls sendRootsListChanged() against a 2026-06 server will crash.

Extended reasoning...

What the bug is

Client.sendRootsListChanged() (client.ts:1379-1381) is implemented as:

async sendRootsListChanged() {
    return this._legacyImpl.sendRootsListChanged();
}

Unlike every other high-level method on the Client facade (ping, callTool, listTools, notification, etc.), it has no if (this._modernImpl) branch — it always calls into _legacyImpl.

Code path that triggers it

  1. client.connect(transport) is called with a version-probing transport whose probe succeeds → transport.mode === 'modern'.
  2. The facade constructs a ModernClientImpl, calls modern.connect(transport), sets this._modernImpl = modern, and returns earlythis._legacyImpl.connect() is never called.
  3. Because _legacyImpl.connect() never ran, _legacyImpl's underlying Protocol._transport is still undefined.
  4. User code calls client.sendRootsListChanged().
  5. _legacyImpl.sendRootsListChanged()this.notification({ method: 'notifications/roots/list_changed' })Protocol.notification().
  6. Protocol.notification() (protocol.ts:824-826) begins with:
    if (!this._transport) {
        throw new SdkError(SdkErrorCode.NotConnected, 'Not connected');
    }
    Since _transport is undefined, this throws synchronously.

Why existing code doesn't prevent it

The facade was clearly designed to short-circuit on the modern path — Client.notification() at line ~1403 does exactly that:

async notification(notification: Notification, options?: NotificationOptions): Promise<void> {
    if (this._modernImpl) {
        return;
    }
    return this._legacyImpl.notification(notification, options);
}

sendRootsListChanged() was simply missed when adding these guards. There's no other safety net: LegacyClient.sendRootsListChanged() doesn't check connection state before calling notification(), and Protocol.notification() throws rather than no-opping when disconnected.

Impact

This is a regression for existing user code. Any client that declares the roots: { listChanged: true } capability and calls client.sendRootsListChanged() (the documented way to inform the server that roots changed) will crash with an unhandled 'Not connected' error the moment it talks to a 2026-06 server — even though the connection is perfectly healthy via _modernImpl. The same code works fine against legacy servers, so users will see intermittent failures depending on which server generation they hit.

Step-by-step proof

const client = new Client({ name: 'c', version: '1' }, { capabilities: { roots: { listChanged: true } } });
const transport = new StreamableHTTPClientTransport(modernServerUrl);  // probe succeeds → mode='modern'
await client.connect(transport);
// At this point: client._modernImpl is set, client._legacyImpl._transport === undefined

await client.sendRootsListChanged();
// → _legacyImpl.sendRootsListChanged()
// → LegacyClient.notification({ method: 'notifications/roots/list_changed' })
// → Protocol.notification(): if (!this._transport) throw new SdkError(NotConnected, 'Not connected')
// 💥 throws

Fix

Add the same guard the other facade methods use:

async sendRootsListChanged() {
    if (this._modernImpl) {
        return;  // modern path: client→server notifications are no-ops (same as Client.notification())
    }
    return this._legacyImpl.sendRootsListChanged();
}

(Or route it through _modernImpl once the modern notification channel exists — but at minimum it must not crash.)

Comment on lines +270 to +293
private _request<T>(method: string, params?: Record<string, unknown>, options?: RequestOptions): Promise<T> {
const id = this._nextId++;
const timeout = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC;

const message: JSONRPCRequest = {
jsonrpc: '2.0',
id,
method,
params: {
...params,
_meta: {
...(params?._meta as Record<string, unknown> | undefined),
protocolVersion: '2026-06-30',
clientCapabilities: this._clientCapabilities,
clientInfo: this._clientInfo
}
}
};

return new Promise<T>((resolve, reject) => {
const timer = setTimeout(() => {
this._pending.delete(id);
reject(new SdkError(SdkErrorCode.RequestTimeout, 'Request timed out', { timeout }));
}, timeout);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 _request() only reads options?.timeoutoptions?.signal is never inspected, no abort listener is registered, and no notifications/cancelled is sent. Since RequestOptions.signal is part of the public API and the legacy Protocol path honors it, client.callTool(params, { signal }) followed by abort() works against a 2025-11 server but silently hangs until timeout against a 2026-06 server. (onprogress, resetTimeoutOnProgress, and maxTotalTimeout are likewise dropped on this path.)

Extended reasoning...

What the bug is

ModernClientImpl._request() (modernClientImpl.ts:270-313) accepts a RequestOptions object but consumes only one field from it:

private _request<T>(method: string, params?: ..., options?: RequestOptions): Promise<T> {
    const id = this._nextId++;
    const timeout = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC;
    ...
    return new Promise<T>((resolve, reject) => {
        const timer = setTimeout(() => { ... }, timeout);
        this._pending.set(id, { resolve, reject, timer });
        this._transport!.send(message).catch(...);
    });
}

There is no options?.signal?.throwIfAborted(), no options?.signal?.addEventListener('abort', ...), no early reject on abort, and no notifications/cancelled emission. The only AbortSignal reference in the entire file is the synthetic one created at line 363 for inbound server-to-client requests — unrelated to outbound cancellation.

Code path that triggers it

  1. User constructs Client and connects via StreamableHTTPClientTransport to a server that supports the 2026-06 protocol.
  2. Client.connect() (client.ts:1175-1190) detects transport.mode === 'modern' and instantiates ModernClientImpl, setting this._modernImpl.
  3. User calls client.callTool({ name: 'long-running' }, { signal: ac.signal }).
  4. Client.callTool() delegates to this._modernImpl.callTool(params, options), which calls this._request('tools/call', params, options).
  5. _request() reads options.timeout only and ignores options.signal entirely.
  6. User calls ac.abort(). Nothing happens — the entry remains in _pending and the promise is not rejected until the setTimeout fires (default 60 s).

Why existing code doesn't prevent it

The type system gives no warning: RequestOptions.signal?: AbortSignal (protocol.ts:97) is optional, so passing it type-checks fine even though the modern impl never reads it. The Client facade forwards options verbatim to whichever impl is active, so the caller has no way to know the option is being dropped.

By contrast, the legacy path (Protocol._requestWithSchema, protocol.ts:796-797) explicitly wires the signal:

onAbort = () => cancel(options?.signal?.reason);
options?.signal?.addEventListener('abort', onAbort, { once: true });

and cancel() both rejects the local promise and sends notifications/cancelled to the peer. So the same user code behaves correctly when the same Client is connected to a legacy server, and silently breaks when connected to a modern one.

The PR description's "NOT yet implemented" table lists subscriptions/listen, MRTR, header enforcement, etc., but does not list request cancellation as deferred — so this looks like an oversight rather than an intentional MVP cut.

Impact

  • Cancellation is a no-op on the modern path. Long-running tool calls cannot be aborted; UIs that show a "Cancel" button will appear to do nothing, and the request consumes the full timeout window.
  • No notifications/cancelled is sent, so the server keeps doing work it no longer needs to.
  • Behavioral divergence: identical client code behaves differently depending on which protocol version the server negotiated, which is exactly what the dual-protocol facade is supposed to hide.
  • The same applies to onprogress (no progressToken is injected into _meta and no progress handler is registered), resetTimeoutOnProgress, and maxTotalTimeout — all are silently ignored.

Step-by-step proof

const ac = new AbortController();
const client = new Client({ name: 'c', version: '1' });
await client.connect(new StreamableHTTPClientTransport(modernUrl)); // mode === 'modern'

const p = client.callTool({ name: 'slow' }, { signal: ac.signal, timeout: 60_000 });
// → ModernClientImpl._request('tools/call', params, { signal, timeout })
//   line 272: const timeout = 60_000           ← signal not read
//   line 290: setTimeout(reject, 60_000)
//   line 295: _pending.set(id, {...})
//   line 307: transport.send(message)          ← no signal passed to transport either

ac.abort();
// Nothing references ac.signal → no listener fires → _pending still has the entry
// p does not reject; server is not told to cancel.

// 60 s later: timer fires → reject(SdkError RequestTimeout)

Against a legacy server the same code rejects immediately on ac.abort() with the abort reason and sends notifications/cancelled.

How to fix

In _request(), mirror the legacy behavior:

if (options?.signal?.aborted) {
    return Promise.reject(options.signal.reason ?? new Error('Aborted'));
}
...
return new Promise<T>((resolve, reject) => {
    const cleanup = () => {
        clearTimeout(timer);
        this._pending.delete(id);
        options?.signal?.removeEventListener('abort', onAbort);
    };
    const onAbort = () => {
        cleanup();
        this._transport?.send({
            jsonrpc: '2.0',
            method: 'notifications/cancelled',
            params: { requestId: id, reason: String(options?.signal?.reason ?? '') }
        }).catch(e => this.onerror?.(e));
        reject(options?.signal?.reason ?? new Error('Aborted'));
    };
    options?.signal?.addEventListener('abort', onAbort, { once: true });
    ...
});

If onprogress / resetTimeoutOnProgress / maxTotalTimeout are intentionally out of scope for this MVP, they should at least be added to the "not yet implemented" table so callers know not to rely on them; otherwise they need similar wiring (inject progressToken into _meta, register a progress handler, and reset the timer on progress).

Comment on lines +154 to +160
this._legacyServer.fallbackRequestHandler = async (request, ctx) => {
const handler = config.requestHandlers.get(request.method);
if (!handler) {
throw new Error(`Method not found: ${request.method}`);
}
return handler(request, ctx);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 This fallback throws a plain new Error(...), which Protocol._onrequest maps to JSON-RPC error -32603 (InternalError) instead of -32601 (MethodNotFound) — so legacy stdio clients calling an unregistered method get the wrong error code. The sibling HTTP routing transport (modernStreamableHttp.ts:176) already does this correctly with new ProtocolError(ProtocolErrorCode.MethodNotFound, ...); this should match.

Extended reasoning...

What the bug is

In StdioServerTransport._initLegacyPath(), the fallbackRequestHandler installed on the inner LegacyServer throws a plain Error when no handler is registered for the incoming method:

this._legacyServer.fallbackRequestHandler = async (request, ctx) => {
    const handler = config.requestHandlers.get(request.method);
    if (!handler) {
        throw new Error(`Method not found: ${request.method}`);
    }
    return handler(request, ctx);
};

When this throws, it lands in Protocol._onrequest's error path (protocol.ts:571), which builds the JSON-RPC error response with:

code: Number.isSafeInteger(error['code']) ? error['code'] : ProtocolErrorCode.InternalError

A plain Error has no numeric .code, so the wire response is { code: -32603, message: "Method not found: ..." } — i.e. InternalError, not MethodNotFound.

Why existing code doesn't prevent it

Before this PR, an unknown method on the stdio server fell through Protocol._onrequest's "no handler" branch, which explicitly emits ProtocolErrorCode.MethodNotFound (-32601). The new routing transport replaces that path by installing a fallbackRequestHandler that always matches, so the request reaches the handler and the error code is now determined by what the handler throws. Throwing a plain Error loses the code.

The sibling HTTP routing transport added in the same PR gets this right — modernStreamableHttp.ts:176 throws new ProtocolError(ProtocolErrorCode.MethodNotFound, ...) in the identical position. The two transports are inconsistent on the same code path.

Step-by-step proof

  1. A legacy (2025-11) client connects over stdio to the new routing StdioServerTransport.
  2. The first message is initialize, so _detectVersion returns 'legacy' and _initLegacyPath() runs, installing the fallback handler above.
  3. The client sends { jsonrpc: '2.0', id: 2, method: 'resources/subscribe', params: {...} } to a server that registered no resource handlers.
  4. Protocol._onrequest looks up 'resources/subscribe' in the registry, finds nothing, falls back to fallbackRequestHandler.
  5. fallbackRequestHandler looks up config.requestHandlers (the outer Server's map), finds nothing, throws new Error('Method not found: resources/subscribe').
  6. Protocol._onrequest's catch block sees error.code === undefined, so it sends { error: { code: -32603, message: 'Method not found: resources/subscribe' } }.
  7. The client receives -32603 InternalError. Per JSON-RPC 2.0 and the MCP spec it should have received -32601 MethodNotFound.

Impact

This is a regression for stdio: pre-PR, unknown methods correctly returned -32601. Clients commonly use MethodNotFound to feature-detect optional methods (e.g. probing for resources/subscribe or custom methods); receiving InternalError instead makes the server look broken rather than simply not supporting the method. It also makes the stdio and HTTP routing transports behave differently for the same condition.

Fix

Match the HTTP transport: import ProtocolError / ProtocolErrorCode and throw new ProtocolError(ProtocolErrorCode.MethodNotFound, Method not found: ${request.method}) at modernStdio.ts:157.

Comment on lines +1396 to +1399
request(request: { method: string; params?: Record<string, unknown> }, ...args: unknown[]): Promise<unknown> {
if (this._modernImpl) {
return this._modernImpl.request(request, args[0] as RequestOptions | undefined);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 On the modern path, request() unconditionally passes args[0] as RequestOptions, but for the 3-arg overload request(req, resultSchema, options), args[0] is the schema and args[1] is the actual options — so the caller's {timeout, signal, ...} is silently dropped and the schema object is treated as options. Detect isStandardSchema(args[0]) and use args[1] as the options in that case (the modern impl doesn't validate results, so the schema can simply be discarded).

Extended reasoning...

What the bug is

The Client facade declares two overloads for request():

request(request, options?: RequestOptions): Promise<...>;
request(request, resultSchema: T, options?: RequestOptions): Promise<...>;

The implementation collects the trailing arguments via ...args and, on the modern path, does:

if (this._modernImpl) {
    return this._modernImpl.request(request, args[0] as RequestOptions | undefined);
}

For the 3-arg overload, args[0] is the StandardSchemaV1 result schema and args[1] is the actual RequestOptions. So the schema object is passed where RequestOptions should go, and the caller's real options are silently discarded. The legacy branch is fine because it forwards ...args verbatim to LegacyClient.request(), which has the same overload set.

Code path that triggers it

  1. User connects a Client to a version-probing transport against a 2026-06 server → transport.mode === 'modern'this._modernImpl is set.
  2. User calls client.request({ method: 'custom/op' }, MySchema, { timeout: 5000, signal }).
  3. At client.ts:1397-1398, this._modernImpl is truthy, so we call this._modernImpl.request(request, args[0]) where args[0] === MySchema.
  4. ModernClientImpl.request() forwards to _request(method, params, options).
  5. _request() (modernClientImpl.ts:272) reads options?.timeout — but options is the schema object, which has no .timeout, so it falls back to DEFAULT_REQUEST_TIMEOUT_MSEC.
  6. The user's { timeout: 5000, signal } at args[1] is never read.

Why existing code doesn't prevent it

The as RequestOptions | undefined cast suppresses the type error, and ModernClientImpl._request only does optional-chaining reads (options?.timeout), so passing an arbitrary object doesn't throw — it just silently produces undefined and falls back to defaults. No test exercises the 3-arg request() overload against a modern server.

Impact

This is a public-API regression introduced by this PR. Any existing code using the 3-arg form (client.request(req, schema, options)) — which was the canonical low-level API on Protocol — loses its timeout, signal, onprogress, etc. when talking to a modern server. Cancellation via AbortSignal stops working, and custom timeouts are ignored in favor of the 60s default.

Step-by-step proof

const client = new Client({ name: 'c', version: '1' });
await client.connect(modernTransport);          // probe succeeds → _modernImpl set
await client.request(
  { method: 'tools/call', params: { name: 't' } },
  CallToolResultSchema,                          // args[0]
  { timeout: 5000 }                              // args[1]
);
  • request() impl: args = [CallToolResultSchema, { timeout: 5000 }]
  • Modern branch: this._modernImpl.request(request, CallToolResultSchema /* as RequestOptions */)
  • _request: const timeout = (CallToolResultSchema as any)?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MSEC60_000, not 5000.
  • { timeout: 5000 } is never referenced.

How to fix

Mirror the dispatch already used in Protocol's mcpReq.send:

request(request, ...args: unknown[]): Promise<unknown> {
    if (this._modernImpl) {
        const opts = isStandardSchema(args[0]) ? (args[1] as RequestOptions | undefined)
                                               : (args[0] as RequestOptions | undefined);
        return this._modernImpl.request(request, opts);
    }
    return (this._legacyImpl.request as ...).call(this._legacyImpl, request, ...args);
}

ModernClientImpl doesn't validate results, so dropping the schema on that path is correct; only the options need to be routed properly.

Comment on lines +1180 to +1188
const modern = new ModernClientImpl(
this._clientInfo,
this._options?.capabilities ?? {},
transport.getDiscoverResult()!,
this._registry
);
await modern.connect(transport);
this._modernImpl = modern;
return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 When connect() takes the modern path it constructs ModernClientImpl from this._options?.capabilities (the constructor-time snapshot) and never copies onclose/onerror from _legacyImpl — so capabilities added via registerCapabilities() before connect are never sent in _meta.clientCapabilities, and client.onerror/client.onclose handlers assigned before connect() (the documented pattern, used in examples/client/src/simpleStreamableHttp.ts) are silently dropped against a 2026-06 server. Fix: pass this._registry.getCapabilities() to the ModernClientImpl constructor, and after creating it set modern.onclose = this._legacyImpl.onclose / modern.onerror = this._legacyImpl.onerror.

Extended reasoning...

What the bug is

The new Client facade lazily creates ModernClientImpl inside connect() (client.ts:1180-1187), but two pieces of pre-connect state are not propagated to it:

  1. onclose / onerror handlers — the setters at lines 1123-1138 store the handler on _legacyImpl and only forward to _modernImpl if it already exists. Since _modernImpl is undefined until connect() runs, and connect() never copies _legacyImpl.onclose / _legacyImpl.onerror onto the freshly created modern instance, those callbacks are lost on the modern path.
  2. Registered capabilitiesClient.registerCapabilities() delegates to _legacyImpl.registerCapabilities(), which merges into both _legacyImpl._capabilities and the shared _registry. But connect() constructs ModernClientImpl with this._options?.capabilities ?? {} — the immutable original constructor argument — instead of this._registry.getCapabilities().

Code path

// client.ts:1133-1138
set onerror(h) {
    this._legacyImpl.onerror = h;
    if (this._modernImpl) {        // ← undefined before connect()
        this._modernImpl.onerror = h;
    }
}
// client.ts:1180-1187
const modern = new ModernClientImpl(
    this._clientInfo,
    this._options?.capabilities ?? {},   // ← stale snapshot, ignores registerCapabilities()
    transport.getDiscoverResult()!,
    this._registry
);
await modern.connect(transport);          // ← onclose/onerror never copied
this._modernImpl = modern;

ModernClientImpl.connect() then wires transport.onerror = (e) => this.onerror?.(e) and transport.onclose = () => { …; this.onclose?.(); } (modernClientImpl.ts), where this.onerror / this.onclose are still undefined. And ModernClientImpl._request() injects _meta.clientCapabilities: this._clientCapabilities into every request — using only the constructor-time options.

Why nothing else catches it

The facade shares the HandlerRegistry between both impls, so request/notification handlers survive — but onclose/onerror are plain instance fields on Protocol / ModernClientImpl, not stored in the registry, so the registry sharing doesn't help. Likewise, registerCapabilities() correctly updates _registry, but connect() reads from this._options instead of this._registry, so the merged set is bypassed.

Step-by-step proof

const client = new Client({ name: 'c', version: '1' });
client.registerCapabilities({ sampling: {} });   // merged into _registry, NOT into _options
client.onerror = e => console.error('got', e);   // stored on _legacyImpl only
await client.connect(modernTransport);            // probe succeeds → mode === 'modern'
  1. connect() enters the transport.mode === 'modern' branch.
  2. new ModernClientImpl(..., this._options?.capabilities ?? {}, ...)_clientCapabilities = {} (no sampling).
  3. modern.connect(transport) sets transport.onerror = e => this.onerror?.(e) with this.onerror === undefined.
  4. this._modernImpl = modern — but _legacyImpl.onerror is never copied over.
  5. Later, the transport emits an error → undefined?.(e)silently swallowed.
  6. Every outgoing request carries _meta.clientCapabilities: {} → the server never learns the client supports sampling, so server-initiated sampling/createMessage is effectively disabled even though the user registered it.

This is the common usage pattern, not an edge case — examples/client/src/simpleStreamableHttp.ts in this very PR does client.onerror = …; await client.connect(transport), and registerCapabilities() is documented as "can only be called before connecting". On the legacy path both work (the legacy initialize sends merged caps, and _legacyImpl already holds the handlers); on the modern path both silently break.

Impact

  • Transport errors and uncaught handler errors against 2026-06 servers are dropped on the floor with no observability.
  • onclose callbacks (cleanup logic, reconnect triggers) never fire.
  • Capabilities registered via the public registerCapabilities() API are never advertised to modern servers, so capability-gated server-to-client features (sampling, elicitation, roots) won't be offered.

Fix

In Client.connect():

const modern = new ModernClientImpl(
    this._clientInfo,
    this._registry.getCapabilities(),     // ← use merged caps
    transport.getDiscoverResult()!,
    this._registry
);
modern.onclose = this._legacyImpl.onclose; // ← propagate pre-connect handlers
modern.onerror = this._legacyImpl.onerror;
await modern.connect(transport);
this._modernImpl = modern;

}

const ct = req.headers.get('content-type');
if (!ct || !ct.includes('application/json')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this require both application/json and text/event-stream? I think clients have to provide both

import { isStandardSchema, validateStandardSchema } from '../util/standardSchema.js';
import type { TaskContext, TaskManagerHost, TaskManagerOptions, TaskRequestOptions } from './taskManager.js';
import { NullTaskManager, TaskManager } from './taskManager.js';
import type { HandlerRegistry, InferHandlerResult, RequestHandlerSchemas } from './handlerRegistry.js';
Copy link
Copy Markdown
Contributor

@felixweinberger felixweinberger May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we do the registry extraction ONLY as its own PR? that feels valuable to get completely right

export class HandlerRegistry<ContextT extends BaseContext, Caps extends ServerCapabilities | ClientCapabilities> {
private _requestHandlers: Map<string, RequestHandler<ContextT>> = new Map();
private _notificationHandlers: Map<string, NotificationHandler> = new Map();
private _capabilities: Caps;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does HandlerRegistry need to know about capabilities?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually this makes sense - the HandlerRegistry has the responsibility of not only taking the map of handlers, but also verifying that those handlers are supported.

So setting up a handlerRegistry involes configuring the capabilities so that the handlerRegistry can check them

export function createServerRegistry(capabilities?: ServerCapabilities): HandlerRegistry<ServerContext, ServerCapabilities> {
const registry = new HandlerRegistry<ServerContext, ServerCapabilities>({
capabilities,
assertRequestHandlerCapability: method => assertServerHandlerCapability(method, registry.getCapabilities()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks a bit odd, passing registry to itself in the construction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants