diff --git a/packages/core/src/mcp/client.ts b/packages/core/src/mcp/client.ts index f0051c31833..69b87bd7d07 100644 --- a/packages/core/src/mcp/client.ts +++ b/packages/core/src/mcp/client.ts @@ -245,7 +245,8 @@ export const connect = Effect.fnUntraced(function* ( return yield* open(url).pipe( Effect.catch((error) => { - if (!addedCodemode || !(error instanceof StreamableHTTPError) || error.code !== 404) return Effect.fail(error) + if (!addedCodemode || !(error instanceof StreamableHTTPError) || (error.code !== 400 && error.code !== 404)) + return Effect.fail(error) // Some servers reject unknown query params. Retry once with the user's original URL. return open(new URL(config.url)) }), diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index 79d6259cbd8..68039b86ce1 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -906,14 +906,16 @@ for (const entry of [ ) } -for (const query of ["", "?source=hello%20world&tag=a&tag=b"]) { - testEffect(Layer.empty).live(`retries an MCP initialization 404 with the original URL: ${query || "no query"}`, () => +for (const { status, query } of [400, 404].flatMap((status) => + ["", "?source=hello%20world&tag=a&tag=b"].map((query) => ({ status, query })), +)) { + testEffect(Layer.empty).live(`retries MCP initialization ${status} at the original URL: ${query || "no query"}`, () => Effect.gen(function* () { const headers: Array = [] const server = yield* resourceServer({ respond: (request) => { headers.push(request.headers.get("x-mcp-test")) - return new URL(request.url).searchParams.has("codemode") ? new Response(null, { status: 404 }) : undefined + return new URL(request.url).searchParams.has("codemode") ? new Response(null, { status }) : undefined }, }) const config = new ConfigMCP.Remote({ @@ -939,7 +941,7 @@ for (const query of ["", "?source=hello%20world&tag=a&tag=b"]) { for (const entry of [ { name: "second 404", status: 404, query: "", codemode: undefined, attempts: 2 }, - { name: "400", status: 400, query: "", codemode: undefined, attempts: 1 }, + { name: "second 400", status: 400, query: "", codemode: undefined, attempts: 2 }, { name: "401", status: 401, query: "", codemode: undefined, attempts: 1 }, { name: "403", status: 403, query: "", codemode: undefined, attempts: 1 }, { name: "500", status: 500, query: "", codemode: undefined, attempts: 1 }, @@ -947,6 +949,10 @@ for (const entry of [ { name: "user codemode=false", status: 404, query: "?codemode=false", codemode: undefined, attempts: 1 }, { name: "empty user codemode", status: 404, query: "?codemode=", codemode: undefined, attempts: 1 }, { name: "direct tools", status: 404, query: "", codemode: false, attempts: 1 }, + { name: "400 with user codemode=true", status: 400, query: "?codemode=true", codemode: undefined, attempts: 1 }, + { name: "400 with user codemode=false", status: 400, query: "?codemode=false", codemode: undefined, attempts: 1 }, + { name: "400 with empty user codemode", status: 400, query: "?codemode=", codemode: undefined, attempts: 1 }, + { name: "400 with direct tools", status: 400, query: "", codemode: false, attempts: 1 }, ]) { testEffect(Layer.empty).live(`does not retry MCP beyond the query fallback: ${entry.name}`, () => Effect.gen(function* () { @@ -970,23 +976,25 @@ for (const entry of [ ) } -testEffect(Layer.empty).live("does not strip codemode for an MCP 404 after initialization", () => - Effect.gen(function* () { - let expired = false - const server = yield* resourceServer({ - respond: (request) => (expired && request.method === "POST" ? new Response(null, { status: 404 }) : undefined), - }) - const config = new ConfigMCP.Remote({ type: "remote", url: server.url, oauth: false }) - const connection = yield* connect("resources", config, import.meta.dir) - expired = true - expect(yield* connection.tools().pipe(Effect.flip)).toBeInstanceOf(Error) +for (const status of [400, 404]) { + testEffect(Layer.empty).live(`does not strip codemode for an MCP ${status} after initialization`, () => + Effect.gen(function* () { + let expired = false + const server = yield* resourceServer({ + respond: (request) => (expired && request.method === "POST" ? new Response(null, { status }) : undefined), + }) + const config = new ConfigMCP.Remote({ type: "remote", url: server.url, oauth: false }) + const connection = yield* connect("resources", config, import.meta.dir) + expired = true + expect(yield* connection.tools().pipe(Effect.flip)).toBeInstanceOf(Error) - // The SDK tries to recover the expired session, but must keep the same URL. - expect(server.state.initializations).toBe(2) - expect(new Set(server.state.urls)).toEqual(new Set([server.url + "?codemode=false"])) - expect(server.state.toolLists).toBe(0) - }), -) + // The SDK tries to recover an expired session on 404, but must keep the same URL. + expect(server.state.initializations).toBe(status === 404 ? 2 : 1) + expect(new Set(server.state.urls)).toEqual(new Set([server.url + "?codemode=false"])) + expect(server.state.toolLists).toBe(0) + }), + ) +} test("lists, reads, and reports MCP resource changes", async () => { await Effect.runPromise(