From 0ef7e43fa3db1a830616509821d4807beff3226f Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Sun, 30 Aug 2026 14:50:45 +0530 Subject: [PATCH 1/4] 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/4] 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/4] 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", From 2a5c2a9b1ca7bd1ace0c1f7ca930afd9173f2ca5 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:36:03 -0700 Subject: [PATCH 4/4] Exercise discovery teardown with real MCP connections --- .../mcp/src/sdk/discover-close.test.ts | 83 +++++++++++++------ 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/packages/plugins/mcp/src/sdk/discover-close.test.ts b/packages/plugins/mcp/src/sdk/discover-close.test.ts index 835b7d9dc0..89f8e78e37 100644 --- a/packages/plugins/mcp/src/sdk/discover-close.test.ts +++ b/packages/plugins/mcp/src/sdk/discover-close.test.ts @@ -1,39 +1,68 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect } from "effect"; -import type { McpConnection, McpConnector } from "./connection"; +import { createMcpConnector, type McpConnector } from "./connection"; import { discoverTools } from "./discover"; +import { makeEchoMcpServer, serveMcpServer } from "../testing"; -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; +// 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("does not strand discovery when close never settles", () => - Effect.gen(function* () { - const state = { closeStarted: false }; - const manifest = yield* discoverTools(hangingCloseConnector(state)); + 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); + }), + ), + ); - expect(state.closeStarted).toBe(true); - expect(manifest.server).toEqual({ - name: "hanging-close", - version: "1.0.0", - instructions: null, - }); - expect(manifest.tools).toEqual([]); - }), + 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 }, + }); + }), + ), ); });