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..835b7d9dc --- /dev/null +++ b/packages/plugins/mcp/src/sdk/discover-close.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import type { McpConnection, McpConnector } from "./connection"; +import { discoverTools } from "./discover"; + +const discoveryClient = (): McpConnection["client"] => + Object.assign(Object.create(null) as McpConnection["client"], { + listTools: () => Promise.resolve({ tools: [] }), + getServerVersion: () => ({ name: "hanging-close", version: "1.0.0" }), + getInstructions: () => undefined, + setRequestHandler: () => undefined, + }); + +const hangingCloseConnector = (state: { closeStarted: boolean }): McpConnector => + Effect.succeed({ + client: discoveryClient(), + close: () => { + state.closeStarted = true; + return new Promise(() => {}); + }, + }); + +describe("MCP discovery teardown", () => { + it.live("does not strand discovery when close never settles", () => + Effect.gen(function* () { + const state = { closeStarted: false }; + const manifest = yield* discoverTools(hangingCloseConnector(state)); + + expect(state.closeStarted).toBe(true); + expect(manifest.server).toEqual({ + name: "hanging-close", + version: "1.0.0", + instructions: null, + }); + expect(manifest.tools).toEqual([]); + }), + ); +}); 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);