Skip to content

Commit 0e9d800

Browse files
fix(mcp): pause active timeout during elicitation (#1956)
* fix(mcp): pause active timeout during elicitation * Test approval waits beyond the MCP active-work deadline * Test queue timeout with a controlled clock --------- Co-authored-by: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com>
1 parent 5c1d1b4 commit 0e9d800

4 files changed

Lines changed: 370 additions & 34 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@executor-js/plugin-mcp": patch
3+
---
4+
5+
Exclude time spent waiting for elicitation from the MCP tool invocation deadline.
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { randomBytes } from "node:crypto";
2+
import { expect } from "@effect/vitest";
3+
import { Effect } from "effect";
4+
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
5+
import { composePluginApi } from "@executor-js/api/server";
6+
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
7+
import { serveMcpServer } from "@executor-js/plugin-mcp/testing";
8+
import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared";
9+
10+
import { scenario } from "../src/scenario";
11+
import { Api, Mcp, Target } from "../src/services";
12+
13+
const api = composePluginApi([mcpHttpPlugin()] as const);
14+
15+
scenario(
16+
"MCP · a human can approve after the active-work deadline without losing the tool call",
17+
{ timeout: 180_000 },
18+
Effect.scoped(
19+
Effect.gen(function* () {
20+
const target = yield* Target;
21+
const mcp = yield* Mcp;
22+
const { client: makeClient } = yield* Api;
23+
const identity = yield* target.newIdentity();
24+
const client = yield* makeClient(api, identity);
25+
const slug = IntegrationSlug.make(`deadline_${randomBytes(4).toString("hex")}`);
26+
const server = yield* serveMcpServer(() => {
27+
const upstream = new McpServer({ name: "Human approval", version: "1" });
28+
upstream.registerTool("approve", { inputSchema: {} }, async () => {
29+
const reply = await upstream.server.elicitInput(
30+
{
31+
mode: "form",
32+
message: "Approve the delayed call?",
33+
requestedSchema: { type: "object", properties: {} },
34+
},
35+
{ timeout: 150_000 },
36+
);
37+
return { content: [{ type: "text", text: `decision:${reply.action}` }] };
38+
});
39+
return upstream;
40+
});
41+
yield* client.mcp.addServer({
42+
payload: {
43+
transport: "remote",
44+
name: "Human approval",
45+
endpoint: server.url,
46+
slug,
47+
remoteTransport: "streamable-http",
48+
},
49+
});
50+
yield* Effect.gen(function* () {
51+
yield* client.connections.create({
52+
payload: {
53+
owner: "org",
54+
name: ConnectionName.make("main"),
55+
integration: slug,
56+
template: AuthTemplateSlug.make("none"),
57+
value: "",
58+
},
59+
});
60+
const session = mcp.session(identity, { elicitationMode: "model" });
61+
yield* session.listTools();
62+
const paused = yield* session.call("execute", {
63+
code: `return await tools.${slug}.org.main.approve({});`,
64+
});
65+
expect(paused.text).toContain("executionId:");
66+
// Cross the production 60-second active-work deadline. This is the
67+
// behavior under test: a human waiting must consume none of that budget.
68+
yield* Effect.sleep("65 seconds");
69+
const completed = yield* session.approvePaused(paused.text);
70+
expect(completed.ok).toBe(true);
71+
expect(completed.text).toContain("decision:accept");
72+
}).pipe(Effect.ensuring(client.mcp.removeServer({ params: { slug } }).pipe(Effect.orDie)));
73+
}),
74+
),
75+
);

‎packages/plugins/mcp/src/sdk/invoke.test.ts‎

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import { beforeAll, describe, expect, it } from "@effect/vitest";
22
import { Effect, Predicate } from "effect";
33
import { HttpServerResponse } from "effect/unstable/http";
4+
// oxlint-disable-next-line executor/no-vitest-import -- boundary: fake-clock coverage for the active-work deadline
5+
import { afterEach, vi } from "vitest";
46

57
import {
68
ProtocolError,
79
SdkErrorCode,
810
SdkHttpError,
911
type OAuthClientProvider,
12+
type ClientContext,
1013
} from "@modelcontextprotocol/client";
1114
import { ElicitationResponse } from "@executor-js/sdk";
1215
import { serveTestHttpApp } from "@executor-js/sdk/testing";
@@ -19,7 +22,7 @@ import { createMcpConnector, type McpConnection, type McpConnector } from "./con
1922
// that precondition here — these tests construct SDK errors directly.
2023
beforeAll(() => loadMcpClientSdk());
2124
import { McpInvocationError, McpOAuthReauthorizationRequired } from "./errors";
22-
import { invokeMcpTool } from "./invoke";
25+
import { invokeMcpTool, makeActiveWorkDeadline, MCP_ACTIVE_WORK_TIMEOUT_MS } from "./invoke";
2326

2427
const acceptAll = () => Effect.succeed(ElicitationResponse.make({ action: "accept" }));
2528

@@ -148,6 +151,145 @@ const invocationRejectionCases = [
148151
];
149152

150153
describe("invokeMcpTool", () => {
154+
afterEach(() => vi.useRealTimers());
155+
156+
it("pauses the active-work deadline across overlapping elicitations", () => {
157+
vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"] });
158+
const deadline = makeActiveWorkDeadline(100);
159+
160+
vi.advanceTimersByTime(40);
161+
deadline.pause();
162+
deadline.pause();
163+
vi.advanceTimersByTime(1_000);
164+
expect(deadline.signal.aborted).toBe(false);
165+
166+
deadline.resume();
167+
vi.advanceTimersByTime(100);
168+
expect(deadline.signal.aborted).toBe(false);
169+
170+
deadline.resume();
171+
vi.advanceTimersByTime(59);
172+
expect(deadline.signal.aborted).toBe(false);
173+
vi.advanceTimersByTime(1);
174+
expect(deadline.signal.aborted).toBe(true);
175+
deadline.dispose();
176+
});
177+
178+
it("uses the active signal for a tool call and excludes elicitation from its deadline", async () => {
179+
vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"] });
180+
181+
let requestHandler:
182+
| ((request: { params: unknown }, context: ClientContext) => Promise<unknown>)
183+
| undefined;
184+
let callOptions: { signal: AbortSignal; timeout: number } | undefined;
185+
let finishElicitation: (() => void) | undefined;
186+
let resolveElicitationStarted: (() => void) | undefined;
187+
const elicitationStarted = new Promise<void>((resolve) => {
188+
resolveElicitationStarted = resolve;
189+
});
190+
const connectionAbort = new AbortController();
191+
192+
const client = {
193+
setRequestHandler: (_method: string, handler: unknown) => {
194+
requestHandler = handler as typeof requestHandler;
195+
},
196+
callTool: async (_request: unknown, options: { signal: AbortSignal; timeout: number }) => {
197+
callOptions = options;
198+
await requestHandler!(
199+
{
200+
params: { mode: "form", message: "Approve?", requestedSchema: {} },
201+
},
202+
{ mcpReq: { signal: connectionAbort.signal } } as ClientContext,
203+
);
204+
// oxlint-disable-next-line executor/no-promise-reject -- boundary: fake MCP client models SDK abort rejection
205+
return await new Promise<never>((_resolve, reject) => {
206+
// oxlint-disable-next-line executor/no-promise-reject -- boundary: fake MCP client models SDK abort rejection
207+
options.signal.addEventListener("abort", () => reject(options.signal.reason), {
208+
once: true,
209+
});
210+
});
211+
},
212+
};
213+
214+
const invocation = Effect.runPromise(
215+
invokeMcpTool({
216+
toolId: "slow",
217+
toolName: "slow",
218+
args: {},
219+
transport: "streamable-http",
220+
connector: Effect.succeed({
221+
// oxlint-disable-next-line executor/no-double-cast -- boundary: minimal fake MCP client implements only invokeMcpTool's surface
222+
client: client as unknown as McpConnection["client"],
223+
close: () => Promise.resolve(),
224+
}),
225+
elicit: () =>
226+
Effect.callback((resume) => {
227+
resolveElicitationStarted!();
228+
finishElicitation = () =>
229+
resume(Effect.succeed(ElicitationResponse.make({ action: "accept" })));
230+
}),
231+
}),
232+
).then(
233+
() => "completed" as const,
234+
() => "failed" as const,
235+
);
236+
237+
await elicitationStarted;
238+
expect(callOptions?.timeout).toBeGreaterThan(MCP_ACTIVE_WORK_TIMEOUT_MS);
239+
vi.advanceTimersByTime(MCP_ACTIVE_WORK_TIMEOUT_MS);
240+
expect(callOptions?.signal.aborted).toBe(false);
241+
242+
finishElicitation!();
243+
await Promise.resolve();
244+
await Promise.resolve();
245+
vi.advanceTimersByTime(MCP_ACTIVE_WORK_TIMEOUT_MS);
246+
expect(callOptions?.signal.aborted).toBe(true);
247+
expect(await invocation).toBe("failed");
248+
});
249+
250+
it("interrupts an elicitation when the MCP connection closes", async () => {
251+
let requestHandler:
252+
| ((request: { params: unknown }, context: ClientContext) => Promise<unknown>)
253+
| undefined;
254+
const connectionAbort = new AbortController();
255+
const client = {
256+
setRequestHandler: (_method: string, handler: unknown) => {
257+
requestHandler = handler as typeof requestHandler;
258+
},
259+
callTool: async () => {
260+
await requestHandler!(
261+
{
262+
params: { mode: "form", message: "Approve?", requestedSchema: {} },
263+
},
264+
{ mcpReq: { signal: connectionAbort.signal } } as ClientContext,
265+
);
266+
return { content: [] };
267+
},
268+
};
269+
270+
const invocation = Effect.runPromise(
271+
invokeMcpTool({
272+
toolId: "closed",
273+
toolName: "closed",
274+
args: {},
275+
transport: "streamable-http",
276+
connector: Effect.succeed({
277+
// oxlint-disable-next-line executor/no-double-cast -- boundary: minimal fake MCP client implements only invokeMcpTool's surface
278+
client: client as unknown as McpConnection["client"],
279+
close: () => Promise.resolve(),
280+
}),
281+
elicit: () => Effect.callback(() => undefined),
282+
}),
283+
).then(
284+
() => "completed" as const,
285+
() => "failed" as const,
286+
);
287+
288+
await Promise.resolve();
289+
connectionAbort.abort();
290+
expect(await invocation).toBe("failed");
291+
});
292+
151293
for (const testCase of invocationRejectionCases) {
152294
it.effect(testCase.name, () =>
153295
Effect.gen(function* () {

0 commit comments

Comments
 (0)