diff --git a/.changeset/mcp-pre-initialize-method-not-found.md b/.changeset/mcp-pre-initialize-method-not-found.md new file mode 100644 index 0000000000..f32473666b --- /dev/null +++ b/.changeset/mcp-pre-initialize-method-not-found.md @@ -0,0 +1,10 @@ +--- +"@executor-js/host-mcp": patch +"@executor-js/local": patch +--- + +**An unsupported method probed before `initialize` no longer kills the MCP connection** + +Only `initialize` can open a session, so the streamable-HTTP transport answered every other pre-session method with HTTP 400 + `-32000 Server not initialized`. A 400 is a transport-level failure, so clients dropped the connection instead of treating it as one request failing — a client that opens with an optional probe (MCP 2026-07-28 clients lead with `server/discover`) was disconnected before it could fall back to `initialize`. Over `executor mcp`, which bridges this endpoint to stdio, that closed the client's pipe outright. + +Pre-session dispatch now answers any method other than `initialize` with `-32601 Method not found` on a normal 200, which is a per-request error, so the connection survives and the handshake proceeds. This replaces only that one answer: a POST with a bad `Accept` or `Content-Type` still gets the transport's 406 or 415, and a message that is not a valid JSON-RPC request still gets its parse error. diff --git a/apps/local/src/mcp-pre-initialize.test.ts b/apps/local/src/mcp-pre-initialize.test.ts new file mode 100644 index 0000000000..2a97e1c8e9 --- /dev/null +++ b/apps/local/src/mcp-pre-initialize.test.ts @@ -0,0 +1,110 @@ +// --------------------------------------------------------------------------- +// Local app × the pre-initialize dispatch guard — through the real handler +// --------------------------------------------------------------------------- +// +// `executor mcp` bridges a stdio client to this handler's `/mcp` endpoint, so +// what the handler answers on a session-less POST decides whether the client's +// connection survives its first probe: +// +// test → createMcpRequestHandler().handleRequest(Request) +// → pre-initialize guard (an unknown method -> -32601 on a 200) +// → WebStandardStreamableHTTPServerTransport (everything else) +// +// The guard replaces exactly ONE transport answer — the connection-killing +// `400 -32000 Server not initialized` for a method that is not `initialize`. +// These assert that replacement AND that no other transport answer is shadowed: +// a request that fails content negotiation still gets the transport's 415/406, +// and a structurally invalid JSON-RPC message still gets its parse error, not a +// "method not found" that was never true. +// --------------------------------------------------------------------------- + +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import type { ExecutionEngine } from "@executor-js/execution"; + +import { createMcpRequestHandler } from "./mcp"; + +/** The headers a streamable-HTTP client must send on a POST; less is a 406/415. */ +const MCP_POST_HEADERS = { + "content-type": "application/json", + accept: "application/json, text/event-stream", +} as const; + +/** No code ever runs here: these requests are answered before any tool call. */ +const stubEngine: ExecutionEngine = { + execute: () => Effect.succeed({ result: "unused" }), + executeWithPause: () => Effect.succeed({ status: "completed", result: { result: "unused" } }), + resume: () => Effect.succeed(null), + getPausedExecution: () => Effect.succeed(null), + pausedExecutionCount: () => Effect.succeed(0), + hasPausedExecutions: () => Effect.succeed(false), + getDescription: Effect.succeed("test executor"), + shutdown: Effect.void, +}; + +const post = ( + body: unknown, + headers: Record = MCP_POST_HEADERS, +): Promise => + createMcpRequestHandler({ engine: stubEngine }).handleRequest( + new Request("http://local.test/mcp", { method: "POST", headers, body: JSON.stringify(body) }), + ); + +interface JsonRpcErrorBody { + readonly error: { readonly code: number; readonly message: string }; +} + +describe("local MCP handler, pre-initialize", () => { + it("answers a valid unknown pre-session method with -32601 on a 200", async () => { + const response = await post({ jsonrpc: "2.0", id: 7, method: "server/discover", params: {} }); + + // 200, not the transport's 400: a per-request error the bridged client + // survives, so it can still fall back to `initialize`. + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ + jsonrpc: "2.0", + id: 7, + error: { code: -32601, message: "Method not found" }, + }); + }); + + it("passes a pre-session notification to the transport", async () => { + const response = await post({ jsonrpc: "2.0", method: "notifications/initialized" }); + + // A notification carries no id, so the guard may not answer it at all; + // whatever comes back is the transport's own answer. + const body = (await response.json()) as JsonRpcErrorBody; + expect(body.error.code).not.toBe(-32601); + expect(body.error.code).toBe(-32000); + }); + + it("leaves a structurally invalid request to the transport's parse error", async () => { + // A fractional id is not a JSON-RPC id, so this is not a request the guard + // may report an unknown method for. + const response = await post({ jsonrpc: "2.0", id: 1.5, method: "server/discover" }); + + expect(response.status).toBe(400); + const body = (await response.json()) as JsonRpcErrorBody; + expect(body.error.code).toBe(-32700); + expect(body.error.code).not.toBe(-32601); + }); + + it("leaves a wrong Content-Type to the transport's 415", async () => { + const response = await post( + { jsonrpc: "2.0", id: 1, method: "server/discover" }, + { "content-type": "text/plain", accept: MCP_POST_HEADERS.accept }, + ); + + expect(response.status).toBe(415); + }); + + it("leaves an incomplete Accept to the transport's 406", async () => { + const response = await post( + { jsonrpc: "2.0", id: 1, method: "server/discover" }, + { "content-type": "application/json", accept: "application/json" }, + ); + + expect(response.status).toBe(406); + }); +}); diff --git a/apps/local/src/mcp.ts b/apps/local/src/mcp.ts index a91a56eb9e..2de3221985 100644 --- a/apps/local/src/mcp.ts +++ b/apps/local/src/mcp.ts @@ -7,6 +7,7 @@ import { defaultMcpResource, jsonRpcErrorBody, mcpResourceKey, + preInitializeMethodNotFound, type McpResource, } from "@executor-js/host-mcp"; import { @@ -181,6 +182,13 @@ export const createMcpRequestHandler = ( return transport.handleRequest(request); } + // Pre-initialize dispatch: only `initialize` opens a session here, so a + // probe for anything else is answered -32601 instead of the transport's + // fatal 400. `executor mcp` bridges this endpoint to stdio, so that 400 + // would close the client's pipe before it could fall back to initialize. + const unsupported = await Effect.runPromise(preInitializeMethodNotFound(request)); + if (unsupported) return unsupported; + let created: McpServer | undefined; let createdSessionId: string | null = null; let resourceConfig: LocalMcpServerConfig | null = null; diff --git a/packages/hosts/mcp/src/envelope.test.ts b/packages/hosts/mcp/src/envelope.test.ts index 523dc60a0b..bdd31aaa4e 100644 --- a/packages/hosts/mcp/src/envelope.test.ts +++ b/packages/hosts/mcp/src/envelope.test.ts @@ -22,6 +22,7 @@ import { McpServingRoutes, McpDiscoveryRoutes, McpSessionStore, + preInitializeMethodNotFound, type McpResource, type McpDispatchResult, type Principal, @@ -204,6 +205,145 @@ it("dispatches toolkit MCP routes with the parsed toolkit resource", async () => }); }); +// --------------------------------------------------------------------------- +// The pre-initialize dispatch guard. Session-less, only `initialize` is servable, +// and the transport's answer for everything else is a connection-killing HTTP +// 400. These lock in the -32601-on-200 replacement and, just as importantly, +// everything it must NOT intercept. +// --------------------------------------------------------------------------- + +/** The headers a streamable-HTTP client must send on a POST; less is a 406/415. */ +const MCP_POST_HEADERS = { + "content-type": "application/json", + accept: "application/json, text/event-stream", +} as const; + +const postBody = (body: unknown, headers: Record = MCP_POST_HEADERS): Request => + new Request("https://host.test/mcp", { + method: "POST", + headers, + body: JSON.stringify(body), + }); + +const guard = (request: Request): Promise => + Effect.runPromise(preInitializeMethodNotFound(request)); + +describe("preInitializeMethodNotFound", () => { + it("answers an unknown pre-init method with -32601 on a 200, echoing the id", async () => { + const response = await guard( + postBody({ jsonrpc: "2.0", id: 7, method: "server/discover", params: {} }), + ); + expect(response).not.toBeNull(); + // 200, not 400: a per-request error the client can survive, which is the + // entire point — a 400 makes clients tear the transport down. + expect(response!.status).toBe(200); + expect(response!.headers.get("content-type")).toContain("application/json"); + expect(await response!.json()).toEqual({ + jsonrpc: "2.0", + id: 7, + error: { code: -32601, message: "Method not found" }, + }); + }); + + it("generalizes past server/discover to any unknown method, including a string id", async () => { + const response = await guard( + postBody({ jsonrpc: "2.0", id: "abc", method: "some/futureProbe" }), + ); + expect(await response!.json()).toEqual({ + jsonrpc: "2.0", + id: "abc", + error: { code: -32601, message: "Method not found" }, + }); + }); + + it("lets initialize through to the transport", async () => { + expect( + await guard(postBody({ jsonrpc: "2.0", id: 1, method: "initialize", params: {} })), + ).toBeNull(); + }); + + it("lets a notification through — there is no id to answer", async () => { + expect( + await guard(postBody({ jsonrpc: "2.0", method: "notifications/initialized" })), + ).toBeNull(); + }); + + it("lets non-POST, non-JSON, and non-JSON-RPC bodies through", async () => { + expect(await guard(new Request("https://host.test/mcp"))).toBeNull(); + expect( + await guard( + new Request("https://host.test/mcp", { + method: "POST", + headers: MCP_POST_HEADERS, + body: "not json", + }), + ), + ).toBeNull(); + expect(await guard(postBody({ id: 1, method: "tools/list" }))).toBeNull(); + expect(await guard(postBody([]))).toBeNull(); + }); + + // The guard replaces ONE transport answer (-32000 on a 400) and must not + // shadow the others. A structurally invalid JSON-RPC request is the + // transport's 400 parse error to give, not ours to call "method not found". + it("lets a structurally invalid JSON-RPC request through to the transport", async () => { + // A fractional id is not a request id (the SDK's RequestIdSchema is + // string | integer), so the transport rejects the whole message. + expect(await guard(postBody({ jsonrpc: "2.0", id: 1.5, method: "tools/list" }))).toBeNull(); + // `params` must be an object when present. + expect( + await guard(postBody({ jsonrpc: "2.0", id: 1, method: "tools/list", params: 5 })), + ).toBeNull(); + // The request schema is strict: an unknown top-level field is invalid. + expect( + await guard(postBody({ jsonrpc: "2.0", id: 1, method: "tools/list", extra: true })), + ).toBeNull(); + // A wrong protocol version, and a batch, which the transport unpacks itself. + expect(await guard(postBody({ jsonrpc: "1.0", id: 1, method: "tools/list" }))).toBeNull(); + expect(await guard(postBody([{ jsonrpc: "2.0", id: 1, method: "tools/list" }]))).toBeNull(); + }); + + // Answering 200 here would bypass the transport's content negotiation, which + // runs before it ever looks at the body. + it("lets a request that fails the transport's content negotiation through", async () => { + const valid = { jsonrpc: "2.0", id: 1, method: "server/discover" }; + // Content-Type is not application/json, or is absent -> the 415. + expect( + await guard( + postBody(valid, { "content-type": "text/plain", accept: MCP_POST_HEADERS.accept }), + ), + ).toBeNull(); + expect(await guard(postBody(valid, { accept: MCP_POST_HEADERS.accept }))).toBeNull(); + // Accept misses one of the two required types, or is absent -> the 406. + expect( + await guard(postBody(valid, { ...MCP_POST_HEADERS, accept: "application/json" })), + ).toBeNull(); + expect( + await guard(postBody(valid, { ...MCP_POST_HEADERS, accept: "text/event-stream" })), + ).toBeNull(); + expect(await guard(postBody(valid, { "content-type": "application/json" }))).toBeNull(); + }); + + it("still fires when the negotiated headers carry parameters", async () => { + const response = await guard( + postBody( + { jsonrpc: "2.0", id: 3, method: "server/discover" }, + { + "content-type": "application/json; charset=utf-8", + accept: "application/json;q=0.9, text/event-stream;q=1.0", + }, + ), + ); + expect(response?.status).toBe(200); + }); + + it("leaves the caller's body readable for the transport", async () => { + const request = postBody({ jsonrpc: "2.0", id: 1, method: "server/discover" }); + await guard(request); + expect(await request.json()).toEqual({ jsonrpc: "2.0", id: 1, method: "server/discover" }); + }); +}); + describe("McpDiscoveryRoutes (discovery-only, no session store)", () => { // Builds with the auth seam ALONE — no McpSessionStore. This is the cloud // shape: the Agent bridge serves /mcp transport, the envelope only publishes diff --git a/packages/hosts/mcp/src/envelope.ts b/packages/hosts/mcp/src/envelope.ts index a2aa6756c2..8e0b5f2252 100644 --- a/packages/hosts/mcp/src/envelope.ts +++ b/packages/hosts/mcp/src/envelope.ts @@ -1,3 +1,4 @@ +import { isJSONRPCRequest } from "@modelcontextprotocol/sdk/types.js"; import { Effect, Match, Predicate } from "effect"; import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; @@ -107,25 +108,35 @@ export const jsonRpcErrorBody = ( readonly cors?: boolean; readonly challenge?: string; readonly retryAfterSeconds?: number; + /** + * The id to echo. Envelope-level errors have no request to answer and stay + * at the default `null`; only an error that answers ONE identified JSON-RPC + * request (the pre-initialize guard below) sets it, since a client matches + * the response to its pending request by id. + */ + readonly id?: string | number | null; }, ): Response => { const cors = opts?.cors ?? true; const challenge = opts?.challenge; const retryAfterSeconds = opts?.retryAfterSeconds; - return new Response(JSON.stringify({ jsonrpc: "2.0", error: { code, message }, id: null }), { - status, - headers: { - "content-type": "application/json", - ...(cors ? { "access-control-allow-origin": "*" } : {}), - ...(challenge - ? { - "www-authenticate": challenge, - "access-control-expose-headers": "WWW-Authenticate", - } - : {}), - ...(retryAfterSeconds === undefined ? {} : { "retry-after": String(retryAfterSeconds) }), + return new Response( + JSON.stringify({ jsonrpc: "2.0", error: { code, message }, id: opts?.id ?? null }), + { + status, + headers: { + "content-type": "application/json", + ...(cors ? { "access-control-allow-origin": "*" } : {}), + ...(challenge + ? { + "www-authenticate": challenge, + "access-control-expose-headers": "WWW-Authenticate", + } + : {}), + ...(retryAfterSeconds === undefined ? {} : { "retry-after": String(retryAfterSeconds) }), + }, }, - }); + ); }; /** @@ -135,6 +146,97 @@ export const jsonRpcErrorBody = ( */ export const UNAVAILABLE_RETRY_AFTER_SECONDS = 2; +/** JSON-RPC's own code for "this server does not implement that method". */ +const METHOD_NOT_FOUND = -32601; + +/** + * The transport's POST content negotiation, mirrored. + * + * `WebStandardStreamableHTTPServerTransport.handlePostRequest` refuses a POST + * on its headers alone, BEFORE it reads the body: + * + * - `Accept` must list BOTH `application/json` and `text/event-stream`, else + * 406 Not Acceptable. + * - `Content-Type` must include `application/json`, else 415 Unsupported + * Media Type. + * + * The guard below answers 200 without ever consulting the transport, so it must + * not fire on a request the transport would have refused — that would turn a + * 406 or a 415 into a success. These are the SDK's own substring tests on the + * raw header values, copied so the two agree; a request that fails either falls + * through untouched and gets the transport's real 406/415. + * + * The transport's DNS-rebinding check runs earlier still, but it is inert + * unless a host passes `enableDnsRebindingProtection`, which no host here does. + */ +const passesTransportContentNegotiation = (request: Request): boolean => { + const accept = request.headers.get("accept"); + if (!accept?.includes("application/json") || !accept.includes("text/event-stream")) return false; + const contentType = request.headers.get("content-type"); + return contentType !== null && contentType.includes("application/json"); +}; + +/** + * The pre-initialize dispatch guard, for the session-less POST every host + * handles before it hands the request to a fresh streamable-HTTP transport. + * + * Only `initialize` can open a session, so the transport answers every OTHER + * method with HTTP **400** + `-32000 Server not initialized`. A 400 is a + * transport-level failure: clients tear the connection down rather than treat + * it as one request failing. So a client that opens with an optional probe — + * MCP 2026-07-28 clients lead with `server/discover` — is disconnected before + * it can fall back to `initialize`, and the handshake never happens. + * + * JSON-RPC already has the right answer for a method the dispatcher doesn't + * know: `-32601 Method not found`, carried on a normal HTTP 200. It is a + * per-request error, so the connection survives and the client falls back. + * + * This deliberately covers ANY method other than `initialize` rather than + * naming `server/discover`: pre-session, `initialize` is the only method this + * dispatcher implements, so -32601 is the literal truth for the rest. It also + * can't silently mask a future real `server/discover` — implementing it means + * it stops being unknown here, instead of a special case going stale. + * + * The guard REPLACES one transport answer and must not shadow any of the + * others, so it fires only where it is the whole story: a POST that clears the + * transport's content negotiation AND carries a structurally valid JSON-RPC 2.0 + * request. Everything else succeeds with `null` and reaches the transport + * untouched, keeping the transport's own response — a non-POST, a bad + * `Accept`/`Content-Type` (406/415), unparseable JSON or a message that is not + * a valid JSON-RPC request (400 parse error: a fractional id, a non-object + * `params`, an unknown top-level field), a batch, a notification or a response + * (no id to answer), and `initialize` itself. + * + * Structural validity is decided by the SDK's own `isJSONRPCRequest`, the exact + * predicate behind the transport's `JSONRPCMessageSchema.parse`, rather than a + * hand-rolled re-implementation that could drift from it. + * + * Reads a clone, leaving the caller's request body intact for the transport. + */ +export const preInitializeMethodNotFound = (request: Request): Effect.Effect => + request.method !== "POST" || !passesTransportContentNegotiation(request) + ? Effect.succeed(null) + : Effect.tryPromise({ + try: (): Promise => request.clone().json(), + catch: () => null, + }).pipe( + // A body we cannot read is simply not ours to answer; hand it on. + Effect.orElseSucceed(() => null), + Effect.map(renderPreInitializeMethodNotFound), + ); + +/** The pure decision behind {@link preInitializeMethodNotFound}. */ +const renderPreInitializeMethodNotFound = (body: unknown): Response | null => { + if (!isJSONRPCRequest(body)) return null; + if (body.method === "initialize") return null; + // An INNER response like every other store/handler error body: the host's + // outer envelope owns the CORS headers on the way out. + return jsonRpcErrorBody(200, METHOD_NOT_FOUND, "Method not found", { + cors: false, + id: body.id, + }); +}; + /** The envelope's own CORS-on JSON-RPC error `Response`, optionally carrying a challenge. */ const jsonRpcResponse = ( status: number, diff --git a/packages/hosts/mcp/src/in-memory-session-store.test.ts b/packages/hosts/mcp/src/in-memory-session-store.test.ts index 8d87f56970..8a3db1ea75 100644 --- a/packages/hosts/mcp/src/in-memory-session-store.test.ts +++ b/packages/hosts/mcp/src/in-memory-session-store.test.ts @@ -1,12 +1,16 @@ -import { expect, it } from "@effect/vitest"; +import { describe, expect, it } from "@effect/vitest"; import { Effect } from "effect"; +import type { ExecutionEngine } from "@executor-js/execution"; + import { makeInMemoryMcpSessionStore, McpEngineBuildError, + type McpBuildServer, type McpBuildServerOptions, } from "./in-memory-session-store"; import { defaultMcpResource, type Principal } from "./seams"; +import { createExecutorMcpServer } from "./tool-server"; const TEST_PRINCIPAL: Principal = { accountId: "acct_test", @@ -52,3 +56,153 @@ it("preserves native elicitation mode when creating an in-memory MCP session", a expect((result as Response).status).toBe(500); expect(buildOptions?.elicitationMode).toEqual({ mode: "native" }); }); + +// --------------------------------------------------------------------------- +// The pre-initialize guard, through the real store path. +// +// `store.dispatch` with no session id runs the guard and, when the guard +// declines, builds a real MCP server and drives a real streamable-HTTP +// transport. So these assert BOTH halves of the contract: the one answer the +// guard replaces, and the transport answers it must not shadow. +// --------------------------------------------------------------------------- + +/** The headers a streamable-HTTP client must send on a POST; less is a 406/415. */ +const MCP_POST_HEADERS = { + "content-type": "application/json", + accept: "application/json, text/event-stream", +} as const; + +/** No code ever runs here: these requests are answered before any tool call. */ +const stubEngine: ExecutionEngine = { + execute: () => Effect.succeed({ result: "unused" }), + executeWithPause: () => Effect.succeed({ status: "completed", result: { result: "unused" } }), + resume: () => Effect.succeed(null), + getPausedExecution: () => Effect.succeed(null), + pausedExecutionCount: () => Effect.succeed(0), + hasPausedExecutions: () => Effect.succeed(false), + getDescription: Effect.succeed("test executor"), + shutdown: Effect.void, +}; + +/** A store whose sessions are real: a real MCP server on a real transport. */ +const makeServingStore = () => { + let builds = 0; + const buildServer: McpBuildServer = () => + Effect.sync(() => { + builds += 1; + }).pipe( + Effect.flatMap(() => createExecutorMcpServer({ engine: stubEngine })), + Effect.map((mcpServer) => ({ mcpServer, engine: stubEngine })), + ); + return { sessions: makeInMemoryMcpSessionStore(buildServer), buildCount: (): number => builds }; +}; + +const dispatchPost = ( + sessions: ReturnType["sessions"], + body: unknown, + headers: Record = MCP_POST_HEADERS, +): Promise => + Effect.runPromise( + sessions.store + .dispatch({ + request: new Request("https://executor.test/mcp", { + method: "POST", + headers, + body: JSON.stringify(body), + }), + principal: TEST_PRINCIPAL, + resource: defaultMcpResource, + sessionId: null, + method: "POST", + }) + .pipe( + Effect.map((result) => { + expect(result).toBeInstanceOf(Response); + return result as Response; + }), + ), + ); + +interface JsonRpcErrorBody { + readonly error: { readonly code: number; readonly message: string }; +} + +describe("pre-initialize dispatch through the in-memory session store", () => { + it("answers a valid unknown pre-session method with -32601 on a 200", async () => { + const { sessions, buildCount } = makeServingStore(); + const response = await dispatchPost(sessions, { + jsonrpc: "2.0", + id: 7, + method: "server/discover", + params: {}, + }); + + // 200, not the transport's 400: a per-request error the client survives. + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ + jsonrpc: "2.0", + id: 7, + error: { code: -32601, message: "Method not found" }, + }); + // The guard short-circuits before any engine is built. + expect(buildCount()).toBe(0); + await sessions.close(); + }); + + it("passes a pre-session notification to the transport", async () => { + const { sessions, buildCount } = makeServingStore(); + const response = await dispatchPost(sessions, { + jsonrpc: "2.0", + method: "notifications/initialized", + }); + + // A notification carries no id, so the guard may not answer it at all; + // whatever comes back is the transport's own answer. + const body = (await response.json()) as JsonRpcErrorBody; + expect(body.error.code).not.toBe(-32601); + expect(body.error.code).toBe(-32000); + expect(buildCount()).toBe(1); + await sessions.close(); + }); + + it("leaves a structurally invalid request to the transport's parse error", async () => { + const { sessions } = makeServingStore(); + // A fractional id is not a JSON-RPC id, so this is not a request the guard + // may report an unknown method for. + const response = await dispatchPost(sessions, { + jsonrpc: "2.0", + id: 1.5, + method: "server/discover", + }); + + expect(response.status).toBe(400); + const body = (await response.json()) as JsonRpcErrorBody; + expect(body.error.code).toBe(-32700); + expect(body.error.code).not.toBe(-32601); + await sessions.close(); + }); + + it("leaves a wrong Content-Type to the transport's 415", async () => { + const { sessions } = makeServingStore(); + const response = await dispatchPost( + sessions, + { jsonrpc: "2.0", id: 1, method: "server/discover" }, + { "content-type": "text/plain", accept: MCP_POST_HEADERS.accept }, + ); + + expect(response.status).toBe(415); + await sessions.close(); + }); + + it("leaves an incomplete Accept to the transport's 406", async () => { + const { sessions } = makeServingStore(); + const response = await dispatchPost( + sessions, + { jsonrpc: "2.0", id: 1, method: "server/discover" }, + { "content-type": "application/json", accept: "application/json" }, + ); + + expect(response.status).toBe(406); + await sessions.close(); + }); +}); diff --git a/packages/hosts/mcp/src/in-memory-session-store.ts b/packages/hosts/mcp/src/in-memory-session-store.ts index 869691648f..81cc4aa2de 100644 --- a/packages/hosts/mcp/src/in-memory-session-store.ts +++ b/packages/hosts/mcp/src/in-memory-session-store.ts @@ -16,7 +16,7 @@ import { makeInProcessBrowserApprovalStore, type InProcessBrowserApprovalStore, } from "./browser-approval-store"; -import { jsonRpcErrorBody } from "./envelope"; +import { jsonRpcErrorBody, preInitializeMethodNotFound } from "./envelope"; import { McpSessionStore, defaultMcpResource, @@ -252,7 +252,7 @@ export const makeInMemoryMcpSessionStore = ( }; /** Open a new session: build the server, connect a transport, drive the request. */ - const create = ( + const openSession = ( principal: Principal, resource: McpResource, request: Request, @@ -296,6 +296,25 @@ export const makeInMemoryMcpSessionStore = ( ); }; + /** + * The session-less POST path: answer a probe for anything but `initialize` + * with -32601 (see `preInitializeMethodNotFound`) rather than let the + * transport reject it with a connection-killing 400, and only then build a + * server and open a session. + */ + const create = ( + principal: Principal, + resource: McpResource, + request: Request, + ): Effect.Effect => + preInitializeMethodNotFound(request).pipe( + Effect.flatMap((unsupported) => + unsupported + ? Effect.succeed(unsupported) + : openSession(principal, resource, request), + ), + ); + const store: McpSessionStore["Service"] = { dispatch: ({ request, principal, resource, sessionId }: McpDispatchInput) => sessionId diff --git a/packages/hosts/mcp/src/index.ts b/packages/hosts/mcp/src/index.ts index 2e536296d9..c36fc10987 100644 --- a/packages/hosts/mcp/src/index.ts +++ b/packages/hosts/mcp/src/index.ts @@ -40,5 +40,6 @@ export { McpServingRoutes, McpDiscoveryRoutes, jsonRpcErrorBody, + preInitializeMethodNotFound, UNAVAILABLE_RETRY_AFTER_SECONDS, } from "./envelope"; diff --git a/packages/hosts/mcp/src/stdio-integration.test.ts b/packages/hosts/mcp/src/stdio-integration.test.ts index d66f9893fc..2f28914d59 100644 --- a/packages/hosts/mcp/src/stdio-integration.test.ts +++ b/packages/hosts/mcp/src/stdio-integration.test.ts @@ -1,8 +1,13 @@ import { describe, expect, it } from "@effect/vitest"; -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; -import { Effect } from "effect"; -import { mkdtempSync } from "node:fs"; +import { + ErrorCode, + LATEST_PROTOCOL_VERSION, + type JSONRPCMessage, +} from "@modelcontextprotocol/sdk/types.js"; +import { Effect, Option, Schema } from "effect"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; @@ -10,42 +15,197 @@ const repoRoot = resolve(import.meta.dirname, "../../../.."); const cliEntry = resolve(repoRoot, "apps/cli/src/main.ts"); const testScope = resolve(repoRoot, "apps/local"); +const decodeServerManifest = Schema.decodeUnknownOption( + Schema.fromJsonString(Schema.Struct({ pid: Schema.optional(Schema.Number) })), +); + +const manifestPid = (dataDir: string): number | undefined => + Effect.runSync( + Effect.try({ + try: () => readFileSync(join(dataDir, "server-control", "server.json"), "utf8"), + catch: () => undefined, + }).pipe( + Effect.map((text) => { + const manifest = decodeServerManifest(text); + return Option.isSome(manifest) ? manifest.value.pid : undefined; + }), + Effect.orElseSucceed(() => undefined), + ), + ); + +/** + * Own the daemon this test bridges to, on its own port. + * + * `executor mcp` otherwise elects a daemon on the shared default port, and will + * adopt one that is already listening there — including a daemon belonging to a + * different `EXECUTOR_DATA_DIR`, whose bearer token this test does not have. A + * dedicated port keeps the test hermetic and parallel-safe. `daemon run` falls + * back to a free port if this one is taken, and writes the port it chose into + * the manifest that `executor mcp` reads, so the exact number is not load-bearing. + */ +const startDaemon = (dataDir: string): void => { + const port = 20_000 + Math.floor(Math.random() * 20_000); + const result = spawnSync( + "bun", + ["run", cliEntry, "daemon", "run", "--port", String(port), "--hostname", "127.0.0.1"], + { env: { ...process.env, EXECUTOR_DATA_DIR: dataDir, EXECUTOR_SCOPE_DIR: testScope } }, + ); + expect(result.status, `daemon run failed: ${result.stderr?.toString() ?? ""}`).toBe(0); +}; + +/** Stop the daemon started above; the manifest carries its pid. */ +const stopDaemon = (dataDir: string): Effect.Effect => + Effect.sync(() => manifestPid(dataDir)).pipe( + Effect.flatMap((pid) => + pid + ? Effect.try({ try: () => process.kill(pid, "SIGTERM"), catch: () => undefined }).pipe( + Effect.ignore, + ) + : Effect.void, + ), + Effect.ignore, + ); + +const withDaemon = Effect.acquireRelease( + Effect.sync(() => { + const dataDir = mkdtempSync(join(tmpdir(), "executor-mcp-discover-test-")); + startDaemon(dataDir); + return dataDir; + }), + (dataDir) => + stopDaemon(dataDir).pipe( + Effect.ensuring(Effect.sync(() => rmSync(dataDir, { recursive: true, force: true }))), + ), +); + +const messageQueue = (transport: StdioClientTransport) => { + const messages: Array = []; + const waiters: Array<(message: JSONRPCMessage) => void> = []; + + transport.onmessage = (message) => { + const waiter = waiters.shift(); + if (waiter) { + waiter(message); + } else { + messages.push(message); + } + }; + + return { + next: (): Promise => { + const message = messages.shift(); + return message ? Promise.resolve(message) : new Promise((resolve) => waiters.push(resolve)); + }, + }; +}; + describe("MCP stdio integration", () => { + // The regression: a client that opens with an unsupported probe used to get + // the transport's fatal `-32000 Server not initialized` on an HTTP 400, which + // tore the bridge down before it could fall back to `initialize`. The whole + // handshake must survive the probe on ONE connection, through to a tool call. it.effect( - "execute tool returns result over stdio transport", + "unsupported discovery keeps the connection open for initialization and tool calls", () => Effect.gen(function* () { - // Fresh temp dir so the test doesn't migrate against the developer's - // real ~/.executor/data.db. - const dataDir = mkdtempSync(join(tmpdir(), "executor-mcp-test-")); - + const dataDir = yield* withDaemon; const transport = new StdioClientTransport({ command: "bun", args: ["run", cliEntry, "mcp", "--scope", testScope], env: { ...process.env, EXECUTOR_DATA_DIR: dataDir }, }); - - const client = new Client({ name: "test-client", version: "1.0.0" }, { capabilities: {} }); + const responses = messageQueue(transport); yield* Effect.acquireRelease( - Effect.promise(() => client.connect(transport)), + Effect.promise(() => transport.start()), () => Effect.promise(() => transport.close()), ); - const { tools } = yield* Effect.promise(() => client.listTools()); - expect(tools.map((t) => t.name)).toContain("execute"); + yield* Effect.promise(() => + transport.send({ + jsonrpc: "2.0", + id: 1, + method: "server/discover", + params: { + _meta: { + "io.modelcontextprotocol/protocolVersion": "2026-07-28", + "io.modelcontextprotocol/clientInfo": { + name: "discovery-test-client", + version: "1.0.0", + }, + "io.modelcontextprotocol/clientCapabilities": {}, + }, + }, + }), + ); - const result = yield* Effect.promise(() => - client.callTool({ - name: "execute", - arguments: { code: "return 2+2" }, + expect(yield* Effect.promise(() => responses.next())).toEqual({ + jsonrpc: "2.0", + id: 1, + error: { + code: ErrorCode.MethodNotFound, + message: "Method not found", + }, + }); + + yield* Effect.promise(() => + transport.send({ + jsonrpc: "2.0", + id: 2, + method: "initialize", + params: { + protocolVersion: LATEST_PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { + name: "discovery-test-client", + version: "1.0.0", + }, + }, + }), + ); + + const initialize = yield* Effect.promise(() => responses.next()); + expect(initialize).toHaveProperty("result.protocolVersion"); + + yield* Effect.promise(() => + transport.send({ + jsonrpc: "2.0", + method: "notifications/initialized", + }), + ); + yield* Effect.promise(() => + transport.send({ + jsonrpc: "2.0", + id: 3, + method: "tools/list", + params: {}, }), ); - const text = (result.content as Array<{ type: string; text: string }>)[0]?.text; - expect(text).toContain("4"); - expect(result.isError).toBeFalsy(); + const listed = yield* Effect.promise(() => responses.next()); + expect(listed).toHaveProperty( + "result.tools", + expect.arrayContaining([expect.objectContaining({ name: "execute" })]), + ); + + yield* Effect.promise(() => + transport.send({ + jsonrpc: "2.0", + id: 4, + method: "tools/call", + params: { + name: "execute", + arguments: { code: "return 2+2" }, + }, + }), + ); + + const called = yield* Effect.promise(() => responses.next()); + expect(called).toHaveProperty( + "result.content", + expect.arrayContaining([expect.objectContaining({ text: expect.stringContaining("4") })]), + ); }).pipe(Effect.scoped), - { timeout: 30_000 }, + { timeout: 120_000 }, ); });