diff --git a/packages/plugins/mcp/src/sdk/discover-close.test.ts b/packages/plugins/mcp/src/sdk/discover-close.test.ts new file mode 100644 index 000000000..89f8e78e3 --- /dev/null +++ b/packages/plugins/mcp/src/sdk/discover-close.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { createMcpConnector, type McpConnector } from "./connection"; +import { discoverTools } from "./discover"; +import { makeEchoMcpServer, serveMcpServer } from "../testing"; + +// Exercise the real MCP handshake and catalog. The connector owns teardown, +// so a wrapper can reproduce a transport that closes its sockets but never +// settles its close promise without replacing the protocol client. +const hangingCloseConnector = (connector: McpConnector, state: { closes: number }): McpConnector => + Effect.map(connector, (connection) => ({ + client: connection.client, + close: async () => { + state.closes += 1; + await connection.close(); + return new Promise(() => {}); + }, + })); + +describe("MCP discovery teardown", () => { + it.live("preserves a real catalog when close never settles", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveMcpServer(() => makeEchoMcpServer({ name: "hanging-close" })); + const state = { closes: 0 }; + const manifest = yield* discoverTools( + hangingCloseConnector( + createMcpConnector({ + transport: "remote", + endpoint: server.url, + remoteTransport: "streamable-http", + }), + state, + ), + ); + expect(state.closes).toBe(1); + expect(manifest.server?.name).toBe("hanging-close"); + expect(manifest.tools.length).toBeGreaterThan(0); + }), + ), + ); + + it.live("preserves listing failure when close never settles", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveMcpServer(() => makeEchoMcpServer()); + yield* server.rejectSessionMethod("tools/list", 403); + const state = { closes: 0 }; + const result = yield* discoverTools( + hangingCloseConnector( + createMcpConnector({ + transport: "remote", + endpoint: server.url, + remoteTransport: "streamable-http", + }), + state, + ), + ).pipe(Effect.result); + expect(state.closes).toBe(1); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { stage: "list_tools", httpStatus: 403 }, + }); + }), + ), + ); +}); diff --git a/packages/plugins/mcp/src/sdk/discover.ts b/packages/plugins/mcp/src/sdk/discover.ts index 754333eb3..d3c672361 100644 --- a/packages/plugins/mcp/src/sdk/discover.ts +++ b/packages/plugins/mcp/src/sdk/discover.ts @@ -31,6 +31,12 @@ const MAX_LIST_TOOLS_PAGES = 100; // shape probe's single unauth POST. const DEFAULT_DISCOVER_TIMEOUT = Duration.seconds(15); +// Teardown is best-effort and paid for by the request that performed discovery. +// A remote transport may accept close and then never settle, so use the same +// bound as the invocation connection pool instead of stranding the caller in an +// uninterruptible finalizer after discovery itself has already completed. +const CLOSE_TIMEOUT = Duration.seconds(2); + // --------------------------------------------------------------------------- // Public API // --------------------------------------------------------------------------- @@ -243,13 +249,11 @@ export const discoverTools = ( const closeConnection = (connection: { readonly close: () => Promise; }): Effect.Effect => - Effect.ignore( - Effect.tryPromise({ - try: () => connection.close(), - catch: () => - new McpToolDiscoveryError({ - stage: "list_tools", - message: "Failed closing MCP connection", - }), - }), - ); + Effect.tryPromise({ + try: () => connection.close(), + catch: () => + new McpToolDiscoveryError({ + stage: "list_tools", + message: "Failed closing MCP connection", + }), + }).pipe(Effect.timeout(CLOSE_TIMEOUT), Effect.ignore);