From 3c0081f2ecd8bb9b117330783ad25c2ec127ef60 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Sat, 15 Mar 2025 13:22:15 +0000 Subject: [PATCH 1/6] add improve transport in SSE server example --- README.md | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index fe4caa3f..a959ae3d 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ await server.connect(transport); For remote servers, start a web server with a Server-Sent Events (SSE) endpoint, and a separate endpoint for the client to send its messages to: ```typescript -import express from "express"; +import express, { Request, Response } from "express"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { SSEServerTransport } from "@modelcontextprotocol/sdk/server/sse.js"; @@ -224,16 +224,27 @@ const server = new McpServer({ const app = express(); -app.get("/sse", async (req, res) => { - const transport = new SSEServerTransport("/messages", res); +// to support multiple simultaneous connections we have a lookup object from +// sessionId to transport +const transports: {[sessionId: string]: SSEServerTransport} = {}; + +app.get("/sse", async (_: Request, res: Response) => { + const transport = new SSEServerTransport('/messages', res); + transports[transport.sessionId] = transport; + res.on("close", () => { + delete transports[transport.sessionId]; + }); await server.connect(transport); }); -app.post("/messages", async (req, res) => { - // Note: to support multiple simultaneous connections, these messages will - // need to be routed to a specific matching transport. (This logic isn't - // implemented here, for simplicity.) - await transport.handlePostMessage(req, res); +app.post("/messages", async (req: Request, res: Response) => { + const sessionId = req.query.sessionId as string; + const transport = transports[sessionId]; + if (transport) { + await transport.handlePostMessage(req, res); + } else { + res.status(400).send('No transport found for sessionId'); + } }); app.listen(3001); From d4bf73b06e404d75c974d5d526cb0acf4e9a56a4 Mon Sep 17 00:00:00 2001 From: zhibisora Date: Sun, 16 Mar 2025 05:00:36 +0800 Subject: [PATCH 2/6] Replace native spawn with cross-spawn for improved cross-platform compatibility --- package.json | 1 + src/client/cross-spawn.test.ts | 112 +++++++++++++++++++++++++++++++++ src/client/stdio.ts | 3 +- 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 src/client/cross-spawn.test.ts diff --git a/package.json b/package.json index b7641c07..51c4e0cd 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "dependencies": { "content-type": "^1.0.5", "cors": "^2.8.5", + "cross-spawn": "^7.0.3", "eventsource": "^3.0.2", "express": "^5.0.1", "express-rate-limit": "^7.5.0", diff --git a/src/client/cross-spawn.test.ts b/src/client/cross-spawn.test.ts new file mode 100644 index 00000000..d898aeb6 --- /dev/null +++ b/src/client/cross-spawn.test.ts @@ -0,0 +1,112 @@ +import { StdioClientTransport } from "./stdio.js"; +import spawn from "cross-spawn"; + +// mock cross-spawn +jest.mock("cross-spawn"); +const mockSpawn = spawn as jest.MockedFunction; + +describe("StdioClientTransport using cross-spawn", () => { + beforeEach(() => { + // mock cross-spawn's return value + mockSpawn.mockImplementation(() => { + const mockProcess = { + on: jest.fn((event: string, callback: Function) => { + if (event === "spawn") { + callback(); + } + return mockProcess; + }), + stdin: { + on: jest.fn(), + write: jest.fn().mockReturnValue(true) + }, + stdout: { + on: jest.fn() + }, + stderr: null + }; + return mockProcess as any; + }); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test("should call cross-spawn correctly", async () => { + const transport = new StdioClientTransport({ + command: "test-command", + args: ["arg1", "arg2"] + }); + + await transport.start(); + + // verify spawn is called correctly + expect(mockSpawn).toHaveBeenCalledWith( + "test-command", + ["arg1", "arg2"], + expect.objectContaining({ + shell: false + }) + ); + }); + + test("should pass environment variables correctly", async () => { + const customEnv = { TEST_VAR: "test-value" }; + const transport = new StdioClientTransport({ + command: "test-command", + env: customEnv + }); + + await transport.start(); + + // verify environment variables are passed correctly + expect(mockSpawn).toHaveBeenCalledWith( + "test-command", + [], + expect.objectContaining({ + env: customEnv + }) + ); + }); + + test("should send messages correctly", async () => { + const transport = new StdioClientTransport({ + command: "test-command" + }); + + // get the mock process object + const mockProcess = { + on: jest.fn((event, callback) => { + if (event === "spawn") { + callback(); + } + return mockProcess; + }), + stdin: { + on: jest.fn(), + write: jest.fn().mockReturnValue(true), + once: jest.fn() + }, + stdout: { + on: jest.fn() + }, + stderr: null + }; + + mockSpawn.mockReturnValue(mockProcess as any); + + await transport.start(); + + const message = { + jsonrpc: "2.0", + id: "test-id", + method: "test-method" + }; + + await transport.send(message); + + // verify message is sent correctly + expect(mockProcess.stdin.write).toHaveBeenCalled(); + }); +}); \ No newline at end of file diff --git a/src/client/stdio.ts b/src/client/stdio.ts index 07e9a196..b83bf27c 100644 --- a/src/client/stdio.ts +++ b/src/client/stdio.ts @@ -1,4 +1,5 @@ -import { ChildProcess, IOType, spawn } from "node:child_process"; +import { ChildProcess, IOType } from "node:child_process"; +import spawn from "cross-spawn"; import process from "node:process"; import { Stream } from "node:stream"; import { ReadBuffer, serializeMessage } from "../shared/stdio.js"; From 46ae2a17af4d331d3076071ed0033bfac0fbb9ef Mon Sep 17 00:00:00 2001 From: zhibisora Date: Sun, 16 Mar 2025 05:15:50 +0800 Subject: [PATCH 3/6] Fix test for cross-spawn --- package-lock.json | 24 +++++++++++++++--------- package.json | 1 + src/client/cross-spawn.test.ts | 28 +++++++++++++++++++++++----- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index cfd8a622..73f1cbba 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,16 +1,17 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.5.0", + "version": "1.7.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@modelcontextprotocol/sdk", - "version": "1.5.0", + "version": "1.7.0", "license": "MIT", "dependencies": { "content-type": "^1.0.5", "cors": "^2.8.5", + "cross-spawn": "^7.0.3", "eventsource": "^3.0.2", "express": "^5.0.1", "express-rate-limit": "^7.5.0", @@ -24,6 +25,7 @@ "@jest-mock/express": "^3.0.0", "@types/content-type": "^1.1.8", "@types/cors": "^2.8.17", + "@types/cross-spawn": "^6.0.6", "@types/eslint__js": "^8.42.3", "@types/eventsource": "^1.1.15", "@types/express": "^5.0.0", @@ -1693,6 +1695,16 @@ "@types/node": "*" } }, + "node_modules/@types/cross-spawn": { + "version": "6.0.6", + "resolved": "https://registry.npmjs.org/@types/cross-spawn/-/cross-spawn-6.0.6.tgz", + "integrity": "sha512-fXRhhUkG4H3TQk5dBhQ7m/JDdSNHKwR2BBia62lhwEIq9xGiQKLxd6LymNhn47SjXhsUEPmxi+PKw2OkW4LLjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/eslint": { "version": "9.6.1", "resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-9.6.1.tgz", @@ -2809,7 +2821,6 @@ "version": "7.0.5", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.5.tgz", "integrity": "sha512-ZVJrKKYunU38/76t0RMOulHOnUcbU9GbpWKAOZ0mhjr7CX6FVrH+4FrAapSOekrgFQ3f/8gwMEuIft0aKq6Hug==", - "dev": true, "dependencies": { "path-key": "^3.1.0", "shebang-command": "^2.0.0", @@ -4156,8 +4167,7 @@ "node_modules/isexe": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz", - "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==", - "dev": true + "integrity": "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw==" }, "node_modules/istanbul-lib-coverage": { "version": "3.2.2", @@ -5359,7 +5369,6 @@ "version": "3.1.1", "resolved": "https://registry.npmjs.org/path-key/-/path-key-3.1.1.tgz", "integrity": "sha512-ojmeN0qd+y0jszEtoY48r0Peq5dwMEkIlCOu6Q5f41lfkswXuKtYrhgoTpLnyIcHm24Uhqx+5Tqm2InSwLhE6Q==", - "dev": true, "engines": { "node": ">=8" } @@ -5871,7 +5880,6 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", "integrity": "sha512-kHxr2zZpYtdmrN1qDjrrX/Z1rR1kG8Dx+gkpK1G4eXmvXswmcE1hTWBWYUzlraYw1/yZp6YuDY77YtvbN0dmDA==", - "dev": true, "dependencies": { "shebang-regex": "^3.0.0" }, @@ -5883,7 +5891,6 @@ "version": "3.0.0", "resolved": "https://registry.npmjs.org/shebang-regex/-/shebang-regex-3.0.0.tgz", "integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==", - "dev": true, "engines": { "node": ">=8" } @@ -6480,7 +6487,6 @@ "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", "integrity": "sha512-BLI3Tl1TW3Pvl70l3yq3Y64i+awpwXqsGBYWkkqMtnbXgrMD+yj7rhW0kuEDxzJaYXGjEW5ogapKNMEKNMjibA==", - "dev": true, "dependencies": { "isexe": "^2.0.0" }, diff --git a/package.json b/package.json index 51c4e0cd..7302af05 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "@jest-mock/express": "^3.0.0", "@types/content-type": "^1.1.8", "@types/cors": "^2.8.17", + "@types/cross-spawn": "^6.0.6", "@types/eslint__js": "^8.42.3", "@types/eventsource": "^1.1.15", "@types/express": "^5.0.0", diff --git a/src/client/cross-spawn.test.ts b/src/client/cross-spawn.test.ts index d898aeb6..ae0a4fd6 100644 --- a/src/client/cross-spawn.test.ts +++ b/src/client/cross-spawn.test.ts @@ -1,5 +1,6 @@ import { StdioClientTransport } from "./stdio.js"; import spawn from "cross-spawn"; +import { JSONRPCMessage } from "../types.js"; // mock cross-spawn jest.mock("cross-spawn"); @@ -9,8 +10,13 @@ describe("StdioClientTransport using cross-spawn", () => { beforeEach(() => { // mock cross-spawn's return value mockSpawn.mockImplementation(() => { - const mockProcess = { - on: jest.fn((event: string, callback: Function) => { + const mockProcess: { + on: jest.Mock; + stdin?: { on: jest.Mock; write: jest.Mock }; + stdout?: { on: jest.Mock }; + stderr?: null; + } = { + on: jest.fn((event: string, callback: () => void) => { if (event === "spawn") { callback(); } @@ -76,8 +82,19 @@ describe("StdioClientTransport using cross-spawn", () => { }); // get the mock process object - const mockProcess = { - on: jest.fn((event, callback) => { + const mockProcess: { + on: jest.Mock; + stdin: { + on: jest.Mock; + write: jest.Mock; + once: jest.Mock; + }; + stdout: { + on: jest.Mock; + }; + stderr: null; + } = { + on: jest.fn((event: string, callback: () => void) => { if (event === "spawn") { callback(); } @@ -98,7 +115,8 @@ describe("StdioClientTransport using cross-spawn", () => { await transport.start(); - const message = { + // 关键修复：确保 jsonrpc 是字面量 "2.0" + const message: JSONRPCMessage = { jsonrpc: "2.0", id: "test-id", method: "test-method" From 2a2392be2608a50a8773ca0ffea202fadfa6669e Mon Sep 17 00:00:00 2001 From: Peter Raboud Date: Tue, 25 Mar 2025 13:26:29 -0700 Subject: [PATCH 4/6] Correctly pass redirect_uri to tokens call --- src/client/auth.test.ts | 18 +++++++++++------- src/client/auth.ts | 4 ++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 02026c4f..cc4717ec 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -46,11 +46,11 @@ describe("OAuth Authorization", () => { it("returns metadata when first fetch fails but second without MCP header succeeds", async () => { // Set up a counter to control behavior let callCount = 0; - + // Mock implementation that changes behavior based on call count mockFetch.mockImplementation((_url, _options) => { callCount++; - + if (callCount === 1) { // First call with MCP header - fail with TypeError (simulating CORS error) // We need to use TypeError specifically because that's what the implementation checks for @@ -68,10 +68,10 @@ describe("OAuth Authorization", () => { // Should succeed with the second call const metadata = await discoverOAuthMetadata("https://auth.example.com"); expect(metadata).toEqual(validMetadata); - + // Verify both calls were made expect(mockFetch).toHaveBeenCalledTimes(2); - + // Verify first call had MCP header expect(mockFetch.mock.calls[0][1]?.headers).toHaveProperty("MCP-Protocol-Version"); }); @@ -79,11 +79,11 @@ describe("OAuth Authorization", () => { it("throws an error when all fetch attempts fail", async () => { // Set up a counter to control behavior let callCount = 0; - + // Mock implementation that changes behavior based on call count mockFetch.mockImplementation((_url, _options) => { callCount++; - + if (callCount === 1) { // First call - fail with TypeError return Promise.reject(new TypeError("First failure")); @@ -96,7 +96,7 @@ describe("OAuth Authorization", () => { // Should fail with the second error await expect(discoverOAuthMetadata("https://auth.example.com")) .rejects.toThrow("Second failure"); - + // Verify both calls were made expect(mockFetch).toHaveBeenCalledTimes(2); }); @@ -250,6 +250,7 @@ describe("OAuth Authorization", () => { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", }); expect(tokens).toEqual(validTokens); @@ -271,6 +272,7 @@ describe("OAuth Authorization", () => { expect(body.get("code_verifier")).toBe("verifier123"); expect(body.get("client_id")).toBe("client123"); expect(body.get("client_secret")).toBe("secret123"); + expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); }); it("validates token response schema", async () => { @@ -288,6 +290,7 @@ describe("OAuth Authorization", () => { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", }) ).rejects.toThrow(); }); @@ -303,6 +306,7 @@ describe("OAuth Authorization", () => { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", }) ).rejects.toThrow("Token exchange failed"); }); diff --git a/src/client/auth.ts b/src/client/auth.ts index c7799429..a279121b 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -115,6 +115,7 @@ export async function auth( clientInformation, authorizationCode, codeVerifier, + redirectUri: provider.redirectUrl, }); await provider.saveTokens(tokens); @@ -259,11 +260,13 @@ export async function exchangeAuthorization( clientInformation, authorizationCode, codeVerifier, + redirectUri, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; authorizationCode: string; codeVerifier: string; + redirectUri: string | URL; }, ): Promise { const grantType = "authorization_code"; @@ -290,6 +293,7 @@ export async function exchangeAuthorization( client_id: clientInformation.client_id, code: authorizationCode, code_verifier: codeVerifier, + redirect_uri: String(redirectUri), }); if (clientInformation.client_secret) { From c6635d71d6685a92617ae1b7d7520e56868b2e72 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Wed, 26 Mar 2025 10:12:19 +0000 Subject: [PATCH 5/6] Fix lints --- src/client/cross-spawn.test.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/client/cross-spawn.test.ts b/src/client/cross-spawn.test.ts index ae0a4fd6..11e81bf6 100644 --- a/src/client/cross-spawn.test.ts +++ b/src/client/cross-spawn.test.ts @@ -1,6 +1,7 @@ import { StdioClientTransport } from "./stdio.js"; import spawn from "cross-spawn"; import { JSONRPCMessage } from "../types.js"; +import { ChildProcess } from "node:child_process"; // mock cross-spawn jest.mock("cross-spawn"); @@ -31,7 +32,7 @@ describe("StdioClientTransport using cross-spawn", () => { }, stderr: null }; - return mockProcess as any; + return mockProcess as unknown as ChildProcess; }); }); @@ -44,9 +45,9 @@ describe("StdioClientTransport using cross-spawn", () => { command: "test-command", args: ["arg1", "arg2"] }); - + await transport.start(); - + // verify spawn is called correctly expect(mockSpawn).toHaveBeenCalledWith( "test-command", @@ -63,9 +64,9 @@ describe("StdioClientTransport using cross-spawn", () => { command: "test-command", env: customEnv }); - + await transport.start(); - + // verify environment variables are passed correctly expect(mockSpawn).toHaveBeenCalledWith( "test-command", @@ -80,7 +81,7 @@ describe("StdioClientTransport using cross-spawn", () => { const transport = new StdioClientTransport({ command: "test-command" }); - + // get the mock process object const mockProcess: { on: jest.Mock; @@ -110,20 +111,20 @@ describe("StdioClientTransport using cross-spawn", () => { }, stderr: null }; - - mockSpawn.mockReturnValue(mockProcess as any); - + + mockSpawn.mockReturnValue(mockProcess as unknown as ChildProcess); + await transport.start(); - + // 关键修复：确保 jsonrpc 是字面量 "2.0" const message: JSONRPCMessage = { jsonrpc: "2.0", id: "test-id", method: "test-method" }; - + await transport.send(message); - + // verify message is sent correctly expect(mockProcess.stdin.write).toHaveBeenCalled(); }); From 784012946dbc28f69708061863da6733f91c6696 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Wed, 26 Mar 2025 10:15:46 +0000 Subject: [PATCH 6/6] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7302af05..e2d8b3d7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.7.0", + "version": "1.8.0", "description": "Model Context Protocol implementation for TypeScript", "license": "MIT", "author": "Anthropic, PBC (https://anthropic.com)",