diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..9af5b7a8 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,32 @@ +name: CI + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 60 + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - run: npm ci + - name: Build interactive visualizer example + run: | + npm install --prefix examples/interactive-visualizer --no-package-lock --no-audit --no-fund + npm run --prefix examples/interactive-visualizer build + - run: npx tsc --noEmit + - run: npm test + - run: npm run test:oauth + - run: npm run test:conformance + - run: npm pack --dry-run diff --git a/.gitignore b/.gitignore index 9dc08cd9..1d740cad 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ node_modules/ .pi/ +conformance/results/ *.log .DS_Store diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b2fc4eb..d34dbceb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- Migrated the MCP client and interactive visualizer to the exact-pinned MCP SDK v2 beta.5 packages, with automatic protocol negotiation and client conformance coverage. Thanks Matt Carey (@mattzcarey) for PR #210. - Added disabled MCP server definitions plus `/mcp disable` and `/mcp enable` project-local overrides that preserve visibility while preventing execution. Thanks Ömer Ulusoy (@ulusoyomer) for PR #61. - Added argument completions for `/mcp` subcommands and reconnect/logout server names. Thanks @sting8k for PR #8. - Surfaced MCP connection failure reasons from bounded stdio diagnostics in status output and the `/mcp` panel, with a shortcut to copy the selected failure. Thanks @parkuman for PR #197. diff --git a/OAUTH.md b/OAUTH.md index b742c674..53e64b0e 100644 --- a/OAUTH.md +++ b/OAUTH.md @@ -323,18 +323,15 @@ The OAuth implementation uses the following modules: ## SDK Integration -The implementation uses these SDK exports: +The implementation uses these MCP SDK v2 exports: ```typescript import { auth, - UnauthorizedError, - OAuthClientProvider, -} from "@modelcontextprotocol/sdk/client/auth.js" - -import { StreamableHTTPClientTransport, -} from "@modelcontextprotocol/sdk/client/streamableHttp.js" + UnauthorizedError, + type OAuthClientProvider, +} from "@modelcontextprotocol/client" ``` The `McpOAuthProvider` class implements `OAuthClientProvider` and is passed to `StreamableHTTPClientTransport`: diff --git a/__tests__/abort-signal.test.ts b/__tests__/abort-signal.test.ts index 9dcb560a..06cf4991 100644 --- a/__tests__/abort-signal.test.ts +++ b/__tests__/abort-signal.test.ts @@ -71,7 +71,6 @@ describe("AbortSignal propagation", () => { expect(result.details.error).toBe("aborted"); expect(callTool).toHaveBeenCalledWith( { name: "slow", arguments: {}, _meta: undefined }, - undefined, { signal: controller.signal }, ); expect(state.manager.decrementInFlight).toHaveBeenCalledWith("demo"); @@ -91,7 +90,6 @@ describe("AbortSignal propagation", () => { expect(result.details.error).toBe("aborted"); expect(callTool).toHaveBeenCalledWith( { name: "slow", arguments: {}, _meta: undefined }, - undefined, { signal: controller.signal }, ); expect(state.manager.decrementInFlight).toHaveBeenCalledWith("demo"); diff --git a/__tests__/direct-tools-auto-auth.test.ts b/__tests__/direct-tools-auto-auth.test.ts index 30b387ad..ebd6b7de 100644 --- a/__tests__/direct-tools-auto-auth.test.ts +++ b/__tests__/direct-tools-auto-auth.test.ts @@ -110,7 +110,6 @@ describe("direct tools auto auth", () => { arguments: { q: "hello" }, _meta: undefined, }, - undefined, { timeout: 4321 }, ); expect(result.content[0].text).toContain("ok"); @@ -157,7 +156,6 @@ describe("direct tools auto auth", () => { expect(state.manager.getRequestOptions).toHaveBeenCalledWith("demo", controller.signal); expect(connection.client.callTool).toHaveBeenCalledWith( { name: "search", arguments: {}, _meta: undefined }, - undefined, requestOptions, ); expect(result.details).toMatchObject({ error: "aborted", server: "demo" }); @@ -246,7 +244,7 @@ describe("direct tools auto auth", () => { }); it("runs URL elicitations returned by a URL-required tool error", async () => { - const { UrlElicitationRequiredError } = await import("@modelcontextprotocol/sdk/types.js"); + const { UrlElicitationRequiredError } = await import("@modelcontextprotocol/client"); const { createDirectToolExecutor } = await import("../direct-tools.ts"); const error = new UrlElicitationRequiredError([{ mode: "url", diff --git a/__tests__/elicitation-handler.test.ts b/__tests__/elicitation-handler.test.ts index aaa60edc..eb8a5fd0 100644 --- a/__tests__/elicitation-handler.test.ts +++ b/__tests__/elicitation-handler.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { ElicitRequest } from "@modelcontextprotocol/sdk/types.js"; +import type { ElicitRequest } from "@modelcontextprotocol/client"; const mocks = vi.hoisted(() => ({ open: vi.fn(async () => undefined), diff --git a/__tests__/elicitation-sdk-integration.test.ts b/__tests__/elicitation-sdk-integration.test.ts index 48c85bee..d368a957 100644 --- a/__tests__/elicitation-sdk-integration.test.ts +++ b/__tests__/elicitation-sdk-integration.test.ts @@ -82,7 +82,7 @@ describe("elicitation with the real MCP SDK", () => { const connection = manager.getConnection("real")!; await expect(connection.client.callTool({ name: "url", arguments: {} })).rejects.toThrow( - /does not support URL-mode elicitation requests/, + /does not support (?:URL-mode|url) elicitation/, ); expect(mocks.open).not.toHaveBeenCalled(); }); diff --git a/__tests__/fixtures/delayed-mcp-server.mjs b/__tests__/fixtures/delayed-mcp-server.mjs index f7136d5a..a28af6a9 100644 --- a/__tests__/fixtures/delayed-mcp-server.mjs +++ b/__tests__/fixtures/delayed-mcp-server.mjs @@ -1,8 +1,7 @@ import { rename, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { Server } from "@modelcontextprotocol/sdk/server/index.js"; -import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; -import { ListResourcesRequestSchema, ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js"; +import { Server } from "@modelcontextprotocol/server"; +import { StdioServerTransport } from "@modelcontextprotocol/server/stdio"; const pidPath = process.env.MCP_RELOAD_PID_DIR ? join(process.env.MCP_RELOAD_PID_DIR, `${process.pid}.json`) : undefined; const identity = { pid: process.pid, toolName: "reload_identity" }; @@ -19,11 +18,11 @@ const server = new Server( { name: "delayed-reload-fixture", version: "1.0.0" }, { capabilities: { tools: {}, resources: {} } }, ); -server.setRequestHandler(ListToolsRequestSchema, async () => { +server.setRequestHandler("tools/list", async () => { await new Promise(resolve => setTimeout(resolve, 100)); return { tools: [{ name: identity.toolName, description: "reload identity", inputSchema: { type: "object", properties: {} } }], }; }); -server.setRequestHandler(ListResourcesRequestSchema, async () => ({ resources: [] })); +server.setRequestHandler("resources/list", async () => ({ resources: [] })); await server.connect(new StdioServerTransport()); diff --git a/__tests__/fixtures/elicitation-server.mjs b/__tests__/fixtures/elicitation-server.mjs index 9f28e2cb..d786cff7 100644 --- a/__tests__/fixtures/elicitation-server.mjs +++ b/__tests__/fixtures/elicitation-server.mjs @@ -1,13 +1,5 @@ -import { Server } from "@modelcontextprotocol/sdk/server/index.js"; -import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; -import { - CallToolRequestSchema, - ElicitResultSchema, - ListResourcesRequestSchema, - ListToolsRequestSchema, - ReadResourceRequestSchema, - UrlElicitationRequiredError, -} from "@modelcontextprotocol/sdk/types.js"; +import { Server, UrlElicitationRequiredError } from "@modelcontextprotocol/server"; +import { StdioServerTransport } from "@modelcontextprotocol/server/stdio"; const server = new Server( { name: "elicitation-integration-server", version: "1.0.0" }, @@ -23,7 +15,7 @@ function urlRequiredError() { }]); } -server.setRequestHandler(ListToolsRequestSchema, async () => ({ +server.setRequestHandler("tools/list", async () => ({ tools: [ { name: "capabilities", inputSchema: { type: "object", properties: {} } }, { name: "form", inputSchema: { type: "object", properties: {} } }, @@ -32,21 +24,21 @@ server.setRequestHandler(ListToolsRequestSchema, async () => ({ ], })); -server.setRequestHandler(ListResourcesRequestSchema, async () => ({ +server.setRequestHandler("resources/list", async () => ({ resources: [ { name: "URL-required resource", uri: "test://url-required" }, { name: "URL-required UI resource", uri: "ui://url-required" }, ], })); -server.setRequestHandler(ReadResourceRequestSchema, async request => { +server.setRequestHandler("resources/read", async request => { if (request.params.uri === "test://url-required" || request.params.uri === "ui://url-required") { throw urlRequiredError(); } return { contents: [] }; }); -server.setRequestHandler(CallToolRequestSchema, async request => { +server.setRequestHandler("tools/call", async request => { if (request.params.name === "capabilities") { return { content: [{ type: "text", text: JSON.stringify(server.getClientCapabilities()?.elicitation ?? null) }], @@ -56,31 +48,25 @@ server.setRequestHandler(CallToolRequestSchema, async request => { if (request.params.name === "url-required") throw urlRequiredError(); if (request.params.name === "form") { - const result = await server.request({ - method: "elicitation/create", - params: { - mode: "form", - message: "Provide a name", - requestedSchema: { - type: "object", - properties: { name: { type: "string", minLength: 1 } }, - required: ["name"], - }, + const result = await server.elicitInput({ + mode: "form", + message: "Provide a name", + requestedSchema: { + type: "object", + properties: { name: { type: "string", minLength: 1 } }, + required: ["name"], }, - }, ElicitResultSchema); + }); return { content: [{ type: "text", text: JSON.stringify(result) }] }; } if (request.params.name === "url") { - const result = await server.request({ - method: "elicitation/create", - params: { - mode: "url", - message: "Connect your account", - elicitationId: "requested-1", - url: "https://example.com/authorize", - }, - }, ElicitResultSchema); + const result = await server.elicitInput({ + mode: "url", + message: "Connect your account", + elicitationId: "requested-1", + url: "https://example.com/authorize", + }); if (result.action === "accept") { for (const elicitationId of ["unknown", "requested-1", "requested-1"]) { await server.notification({ diff --git a/__tests__/mcp-auth-flow-client-credentials.test.ts b/__tests__/mcp-auth-flow-client-credentials.test.ts index be7f3418..79256c98 100644 --- a/__tests__/mcp-auth-flow-client-credentials.test.ts +++ b/__tests__/mcp-auth-flow-client-credentials.test.ts @@ -17,7 +17,8 @@ const mocks = vi.hoisted(() => ({ class MockUnauthorizedError extends Error {} -vi.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), auth: mocks.sdkAuth, extractWWWAuthenticateParams: (response: Response) => { const header = response.headers.get("www-authenticate") ?? ""; @@ -327,7 +328,7 @@ describe("mcp-auth-flow explicit auth", () => { it("preserves stored dynamic client info when tokens exist", async () => { mocks.sdkAuth.mockImplementationOnce(async (provider) => { - expect(await provider.clientInformation()).toEqual({ client_id: "stored-client", client_secret: "stored-secret" }); + expect(await provider.clientInformation()).toEqual({ client_id: "stored-client", client_secret: "stored-secret", redirect_uris: ["http://localhost:19876/callback"] }); await provider.redirectToAuthorization(new URL("https://auth.example.com/authorize")); return "REDIRECT"; }); @@ -474,7 +475,7 @@ describe("mcp-auth-flow explicit auth", () => { it("refreshes expired tokens even when cached dynamic redirect URIs are stale", async () => { mocks.sdkAuth.mockImplementationOnce(async (provider) => { - expect(await provider.clientInformation()).toEqual({ client_id: "refresh-client", client_secret: "refresh-secret" }); + expect(await provider.clientInformation()).toEqual({ client_id: "refresh-client", client_secret: "refresh-secret", redirect_uris: ["http://localhost:19876/callback"] }); await provider.saveTokens({ access_token: "new-access", token_type: "Bearer", diff --git a/__tests__/mcp-oauth-provider.test.ts b/__tests__/mcp-oauth-provider.test.ts index 199a790f..891b22c1 100644 --- a/__tests__/mcp-oauth-provider.test.ts +++ b/__tests__/mcp-oauth-provider.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { mkdtempSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"; +import { UnauthorizedError } from "@modelcontextprotocol/client"; import { McpOAuthProvider } from "../mcp-oauth-provider.ts"; import { saveAuthEntry } from "../mcp-auth.ts"; @@ -158,6 +158,87 @@ describe("McpOAuthProvider addClientAuthentication", () => { expect([...params.entries()]).toEqual([["grant_type", "authorization_code"]]); expect([...headers.entries()]).toEqual([]); }); + + it("does not persist a pre-registered issuer stub after deactivation", async () => { + const provider = new McpOAuthProvider( + "inactive-client-info", + serverUrl, + { clientId: "my-client", clientSecret: "my-secret" }, + { onRedirect: async () => {} }, + ); + provider.deactivate(); + + await expect(provider.saveClientInformation({ + client_id: "my-client", + issuer: "https://auth.example.com", + })).rejects.toThrow("OAuth flow is no longer active"); + + const { getAuthForUrl } = await import("../mcp-auth.ts"); + expect(getAuthForUrl("inactive-client-info", serverUrl)).toBeUndefined(); + }); +}); + +describe("McpOAuthProvider discovery state", () => { + const originalOAuthDir = process.env.MCP_OAUTH_DIR; + const serverUrl = "https://api.example.com/mcp"; + let authDir: string; + + beforeEach(() => { + authDir = mkdtempSync(join(tmpdir(), "pi-mcp-oauth-discovery-")); + process.env.MCP_OAUTH_DIR = authDir; + }); + + afterEach(() => { + rmSync(authDir, { recursive: true, force: true }); + if (originalOAuthDir === undefined) { + delete process.env.MCP_OAUTH_DIR; + } else { + process.env.MCP_OAUTH_DIR = originalOAuthDir; + } + }); + + it("round-trips callback-leg discovery state and invalidates it independently", async () => { + const provider = new McpOAuthProvider( + "discovery-state", + serverUrl, + {}, + { onRedirect: async () => {} }, + ); + const discoveryState = { + authorizationServerUrl: "https://auth.example.com", + resourceMetadataUrl: "https://api.example.com/.well-known/oauth-protected-resource/mcp", + authorizationServerMetadata: { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"], + }, + }; + + await provider.saveDiscoveryState(discoveryState); + expect(await provider.discoveryState()).toEqual(discoveryState); + + const otherRuntimeProvider = new McpOAuthProvider( + "discovery-state", + serverUrl, + {}, + { onRedirect: async () => {} }, + ); + expect(await otherRuntimeProvider.discoveryState()).toBeUndefined(); + + await provider.saveTokens({ + access_token: "access-token", + token_type: "Bearer", + issuer: "https://auth.example.com", + }); + expect(await provider.discoveryState()).toBeUndefined(); + + await provider.saveDiscoveryState(discoveryState); + await provider.invalidateCredentials("discovery"); + + expect(await provider.discoveryState()).toBeUndefined(); + expect((await provider.tokens())?.access_token).toBe("access-token"); + }); }); describe("McpOAuthProvider authorization fallback", () => { diff --git a/__tests__/proxy-modes-auto-auth.test.ts b/__tests__/proxy-modes-auto-auth.test.ts index 2f42382c..d387ac9d 100644 --- a/__tests__/proxy-modes-auto-auth.test.ts +++ b/__tests__/proxy-modes-auto-auth.test.ts @@ -35,7 +35,8 @@ vi.mock("../init.ts", () => ({ recordFailure: mocks.recordFailure, })); -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), Client: vi.fn().mockImplementation(function (this: any, info: unknown, options: unknown) { this.info = info; this.options = options; @@ -56,9 +57,11 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ this.close = vi.fn(async () => undefined); mocks.clients.push(this); }), + StreamableHTTPClientTransport: vi.fn(), + SSEClientTransport: vi.fn(), })); -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ +vi.mock("@modelcontextprotocol/client/stdio", () => ({ StdioClientTransport: vi.fn().mockImplementation(function (this: any, options: unknown) { this.options = options; this.close = vi.fn(async () => undefined); @@ -66,21 +69,6 @@ vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ }), })); -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ - StreamableHTTPClientTransport: vi.fn(), - StreamableHTTPError: class StreamableHTTPError extends Error { - code: number; - constructor(code: number, message: string) { - super(`Streamable HTTP error: ${message}`); - this.code = code; - } - }, -})); - -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: vi.fn(), -})); - vi.mock("../npx-resolver.ts", () => ({ resolveNpxBinary: vi.fn(async () => null), })); @@ -227,7 +215,7 @@ describe("proxy auto auth", () => { }); it("runs URL elicitations returned by proxy tool calls", async () => { - const { UrlElicitationRequiredError } = await import("@modelcontextprotocol/sdk/types.js"); + const { UrlElicitationRequiredError } = await import("@modelcontextprotocol/client"); const { executeCall } = await import("../proxy-modes.ts"); const error = new UrlElicitationRequiredError([{ mode: "url", @@ -339,7 +327,6 @@ describe("proxy auto auth", () => { arguments: { q: "hello" }, _meta: undefined, }, - undefined, { timeout: 1234 }, ); expect(result.content[0].text).toContain("ok"); @@ -417,7 +404,6 @@ describe("proxy auto auth", () => { expect(manager.getRequestOptions).toHaveBeenCalledWith("demo", controller.signal); expect(connection.client.callTool).toHaveBeenCalledWith( { name: "search", arguments: {}, _meta: undefined }, - undefined, requestOptions, ); expect(result.details).toMatchObject({ error: "aborted", message: "request aborted" }); @@ -521,13 +507,11 @@ describe("proxy auto auth", () => { expect(client.callTool).toHaveBeenNthCalledWith( 1, { name: "search", arguments: { q: "one" }, _meta: undefined }, - undefined, { timeout: 5000 }, ); expect(client.callTool).toHaveBeenNthCalledWith( 2, { name: "search", arguments: { q: "two" }, _meta: undefined }, - undefined, { timeout: 5000 }, ); expect(first.content[0].text).toContain("ok"); diff --git a/__tests__/sampling-handler.test.ts b/__tests__/sampling-handler.test.ts index c820f2b2..3c4b0e43 100644 --- a/__tests__/sampling-handler.test.ts +++ b/__tests__/sampling-handler.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Api, Model } from "@earendil-works/pi-ai"; -import type { CreateMessageRequest, ModelPreferences } from "@modelcontextprotocol/sdk/types.js"; +import type { CreateMessageRequest, ModelPreferences } from "@modelcontextprotocol/client"; import type { SamplingHandlerOptions } from "../sampling-handler.ts"; const mocks = vi.hoisted(() => ({ diff --git a/__tests__/server-manager-http-auth.test.ts b/__tests__/server-manager-http-auth.test.ts index b5219970..96b7970c 100644 --- a/__tests__/server-manager-http-auth.test.ts +++ b/__tests__/server-manager-http-auth.test.ts @@ -9,6 +9,10 @@ type OAuthProviderLike = { }; }; +type ClientOptions = { + versionNegotiation?: { mode?: string }; +}; + type TransportOptions = { requestInit?: { headers?: Record; @@ -27,8 +31,9 @@ const mocks = vi.hoisted(() => ({ httpTransports: [] as HttpTransportMock[], })); -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ - Client: vi.fn().mockImplementation((info: unknown, options: unknown) => { +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), + Client: vi.fn().mockImplementation((info: unknown, options: ClientOptions) => { const client = { info, options, @@ -42,29 +47,16 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ mocks.clients.push(client); return client; }), -})); - -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ - StdioClientTransport: vi.fn(), -})); - -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ StreamableHTTPClientTransport: vi.fn().mockImplementation((url: URL, options: TransportOptions) => { const transport = { url, options, close: vi.fn(async () => undefined) }; mocks.httpTransports.push(transport); return transport; }), - StreamableHTTPError: class StreamableHTTPError extends Error { - code: number; - constructor(code: number, message: string) { - super(`Streamable HTTP error: ${message}`); - this.code = code; - } - }, + SSEClientTransport: vi.fn(), })); -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: vi.fn(), +vi.mock("@modelcontextprotocol/client/stdio", () => ({ + StdioClientTransport: vi.fn(), })); vi.mock("../npx-resolver.ts", () => ({ @@ -93,6 +85,19 @@ describe("McpServerManager HTTP bearer auth", () => { } }); + it("enables automatic v2 protocol negotiation for both HTTP probe and live clients", async () => { + const { McpServerManager } = await import("../server-manager.ts"); + const manager = new McpServerManager(); + + await manager.connect("remote", { url: "https://example.test/mcp" }); + + expect(mocks.clients).toHaveLength(2); + expect(mocks.clients.map(client => client.options)).toEqual([ + expect.objectContaining({ versionNegotiation: { mode: "auto" } }), + expect.objectContaining({ versionNegotiation: { mode: "auto" } }), + ]); + }); + it("interpolates ${VAR} URL placeholders", async () => { const { McpServerManager } = await import("../server-manager.ts"); process.env.MCP_TEST_URL = "https://example.test/mcp"; diff --git a/__tests__/server-manager-instructions.test.ts b/__tests__/server-manager-instructions.test.ts index e3df44d0..ff07b5c3 100644 --- a/__tests__/server-manager-instructions.test.ts +++ b/__tests__/server-manager-instructions.test.ts @@ -5,7 +5,8 @@ const mocks = vi.hoisted(() => ({ instructions: undefined as string | undefined, })); -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), Client: vi.fn().mockImplementation(function (this: any, info: unknown, options: unknown) { this.info = info; this.options = options; @@ -18,23 +19,17 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ this.close = vi.fn(async () => undefined); mocks.clients.push(this); }), + StreamableHTTPClientTransport: vi.fn(), + SSEClientTransport: vi.fn(), })); -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ +vi.mock("@modelcontextprotocol/client/stdio", () => ({ StdioClientTransport: vi.fn().mockImplementation(function (this: any, options: unknown) { this.options = options; this.close = vi.fn(async () => undefined); }), })); -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ - StreamableHTTPClientTransport: vi.fn(), -})); - -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: vi.fn(), -})); - vi.mock("../npx-resolver.ts", () => ({ resolveNpxBinary: vi.fn(async () => null), })); @@ -63,4 +58,15 @@ describe("McpServerManager instructions", () => { expect(connection.instructions).toBeUndefined(); }); + + it("enables automatic v2 protocol negotiation for stdio clients", async () => { + const { McpServerManager } = await import("../server-manager.ts"); + const manager = new McpServerManager(); + + await manager.connect("demo", { command: "node", args: ["server.js"] }); + + expect(mocks.clients[0]?.options).toMatchObject({ + versionNegotiation: { mode: "auto" }, + }); + }); }); diff --git a/__tests__/server-manager-reconnect.test.ts b/__tests__/server-manager-reconnect.test.ts index 019a21d0..a8c49d4d 100644 --- a/__tests__/server-manager-reconnect.test.ts +++ b/__tests__/server-manager-reconnect.test.ts @@ -15,7 +15,8 @@ const mocks = vi.hoisted(() => ({ httpTransports: [] as HttpTransportMock[], })); -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), Client: vi.fn().mockImplementation((info: unknown, options: unknown) => { const client: any = { info, @@ -31,29 +32,16 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ mocks.clients.push(client); return client; }), -})); - -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ - StdioClientTransport: vi.fn(), -})); - -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ StreamableHTTPClientTransport: vi.fn().mockImplementation((url: URL, options: TransportOptions) => { const transport = { url, options, close: vi.fn(async () => undefined) }; mocks.httpTransports.push(transport); return transport; }), - StreamableHTTPError: class StreamableHTTPError extends Error { - code: number; - constructor(code: number, message: string) { - super(`Streamable HTTP error: ${message}`); - this.code = code; - } - }, + SSEClientTransport: vi.fn(), })); -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: vi.fn(), +vi.mock("@modelcontextprotocol/client/stdio", () => ({ + StdioClientTransport: vi.fn(), })); vi.mock("../npx-resolver.ts", () => ({ diff --git a/__tests__/server-manager-runtime-owner.test.ts b/__tests__/server-manager-runtime-owner.test.ts index 2a2f30aa..75a5dd6c 100644 --- a/__tests__/server-manager-runtime-owner.test.ts +++ b/__tests__/server-manager-runtime-owner.test.ts @@ -14,7 +14,8 @@ function gate() { return { promise, resolve }; } -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), Client: vi.fn().mockImplementation(function (this: any) { this.setRequestHandler = vi.fn(); this.setNotificationHandler = vi.fn(); @@ -27,15 +28,15 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ this.close = vi.fn(async () => undefined); mocks.clients.push(this); }), + StreamableHTTPClientTransport: vi.fn(), + SSEClientTransport: vi.fn(), })); -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ +vi.mock("@modelcontextprotocol/client/stdio", () => ({ StdioClientTransport: vi.fn().mockImplementation(function (this: any) { this.close = vi.fn(async () => undefined); mocks.transports.push(this); }), })); -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ StreamableHTTPClientTransport: vi.fn() })); -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ SSEClientTransport: vi.fn() })); vi.mock("../npx-resolver.ts", () => ({ resolveNpxBinary: vi.fn(async () => { await mocks.resolveGate?.promise; diff --git a/__tests__/server-manager-sampling.test.ts b/__tests__/server-manager-sampling.test.ts index fd6ca0d7..1762d561 100644 --- a/__tests__/server-manager-sampling.test.ts +++ b/__tests__/server-manager-sampling.test.ts @@ -10,7 +10,8 @@ const mocks = vi.hoisted(() => ({ vi.mock("open", () => ({ default: mocks.open })); -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), Client: vi.fn().mockImplementation(function (this: any, info: unknown, options: unknown) { this.info = info; this.options = options; @@ -22,9 +23,11 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ this.close = vi.fn(async () => undefined); mocks.clients.push(this); }), + StreamableHTTPClientTransport: vi.fn(), + SSEClientTransport: vi.fn(), })); -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ +vi.mock("@modelcontextprotocol/client/stdio", () => ({ StdioClientTransport: vi.fn().mockImplementation(function (this: any, options: unknown) { this.options = options; this.close = vi.fn(async () => undefined); @@ -32,21 +35,6 @@ vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ }), })); -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ - StreamableHTTPClientTransport: vi.fn(), - StreamableHTTPError: class StreamableHTTPError extends Error { - code: number; - constructor(code: number, message: string) { - super(`Streamable HTTP error: ${message}`); - this.code = code; - } - }, -})); - -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: vi.fn(), -})); - vi.mock("../npx-resolver.ts", () => ({ resolveNpxBinary: vi.fn(async () => null), })); @@ -81,7 +69,11 @@ describe("McpServerManager sampling", () => { await manager.connect("demo", { command: "node", args: ["server.js"] }); const client = mocks.clients[0]; - expect(client.options).toEqual({ capabilities: { sampling: {} } }); + expect(client.options).toEqual({ + capabilities: { sampling: {} }, + versionNegotiation: { mode: "auto" }, + inputRequired: { autoFulfill: true }, + }); expect(client.setRequestHandler).toHaveBeenCalledTimes(1); expect(client.setRequestHandler.mock.invocationCallOrder[0]).toBeLessThan( client.connect.mock.invocationCallOrder[0], @@ -106,6 +98,8 @@ describe("McpServerManager sampling", () => { url: {}, }, }, + versionNegotiation: { mode: "auto" }, + inputRequired: { autoFulfill: true }, }); expect(client.setRequestHandler).toHaveBeenCalledTimes(1); expect(client.setRequestHandler.mock.invocationCallOrder[0]).toBeLessThan( @@ -122,6 +116,8 @@ describe("McpServerManager sampling", () => { expect(mocks.clients[0].options).toEqual({ capabilities: { elicitation: { form: {} } }, + versionNegotiation: { mode: "auto" }, + inputRequired: { autoFulfill: true }, }); }); @@ -161,7 +157,7 @@ describe("McpServerManager sampling", () => { }); it("handles every URL in a URL-required error", async () => { - const { UrlElicitationRequiredError } = await import("@modelcontextprotocol/sdk/types.js"); + const { UrlElicitationRequiredError } = await import("@modelcontextprotocol/client"); const { McpServerManager } = await import("../server-manager.ts"); const ui = { select: vi.fn().mockResolvedValue("Open"), @@ -204,6 +200,8 @@ describe("McpServerManager sampling", () => { url: {}, }, }, + versionNegotiation: { mode: "auto" }, + inputRequired: { autoFulfill: true }, }); expect(mocks.clients[0].setRequestHandler).toHaveBeenCalledTimes(2); }); @@ -215,7 +213,10 @@ describe("McpServerManager sampling", () => { await manager.connect("demo", { command: "node", args: ["server.js"] }); const client = mocks.clients[0]; - expect(client.options).toBeUndefined(); + expect(client.options).toEqual({ + versionNegotiation: { mode: "auto" }, + inputRequired: { autoFulfill: true }, + }); expect(client.setRequestHandler).not.toHaveBeenCalled(); }); diff --git a/__tests__/server-manager-stderr-capture.test.ts b/__tests__/server-manager-stderr-capture.test.ts index bc9f8c74..cd2073c6 100644 --- a/__tests__/server-manager-stderr-capture.test.ts +++ b/__tests__/server-manager-stderr-capture.test.ts @@ -6,7 +6,8 @@ const mocks = vi.hoisted(() => ({ connectImpl: null as null | ((transport: any) => Promise), })); -vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ +vi.mock("@modelcontextprotocol/client", async (importOriginal) => ({ + ...(await importOriginal>()), Client: vi.fn().mockImplementation(function (this: any) { this.setRequestHandler = vi.fn(); this.setNotificationHandler = vi.fn(); @@ -17,26 +18,20 @@ vi.mock("@modelcontextprotocol/sdk/client/index.js", () => ({ this.listResources = vi.fn(async () => ({ resources: [] })); this.close = vi.fn(async () => undefined); }), -})); - -vi.mock("@modelcontextprotocol/sdk/client/stdio.js", () => ({ - StdioClientTransport: vi.fn().mockImplementation(function (this: any, options: any) { - this.options = options; - this.stderr = options?.stderr === "pipe" ? new PassThrough() : null; + StreamableHTTPClientTransport: vi.fn().mockImplementation(function (this: any) { this.close = vi.fn(async () => undefined); mocks.transports.push(this); }), -})); - -vi.mock("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({ - StreamableHTTPClientTransport: vi.fn().mockImplementation(function (this: any) { + SSEClientTransport: vi.fn().mockImplementation(function (this: any) { this.close = vi.fn(async () => undefined); mocks.transports.push(this); }), })); -vi.mock("@modelcontextprotocol/sdk/client/sse.js", () => ({ - SSEClientTransport: vi.fn().mockImplementation(function (this: any) { +vi.mock("@modelcontextprotocol/client/stdio", () => ({ + StdioClientTransport: vi.fn().mockImplementation(function (this: any, options: any) { + this.options = options; + this.stderr = options?.stderr === "pipe" ? new PassThrough() : null; this.close = vi.fn(async () => undefined); mocks.transports.push(this); }), diff --git a/__tests__/server-manager-streamable-http.test.ts b/__tests__/server-manager-streamable-http.test.ts index 9c77bcdd..100457dd 100644 --- a/__tests__/server-manager-streamable-http.test.ts +++ b/__tests__/server-manager-streamable-http.test.ts @@ -89,6 +89,8 @@ describe("McpServerManager StreamableHTTP transport", () => { expect(connection.tools).toEqual([]); expect(connection.resources).toEqual([]); expect(requests).toContain("GET /mcp"); + // SDK v2 probes the optional GET stream once, then keeps the successful + // POST-based session without an SSE fallback or retry storm. expect(requests.filter(request => request === "GET /mcp")).toHaveLength(1); } finally { await manager.close("post-only").catch(() => {}); diff --git a/__tests__/session-recovery-integration.test.ts b/__tests__/session-recovery-integration.test.ts index 415ee942..66751625 100644 --- a/__tests__/session-recovery-integration.test.ts +++ b/__tests__/session-recovery-integration.test.ts @@ -129,13 +129,13 @@ describe("session recovery — Streamable HTTP wire path", () => { describe("session recovery — proxy path (proxy-modes.ts executeCall)", () => { it("recovers a terminated Streamable HTTP session transparently mid tool-call", async () => { const { executeCall } = await import("../proxy-modes.ts"); - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, client: { - callTool: vi.fn().mockRejectedValueOnce(new StreamableHTTPError(404, "Session not found")), + callTool: vi.fn().mockRejectedValueOnce(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })), }, }; const fresh = { @@ -176,13 +176,13 @@ describe("session recovery — proxy path (proxy-modes.ts executeCall)", () => { it("recovers a server-not-initialized MCP error transparently mid tool-call", async () => { const { executeCall } = await import("../proxy-modes.ts"); - const { ErrorCode, McpError } = await import("@modelcontextprotocol/sdk/types.js"); + const { ProtocolError } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, client: { - callTool: vi.fn().mockRejectedValueOnce(new McpError(ErrorCode.ConnectionClosed, "Server not initialized")), + callTool: vi.fn().mockRejectedValueOnce(new ProtocolError(-32000, "Server not initialized")), }, }; const fresh = { @@ -223,17 +223,17 @@ describe("session recovery — proxy path (proxy-modes.ts executeCall)", () => { it("gives up after one reconnect attempt: a second server-not-initialized MCP error propagates as call_failed", async () => { const { executeCall } = await import("../proxy-modes.ts"); - const { ErrorCode, McpError } = await import("@modelcontextprotocol/sdk/types.js"); + const { ProtocolError } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValue(new McpError(ErrorCode.ConnectionClosed, "Server not initialized")) }, + client: { callTool: vi.fn().mockRejectedValue(new ProtocolError(-32000, "Server not initialized")) }, }; const fresh = { status: "connected" as const, transport: { sessionId: "session-2" }, - client: { callTool: vi.fn().mockRejectedValue(new McpError(ErrorCode.ConnectionClosed, "Server not initialized")) }, + client: { callTool: vi.fn().mockRejectedValue(new ProtocolError(-32000, "Server not initialized")) }, }; const manager = { @@ -265,17 +265,17 @@ describe("session recovery — proxy path (proxy-modes.ts executeCall)", () => { it("gives up after one reconnect attempt: a second terminated session propagates as call_failed", async () => { const { executeCall } = await import("../proxy-modes.ts"); - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValue(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValue(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, }; const fresh = { status: "connected" as const, transport: { sessionId: "session-2" }, - client: { callTool: vi.fn().mockRejectedValue(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValue(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, }; const manager = { @@ -314,13 +314,13 @@ describe("session recovery — direct-tools path (direct-tools.ts createDirectTo it("recovers a terminated Streamable HTTP session transparently for a direct tool call", async () => { const { createDirectToolExecutor } = await import("../direct-tools.ts"); - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, client: { - callTool: vi.fn().mockRejectedValueOnce(new StreamableHTTPError(404, "Session not found")), + callTool: vi.fn().mockRejectedValueOnce(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })), }, }; const fresh = { @@ -365,13 +365,13 @@ describe("session recovery — direct-tools path (direct-tools.ts createDirectTo it("recovers a server-not-initialized MCP error transparently for a direct tool call", async () => { const { createDirectToolExecutor } = await import("../direct-tools.ts"); - const { ErrorCode, McpError } = await import("@modelcontextprotocol/sdk/types.js"); + const { ProtocolError } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, client: { - callTool: vi.fn().mockRejectedValueOnce(new McpError(ErrorCode.ConnectionClosed, "Server not initialized")), + callTool: vi.fn().mockRejectedValueOnce(new ProtocolError(-32000, "Server not initialized")), }, }; const fresh = { @@ -416,17 +416,17 @@ describe("session recovery — direct-tools path (direct-tools.ts createDirectTo it("gives up after one reconnect attempt: a second terminated session propagates as call_failed", async () => { const { createDirectToolExecutor } = await import("../direct-tools.ts"); - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValue(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValue(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, }; const fresh = { status: "connected" as const, transport: { sessionId: "session-2" }, - client: { callTool: vi.fn().mockRejectedValue(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValue(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, }; const manager = { diff --git a/__tests__/session-recovery.test.ts b/__tests__/session-recovery.test.ts index 318c43d1..f4fbb9a9 100644 --- a/__tests__/session-recovery.test.ts +++ b/__tests__/session-recovery.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; -import { StreamableHTTPError } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; -import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js"; +import { SdkHttpError, SdkErrorCode } from "@modelcontextprotocol/client"; +import { ProtocolError } from "@modelcontextprotocol/client"; import { SessionRecoveryAuthRequiredError, isTerminatedSession, withSessionRecovery } from "../session-recovery.ts"; import type { ServerConnection } from "../server-manager.ts"; import type { McpConfig } from "../types.ts"; @@ -20,48 +20,42 @@ function makeConnection(sessionId: string | undefined): ServerConnection { describe("isTerminatedSession", () => { it("is true for a 404 StreamableHTTPError carrying a session id", () => { - const err = new StreamableHTTPError(404, "Session not found"); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); expect(isTerminatedSession(err, true)).toBe(true); }); it("is false for a 404 with no session id (never initialized / wrong URL)", () => { - const err = new StreamableHTTPError(404, "Not found"); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Not found", { status: 404 }); expect(isTerminatedSession(err, false)).toBe(false); }); it("is true for a server-not-initialized MCP error carrying a session id", () => { - const err = new McpError(ErrorCode.ConnectionClosed, "Server not initialized"); + const err = new ProtocolError(-32000, "Server not initialized"); expect(isTerminatedSession(err, true)).toBe(true); }); it("is true for the SDK's bad-request server-not-initialized MCP error", () => { - const err = new McpError(ErrorCode.ConnectionClosed, "Bad Request: Server not initialized"); + const err = new ProtocolError(-32000, "Bad Request: Server not initialized"); expect(isTerminatedSession(err, true)).toBe(true); }); it("is true for the SDK's bad-request server-not-initialized HTTP error body", () => { - const err = new StreamableHTTPError( - 400, - 'Error POSTing to endpoint: {"jsonrpc":"2.0","error":{"code":-32000,"message":"Bad Request: Server not initialized"},"id":null}', - ); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'Error POSTing to endpoint: {"jsonrpc":"2.0","error":{"code":-32000,"message":"Bad Request: Server not initialized"},"id":null}', { status: 400 }); expect(isTerminatedSession(err, true)).toBe(true); }); it("is false for other -32000 HTTP 400 error bodies", () => { - const err = new StreamableHTTPError( - 400, - 'Error POSTing to endpoint: {"jsonrpc":"2.0","error":{"code":-32000,"message":"Bad Request: Mcp-Session-Id header is required"},"id":null}', - ); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'Error POSTing to endpoint: {"jsonrpc":"2.0","error":{"code":-32000,"message":"Bad Request: Mcp-Session-Id header is required"},"id":null}', { status: 400 }); expect(isTerminatedSession(err, true)).toBe(false); }); it("is false for server-not-initialized without a session id", () => { - const err = new McpError(ErrorCode.ConnectionClosed, "Server not initialized"); + const err = new ProtocolError(-32000, "Server not initialized"); expect(isTerminatedSession(err, false)).toBe(false); }); it("is false for other -32000 MCP errors, even with a session id", () => { - const err = new McpError(ErrorCode.ConnectionClosed, "Connection closed"); + const err = new ProtocolError(-32000, "Connection closed"); expect(isTerminatedSession(err, true)).toBe(false); }); @@ -71,12 +65,12 @@ describe("isTerminatedSession", () => { }); it("is false for the right message with the wrong MCP error code", () => { - const err = new McpError(ErrorCode.InternalError, "Server not initialized"); + const err = new ProtocolError(-32603, "Server not initialized"); expect(isTerminatedSession(err, true)).toBe(false); }); it("is false for 400, even with a session id — ambiguous, never treated as expiry", () => { - const err = new StreamableHTTPError(400, "Bad request"); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Bad request", { status: 400 }); expect(isTerminatedSession(err, true)).toBe(false); }); @@ -121,7 +115,7 @@ describe("withSessionRecovery", () => { const fn = vi.fn(async (conn: ServerConnection) => { if (conn === stale) { - throw new StreamableHTTPError(404, "Session not found"); + throw new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); } return "ok"; }); @@ -146,7 +140,7 @@ describe("withSessionRecovery", () => { const fn = vi.fn(async (conn: ServerConnection) => { if (conn === stale) { - throw new McpError(ErrorCode.ConnectionClosed, "Server not initialized"); + throw new ProtocolError(-32000, "Server not initialized"); } return "ok"; }); @@ -164,7 +158,7 @@ describe("withSessionRecovery", () => { it("does not recover unrelated -32000 MCP errors", async () => { const connection = makeConnection("session-1"); const manager = makeManager({ getConnection: () => connection, reconnect: async () => connection }); - const err = new McpError(ErrorCode.ConnectionClosed, "Connection closed"); + const err = new ProtocolError(-32000, "Connection closed"); const fn = vi.fn().mockRejectedValue(err); await expect(withSessionRecovery({ manager: manager as any, config }, "demo", fn)).rejects.toBe(err); @@ -184,7 +178,7 @@ describe("withSessionRecovery", () => { const fn = vi.fn(async (conn: ServerConnection) => { if (conn === stale) { - throw new StreamableHTTPError(404, "Session not found"); + throw new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); } return conn === authed ? "ok" : "wrong-connection"; }); @@ -204,7 +198,7 @@ describe("withSessionRecovery", () => { reconnect: async () => needsAuth, }); const fn = vi.fn(async () => { - throw new StreamableHTTPError(404, "Session not found"); + throw new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); }); await expect(withSessionRecovery({ manager: manager as any, config }, "demo", fn)) @@ -221,7 +215,7 @@ describe("withSessionRecovery", () => { }); const fn = vi.fn(async (conn: ServerConnection) => { if (conn === stale) { - throw new StreamableHTTPError(404, "Session not found"); + throw new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); } return "ok"; }); @@ -241,7 +235,7 @@ describe("withSessionRecovery", () => { }); const fn = vi.fn(async () => { controller.abort(reason); - throw new StreamableHTTPError(404, "Session not found"); + throw new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); }); await expect(withSessionRecovery({ manager: manager as any, config, signal: controller.signal }, "demo", fn)) @@ -257,8 +251,8 @@ describe("withSessionRecovery", () => { reconnect: async () => fresh, }); - const err1 = new McpError(ErrorCode.ConnectionClosed, "Server not initialized"); - const err2 = new McpError(ErrorCode.ConnectionClosed, "Server not initialized"); + const err1 = new ProtocolError(-32000, "Server not initialized"); + const err2 = new ProtocolError(-32000, "Server not initialized"); const fn = vi.fn().mockRejectedValueOnce(err1).mockRejectedValueOnce(err2); await expect(withSessionRecovery({ manager: manager as any, config }, "demo", fn)).rejects.toBe(err2); @@ -275,8 +269,8 @@ describe("withSessionRecovery", () => { reconnect: async () => fresh, }); - const err1 = new StreamableHTTPError(404, "Session not found"); - const err2 = new StreamableHTTPError(404, "Session not found"); + const err1 = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); + const err2 = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); const fn = vi.fn().mockRejectedValueOnce(err1).mockRejectedValueOnce(err2); await expect(withSessionRecovery({ manager: manager as any, config }, "demo", fn)).rejects.toBe(err2); @@ -300,7 +294,7 @@ describe("withSessionRecovery", () => { it("does not recover a 404 without a session id (never initialized / wrong URL)", async () => { const connection = makeConnection(undefined); const manager = makeManager({ getConnection: () => connection, reconnect: async () => connection }); - const err = new StreamableHTTPError(404, "Not found"); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Not found", { status: 404 }); const fn = vi.fn().mockRejectedValue(err); await expect(withSessionRecovery({ manager: manager as any, config }, "demo", fn)).rejects.toBe(err); @@ -322,7 +316,7 @@ describe("withSessionRecovery", () => { it("does not recover a 400 (ambiguous status, never matched)", async () => { const connection = makeConnection("session-1"); const manager = makeManager({ getConnection: () => connection, reconnect: async () => connection }); - const err = new StreamableHTTPError(400, "Bad request"); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Bad request", { status: 400 }); const fn = vi.fn().mockRejectedValue(err); await expect(withSessionRecovery({ manager: manager as any, config }, "demo", fn)).rejects.toBe(err); @@ -333,7 +327,7 @@ describe("withSessionRecovery", () => { it("rethrows the original error when the server was removed from config before reconnecting", async () => { const connection = makeConnection("session-1"); const manager = makeManager({ getConnection: () => connection, reconnect: async () => connection }); - const err = new StreamableHTTPError(404, "Session not found"); + const err = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); const fn = vi.fn().mockRejectedValue(err); const emptyConfig: McpConfig = { mcpServers: {} }; @@ -357,7 +351,7 @@ describe("withSessionRecovery", () => { const fn = vi.fn(async (conn: ServerConnection) => { if (conn === stale) { - throw new StreamableHTTPError(404, "Session not found"); + throw new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 }); } return conn === fresh ? "ok" : "unexpected"; }); diff --git a/__tests__/ui-resource-handler.test.ts b/__tests__/ui-resource-handler.test.ts index 35462739..9e155e6d 100644 --- a/__tests__/ui-resource-handler.test.ts +++ b/__tests__/ui-resource-handler.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { UiResourceHandler } from "../ui-resource-handler.ts"; import { buildCspMetaContent } from "../host-html-template.ts"; -import { UrlElicitationRequiredError } from "@modelcontextprotocol/sdk/types.js"; +import { UrlElicitationRequiredError } from "@modelcontextprotocol/client"; import type { McpServerManager } from "../server-manager.ts"; // Mock the manager @@ -38,12 +38,12 @@ describe("UiResourceHandler", () => { }); it("recovers a terminated HTTP session while loading a UI resource", async () => { - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected" as const, transport: { sessionId: "session-1" }, client: { - readResource: vi.fn().mockRejectedValueOnce(new StreamableHTTPError(404, "Session not found")), + readResource: vi.fn().mockRejectedValueOnce(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })), }, resources: [], }; diff --git a/__tests__/ui-server-session-recovery.test.ts b/__tests__/ui-server-session-recovery.test.ts index 0611d0ad..cb8bcd05 100644 --- a/__tests__/ui-server-session-recovery.test.ts +++ b/__tests__/ui-server-session-recovery.test.ts @@ -70,12 +70,12 @@ describe("UiServer /proxy/tools/call session recovery", () => { }); it("recovers a terminated Streamable HTTP session transparently, when config is supplied", async () => { - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected", transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValueOnce(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValueOnce(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, } as unknown as ServerConnection; const fresh = { status: "connected", @@ -119,12 +119,12 @@ describe("UiServer /proxy/tools/call session recovery", () => { }); it("returns auth guidance when recovery reconnect needs auth and no callback can refresh it", async () => { - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected", transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValueOnce(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValueOnce(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, } as unknown as ServerConnection; const needsAuth = { status: "needs-auth", @@ -164,12 +164,12 @@ describe("UiServer /proxy/tools/call session recovery", () => { }); it("uses the supplied auth callback when recovery reconnect returns needs-auth", async () => { - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected", transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValueOnce(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValueOnce(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, } as unknown as ServerConnection; const needsAuth = { status: "needs-auth", @@ -215,12 +215,12 @@ describe("UiServer /proxy/tools/call session recovery", () => { }); it("runs without recovery (unchanged pre-existing behavior) when config is not supplied", async () => { - const { StreamableHTTPError } = await import("@modelcontextprotocol/sdk/client/streamableHttp.js"); + const { SdkHttpError, SdkErrorCode } = await import("@modelcontextprotocol/client"); const stale = { status: "connected", transport: { sessionId: "session-1" }, - client: { callTool: vi.fn().mockRejectedValue(new StreamableHTTPError(404, "Session not found")) }, + client: { callTool: vi.fn().mockRejectedValue(new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, "Session not found", { status: 404 })) }, } as unknown as ServerConnection; const manager = { diff --git a/__tests__/ui-server.test.ts b/__tests__/ui-server.test.ts index 59d1baef..4d722727 100644 --- a/__tests__/ui-server.test.ts +++ b/__tests__/ui-server.test.ts @@ -554,7 +554,6 @@ describe("UiServer", () => { name: "some_tool", arguments: { arg1: "value1" }, }, - undefined, requestOptions, ); }); diff --git a/__tests__/ui-streaming.test.ts b/__tests__/ui-streaming.test.ts index 88edd96e..c756135e 100644 --- a/__tests__/ui-streaming.test.ts +++ b/__tests__/ui-streaming.test.ts @@ -198,13 +198,17 @@ describe("UI Streaming", () => { attachAdapterNotificationHandlers: (serverName: string, client: { setNotificationHandler: typeof client.setNotificationHandler }) => void; }).attachAdapterNotificationHandlers(serverName, client); expect(client.setNotificationHandler).toHaveBeenCalledOnce(); - return client.setNotificationHandler.mock.calls[0][1] as (notification: { + const handler = client.setNotificationHandler.mock.calls[0][2] as (params: { + streamToken: string; + result: { content?: unknown[]; structuredContent?: Record }; + }) => void; + return (notification: { method: string; params: { streamToken: string; result: { content?: unknown[]; structuredContent?: Record }; }; - }) => void; + }) => handler(notification.params); } it("routes notifications to the matching listener", () => { diff --git a/conformance/README.md b/conformance/README.md new file mode 100644 index 00000000..9a5d4853 --- /dev/null +++ b/conformance/README.md @@ -0,0 +1,51 @@ +# MCP client conformance tests + +This directory runs the official `@modelcontextprotocol/conformance` client suite against pi-mcp-adapter. + +The driver uses the adapter's own code rather than constructing a bare SDK client: + +```text +conformance referee + -> conformance/driver.sh (isolated token store and callback port) + -> conformance/driver.ts + -> McpServerManager + -> mcp-auth-flow + McpOAuthProvider + localhost callback server + -> referee MCP and OAuth servers +``` + +It covers the four core client scenarios and the full OAuth matrix shipped by conformance 0.1.16, 26 scenarios in total. The OAuth driver follows the referee's authorization redirect into the adapter's real callback server, then completes the pending flow through `completeAuthFromInput()`. + +## Run the tests + +From the repository root: + +```sh +npm run test:conformance +``` + +Run one scenario while developing: + +```sh +bash conformance/run.sh --scenario initialize +bash conformance/run.sh --scenario auth/metadata-default --verbose +``` + +Results are written to `conformance/results/` and ignored by git. Set `CONFORMANCE_RESULTS_DIR` to use another directory, or `CONFORMANCE_TIMEOUT_MS` to change the 90-second per-scenario timeout. + +The full runner is sequential. The upstream CLI's `--suite` mode runs scenarios in parallel, but pre-registered OAuth clients bind an exact localhost callback port. Parallel runs can therefore fail because another scenario briefly owns that port, which tests port contention rather than MCP behavior. + +## Expected failures + +`baseline-client.yml` lists known adapter or SDK gaps. A failure in that file keeps the suite green; an unexpected failure or a baseline entry that starts passing fails the run. + +Current gaps: + +| Scenario | Reason | +| --- | --- | +| `auth/basic-cimd` | The adapter uses dynamic client registration rather than an HTTPS Client ID Metadata Document. | +| `auth/scope-step-up` | Pi's user-gated OAuth flow cannot yet resume the SDK's in-call 403 scope challenge with the widened scope. | +| `auth/2025-03-26-oauth-metadata-backcompat` | SDK beta.5 rejects the referee's legacy metadata issuer mismatch under RFC 8414 validation. | +| `auth/client-credentials-jwt` | Private-key JWT client authentication is not configured by the adapter. | +| `auth/cross-app-access-complete-flow` | The adapter does not implement SEP-990 token exchange and JWT bearer grants. | + +Baseline comments contain the protocol-level details. Do not add a failure caused by the driver, callback-port contention, or test setup. diff --git a/conformance/baseline-client.yml b/conformance/baseline-client.yml new file mode 100644 index 00000000..eb79f45e --- /dev/null +++ b/conformance/baseline-client.yml @@ -0,0 +1,25 @@ +# Known gaps in pi-mcp-adapter's MCP client conformance coverage. +# Remove an entry as soon as its scenario passes. The conformance CLI treats a +# passing baseline entry as stale and fails the run. +client: + # The adapter dynamically registers instead of publishing an HTTPS Client ID + # Metadata Document. The scenario otherwise passes and reports one SHOULD-level warning. + - auth/basic-cimd + + # The SDK receives the widened scope from the 403 challenge, but its in-call + # step-up cannot open Pi's user-gated OAuth flow. A manual restart loses the + # challenge scope and requests only the original PRM scope. + - auth/scope-step-up + + # beta.5 applies RFC 8414 issuer-echo validation to the 2025-03-26 fallback. + # The referee serves metadata at the MCP origin while advertising an /oauth + # issuer, so the secure default rejects it before registration. + - auth/2025-03-26-oauth-metadata-backcompat + + # The adapter supports client_secret_basic client credentials, but its config + # has no private-key JWT credentials or assertion signing hook. + - auth/client-credentials-jwt + + # SEP-990 needs an IdP token exchange followed by a JWT bearer grant. The + # adapter currently implements authorization_code and client_credentials only. + - auth/cross-app-access-complete-flow diff --git a/conformance/driver.sh b/conformance/driver.sh new file mode 100755 index 00000000..814d30a5 --- /dev/null +++ b/conformance/driver.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Per-scenario wrapper spawned by the conformance CLI. Each process gets an +# isolated OAuth store and callback port, then launches the TypeScript driver. +set -euo pipefail + +cd "$(dirname "$0")/.." + +AUTH_DIR="$(mktemp -d "${TMPDIR:-/tmp}/pi-mcp-conformance-auth.XXXXXX")" +cleanup() { + rm -rf "$AUTH_DIR" +} +trap cleanup EXIT HUP INT TERM + +export MCP_OAUTH_DIR="$AUTH_DIR" + +# Pre-registered browser clients require an exact callback port. Allocate one +# per process. The full runner is sequential, avoiding the race between probing +# a free port and binding it. +export MCP_OAUTH_CALLBACK_PORT="$( + node -e 'const s=require("node:net").createServer();s.listen(0,"127.0.0.1",()=>{console.log(s.address().port);s.close();})' +)" + +node --import tsx conformance/driver.ts "$@" diff --git a/conformance/driver.ts b/conformance/driver.ts new file mode 100644 index 00000000..39ee3039 --- /dev/null +++ b/conformance/driver.ts @@ -0,0 +1,270 @@ +/** + * Conformance client driver. + * + * Spawned by `@modelcontextprotocol/conformance` once per scenario (via + * conformance/driver.sh, which provisions an isolated MCP_OAUTH_DIR): + * + * MCP_CONFORMANCE_SCENARIO= node --import tsx conformance/driver.ts + * + * The MCP client under test is the adapter's real client stack: + * McpServerManager (transport probe, StreamableHTTP/SSE fallback, needs-auth + * detection, elicitation handler) plus mcp-auth-flow (SDK OAuth discovery, + * DCR, PKCE, token exchange, the real localhost callback server). + * + * For OAuth scenarios this process also plays the role of the user's + * browser: the conformance authorization endpoint auto-approves, so the + * driver fetches the authorization URL with `redirect: "manual"`, follows + * the Location into the adapter's real callback server, and then completes + * the flow via completeAuthFromInput with the full redirect URL. + * + * Exit code: 0 = scenario steps completed, non-zero = client failure. + * Grading itself is done server-side by the conformance harness. + */ + +import { rmSync } from "node:fs" +import { UnauthorizedError } from "@modelcontextprotocol/client" +import type { ExtensionUIContext } from "@earendil-works/pi-coding-agent" +import { McpServerManager } from "../server-manager.ts" +import { + completeAuthFromInput, + initializeOAuth, + shutdownOAuth, + startAuth, +} from "../mcp-auth-flow.ts" +import type { ServerEntry } from "../types.ts" + +const scenario = process.env.MCP_CONFORMANCE_SCENARIO ?? "" +const serverUrl = process.argv[2] ?? "" +const authDir = process.env.MCP_OAUTH_DIR ?? "" + +if (!scenario || !serverUrl || !authDir) { + console.error( + "Usage: MCP_OAUTH_DIR= MCP_CONFORMANCE_SCENARIO= driver.ts ", + ) + process.exit(1) +} + +interface ScenarioContext { + name?: string + client_id?: string + client_secret?: string + private_key_pem?: string + signing_algorithm?: string + [key: string]: unknown +} + +const context: ScenarioContext = process.env.MCP_CONFORMANCE_CONTEXT + ? JSON.parse(process.env.MCP_CONFORMANCE_CONTEXT) + : {} + +// Keep this list explicit. If a later conformance release adds a scenario, +// the driver must learn its required action rather than passing from incidental +// initialize traffic. +const SUPPORTED_SCENARIOS = new Set([ + "initialize", + "tools_call", + "elicitation-sep1034-client-defaults", + "sse-retry", + "auth/metadata-default", + "auth/metadata-var1", + "auth/metadata-var2", + "auth/metadata-var3", + "auth/basic-cimd", + "auth/scope-from-www-authenticate", + "auth/scope-from-scopes-supported", + "auth/scope-omitted-when-undefined", + "auth/scope-step-up", + "auth/scope-retry-limit", + "auth/token-endpoint-auth-basic", + "auth/token-endpoint-auth-post", + "auth/token-endpoint-auth-none", + "auth/pre-registration", + "auth/2025-03-26-oauth-metadata-backcompat", + "auth/2025-03-26-oauth-endpoint-fallback", + "auth/resource-mismatch", + "auth/offline-access-scope", + "auth/offline-access-not-supported", + "auth/client-credentials-jwt", + "auth/client-credentials-basic", + "auth/cross-app-access-complete-flow", +]) + +if (!SUPPORTED_SCENARIOS.has(scenario)) { + console.error(`Unsupported MCP conformance scenario: ${scenario}`) + process.exit(1) +} + +/** + * Scripted UI for elicitation: accept the request, take every field's + * default, submit. Exercises the adapter's real form-elicitation handler. + */ +const scriptedUi = { + select: async (_title: string, options: string[]) => { + for (const preferred of ["Use default", "Submit", "Continue"]) { + const match = options.find((option) => option === preferred) + if (match) return match + } + return options[0] + }, + input: async () => undefined, + confirm: async () => true, + notify: () => {}, +} as unknown as ExtensionUIContext + +const SERVER_NAME = "conformance" +const MAX_AUTH_ROUND_TRIPS = 3 +const debugLog = (message: string): void => { + if (process.env.CONFORMANCE_DRIVER_DEBUG) console.error(`[driver] ${message}`) +} + +const definition: ServerEntry = { url: serverUrl } +if (context.client_id) { + definition.oauth = { + clientId: context.client_id, + ...(context.client_secret ? { clientSecret: context.client_secret } : {}), + } +} +if (scenario.startsWith("auth/client-credentials")) { + definition.oauth = { ...(definition.oauth ?? {}), grantType: "client_credentials" } +} + +const oauthRuntime = await initializeOAuth() +const manager = new McpServerManager(process.cwd()) +manager.setOAuthRuntime(oauthRuntime) +manager.setElicitationConfig({ allowUrl: false, ui: scriptedUi }) +manager.setDefaultRequestTimeoutMs(60_000) + +/** + * Run one OAuth round-trip through the adapter's real auth flow, playing + * the browser headlessly: the conformance AS auto-approves a plain GET on + * the authorization URL and 302s to the adapter's localhost callback. + */ +async function runAuthRoundTrip(): Promise { + debugLog("startAuth") + const { authorizationUrl } = await startAuth(SERVER_NAME, serverUrl, definition, { runtime: oauthRuntime }) + debugLog(`authorizationUrl=${authorizationUrl || "(none)"}`) + if (!authorizationUrl) return // e.g. client_credentials — already authorized + + const authResponse = await fetch(authorizationUrl, { redirect: "manual" }) + const location = authResponse.headers.get("location") + if (!location) { + throw new Error( + `Authorization endpoint did not redirect (status ${authResponse.status}): ${await authResponse.text()}`, + ) + } + + // Follow the redirect into the adapter's real callback server (it serves + // the "return to terminal" page), then complete the flow with the full + // redirect URL. The adapter validates state and exchanges the code. + const callbackUrl = new URL(location, authorizationUrl).toString() + const callbackResponse = await fetch(callbackUrl, { redirect: "manual" }) + if (callbackResponse.status >= 400) { + throw new Error( + `OAuth callback failed with ${callbackResponse.status}: ${await callbackResponse.text()}`, + ) + } + const status = await completeAuthFromInput(SERVER_NAME, callbackUrl, { runtime: oauthRuntime }) + if (status !== "authenticated") { + throw new Error(`OAuth completion returned status: ${status}`) + } +} + +async function connectWithAuth() { + for (let attempt = 0; attempt <= MAX_AUTH_ROUND_TRIPS; attempt++) { + let needsAuth = false + try { + const connection = await manager.connect(SERVER_NAME, definition) + debugLog(`connect attempt ${attempt}: status=${connection.status}`) + if (connection.status === "connected") return connection + if (connection.status !== "needs-auth") { + throw new Error(`Unexpected connection status: ${connection.status}`) + } + needsAuth = true + } catch (error) { + // The HTTP transport probe throws UnauthorizedError out of connect() + // when the server requires OAuth; the host treats this as needs-auth. + debugLog(`connect attempt ${attempt}: threw ${(error as Error)?.constructor?.name}: ${(error as Error)?.message}`) + // For auth scenarios, any other connect failure (e.g. the SSE-fallback + // error seen on 2025-03-26 backcompat servers) is handled the way the + // Pi host would: by offering manual authentication. If the auth flow + // itself cannot proceed either, surface the original connect error. + if (!(error instanceof UnauthorizedError) && !scenario.startsWith("auth/")) throw error + needsAuth = true + } + if (!needsAuth || attempt === MAX_AUTH_ROUND_TRIPS) break + await manager.close(SERVER_NAME) + await runAuthRoundTrip() + } + throw new Error(`Gave up after ${MAX_AUTH_ROUND_TRIPS} OAuth round-trips`) +} + +function isAuthFailure(error: unknown): boolean { + if (error instanceof UnauthorizedError) return true + const message = error instanceof Error ? error.message : String(error) + return /unauthorized|re-authentication required|insufficient_scope|invalid_token|\b40[13]\b/i.test(message) +} + +async function callTool(toolName: string, args: Record) { + for (let attempt = 0; ; attempt++) { + const connection = await connectWithAuth() + try { + const result = await connection.client.callTool( + { name: toolName, arguments: args }, + manager.getRequestOptions(SERVER_NAME), + ) + if (result.isError) { + throw new Error(`Tool ${toolName} returned an error result: ${JSON.stringify(result.content)}`) + } + return result + } catch (error) { + // Mid-session auth failure (e.g. scope step-up): re-run the OAuth + // flow through the adapter and retry, capped like a real host would. + if (attempt >= MAX_AUTH_ROUND_TRIPS - 1 || !isAuthFailure(error)) throw error + await manager.close(SERVER_NAME) + await runAuthRoundTrip() + } + } +} + +async function runScenario(): Promise { + if (scenario === "initialize") { + await connectWithAuth() + return + } + if (scenario === "tools_call") { + await callTool("add_numbers", { a: 5, b: 3 }) + return + } + if (scenario === "elicitation-sep1034-client-defaults") { + await callTool("test_client_elicitation_defaults", {}) + return + } + if (scenario === "sse-retry") { + await callTool("test_reconnection", {}) + return + } + if (scenario.startsWith("auth/")) { + await callTool("test-tool", {}) + return + } + throw new Error(`Unsupported MCP conformance scenario: ${scenario}`) +} + +let exitCode = 0 +try { + await runScenario() +} catch (error) { + console.error(error instanceof Error ? (error.stack ?? error.message) : String(error)) + exitCode = 1 +} finally { + try { + await manager.closeAll() + } catch {} + try { + await shutdownOAuth(oauthRuntime) + } catch {} + try { + rmSync(authDir, { recursive: true, force: true }) + } catch {} +} +process.exit(exitCode) diff --git a/conformance/run.sh b/conformance/run.sh new file mode 100755 index 00000000..89504886 --- /dev/null +++ b/conformance/run.sh @@ -0,0 +1,130 @@ +#!/usr/bin/env bash +# Run the official MCP client conformance scenarios against pi-mcp-adapter's +# real McpServerManager and OAuth stack. +# +# Usage: +# npm run test:conformance +# bash conformance/run.sh --scenario initialize +# bash conformance/run.sh --scenario auth/metadata-default --verbose +set -euo pipefail + +cd "$(dirname "$0")/.." + +RESULTS_DIR="${CONFORMANCE_RESULTS_DIR:-conformance/results}" +BASELINE="conformance/baseline-client.yml" +DRIVER="bash conformance/driver.sh" +TIMEOUT="${CONFORMANCE_TIMEOUT_MS:-90000}" + +run_scenario() { + local scenario="$1" + shift + npx conformance client \ + --command "$DRIVER" \ + --scenario "$scenario" \ + --expected-failures "$BASELINE" \ + --timeout "$TIMEOUT" \ + --output-dir "$RESULTS_DIR" \ + "$@" +} + +is_baselined() { + grep -Fqx " - $1" "$BASELINE" +} + +allows_client_error() { + [[ "$1" == "auth/scope-retry-limit" || "$1" == "auth/resource-mismatch" ]] +} + +# Preserve the official CLI's focused-scenario workflow. Full-suite execution +# below is deliberately sequential: pre-registered clients bind an exact local +# callback port, so the CLI's parallel --suite mode creates false port-contention +# failures between otherwise independent scenarios. +focused_scenario="" +previous="" +for argument in "$@"; do + if [[ "$previous" == "--scenario" ]]; then + focused_scenario="$argument" + break + fi + previous="$argument" +done + +if [[ -n "$focused_scenario" ]]; then + focused_log="$(mktemp "${TMPDIR:-/tmp}/pi-mcp-conformance-log.XXXXXX")" + trap 'rm -f "$focused_log"' EXIT + set +e + npx conformance client \ + --command "$DRIVER" \ + --expected-failures "$BASELINE" \ + --timeout "$TIMEOUT" \ + --output-dir "$RESULTS_DIR" \ + "$@" 2>&1 | tee "$focused_log" + focused_status=${PIPESTATUS[0]} + set -e + if [[ "$focused_status" -ne 0 ]]; then + exit "$focused_status" + fi + if grep -q "Client timed out after" "$focused_log"; then + echo "MCP client conformance timed out" >&2 + exit 1 + fi + if grep -q "Client exited with code" "$focused_log" \ + && ! is_baselined "$focused_scenario" \ + && ! allows_client_error "$focused_scenario"; then + echo "MCP client exited unexpectedly despite passing wire checks" >&2 + exit 1 + fi + exit 0 +fi + +if [[ " $* " == *" --suite "* ]]; then + echo "conformance/run.sh runs the complete client matrix sequentially; use --scenario for a focused run" >&2 + exit 2 +fi + +rm -rf "$RESULTS_DIR" +mkdir -p "$RESULTS_DIR" + +scenarios="$({ npx conformance list --client 2>/dev/null || true; } | + awk '/^Client scenarios/{found=1; next} found && /^ - /{print $2}')" +if [[ -z "$scenarios" ]]; then + echo "Unable to list MCP client conformance scenarios" >&2 + exit 1 +fi + +failed=0 +log_file="$(mktemp "${TMPDIR:-/tmp}/pi-mcp-conformance-log.XXXXXX")" +trap 'rm -f "$log_file"' EXIT + +while IFS= read -r scenario; do + [[ -z "$scenario" ]] && continue + printf '%-52s' "$scenario" + if run_scenario "$scenario" "$@" >"$log_file" 2>&1; then + # conformance 0.1.16's baseline check only evaluates wire checks. Do not + # let it hide an unexpected client-process failure after those checks ran. + if grep -q "Client timed out after" "$log_file"; then + echo "FAIL" + tail -40 "$log_file" + failed=1 + elif grep -q "Client exited with code" "$log_file" \ + && ! is_baselined "$scenario" \ + && ! allows_client_error "$scenario"; then + echo "FAIL" + tail -40 "$log_file" + failed=1 + else + echo "PASS" + fi + else + echo "FAIL" + tail -40 "$log_file" + failed=1 + fi +done <<< "$scenarios" + +if [[ "$failed" -ne 0 ]]; then + echo "MCP client conformance failed; inspect $RESULTS_DIR" >&2 + exit 1 +fi + +echo "All MCP client conformance scenarios passed or matched the reviewed baseline." diff --git a/direct-tools.ts b/direct-tools.ts index be63e0b7..48415095 100644 --- a/direct-tools.ts +++ b/direct-tools.ts @@ -1,5 +1,5 @@ import type { AgentToolResult, AgentToolUpdateCallback, ExtensionContext } from "@earendil-works/pi-coding-agent"; -import { UrlElicitationRequiredError } from "@modelcontextprotocol/sdk/types.js"; +import { UrlElicitationRequiredError } from "@modelcontextprotocol/client"; import type { McpExtensionState } from "./state.ts"; import type { DirectToolSpec, McpConfig, McpContent, ToolPrefix } from "./types.ts"; import type { MetadataCache } from "./metadata-cache.ts"; @@ -474,9 +474,9 @@ export function createDirectToolExecutor( name: spec.originalName, arguments: params ?? {}, _meta: uiSession?.requestMeta, - }, undefined, requestOptions), ownedSignal), + }, requestOptions), ownedSignal), ); - uiSession?.sendToolResult(result as unknown as import("@modelcontextprotocol/sdk/types.js").CallToolResult); + uiSession?.sendToolResult(result as unknown as import("@modelcontextprotocol/client").CallToolResult); if (result.isError) { const mcpContent = (result.content ?? []) as McpContent[]; diff --git a/elicitation-handler.ts b/elicitation-handler.ts index 2b44ed2e..b5fa8371 100644 --- a/elicitation-handler.ts +++ b/elicitation-handler.ts @@ -1,16 +1,15 @@ import type { ExtensionUIContext } from "@earendil-works/pi-coding-agent"; -import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { - ElicitRequestSchema, - ErrorCode, - McpError, + ProtocolError, + ProtocolErrorCode, + type Client, type ElicitRequest, type ElicitRequestFormParams, type ElicitRequestURLParams, type ElicitResult, -} from "@modelcontextprotocol/sdk/types.js"; -import { AjvJsonSchemaValidator } from "@modelcontextprotocol/sdk/validation/ajv"; -import type { JsonSchemaType } from "@modelcontextprotocol/sdk/validation/types.js"; + type JsonSchemaType, +} from "@modelcontextprotocol/client"; +import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv"; import open from "open"; export type ElicitationValue = string | number | boolean | string[] | undefined; @@ -28,7 +27,7 @@ export interface ElicitationHandlerOptions { export type ServerElicitationConfig = Omit; export function registerElicitationHandler(client: Client, options: ElicitationHandlerOptions): void { - client.setRequestHandler(ElicitRequestSchema, (request) => + client.setRequestHandler("elicitation/create", (request) => handleElicitationRequest(options, request)); } @@ -305,16 +304,16 @@ export async function handleUrlElicitation( options: ElicitationHandlerOptions, params: ElicitRequestURLParams, ): Promise { - if (!options.allowUrl) throw new McpError(ErrorCode.InvalidParams, "URL elicitation is not supported"); + if (!options.allowUrl) throw new ProtocolError(ProtocolErrorCode.InvalidParams, "URL elicitation is not supported"); let parsed: URL; try { parsed = new URL(params.url); } catch { - throw new McpError(ErrorCode.InvalidParams, "URL elicitation supplied an invalid URL"); + throw new ProtocolError(ProtocolErrorCode.InvalidParams, "URL elicitation supplied an invalid URL"); } if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { - throw new McpError(ErrorCode.InvalidParams, "URL elicitation only supports HTTP and HTTPS URLs"); + throw new ProtocolError(ProtocolErrorCode.InvalidParams, "URL elicitation only supports HTTP and HTTPS URLs"); } const decision = await options.ui.select([ diff --git a/examples/interactive-visualizer/package.json b/examples/interactive-visualizer/package.json index 521f9bd7..76ea35da 100644 --- a/examples/interactive-visualizer/package.json +++ b/examples/interactive-visualizer/package.json @@ -11,10 +11,10 @@ }, "dependencies": { "@modelcontextprotocol/ext-apps": "^1.2.0", - "@modelcontextprotocol/sdk": "^1.25.2", + "@modelcontextprotocol/server": "2.0.0-beta.5", "chart.js": "^4.5.1", "mermaid": "^11.12.0", - "zod": "^3.25.76" + "zod": "^4.1.0" }, "devDependencies": { "esbuild": "^0.25.12", diff --git a/examples/interactive-visualizer/src/server.ts b/examples/interactive-visualizer/src/server.ts index a110a4dd..dd90f954 100644 --- a/examples/interactive-visualizer/src/server.ts +++ b/examples/interactive-visualizer/src/server.ts @@ -1,5 +1,5 @@ -import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; +import { McpServer, ResourceTemplate } from "@modelcontextprotocol/server"; +import { StdioServerTransport } from "@modelcontextprotocol/server/stdio"; import { readFileSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; @@ -25,7 +25,7 @@ const server = new McpServer({ version: "0.1.0", }); -server.resource( +server.registerResource( "app-html", new ResourceTemplate("ui://interactive-visualizer/app.html", { list: undefined }), { mimeType: "text/html;profile=mcp-app" }, @@ -51,15 +51,15 @@ server.registerTool( "show_chart", { description: "Display an interactive chart. The chart opens in a UI window.", - inputSchema: { + inputSchema: z.object({ type: z.string().describe("Chart type: bar, line, pie, or doughnut"), title: z.string().describe("Chart title"), labels: z.string().describe("Comma-separated labels for the x-axis or segments"), datasets: z.string().describe("JSON array of datasets: [{label, data: number[], color?}]"), - }, + }), _meta: { ui: { resourceUri: "ui://interactive-visualizer/app.html" } }, }, - async (args, extra) => { + async (args, ctx) => { const spec = { type: args.type || "bar", title: args.title || "Chart", @@ -67,10 +67,10 @@ server.registerTool( datasets: JSON.parse(args.datasets || "[]"), }; - const streamToken = extra._meta?.["pi-mcp-adapter/stream-token"] as string | undefined; + const streamToken = ctx.mcpReq._meta?.["pi-mcp-adapter/stream-token"] as string | undefined; if (streamToken) { const sendAdapterNotification = (notification: StreamNotification) => - extra.sendNotification(notification as never); + ctx.mcpReq.notify(notification as never); for (let i = 0; i < spec.datasets.length; i++) { const partial = { ...spec, datasets: spec.datasets.slice(0, i + 1) }; const isLast = i === spec.datasets.length - 1; @@ -182,15 +182,15 @@ server.registerTool( "stream_adventure", { description: "Stream an interactive choose-your-own-adventure decision tree. The story builds tier by tier. Click any choice node to send your decision back to the agent.", - inputSchema: {}, + inputSchema: z.object({}), _meta: { ui: { resourceUri: "ui://interactive-visualizer/app.html" } }, }, - async (_args, extra) => { - const streamToken = extra._meta?.["pi-mcp-adapter/stream-token"] as string | undefined; + async (_args, ctx) => { + const streamToken = ctx.mcpReq._meta?.["pi-mcp-adapter/stream-token"] as string | undefined; if (streamToken) { const sendAdapterNotification = (notification: StreamNotification) => - extra.sendNotification(notification as never); + ctx.mcpReq.notify(notification as never); for (let t = 1; t <= STORY.length; t++) { const isLast = t === STORY.length; await sendStreamFrame(streamToken, sendAdapterNotification, { diff --git a/mcp-auth-flow.test.ts b/mcp-auth-flow.test.ts index 6239d778..4819ac1f 100644 --- a/mcp-auth-flow.test.ts +++ b/mcp-auth-flow.test.ts @@ -18,6 +18,7 @@ import { startAuth, completeAuth, getAuthStatus, + getValidToken, removeAuth, supportsOAuth, extractOAuthConfig, @@ -26,7 +27,7 @@ import { type AuthStatus, } from "./mcp-auth-flow.ts" import { isCallbackServerRunning } from "./mcp-callback-server.ts" -import { updateTokens, clearAllCredentials } from "./mcp-auth.ts" +import { updateTokens, updateClientInfo, getAuthForUrl, clearAllCredentials } from "./mcp-auth.ts" import type { ServerEntry } from "./types.ts" describe("mcp-auth-flow", () => { @@ -139,6 +140,45 @@ describe("mcp-auth-flow", () => { }) }) + describe("getValidToken", () => { + it("should not attempt refresh or wipe credentials when stored client info is a config-pre-registered stub", async () => { + const serverName = "stub-refresh-test" + const serverUrl = "https://stub-refresh.example.com/mcp" + + // Expired tokens with a refresh token: normally getValidToken would + // attempt an SDK refresh. + await updateTokens(serverName, { + accessToken: "expired-token", + refreshToken: "refresh-token", + expiresAt: Date.now() / 1000 - 3600, + }, serverUrl) + + // Secretless SEP-2352 issuer stub written for a config-pre-registered + // client. getValidToken builds its provider with an empty config, so + // this stub must not be served as client information; otherwise a + // refresh goes out without a client secret, the AS returns + // invalid_client, and the SDK invalidates stored credentials. + await updateClientInfo(serverName, { + clientId: "config-client", + issuer: "https://auth.example.com", + configPreRegistered: true, + }, serverUrl) + + const result = await getValidToken(serverName, serverUrl) + + // Bails via the "no client info" guard before any network refresh. + assert.strictEqual(result, null) + + // Stored credentials must remain intact - nothing was invalidated. + const entry = await getAuthForUrl(serverName, serverUrl) + assert.strictEqual(entry?.tokens?.accessToken, "expired-token") + assert.strictEqual(entry?.tokens?.refreshToken, "refresh-token") + assert.strictEqual(entry?.clientInfo?.clientId, "config-client") + + clearAllCredentials(serverName) + }) + }) + describe("initializeOAuth / shutdownOAuth", () => { it("should not start callback server on initialize", async () => { await shutdownOAuth() diff --git a/mcp-auth-flow.ts b/mcp-auth-flow.ts index 137b6f13..e3648592 100644 --- a/mcp-auth-flow.ts +++ b/mcp-auth-flow.ts @@ -7,9 +7,10 @@ import { auth as runSdkAuth, extractWWWAuthenticateParams, + IssuerMismatchError, UnauthorizedError, -} from "@modelcontextprotocol/sdk/client/auth.js" -import { LATEST_PROTOCOL_VERSION } from "@modelcontextprotocol/sdk/types.js" + LATEST_PROTOCOL_VERSION, +} from "@modelcontextprotocol/client" import open from "open" import { McpOAuthProvider, type McpOAuthConfig } from "./mcp-oauth-provider.ts" import { @@ -458,11 +459,18 @@ function getSearchParamsFromInput(input: string): URLSearchParams | undefined { } } +/** Authorization code plus the optional RFC 9207 `iss` callback parameter. */ +export interface AuthorizationCodeInput { + code: string + iss?: string +} + /** - * Extract an OAuth authorization code from either a raw code, a query string, - * or the full localhost redirect URL copied from the browser address bar. + * Extract an OAuth authorization code (and the RFC 9207 `iss` parameter, when + * present) from either a raw code, a query string, or the full localhost + * redirect URL copied from the browser address bar. */ -export function parseAuthorizationCodeInput(input: string, expectedState?: string): string { +export function parseAuthorizationRedirectInput(input: string, expectedState?: string): AuthorizationCodeInput { const trimmed = input.trim() if (!trimmed) { throw new Error("Authorization code or redirect URL is required") @@ -485,16 +493,27 @@ export function parseAuthorizationCodeInput(input: string, expectedState?: strin } const code = params.get("code") - if (code) return code + if (code) { + const iss = params.get("iss") + return { code, ...(iss !== null ? { iss } : {}) } + } } if (/^[A-Za-z0-9._~+/=-]+$/.test(trimmed)) { - return trimmed + return { code: trimmed } } throw new Error("Could not find an OAuth authorization code in the provided input") } +/** + * Extract an OAuth authorization code from either a raw code, a query string, + * or the full localhost redirect URL copied from the browser address bar. + */ +export function parseAuthorizationCodeInput(input: string, expectedState?: string): string { + return parseAuthorizationRedirectInput(input, expectedState).code +} + /** * Complete OAuth authentication from manual user input. */ @@ -512,8 +531,8 @@ export async function completeAuthFromInput( const authStorageOptions = runtimeState.pendingAuths.get(key)?.authStorageOptions ?? fallbackAuthStorageOptions const oauthState = runtimeState.pendingAuthStates.get(key) throwIfAborted(signal) - const code = parseAuthorizationCodeInput(input, oauthState) - return completeAuth(serverName, code, options) + const parsed = parseAuthorizationRedirectInput(input, oauthState) + return completeAuth(serverName, parsed, options) } /** @@ -521,11 +540,14 @@ export async function completeAuthFromInput( */ export async function completeAuth( serverName: string, - authorizationCode: string, + authorizationCode: string | AuthorizationCodeInput, options: AuthenticateOptions = {}, ): Promise { const runtime = getRuntime(options) const runtimeState = getRuntimeState(runtime) + const { code, iss } = typeof authorizationCode === "string" + ? { code: authorizationCode, iss: undefined } + : authorizationCode const fallbackAuthStorageOptions = options.authStorageOptions ?? {} const signal = combineAbortSignals(runtime.signal, options.signal) throwIfAborted(signal) @@ -539,27 +561,46 @@ export async function completeAuth( const oauthState = runtimeState.pendingAuthStates.get(key) throwIfAborted(signal) + let keepPendingForRetry = false + let caughtError: unknown try { const result = await abortable(runSdkAuth(pendingAuth.authProvider, { serverUrl: pendingAuth.serverUrl, - authorizationCode, + authorizationCode: code, + ...(iss !== undefined ? { iss } : {}), ...pendingAuth.discovery, }), signal) throwIfAborted(signal) if (result !== "AUTHORIZED") { throw new UnauthorizedError("Failed to authorize") } + return "authenticated" } catch (error) { - try { - await clearPendingAuth(runtime, serverName, oauthState, authStorageOptions) - } catch (cleanupError) { - throw new AggregateError([error, cleanupError], "OAuth completion cleanup failed") + caughtError = error + // RFC 9207: the AS advertises authorization_response_iss_parameter_supported + // but no `iss` accompanied the pasted code (e.g. the user pasted only the + // raw code). Keep the pending flow alive so the user can re-paste the full + // redirect URL instead of restarting authentication from scratch. + if (iss === undefined && error instanceof IssuerMismatchError && error.kind === "authorization_response") { + keepPendingForRetry = true + throw new Error( + `The authorization server for ${serverName} requires the RFC 9207 "iss" parameter. ` + + "Paste the full redirect URL from the browser address bar (not just the authorization code).", + ) } throw error + } finally { + if (!keepPendingForRetry) { + try { + await clearPendingAuth(runtime, serverName, oauthState, authStorageOptions) + } catch (cleanupError) { + if (caughtError !== undefined) { + throw new AggregateError([caughtError, cleanupError], "OAuth completion cleanup failed") + } + throw cleanupError + } + } } - - await clearPendingAuth(runtime, serverName, oauthState, authStorageOptions) - return "authenticated" } /** @@ -627,13 +668,13 @@ export async function authenticate( } // Wait for callback - const code = await abortable(callbackPromise, signal) + const callbackResult = await abortable(callbackPromise, signal) // The callback server accepted only the flow-local reserved state. throwIfAborted(signal) // Complete the auth - return await completeAuth(serverName, code, { ...options, signal, runtime }) + return await completeAuth(serverName, callbackResult, { ...options, signal, runtime }) } catch (error) { if (oauthState) cancelPendingCallback(oauthState) try { diff --git a/mcp-auth.ts b/mcp-auth.ts index 8dd53f4a..6a6d2e92 100644 --- a/mcp-auth.ts +++ b/mcp-auth.ts @@ -20,6 +20,8 @@ export interface StoredTokens { refreshToken?: string; expiresAt?: number; // Unix timestamp in seconds scope?: string; + /** SEP-2352 authorization-server issuer stamp from the SDK */ + issuer?: string; } /** OAuth client information from dynamic or static registration */ @@ -29,6 +31,16 @@ export interface StoredClientInfo { clientIdIssuedAt?: number; clientSecretExpiresAt?: number; redirectUris?: string[]; + /** SEP-2352 authorization-server issuer stamp from the SDK */ + issuer?: string; + /** + * True when this entry is a secretless SEP-2352 issuer stub persisted for a + * config-pre-registered client (written by the config-clientId path of + * saveClientInformation). Such a stub is only usable when paired with the + * config that supplies the client secret; it must never be served as + * standalone client information. + */ + configPreRegistered?: boolean; } /** Complete auth entry for a server */ diff --git a/mcp-callback-server.test.ts b/mcp-callback-server.test.ts index c3837f30..4b580fc5 100644 --- a/mcp-callback-server.test.ts +++ b/mcp-callback-server.test.ts @@ -126,7 +126,7 @@ describe("mcp-callback-server", () => { const callbackPromise = waitForCallback("custom-state") const response = await fetch(`http://127.0.0.1:${port}/custom/callback?code=ok&state=custom-state`) assert.strictEqual(response.status, 200) - assert.strictEqual(await callbackPromise, "ok") + assert.strictEqual((await callbackPromise).code, "ok") }) it("should reject an occupied explicit strict port", async () => { @@ -177,7 +177,7 @@ describe("mcp-callback-server", () => { const callbackPromise = waitForCallback(state) const response = await fetch(`http://localhost:${callbackPort}/callback?code=ok&state=${state}`) assert.strictEqual(response.status, 200) - assert.strictEqual(await callbackPromise, "ok") + assert.strictEqual((await callbackPromise).code, "ok") await assert.rejects( async () => await ensureCallbackServer({ strictPort: true }), @@ -211,8 +211,30 @@ describe("mcp-callback-server", () => { assert.ok(html.includes("Authorization Successful")) // Callback promise should resolve - const code = await callbackPromise - assert.strictEqual(code, expectedCode) + const result = await callbackPromise + assert.strictEqual(result.code, expectedCode) + assert.strictEqual(result.iss, undefined) + }) + + it("should propagate the RFC 9207 iss parameter when present", async () => { + await ensureCallbackServer() + + const state = "test-state-iss" + const expectedCode = "auth-code-iss" + const expectedIss = "https://auth.example.com" + + const callbackPromise = waitForCallback(state) + + const callbackPort = getOAuthCallbackPort() + const response = await fetch( + `http://localhost:${callbackPort}/callback?code=${expectedCode}&state=${state}&iss=${encodeURIComponent(expectedIss)}` + ) + assert.strictEqual(response.status, 200) + await response.text() + + const result = await callbackPromise + assert.strictEqual(result.code, expectedCode) + assert.strictEqual(result.iss, expectedIss) }) it("should reject on error parameter", async () => { diff --git a/mcp-callback-server.ts b/mcp-callback-server.ts index ebccc135..7f09983e 100644 --- a/mcp-callback-server.ts +++ b/mcp-callback-server.ts @@ -85,9 +85,16 @@ const HTML_ERROR = (error: string) => ` ` +/** Result of a successful OAuth callback */ +export interface OAuthCallbackResult { + code: string + /** RFC 9207 `iss` authorization response parameter, when provided */ + iss?: string +} + /** Pending authorization request */ interface PendingAuth { - resolve: (code: string) => void + resolve: (result: OAuthCallbackResult) => void reject: (error: Error) => void timeout: ReturnType } @@ -129,6 +136,7 @@ function handleRequest(req: IncomingMessage, res: ServerResponse): void { } const code = url.searchParams.get("code") + const iss = url.searchParams.get("iss") const state = url.searchParams.get("state") const error = url.searchParams.get("error") const errorDescription = url.searchParams.get("error_description") @@ -191,7 +199,7 @@ function handleRequest(req: IncomingMessage, res: ServerResponse): void { // Clear timeout and resolve the pending promise clearTimeout(pending.timeout) pendingAuths.delete(state) - pending.resolve(code) + pending.resolve({ code, ...(iss !== null ? { iss } : {}) }) res.writeHead(200, { "Content-Type": "text/html" }) res.end(HTML_SUCCESS) @@ -336,9 +344,10 @@ export function releaseCallbackServer(oauthState: string): void { /** * Wait for a callback with the given OAuth state. - * Returns a promise that resolves with the authorization code. + * Returns a promise that resolves with the authorization code and, when the + * authorization server sends one, the RFC 9207 `iss` parameter. */ -export function waitForCallback(oauthState: string): Promise { +export function waitForCallback(oauthState: string): Promise { reservedAuthStates.delete(oauthState) return new Promise((resolve, reject) => { const timeout = setTimeout(() => { diff --git a/mcp-oauth-provider.test.ts b/mcp-oauth-provider.test.ts index f318e19d..2f85a0b3 100644 --- a/mcp-oauth-provider.test.ts +++ b/mcp-oauth-provider.test.ts @@ -22,8 +22,8 @@ import { type McpOAuthConfig, } from "./mcp-oauth-provider.ts" import { getAuthForUrl, saveAuthEntry } from "./mcp-auth.ts" -import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js" -import type { OAuthClientInformationFull, OAuthTokens } from "@modelcontextprotocol/sdk/shared/auth.js" +import { UnauthorizedError } from "@modelcontextprotocol/client" +import type { OAuthClientInformationFull, OAuthTokens } from "@modelcontextprotocol/client" describe("McpOAuthProvider", () => { const serverName = "test-server" @@ -203,6 +203,51 @@ describe("McpOAuthProvider", () => { assert.strictEqual(info, undefined) }) + it("should not serve a config-pre-registered stub when no config clientId is present", async () => { + // Stub written by the config-clientId path of saveClientInformation + // (SEP-2352 stamp-and-resave): {clientId, issuer} with the marker. + const provider = createProvider() + saveAuthEntry(serverName, { + clientInfo: { + clientId: "config-client", + issuer: "https://auth.example.com", + configPreRegistered: true, + }, + serverUrl, + }, serverUrl) + + assert.strictEqual(await provider.clientInformation(), undefined) + }) + + it("should not serve a legacy unmarked {clientId, issuer} stub when no config clientId is present", async () => { + const provider = createProvider() + saveAuthEntry(serverName, { + clientInfo: { + clientId: "config-client", + issuer: "https://auth.example.com", + }, + serverUrl, + }, serverUrl) + + assert.strictEqual(await provider.clientInformation(), undefined) + }) + + it("should still serve a dynamically-registered public client (no secret) with registration metadata", async () => { + const provider = createProvider() + saveAuthEntry(serverName, { + clientInfo: { + clientId: "public-client", + clientIdIssuedAt: Math.floor(Date.now() / 1000), + redirectUris: ["http://localhost:19876/callback"], + }, + serverUrl, + }, serverUrl) + + const info = await provider.clientInformation() + assert.strictEqual(info?.client_id, "public-client") + assert.strictEqual(info?.client_secret, undefined) + }) + it("should prefer config over stored", async () => { const provider = createProvider({ clientId: "config-client" }) diff --git a/mcp-oauth-provider.ts b/mcp-oauth-provider.ts index 2b14532c..1f9b4868 100644 --- a/mcp-oauth-provider.ts +++ b/mcp-oauth-provider.ts @@ -5,20 +5,22 @@ * Handles OAuth client registration, token storage, and authorization redirection. */ -import type { AddClientAuthentication, OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js" -import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js" +import { UnauthorizedError } from "@modelcontextprotocol/client" import type { + AddClientAuthentication, + OAuthClientProvider, OAuthClientMetadata, - OAuthTokens, - OAuthClientInformation, - OAuthClientInformationFull, -} from "@modelcontextprotocol/sdk/shared/auth.js" + OAuthDiscoveryState, + StoredOAuthClientInformation, + StoredOAuthTokens, +} from "@modelcontextprotocol/client" import { getAuthForUrl, updateTokens, updateClientInfo, clearAllCredentials, clearClientInfo, + clearCodeVerifier, clearTokens, type AuthStorageOptions, type StoredTokens, @@ -86,6 +88,7 @@ export class McpOAuthProvider implements OAuthClientProvider { private active = true private flowClientInfo: StoredClientInfo | undefined private flowCodeVerifier: string | undefined + private flowDiscoveryState: OAuthDiscoveryState | undefined private flowState: string | undefined constructor( @@ -159,12 +162,17 @@ export class McpOAuthProvider implements OAuthClientProvider { * Get client information (for pre-registered or dynamically registered clients). * Returns undefined if no client info exists or if the server URL has changed. */ - async clientInformation(): Promise { + async clientInformation(): Promise { // Check config first (pre-registered client) if (this.config.clientId) { + // Reuse a previously stamped issuer (SEP-2352) if the SDK re-saved + // this pre-registered client with one. + const stored = await getAuthForUrl(this.serverName, this.serverUrl, this.storageOptions) + const issuer = stored?.clientInfo?.clientId === this.config.clientId ? stored.clientInfo.issuer : undefined return { client_id: this.config.clientId, client_secret: this.config.clientSecret, + ...(issuer !== undefined ? { issuer } : {}), } } @@ -172,13 +180,43 @@ export class McpOAuthProvider implements OAuthClientProvider { // another runtime writes the shared persistent entry for the same name. const clientInfo = this.flowClientInfo ?? (await getAuthForUrl(this.serverName, this.serverUrl, this.storageOptions))?.clientInfo if (clientInfo) { + // A stored SEP-2352 issuer stub for a config-pre-registered client + // (identified by the explicit marker, or by the legacy stub shape of + // {clientId, issuer} with no registration metadata) is only meaningful + // when the config supplies the matching client secret. Since we reach + // this branch only when config.clientId is absent, serving the stub + // would let a token refresh go out with a client_id but no secret, + // causing invalid_client and credential invalidation. Return undefined + // so callers treat this as "no client info". + const isConfigStub = clientInfo.configPreRegistered === true + || (clientInfo.clientSecret === undefined + && clientInfo.clientIdIssuedAt === undefined + && clientInfo.clientSecretExpiresAt === undefined + && clientInfo.redirectUris === undefined) + if (isConfigStub) { + return undefined + } // Check if client secret has expired if (clientInfo.clientSecretExpiresAt && clientInfo.clientSecretExpiresAt < Date.now() / 1000) { return undefined } + // Return all stored registration metadata so the SDK's SEP-2352 issuer + // stamp-and-resave (which round-trips clientInformation() back through + // saveClientInformation()) does not drop client_id_issued_at, + // client_secret_expires_at, or the registered redirect_uris. return { client_id: clientInfo.clientId, client_secret: clientInfo.clientSecret, + ...(clientInfo.clientIdIssuedAt !== undefined + ? { client_id_issued_at: clientInfo.clientIdIssuedAt } + : {}), + ...(clientInfo.clientSecretExpiresAt !== undefined + ? { client_secret_expires_at: clientInfo.clientSecretExpiresAt } + : {}), + ...(clientInfo.redirectUris !== undefined + ? { redirect_uris: clientInfo.redirectUris } + : {}), + ...(clientInfo.issuer !== undefined ? { issuer: clientInfo.issuer } : {}), } } @@ -189,16 +227,32 @@ export class McpOAuthProvider implements OAuthClientProvider { /** * Save client information from dynamic registration. */ - async saveClientInformation(info: OAuthClientInformationFull): Promise { - const redirectUris = info.redirect_uris ?? (this.redirectUrl ? [this.redirectUrl] : undefined) + async saveClientInformation(info: StoredOAuthClientInformation): Promise { + this.throwIfInactive() + // Pre-registered client from config: the SDK's SEP-2352 issuer stamp + // re-saves whatever clientInformation() returned. Persist only the + // issuer binding - never copy the config-supplied client secret into + // the on-disk auth store. + if (this.config.clientId && info.client_id === this.config.clientId) { + updateClientInfo( + this.serverName, + { clientId: info.client_id, issuer: info.issuer, configPreRegistered: true }, + this.serverUrl, + this.storageOptions, + ) + return + } + + const redirectUris = ("redirect_uris" in info ? info.redirect_uris : undefined) + ?? (this.redirectUrl ? [this.redirectUrl] : undefined) const clientInfo: StoredClientInfo = { clientId: info.client_id, clientSecret: info.client_secret, clientIdIssuedAt: info.client_id_issued_at, clientSecretExpiresAt: info.client_secret_expires_at, redirectUris, + issuer: info.issuer, } - this.throwIfInactive() this.flowClientInfo = clientInfo updateClientInfo(this.serverName, clientInfo, this.serverUrl, this.storageOptions) } @@ -207,7 +261,7 @@ export class McpOAuthProvider implements OAuthClientProvider { * Get stored OAuth tokens. * Returns undefined if no tokens exist or if the server URL has changed. */ - async tokens(): Promise { + async tokens(): Promise { // Use getAuthForUrl to validate tokens are for the current server URL const entry = await getAuthForUrl(this.serverName, this.serverUrl, this.storageOptions) if (!entry?.tokens) return undefined @@ -220,21 +274,31 @@ export class McpOAuthProvider implements OAuthClientProvider { ? Math.max(0, Math.floor(entry.tokens.expiresAt - Date.now() / 1000)) : undefined, scope: entry.tokens.scope, + ...(entry.tokens.issuer !== undefined ? { issuer: entry.tokens.issuer } : {}), } } /** * Save OAuth tokens. */ - async saveTokens(tokens: OAuthTokens): Promise { + async saveTokens(tokens: StoredOAuthTokens): Promise { const storedTokens: StoredTokens = { accessToken: tokens.access_token, refreshToken: tokens.refresh_token, - expiresAt: tokens.expires_in ? Date.now() / 1000 + tokens.expires_in : undefined, + // Preserve expiry even when expires_in is 0 (e.g. the SDK re-saving an + // already-expired token) so expired tokens stay expired instead of + // being persisted as never-expiring. + expiresAt: tokens.expires_in !== undefined ? Date.now() / 1000 + tokens.expires_in : undefined, scope: tokens.scope, + issuer: tokens.issuer, } this.throwIfInactive() updateTokens(this.serverName, storedTokens, this.serverUrl, this.storageOptions) + // Discovery must survive the browser redirect so the callback can verify + // the authorization server that minted the code. Once token issuance + // succeeds, clear it so a later 401 re-reads PRM and can observe an + // authorization-server migration. + this.flowDiscoveryState = undefined } /** @@ -284,6 +348,20 @@ export class McpOAuthProvider implements OAuthClientProvider { return this.flowCodeVerifier } + /** + * Persist discovery with the same durability as the PKCE verifier. SDK v2 + * reads this on the callback leg to prevent authorization-code mix-up. + */ + async saveDiscoveryState(state: OAuthDiscoveryState): Promise { + this.throwIfInactive() + this.flowDiscoveryState = structuredClone(state) + } + + async discoveryState(): Promise { + this.throwIfInactive() + return this.flowDiscoveryState ? structuredClone(this.flowDiscoveryState) : undefined + } + /** * Save the OAuth state parameter for CSRF protection. */ @@ -313,12 +391,13 @@ export class McpOAuthProvider implements OAuthClientProvider { * Invalidate credentials when authentication fails. * Clears tokens, client info, or all credentials based on the type. */ - async invalidateCredentials(type: "all" | "client" | "tokens"): Promise { + async invalidateCredentials(type: "all" | "client" | "tokens" | "verifier" | "discovery"): Promise { this.throwIfInactive() switch (type) { case "all": this.flowClientInfo = undefined this.flowCodeVerifier = undefined + this.flowDiscoveryState = undefined this.flowState = undefined clearAllCredentials(this.serverName, this.storageOptions) break @@ -329,6 +408,12 @@ export class McpOAuthProvider implements OAuthClientProvider { case "tokens": clearTokens(this.serverName, this.storageOptions) break + case "verifier": + clearCodeVerifier(this.serverName, this.storageOptions) + break + case "discovery": + this.flowDiscoveryState = undefined + break } } diff --git a/oauth-handler.ts b/oauth-handler.ts index 6e9c0423..347225d4 100644 --- a/oauth-handler.ts +++ b/oauth-handler.ts @@ -1,6 +1,6 @@ // oauth-handler.ts - OAuth token management for MCP servers import { existsSync, readFileSync } from "node:fs"; -import type { OAuthTokens } from "@modelcontextprotocol/sdk/shared/auth.js"; +import type { OAuthTokens } from "@modelcontextprotocol/client"; import { getAuthEntryFilePath } from "./mcp-auth.ts"; // Token storage path for a server diff --git a/package-lock.json b/package-lock.json index 42d8305e..0c38ed6b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,9 +8,12 @@ "name": "pi-mcp-adapter", "version": "2.11.0", "license": "MIT", + "engines": { + "node": ">=20" + }, "dependencies": { + "@modelcontextprotocol/client": "2.0.0-beta.5", "@modelcontextprotocol/ext-apps": "^1.2.2", - "@modelcontextprotocol/sdk": "^1.25.1", "cross-spawn": "^7.0.6", "open": "^10.2.0", "recheck": "^4.5.0", @@ -24,6 +27,8 @@ "@earendil-works/pi-ai": "0.74.2", "@earendil-works/pi-coding-agent": "0.79.10", "@earendil-works/pi-tui": "0.74.2", + "@modelcontextprotocol/conformance": "0.1.16", + "@modelcontextprotocol/server": "2.0.0-beta.5", "@types/bun": "^1.0.0", "@types/node": "^20.0.0", "@types/open": "^6.2.1", @@ -2945,6 +2950,57 @@ } } }, + "node_modules/@modelcontextprotocol/client": { + "version": "2.0.0-beta.5", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/client/-/client-2.0.0-beta.5.tgz", + "integrity": "sha512-YuuNm5f2TMoFQRje1UqVP8TJRjijCXMz4ckvoVpx1cUXuBEmykWQ2d8R536pek6UKcXT41T5nWc4qR1JFIbEmg==", + "license": "MIT", + "dependencies": { + "@modelcontextprotocol/core": "2.0.0-beta.5", + "cross-spawn": "^7.0.5", + "eventsource": "^3.0.2", + "eventsource-parser": "^3.0.0", + "jose": "^6.1.3", + "pkce-challenge": "^5.0.0", + "zod": "^4.2.0" + }, + "engines": { + "node": ">=20" + } + }, + "node_modules/@modelcontextprotocol/conformance": { + "version": "0.1.16", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/conformance/-/conformance-0.1.16.tgz", + "integrity": "sha512-GI7qiN0r39/MH2srVUR3AXaEN0YLCro20lIBbnvc1frBhszenxvUifBuTzxeVQVagILfBzCIcnungUOma8OrgA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@modelcontextprotocol/sdk": "^1.27.1", + "@octokit/rest": "^22.0.0", + "commander": "^14.0.2", + "eventsource-parser": "^3.0.6", + "express": "^5.1.0", + "jose": "^6.1.2", + "undici": "^7.19.0", + "yaml": "^2.8.2", + "zod": "^4.3.6" + }, + "bin": { + "conformance": "dist/index.js" + } + }, + "node_modules/@modelcontextprotocol/core": { + "version": "2.0.0-beta.5", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/core/-/core-2.0.0-beta.5.tgz", + "integrity": "sha512-HKbY9XTbsDy1Y6r2I55TGE3JEapM0vg96e1MUmBIF9LGjos5gjhcIrTz1yvBPLg2aFKHjwhUAQfRdrCEnPxNew==", + "license": "MIT", + "dependencies": { + "zod": "^4.2.0" + }, + "engines": { + "node": ">=20" + } + }, "node_modules/@modelcontextprotocol/ext-apps": { "version": "1.7.4", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/ext-apps/-/ext-apps-1.7.4.tgz", @@ -3014,6 +3070,201 @@ } } }, + "node_modules/@modelcontextprotocol/server": { + "version": "2.0.0-beta.5", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/server/-/server-2.0.0-beta.5.tgz", + "integrity": "sha512-i1E5l75rQKsgY/AKAIspgMBH1vEL7dqiK7tHr0L+raYcb0SWOziqNGJXGIG6NY4AlXDWIKGJQGB7Nqfs3oUi5g==", + "dev": true, + "license": "MIT", + "dependencies": { + "@modelcontextprotocol/core": "2.0.0-beta.5", + "zod": "^4.2.0" + }, + "engines": { + "node": ">=20" + } + }, + "node_modules/@octokit/auth-token": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-6.0.0.tgz", + "integrity": "sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/core": { + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-7.0.6.tgz", + "integrity": "sha512-DhGl4xMVFGVIyMwswXeyzdL4uXD5OGILGX5N8Y+f6W7LhC1Ze2poSNrkF/fedpVDHEEZ+PHFW0vL14I+mm8K3Q==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/auth-token": "^6.0.0", + "@octokit/graphql": "^9.0.3", + "@octokit/request": "^10.0.6", + "@octokit/request-error": "^7.0.2", + "@octokit/types": "^16.0.0", + "before-after-hook": "^4.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/endpoint": { + "version": "11.0.3", + "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-11.0.3.tgz", + "integrity": "sha512-FWFlNxghg4HrXkD3ifYbS/IdL/mDHjh9QcsNyhQjN8dplUoZbejsdpmuqdA76nxj2xoWPs7p8uX2SNr9rYu0Ag==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/graphql": { + "version": "9.0.3", + "resolved": "https://registry.npmjs.org/@octokit/graphql/-/graphql-9.0.3.tgz", + "integrity": "sha512-grAEuupr/C1rALFnXTv6ZQhFuL1D8G5y8CN04RgrO4FIPMrtm+mcZzFG7dcBm+nq+1ppNixu+Jd78aeJOYxlGA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/request": "^10.0.6", + "@octokit/types": "^16.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/openapi-types": { + "version": "27.0.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-27.0.0.tgz", + "integrity": "sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==", + "dev": true, + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest": { + "version": "14.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-paginate-rest/-/plugin-paginate-rest-14.0.0.tgz", + "integrity": "sha512-fNVRE7ufJiAA3XUrha2omTA39M6IXIc6GIZLvlbsm8QOQCYvpq/LkMNGyFlB1d8hTDzsAXa3OKtybdMAYsV/fw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-request-log": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-request-log/-/plugin-request-log-6.0.0.tgz", + "integrity": "sha512-UkOzeEN3W91/eBq9sPZNQ7sUBvYCqYbrrD8gTbBuGtHEuycE4/awMXcYvx6sVYo7LypPhmQwwpUe4Yyu4QZN5Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods": { + "version": "17.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-rest-endpoint-methods/-/plugin-rest-endpoint-methods-17.0.0.tgz", + "integrity": "sha512-B5yCyIlOJFPqUUeiD0cnBJwWJO8lkJs5d8+ze9QDP6SvfiXSz1BF+91+0MeI1d2yxgOhU/O+CvtiZ9jSkHhFAw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/request": { + "version": "10.0.11", + "resolved": "https://registry.npmjs.org/@octokit/request/-/request-10.0.11.tgz", + "integrity": "sha512-+s7HUxjfFqOMS9VlIwDffq0MikjSAK0gSpG73W+meAvVAvX4MBrHYTK5Bj3Uot55qFT4gzUtfzE4mGWY4Br8/Q==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/endpoint": "^11.0.3", + "@octokit/request-error": "^7.0.2", + "@octokit/types": "^16.0.0", + "content-type": "^2.0.0", + "json-with-bigint": "^3.5.3", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/request-error": { + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-7.1.0.tgz", + "integrity": "sha512-KMQIfq5sOPpkQYajXHwnhjCC0slzCNScLHs9JafXc4RAJI+9f+jNDlBNaIMTvazOPLgb4BnlhGJOTbnN0wIjPw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/request/node_modules/content-type": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.0.0.tgz", + "integrity": "sha512-j/O/d7GcZCyNl7/hwZAb606rzqkyvaDctLmckbxLzHvFBzTJHuGEdodATcP3yIRoDrLHkIATJuvzbFlp/ki2cQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, + "node_modules/@octokit/rest": { + "version": "22.0.1", + "resolved": "https://registry.npmjs.org/@octokit/rest/-/rest-22.0.1.tgz", + "integrity": "sha512-Jzbhzl3CEexhnivb1iQ0KJ7s5vvjMWcmRtq5aUsKmKDrRW6z3r84ngmiFKFvpZjpiU/9/S6ITPFRpn5s/3uQJw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/core": "^7.0.6", + "@octokit/plugin-paginate-rest": "^14.0.0", + "@octokit/plugin-request-log": "^6.0.0", + "@octokit/plugin-rest-endpoint-methods": "^17.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/types": { + "version": "16.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-16.0.0.tgz", + "integrity": "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^27.0.0" + } + }, "node_modules/@opentelemetry/semantic-conventions": { "version": "1.41.1", "resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.41.1.tgz", @@ -3849,6 +4100,13 @@ ], "license": "MIT" }, + "node_modules/before-after-hook": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-4.0.0.tgz", + "integrity": "sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==", + "dev": true, + "license": "Apache-2.0" + }, "node_modules/bignumber.js": { "version": "9.3.1", "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.3.1.tgz", @@ -4010,6 +4268,16 @@ "node": ">= 16" } }, + "node_modules/commander": { + "version": "14.0.3", + "resolved": "https://registry.npmjs.org/commander/-/commander-14.0.3.tgz", + "integrity": "sha512-H+y0Jo/T1RZ9qPP4Eh1pkcQcLRglraJaSLoyOtHxu6AapkjWVCy2Sit1QQ4x3Dng8qDlSsZEet7g5Pq06MvTgw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=20" + } + }, "node_modules/content-disposition": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-1.1.0.tgz", @@ -4902,6 +5170,13 @@ "integrity": "sha512-fQhoXdcvc3V28x7C7BMs4P5+kNlgUURe2jmUT1T//oBRMDrqy1QPelJimwZGo7Hg9VPV3EQV5Bnq4hbFy2vetA==", "license": "BSD-2-Clause" }, + "node_modules/json-with-bigint": { + "version": "3.5.10", + "resolved": "https://registry.npmjs.org/json-with-bigint/-/json-with-bigint-3.5.10.tgz", + "integrity": "sha512-Vcx+JVNEBts/xfcoCS69sKrOhOk/3TVlvlT+XzUOefVKnnrbYSCKpDCm10pohsJFtsJVYnwa/cXRZ4eElzaM6w==", + "dev": true, + "license": "MIT" + }, "node_modules/jwa": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/jwa/-/jwa-2.0.1.tgz", @@ -5966,6 +6241,16 @@ "node": ">=14.17" } }, + "node_modules/undici": { + "version": "7.28.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.28.0.tgz", + "integrity": "sha512-cRZYrTDwWznlnRiPjggAGxZXanty6M8RV1ff8Wm4LWXBp7/IG8v5DnOm74DtUBp9OONpK75YlPnIjQqX0dBDtA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=20.18.1" + } + }, "node_modules/undici-types": { "version": "6.21.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.21.0.tgz", @@ -5973,6 +6258,13 @@ "dev": true, "license": "MIT" }, + "node_modules/universal-user-agent": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/universal-user-agent/-/universal-user-agent-7.0.3.tgz", + "integrity": "sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==", + "dev": true, + "license": "ISC" + }, "node_modules/unpipe": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/unpipe/-/unpipe-1.0.0.tgz", @@ -6247,6 +6539,22 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/yaml": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.0.tgz", + "integrity": "sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==", + "dev": true, + "license": "ISC", + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } + }, "node_modules/zod": { "version": "4.4.3", "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", diff --git a/package.json b/package.json index 3362c459..169d173a 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,9 @@ }, "license": "MIT", "author": "Nico Bailon", + "engines": { + "node": ">=20" + }, "bin": { "pi-mcp-adapter": "cli.js" }, @@ -25,7 +28,9 @@ "test": "vitest run", "test:watch": "vitest", "test:coverage": "vitest run --coverage", - "test:oauth-provider": "node --import tsx --test mcp-oauth-provider.test.ts" + "test:oauth": "node --import tsx --test --test-concurrency=1 mcp-auth.test.ts mcp-auth-flow.test.ts mcp-callback-server.test.ts mcp-oauth-provider.test.ts", + "test:oauth-provider": "node --import tsx --test mcp-oauth-provider.test.ts", + "test:conformance": "bash conformance/run.sh" }, "repository": { "type": "git", @@ -100,8 +105,8 @@ "LICENSE" ], "dependencies": { + "@modelcontextprotocol/client": "2.0.0-beta.5", "@modelcontextprotocol/ext-apps": "^1.2.2", - "@modelcontextprotocol/sdk": "^1.25.1", "cross-spawn": "^7.0.6", "open": "^10.2.0", "recheck": "^4.5.0", @@ -129,6 +134,8 @@ "@earendil-works/pi-ai": "0.74.2", "@earendil-works/pi-coding-agent": "0.79.10", "@earendil-works/pi-tui": "0.74.2", + "@modelcontextprotocol/conformance": "0.1.16", + "@modelcontextprotocol/server": "2.0.0-beta.5", "@types/bun": "^1.0.0", "@types/node": "^20.0.0", "@types/open": "^6.2.1", diff --git a/proxy-modes.ts b/proxy-modes.ts index 13dbed1f..af94d8f7 100644 --- a/proxy-modes.ts +++ b/proxy-modes.ts @@ -1,5 +1,5 @@ import type { AgentToolResult, ToolInfo } from "@earendil-works/pi-coding-agent"; -import { UrlElicitationRequiredError } from "@modelcontextprotocol/sdk/types.js"; +import { UrlElicitationRequiredError } from "@modelcontextprotocol/client"; import { createRequire } from "node:module"; import type { McpExtensionState } from "./state.ts"; import type { ToolMetadata, McpContent } from "./types.ts"; @@ -1050,11 +1050,11 @@ export async function executeCall( name: toolMeta.originalName, arguments: args ?? {}, _meta: uiSession?.requestMeta, - }, undefined, requestOptions), ownedSignal), + }, requestOptions), ownedSignal), ); if (toolMeta.uiResourceUri) { - uiSession?.sendToolResult(result as unknown as import("@modelcontextprotocol/sdk/types.js").CallToolResult); + uiSession?.sendToolResult(result as unknown as import("@modelcontextprotocol/client").CallToolResult); if (result.isError) { const mcpContent = (result.content ?? []) as McpContent[]; diff --git a/sampling-handler.ts b/sampling-handler.ts index 274ca8af..66bb67c6 100644 --- a/sampling-handler.ts +++ b/sampling-handler.ts @@ -2,15 +2,14 @@ import { complete, type Api, type AssistantMessage, type Message, type Model, ty import { truncateAtWord } from "./utils.ts"; import { throwIfAborted } from "./abort.ts"; import type { ExtensionUIContext, ModelRegistry } from "@earendil-works/pi-coding-agent"; -import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { - CreateMessageRequestSchema, - type CreateMessageRequest, - type CreateMessageResult, - type ModelPreferences, - type SamplingMessage, - type SamplingMessageContentBlock, -} from "@modelcontextprotocol/sdk/types.js"; +import type { + Client, + CreateMessageRequest, + CreateMessageResult, + ModelPreferences, + SamplingMessage, + SamplingMessageContentBlock, +} from "@modelcontextprotocol/client"; export interface SamplingHandlerOptions { serverName: string; @@ -24,7 +23,7 @@ export interface SamplingHandlerOptions { export type ServerSamplingConfig = Omit; export function registerSamplingHandler(client: Client, options: SamplingHandlerOptions): void { - client.setRequestHandler(CreateMessageRequestSchema, (request) => { + client.setRequestHandler("sampling/createMessage", (request) => { return handleSamplingRequest(options, request as CreateMessageRequest); }); } diff --git a/server-manager.ts b/server-manager.ts index 98f7a83e..4ba98f87 100644 --- a/server-manager.ts +++ b/server-manager.ts @@ -1,14 +1,13 @@ -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; -import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; -import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; -import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"; -import type { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { - ElicitationCompleteNotificationSchema, + Client, + SSEClientTransport, + StreamableHTTPClientTransport, + UnauthorizedError, + type RequestOptions, type ReadResourceResult, type UrlElicitationRequiredError, -} from "@modelcontextprotocol/sdk/types.js"; +} from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import { isServerDisabled, type McpTool, @@ -16,8 +15,8 @@ import { type ServerDefinition, type ServerStreamResultPatchNotification, type Transport, - serverStreamResultPatchNotificationSchema, } from "./types.ts"; +import { SERVER_STREAM_RESULT_PATCH_METHOD, serverStreamResultPatchNotificationSchema } from "./types.ts"; import { resolveNpxBinary } from "./npx-resolver.ts"; import { logger } from "./logger.ts"; import { McpOAuthProvider } from "./mcp-oauth-provider.ts"; @@ -35,6 +34,10 @@ import { combineAbortSignals } from "./runtime-owner.ts"; const MAX_CAPTURED_STDERR_BYTES = 8 * 1024; const MAX_CAPTURED_STDERR_LINES = 3; +const MCP_CLIENT_OPTIONS = { + versionNegotiation: { mode: "auto" as const }, + inputRequired: { autoFulfill: true }, +}; const abortCleanupPromises = new WeakMap>(); function boundedStderrChunk(chunk: Buffer | string): Buffer { @@ -434,7 +437,10 @@ export class McpServerManager { const capabilities = this.buildClientCapabilities(); const client = new Client( { name: `pi-mcp-${serverName}`, version: "1.0.0" }, - Object.keys(capabilities).length > 0 ? { capabilities } : undefined, + { + ...MCP_CLIENT_OPTIONS, + ...(Object.keys(capabilities).length > 0 ? { capabilities } : {}), + }, ); if (this.samplingConfig) { registerSamplingHandler(client, { ...this.samplingConfig, serverName }); @@ -446,7 +452,7 @@ export class McpServerManager { onUrlAccepted: elicitationId => this.rememberUrlElicitation(serverName, elicitationId), }); if (this.elicitationConfig.allowUrl) { - client.setNotificationHandler(ElicitationCompleteNotificationSchema, notification => { + client.setNotificationHandler("notifications/elicitation/complete", notification => { if (this.runtimeSignal?.aborted) return; const accepted = this.acceptedUrlElicitations.get(serverName); if (!accepted?.delete(notification.params.elicitationId)) return; @@ -534,7 +540,10 @@ export class McpServerManager { authProvider, }); - const testClient = new Client({ name: "pi-mcp-probe", version: "2.1.2" }); + const testClient = new Client( + { name: "pi-mcp-probe", version: "2.1.2" }, + MCP_CLIENT_OPTIONS, + ); let probeCleanupAttempted = false; try { await this.connectClientWithAbort( @@ -623,11 +632,15 @@ export class McpServerManager { } private attachAdapterNotificationHandlers(serverName: string, client: Client): void { - client.setNotificationHandler(serverStreamResultPatchNotificationSchema, (notification) => { - const listener = this.uiStreamListeners.get(notification.params.streamToken); - if (!listener) return; - listener(serverName, notification.params); - }); + client.setNotificationHandler( + SERVER_STREAM_RESULT_PATCH_METHOD, + { params: serverStreamResultPatchNotificationSchema.shape.params }, + (params) => { + const listener = this.uiStreamListeners.get(params.streamToken); + if (!listener) return; + listener(serverName, params); + }, + ); } registerUiStreamListener(streamToken: string, listener: UiStreamListener): void { diff --git a/session-recovery.ts b/session-recovery.ts index 71c3490e..0ac93f2c 100644 --- a/session-recovery.ts +++ b/session-recovery.ts @@ -20,8 +20,7 @@ // many things other than "your session is gone" // - treat generic -32000/ConnectionClosed errors as session expiry // - treat AbortError/cancellation as a session failure -import { StreamableHTTPError } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; -import { ErrorCode, McpError } from "@modelcontextprotocol/sdk/types.js"; +import { ProtocolError, SdkHttpError } from "@modelcontextprotocol/client"; import { logger } from "./logger.ts"; import { throwIfAborted } from "./abort.ts"; import { isServerDisabled, type McpConfig } from "./types.ts"; @@ -34,26 +33,32 @@ import type { McpServerManager, ServerConnection } from "./server-manager.ts"; * gate response some servers emit before dispatching to a handler. * * `hadSessionId` must reflect the transport's session id from *before* the - * call that produced `err` was made. The installed SDK (1.29.0) happens not - * to clear `transport.sessionId` on a 404 response, so checking it at catch - * time currently agrees with checking it up front — but callers should - * capture it before the call rather than rely on that incidental behavior. + * call that produced `err` was made. The installed SDK (2.0.0-beta.5) only + * clears `transport.sessionId` on an explicit `terminateSession()`, so + * checking it at catch time currently agrees with checking it up front — but + * callers should capture it before the call rather than rely on that + * incidental behavior. */ +// JSON-RPC "connection closed" code emitted by servers as -32000 (the SDK v1 +// `ErrorCode.ConnectionClosed`; v2's `ProtocolErrorCode` no longer names it, +// but peers still send it on the wire). +const CONNECTION_CLOSED_CODE = -32000; + const SERVER_NOT_INITIALIZED_MCP_MESSAGES = new Set([ - `MCP error ${ErrorCode.ConnectionClosed}: Server not initialized`, - `MCP error ${ErrorCode.ConnectionClosed}: Bad Request: Server not initialized`, + "Server not initialized", + "Bad Request: Server not initialized", ]); export function isTerminatedSession(err: unknown, hadSessionId: boolean): boolean { if (!hadSessionId) return false; - if (err instanceof StreamableHTTPError) { - return err.code === 404 - || (err.code === 400 + if (err instanceof SdkHttpError) { + return err.status === 404 + || (err.status === 400 && /"code"\s*:\s*-32000/.test(err.message) && /"message"\s*:\s*"Bad Request: Server not initialized"/.test(err.message)); } - return err instanceof McpError - && err.code === ErrorCode.ConnectionClosed + return err instanceof ProtocolError + && err.code === CONNECTION_CLOSED_CODE && SERVER_NOT_INITIALIZED_MCP_MESSAGES.has(err.message); } diff --git a/types.ts b/types.ts index 6279776c..ab498d9c 100644 --- a/types.ts +++ b/types.ts @@ -1,7 +1,6 @@ // types.ts - Core type definitions -import type { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; -import type { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js"; -import type { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import type { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import type { SSEClientTransport, StreamableHTTPClientTransport } from "@modelcontextprotocol/client"; import type { TextContent, ImageContent } from "@earendil-works/pi-ai"; import type { UiStreamMode } from "./ui-stream-types.ts"; diff --git a/ui-resource-handler.ts b/ui-resource-handler.ts index 96702ebf..7a327be3 100644 --- a/ui-resource-handler.ts +++ b/ui-resource-handler.ts @@ -1,5 +1,5 @@ import { RESOURCE_MIME_TYPE } from "@modelcontextprotocol/ext-apps/app-bridge"; -import { UrlElicitationRequiredError, type ReadResourceResult } from "@modelcontextprotocol/sdk/types.js"; +import { UrlElicitationRequiredError, type ReadResourceResult } from "@modelcontextprotocol/client"; import { ResourceFetchError, ResourceParseError } from "./errors.ts"; import { logger } from "./logger.ts"; import { SessionRecoveryAuthRequiredError, withSessionRecovery, type SessionRecoveryDeps } from "./session-recovery.ts"; diff --git a/ui-server.ts b/ui-server.ts index 12b05943..a0370959 100644 --- a/ui-server.ts +++ b/ui-server.ts @@ -6,7 +6,7 @@ import { buildAllowAttribute } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { CallToolRequest, CallToolResult, -} from "@modelcontextprotocol/sdk/types.js"; +} from "@modelcontextprotocol/client"; import type { ConsentManager } from "./consent-manager.ts"; import { ServerError, wrapError } from "./errors.ts"; import { formatAuthRequiredMessage } from "./utils.ts"; @@ -369,9 +369,9 @@ export async function startUiServer(options: UiServerOptions): Promise conn.client.callTool(callArgs, undefined, options.manager.getRequestOptions?.(options.serverName)), + (conn) => conn.client.callTool(callArgs, options.manager.getRequestOptions?.(options.serverName)), ) - : await connection.client.callTool(callArgs, undefined, options.manager.getRequestOptions?.(options.serverName)); + : await connection.client.callTool(callArgs, options.manager.getRequestOptions?.(options.serverName)); sendJson(res, 200, { ok: true, result }); } finally { options.manager.decrementInFlight(options.serverName); diff --git a/ui-session.ts b/ui-session.ts index 768af371..6bc8460d 100644 --- a/ui-session.ts +++ b/ui-session.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { UrlElicitationRequiredError, type CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { UrlElicitationRequiredError, type CallToolResult } from "@modelcontextprotocol/client"; import type { McpExtensionState } from "./state.ts"; import { extractUiPromptText,