From 0ef7e43fa3db1a830616509821d4807beff3226f Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Sun, 30 Aug 2026 14:50:45 +0530 Subject: [PATCH 1/3] fix(mcp): bound discovery connection teardown Discovery cleanup runs in an interruption-masked finalizer. Bound an unresponsive transport close so a completed tool listing cannot strand health checks or other callers indefinitely. --- .../mcp/src/sdk/discover-close.test.ts | 40 +++++++++++++++++++ packages/plugins/mcp/src/sdk/discover.ts | 24 ++++++----- 2 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 packages/plugins/mcp/src/sdk/discover-close.test.ts 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 0000000000..89ffd547fe --- /dev/null +++ b/packages/plugins/mcp/src/sdk/discover-close.test.ts @@ -0,0 +1,40 @@ +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, + }); + +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 startedAt = Date.now(); + const manifest = yield* discoverTools(hangingCloseConnector(state)); + + expect(state.closeStarted).toBe(true); + expect(Date.now() - startedAt).toBeLessThan(4_000); + 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 754333eb35..d3c672361d 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); From 0fce39dcfa66d04d8b7ecb99a3a9d1db20ef3855 Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Sun, 30 Aug 2026 14:55:39 +0530 Subject: [PATCH 2/3] test(mcp): match current discovery client contract --- packages/plugins/mcp/src/sdk/discover-close.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/plugins/mcp/src/sdk/discover-close.test.ts b/packages/plugins/mcp/src/sdk/discover-close.test.ts index 89ffd547fe..8930cf8db5 100644 --- a/packages/plugins/mcp/src/sdk/discover-close.test.ts +++ b/packages/plugins/mcp/src/sdk/discover-close.test.ts @@ -9,6 +9,7 @@ const discoveryClient = (): McpConnection["client"] => listTools: () => Promise.resolve({ tools: [] }), getServerVersion: () => ({ name: "hanging-close", version: "1.0.0" }), getInstructions: () => undefined, + setRequestHandler: () => undefined, }); const hangingCloseConnector = (state: { closeStarted: boolean }): McpConnector => From 08b52281deaf5cae9de82121ed9667eeb5bab08b Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Sun, 30 Aug 2026 15:07:52 +0530 Subject: [PATCH 3/3] test(mcp): avoid wall-clock teardown assertion --- packages/plugins/mcp/src/sdk/discover-close.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/plugins/mcp/src/sdk/discover-close.test.ts b/packages/plugins/mcp/src/sdk/discover-close.test.ts index 8930cf8db5..835b7d9dc0 100644 --- a/packages/plugins/mcp/src/sdk/discover-close.test.ts +++ b/packages/plugins/mcp/src/sdk/discover-close.test.ts @@ -25,11 +25,9 @@ describe("MCP discovery teardown", () => { it.live("does not strand discovery when close never settles", () => Effect.gen(function* () { const state = { closeStarted: false }; - const startedAt = Date.now(); const manifest = yield* discoverTools(hangingCloseConnector(state)); expect(state.closeStarted).toBe(true); - expect(Date.now() - startedAt).toBeLessThan(4_000); expect(manifest.server).toEqual({ name: "hanging-close", version: "1.0.0",