From ee16173865f0a2cf084ac75929f9bbbce3148389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Barthelet?= Date: Tue, 29 Jul 2025 15:06:27 +0200 Subject: [PATCH 1/3] (fix): Update fallbackRequestHandler type to match _requestHandlers leaves type (#784) --- src/shared/protocol.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index 6142140dd..b19a6c5ca 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -217,7 +217,10 @@ export abstract class Protocol< /** * A handler to invoke for any request types that do not have their own handler installed. */ - fallbackRequestHandler?: (request: Request) => Promise; + fallbackRequestHandler?: ( + request: JSONRPCRequest, + extra: RequestHandlerExtra + ) => Promise; /** * A handler to invoke for any notification types that do not have their own handler installed. From 31acdcbb189056ec83d14a4f7a37ae2b1c67680e Mon Sep 17 00:00:00 2001 From: Grimmer Kang Date: Tue, 29 Jul 2025 21:07:01 +0800 Subject: [PATCH 2/3] fix: prevent responses being sent to wrong client when multiple transports connect (#820) --- .../protocol-transport-handling.test.ts | 189 ++++++++++++++++++ src/shared/protocol.ts | 11 +- 2 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 src/shared/protocol-transport-handling.test.ts diff --git a/src/shared/protocol-transport-handling.test.ts b/src/shared/protocol-transport-handling.test.ts new file mode 100644 index 000000000..3baa9b638 --- /dev/null +++ b/src/shared/protocol-transport-handling.test.ts @@ -0,0 +1,189 @@ +import { describe, expect, test, beforeEach } from "@jest/globals"; +import { Protocol } from "./protocol.js"; +import { Transport } from "./transport.js"; +import { Request, Notification, Result, JSONRPCMessage } from "../types.js"; +import { z } from "zod"; + +// Mock Transport class +class MockTransport implements Transport { + id: string; + onclose?: () => void; + onerror?: (error: Error) => void; + onmessage?: (message: unknown) => void; + sentMessages: JSONRPCMessage[] = []; + + constructor(id: string) { + this.id = id; + } + + async start(): Promise {} + + async close(): Promise { + this.onclose?.(); + } + + async send(message: JSONRPCMessage): Promise { + this.sentMessages.push(message); + } +} + +describe("Protocol transport handling bug", () => { + let protocol: Protocol; + let transportA: MockTransport; + let transportB: MockTransport; + + beforeEach(() => { + protocol = new (class extends Protocol { + protected assertCapabilityForMethod(): void {} + protected assertNotificationCapability(): void {} + protected assertRequestHandlerCapability(): void {} + })(); + + transportA = new MockTransport("A"); + transportB = new MockTransport("B"); + }); + + test("should send response to the correct transport when multiple clients are connected", async () => { + // Set up a request handler that simulates processing time + let resolveHandler: (value: Result) => void; + const handlerPromise = new Promise((resolve) => { + resolveHandler = resolve; + }); + + const TestRequestSchema = z.object({ + method: z.literal("test/method"), + params: z.object({ + from: z.string() + }).optional() + }); + + protocol.setRequestHandler( + TestRequestSchema, + async (request) => { + console.log(`Processing request from ${request.params?.from}`); + return handlerPromise; + } + ); + + // Client A connects and sends a request + await protocol.connect(transportA); + + const requestFromA = { + jsonrpc: "2.0" as const, + method: "test/method", + params: { from: "clientA" }, + id: 1 + }; + + // Simulate client A sending a request + transportA.onmessage?.(requestFromA); + + // While A's request is being processed, client B connects + // This overwrites the transport reference in the protocol + await protocol.connect(transportB); + + const requestFromB = { + jsonrpc: "2.0" as const, + method: "test/method", + params: { from: "clientB" }, + id: 2 + }; + + // Client B sends its own request + transportB.onmessage?.(requestFromB); + + // Now complete A's request + resolveHandler!({ data: "responseForA" } as Result); + + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Check where the responses went + console.log("Transport A received:", transportA.sentMessages); + console.log("Transport B received:", transportB.sentMessages); + + // FIXED: Each transport now receives its own response + + // Transport A should receive response for request ID 1 + expect(transportA.sentMessages.length).toBe(1); + expect(transportA.sentMessages[0]).toMatchObject({ + jsonrpc: "2.0", + id: 1, + result: { data: "responseForA" } + }); + + // Transport B should only receive its own response (when implemented) + expect(transportB.sentMessages.length).toBe(1); + expect(transportB.sentMessages[0]).toMatchObject({ + jsonrpc: "2.0", + id: 2, + result: { data: "responseForA" } // Same handler result in this test + }); + }); + + test("demonstrates the timing issue with multiple rapid connections", async () => { + const delays: number[] = []; + const results: { transport: string; response: JSONRPCMessage[] }[] = []; + + const DelayedRequestSchema = z.object({ + method: z.literal("test/delayed"), + params: z.object({ + delay: z.number(), + client: z.string() + }).optional() + }); + + // Set up handler with variable delay + protocol.setRequestHandler( + DelayedRequestSchema, + async (request, extra) => { + const delay = request.params?.delay || 0; + delays.push(delay); + + await new Promise(resolve => setTimeout(resolve, delay)); + + return { + processedBy: `handler-${extra.requestId}`, + delay: delay + } as Result; + } + ); + + // Rapid succession of connections and requests + await protocol.connect(transportA); + transportA.onmessage?.({ + jsonrpc: "2.0" as const, + method: "test/delayed", + params: { delay: 50, client: "A" }, + id: 1 + }); + + // Connect B while A is processing + setTimeout(async () => { + await protocol.connect(transportB); + transportB.onmessage?.({ + jsonrpc: "2.0" as const, + method: "test/delayed", + params: { delay: 10, client: "B" }, + id: 2 + }); + }, 10); + + // Wait for all processing + await new Promise(resolve => setTimeout(resolve, 100)); + + // Collect results + if (transportA.sentMessages.length > 0) { + results.push({ transport: "A", response: transportA.sentMessages }); + } + if (transportB.sentMessages.length > 0) { + results.push({ transport: "B", response: transportB.sentMessages }); + } + + console.log("Timing test results:", results); + + // FIXED: Each transport receives its own responses + expect(transportA.sentMessages.length).toBe(1); + expect(transportB.sentMessages.length).toBe(1); + }); +}); \ No newline at end of file diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index b19a6c5ca..7df190ba1 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -370,8 +370,11 @@ export abstract class Protocol< const handler = this._requestHandlers.get(request.method) ?? this.fallbackRequestHandler; + // Capture the current transport at request time to ensure responses go to the correct client + const capturedTransport = this._transport; + if (handler === undefined) { - this._transport + capturedTransport ?.send({ jsonrpc: "2.0", id: request.id, @@ -393,7 +396,7 @@ export abstract class Protocol< const fullExtra: RequestHandlerExtra = { signal: abortController.signal, - sessionId: this._transport?.sessionId, + sessionId: capturedTransport?.sessionId, _meta: request.params?._meta, sendNotification: (notification) => @@ -414,7 +417,7 @@ export abstract class Protocol< return; } - return this._transport?.send({ + return capturedTransport?.send({ result, jsonrpc: "2.0", id: request.id, @@ -425,7 +428,7 @@ export abstract class Protocol< return; } - return this._transport?.send({ + return capturedTransport?.send({ jsonrpc: "2.0", id: request.id, error: { From 0551cc52b8920d7da46a4519b42f335a0a852b6c Mon Sep 17 00:00:00 2001 From: Inna Harper Date: Thu, 31 Jul 2025 19:33:29 +0100 Subject: [PATCH 3/3] 1.17.1 (#831) --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 303dfbfd4..dd45ff05d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.17.0", + "version": "1.17.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@modelcontextprotocol/sdk", - "version": "1.17.0", + "version": "1.17.1", "license": "MIT", "dependencies": { "ajv": "^6.12.6", diff --git a/package.json b/package.json index c861b5358..7bbb0f173 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.17.0", + "version": "1.17.1", "description": "Model Context Protocol implementation for TypeScript", "license": "MIT", "author": "Anthropic, PBC (https://anthropic.com)",