diff --git a/.changeset/mcp-tool-timeout.md b/.changeset/mcp-tool-timeout.md new file mode 100644 index 0000000000..5e83a2bcd6 --- /dev/null +++ b/.changeset/mcp-tool-timeout.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Let an MCP integration set `toolTimeoutMs` to raise the active-work deadline for its tool calls, so servers whose tools legitimately run past 60 seconds stop failing at the one-minute mark. diff --git a/packages/plugins/mcp/src/sdk/invoke.test.ts b/packages/plugins/mcp/src/sdk/invoke.test.ts index bb48e9908a..aac89d3142 100644 --- a/packages/plugins/mcp/src/sdk/invoke.test.ts +++ b/packages/plugins/mcp/src/sdk/invoke.test.ts @@ -22,7 +22,12 @@ import { createMcpConnector, type McpConnection, type McpConnector } from "./con // that precondition here — these tests construct SDK errors directly. beforeAll(() => loadMcpClientSdk()); import { McpInvocationError, McpOAuthReauthorizationRequired } from "./errors"; -import { invokeMcpTool, makeActiveWorkDeadline, MCP_ACTIVE_WORK_TIMEOUT_MS } from "./invoke"; +import { + invokeMcpTool, + makeActiveWorkDeadline, + MCP_ACTIVE_WORK_TIMEOUT_MS, + resolveActiveWorkTimeout, +} from "./invoke"; const acceptAll = () => Effect.succeed(ElicitationResponse.make({ action: "accept" })); @@ -189,6 +194,64 @@ describe("invokeMcpTool", () => { deadline.dispose(); }); + it("falls back to the default active-work timeout for an unusable value", () => { + expect(resolveActiveWorkTimeout(undefined)).toBe(MCP_ACTIVE_WORK_TIMEOUT_MS); + expect(resolveActiveWorkTimeout(0)).toBe(MCP_ACTIVE_WORK_TIMEOUT_MS); + expect(resolveActiveWorkTimeout(-5)).toBe(MCP_ACTIVE_WORK_TIMEOUT_MS); + expect(resolveActiveWorkTimeout(Number.POSITIVE_INFINITY)).toBe(MCP_ACTIVE_WORK_TIMEOUT_MS); + expect(resolveActiveWorkTimeout(120_000)).toBe(120_000); + }); + + it("gives a tool call the integration's timeout instead of the default", async () => { + vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"] }); + + let callOptions: { signal: AbortSignal; timeout: number } | undefined; + let resolveCallStarted: (() => void) | undefined; + const callStarted = new Promise((resolve) => { + resolveCallStarted = resolve; + }); + const client = { + setRequestHandler: () => undefined, + callTool: (_request: unknown, options: { signal: AbortSignal; timeout: number }) => { + callOptions = options; + resolveCallStarted!(); + // oxlint-disable-next-line executor/no-promise-reject -- boundary: fake MCP client models SDK abort rejection + return new Promise((_resolve, reject) => { + // oxlint-disable-next-line executor/no-promise-reject -- boundary: fake MCP client models SDK abort rejection + options.signal.addEventListener("abort", () => reject(options.signal.reason), { + once: true, + }); + }); + }, + }; + + const invocation = Effect.runPromise( + invokeMcpTool({ + toolId: "slow", + toolName: "slow", + args: {}, + transport: "stdio", + connector: Effect.succeed({ + // oxlint-disable-next-line executor/no-double-cast -- boundary: minimal fake MCP client implements only invokeMcpTool's surface + client: client as unknown as McpConnection["client"], + close: () => Promise.resolve(), + }), + elicit: acceptAll, + activeWorkTimeoutMs: 2 * MCP_ACTIVE_WORK_TIMEOUT_MS, + }), + ).then( + () => "completed" as const, + () => "failed" as const, + ); + + await callStarted; + vi.advanceTimersByTime(MCP_ACTIVE_WORK_TIMEOUT_MS); + expect(callOptions?.signal.aborted).toBe(false); + vi.advanceTimersByTime(MCP_ACTIVE_WORK_TIMEOUT_MS); + expect(callOptions?.signal.aborted).toBe(true); + expect(await invocation).toBe("failed"); + }); + it("uses the active signal for a tool call and excludes elicitation from its deadline", async () => { vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"] }); diff --git a/packages/plugins/mcp/src/sdk/invoke.ts b/packages/plugins/mcp/src/sdk/invoke.ts index ed67dd2312..eb16560846 100644 --- a/packages/plugins/mcp/src/sdk/invoke.ts +++ b/packages/plugins/mcp/src/sdk/invoke.ts @@ -51,6 +51,13 @@ import { * waiting for input. */ export const MCP_ACTIVE_WORK_TIMEOUT_MS = 60_000; + +/** An integration's `toolTimeoutMs` when it is a usable duration, otherwise + * the default. A stored config is not trusted to be positive or finite. */ +export const resolveActiveWorkTimeout = (timeoutMs: number | undefined): number => + timeoutMs !== undefined && Number.isFinite(timeoutMs) && timeoutMs > 0 + ? timeoutMs + : MCP_ACTIVE_WORK_TIMEOUT_MS; const MCP_SDK_TIMEOUT_BACKSTOP_MS = 2_147_483_647; export type ActiveWorkDeadline = { @@ -361,10 +368,11 @@ const useConnection = ( args: Record, elicit: Elicit, onToolListChanged: (() => void) | undefined, + activeWorkTimeoutMs: number | undefined, ): Effect.Effect => Effect.gen(function* () { const deadline = yield* Effect.acquireRelease( - Effect.sync(() => makeActiveWorkDeadline()), + Effect.sync(() => makeActiveWorkDeadline(resolveActiveWorkTimeout(activeWorkTimeoutMs))), (activeWork) => Effect.sync(activeWork.dispose), ); installElicitationHandler(connection.client, elicit, deadline); @@ -452,6 +460,9 @@ export interface InvokeMcpToolInput { * the call window. Synchronous and non-throwing by contract; the caller * uses it to mark the persisted catalog stale. */ readonly onToolListChanged?: () => void; + /** Active-work deadline for this call, from the integration's + * `toolTimeoutMs`. Absent or invalid means `MCP_ACTIVE_WORK_TIMEOUT_MS`. */ + readonly activeWorkTimeoutMs?: number; } export const invokeMcpTool = ( @@ -463,7 +474,14 @@ export const invokeMcpTool = ( Effect.gen(function* () { const args = argsRecord(input.args); const use = (connection: McpConnection) => - useConnection(connection, input.toolName, args, input.elicit, input.onToolListChanged); + useConnection( + connection, + input.toolName, + args, + input.elicit, + input.onToolListChanged, + input.activeWorkTimeoutMs, + ); if (input.connectionPool && input.connectionPoolKey) { return yield* input.connectionPool.withConnection( diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 079b89dce5..30e27f0da2 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -223,6 +223,9 @@ const McpRemoteServerInputSchema = Schema.Struct({ * 2026-07-28 revision and then violate its response contract; the probe * reports when it had to fall back, and the add flow passes that through. */ versionNegotiation: Schema.optional(McpStdioVersionNegotiation), + /** Active-work deadline per tool call in ms; see + * `McpStdioIntegrationConfig.toolTimeoutMs`. */ + toolTimeoutMs: Schema.optional(Schema.Number), }); const McpStdioServerInputSchema = Schema.Struct({ @@ -261,6 +264,9 @@ const McpStdioServerInputSchema = Schema.Struct({ /** Opt out of process reuse — spawn a fresh child for every tool call (see * `McpStdioIntegrationConfig.spawnPerCall`). */ spawnPerCall: Schema.optional(Schema.Boolean), + /** Active-work deadline per tool call in ms; see + * `McpStdioIntegrationConfig.toolTimeoutMs`. */ + toolTimeoutMs: Schema.optional(Schema.Number), /** Reach the server through the Codex app-server bridge: the command spawns * `codex app-server` and `server` names the MCP server inside Codex whose * tools this integration exposes. Set by the Codex plugin add flow. */ @@ -481,6 +487,7 @@ export const toIntegrationConfig = (input: McpServerInput): McpIntegrationConfig cwd: input.cwd, versionNegotiation: input.versionNegotiation, spawnPerCall: input.spawnPerCall, + toolTimeoutMs: input.toolTimeoutMs, appServer: input.appServer, authenticationTemplate: vars.length > 0 @@ -499,6 +506,7 @@ export const toIntegrationConfig = (input: McpServerInput): McpIntegrationConfig ? normalizeMcpAuthMethods(input.authenticationTemplate) : [mcpAuthMethodFromShorthand(input.auth ?? { kind: "none" })], versionNegotiation: input.versionNegotiation, + toolTimeoutMs: input.toolTimeoutMs, }; }; @@ -1714,6 +1722,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { onToolListChanged: () => { toolListChanged = true; }, + activeWorkTimeoutMs: parsed.toolTimeoutMs, }).pipe( Effect.onExit(() => toolListChanged diff --git a/packages/plugins/mcp/src/sdk/types.ts b/packages/plugins/mcp/src/sdk/types.ts index 934d8d2cfb..7922ad3f37 100644 --- a/packages/plugins/mcp/src/sdk/types.ts +++ b/packages/plugins/mcp/src/sdk/types.ts @@ -237,6 +237,11 @@ export const McpRemoteIntegrationConfig = Schema.Struct({ * proposal while emitting 2024-era results, which the modern client * rightly rejects. */ versionNegotiation: Schema.optional(McpStdioVersionNegotiation), + /** Active-work deadline for each tool call, in milliseconds. Absent means + * `MCP_ACTIVE_WORK_TIMEOUT_MS` (60s). Raise it for servers whose tools + * legitimately run longer, such as ones that drive a browser. Time spent + * waiting on an elicitation never counts against it. */ + toolTimeoutMs: Schema.optional(Schema.Number), }); export type McpRemoteIntegrationConfig = typeof McpRemoteIntegrationConfig.Type; @@ -267,6 +272,11 @@ export const McpStdioIntegrationConfig = Schema.Struct({ * this only for a server that genuinely depends on fresh-process * semantics, e.g. one that re-reads state at boot and never afterwards. */ spawnPerCall: Schema.optional(Schema.Boolean), + /** Active-work deadline for each tool call, in milliseconds. Absent means + * `MCP_ACTIVE_WORK_TIMEOUT_MS` (60s). Raise it for servers whose tools + * legitimately run longer, such as ones that drive a browser. Time spent + * waiting on an elicitation never counts against it. */ + toolTimeoutMs: Schema.optional(Schema.Number), /** Present when the spawned command is `codex app-server` rather than an * MCP server itself: the connector then bridges MCP to the Codex * app-server protocol in process, and `server` names the MCP server