Skip to content

Commit ca8bbf2

Browse files
committed
Bound MCP discovery with a probe deadline
discoverTools (the shared connect+listTools path behind resolveTools, detect, probeEndpoint, and checkHealth) had no timeout of its own, and neither the MCP SDK's connect handshake nor listTools call one either. An unresponsive endpoint (a closed loopback port after its e2e scenario's scope exits, a server wedged mid-handshake) hung the calling fiber, and with it the server-side request handling it ran under, indefinitely. Under CI shard load this showed up as auth-methods-ui.test.ts sitting on its 90s auto-probe wait and then hitting the full 120s vitest timeout, followed by every later test failing "login did not redirect to AuthKit (500)": the dev server's request-handling capacity was pinned by fibers stuck in an unbounded MCP connect, starving unrelated routes. Give discoverTools a default 15s deadline (Effect.timeoutOrElse), mapping a timeout to the same McpToolDiscoveryError("connect") shape a failed connect already produces, so callers' existing handling (auth classification, incomplete-tools fallback, health "degraded" status) all keeps working unchanged. Connections that DID get established before the deadline still close via the existing Effect.onExit handler, which fires on interruption too. Note: discoverTools already closed its connection deterministically via Effect.onExit — there was no connector/session leak. The bug was purely missing deadline, not missing cleanup.
1 parent dbc2053 commit ca8bbf2

1 file changed

Lines changed: 35 additions & 2 deletions

File tree

packages/plugins/mcp/src/sdk/discover.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// MCP tool discovery — connect to an MCP server and list its tools
33
// ---------------------------------------------------------------------------
44

5-
import { Effect, Option, Predicate } from "effect";
5+
import { Duration, Effect, Option, Predicate } from "effect";
66

77
import type { McpConnection, McpConnector } from "./connection";
88
import { McpToolDiscoveryError } from "./errors";
@@ -17,6 +17,18 @@ import {
1717
// omitting `nextCursor`, so any real catalog fits well inside this.
1818
const MAX_LIST_TOOLS_PAGES = 100;
1919

20+
// Every caller of `discoverTools` (resolveTools, detect, probeEndpoint,
21+
// checkHealth) is a probe: dial a server we don't control and list its
22+
// tools. Neither the MCP SDK's transport connect nor its `listTools` call
23+
// carries a deadline of its own against a server that never answers (a
24+
// closed/half-open loopback socket, a server wedged mid-handshake), so
25+
// without a bound here a single unresponsive endpoint hangs the calling
26+
// fiber forever. Mirrors the shape-probe fallback's default
27+
// (`probeMcpEndpointShape`'s `timeoutMs = 8_000`) at a slightly longer
28+
// bound since a real handshake + listTools round-trip is heavier than the
29+
// shape probe's single unauth POST.
30+
const DEFAULT_DISCOVER_TIMEOUT = Duration.seconds(15);
31+
2032
// ---------------------------------------------------------------------------
2133
// Public API
2234
// ---------------------------------------------------------------------------
@@ -70,9 +82,19 @@ const listAllTools = (
7082
/**
7183
* Connect to an MCP server and discover all available tools.
7284
* Returns the parsed manifest containing server metadata and tool entries.
85+
*
86+
* Bounded by `timeoutMs` (default `DEFAULT_DISCOVER_TIMEOUT`): every caller
87+
* is dialing a server we don't control, and neither the connect handshake
88+
* nor `listTools` has a deadline of its own, so an unresponsive endpoint
89+
* (dead loopback port, wedged mid-handshake) would otherwise hang the
90+
* calling fiber (and, transitively, the server-side request handling it)
91+
* forever. On timeout, any connection that DID get established is closed
92+
* before the timeout error is raised (`Effect.onExit` still fires for an
93+
* interrupted fiber).
7394
*/
7495
export const discoverTools = (
7596
connector: McpConnector,
97+
timeoutMs: number = Duration.toMillis(DEFAULT_DISCOVER_TIMEOUT),
7698
): Effect.Effect<McpToolManifest, McpToolDiscoveryError> =>
7799
Effect.gen(function* () {
78100
// Acquire connection
@@ -96,7 +118,18 @@ export const discoverTools = (
96118
);
97119

98120
return manifest;
99-
});
121+
}).pipe(
122+
Effect.timeoutOrElse({
123+
duration: Duration.millis(timeoutMs),
124+
orElse: () =>
125+
Effect.fail(
126+
new McpToolDiscoveryError({
127+
stage: "connect",
128+
message: `MCP discovery timed out after ${timeoutMs}ms`,
129+
}),
130+
),
131+
}),
132+
);
100133

101134
const closeConnection = (connection: {
102135
readonly close: () => Promise<void>;

0 commit comments

Comments
 (0)