Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mcp-elicitation-active-deadline.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@executor-js/plugin-mcp": patch
---

Exclude time spent waiting for elicitation from the MCP tool invocation deadline.
11 changes: 10 additions & 1 deletion apps/cloud/src/mcp/session-build-semaphore.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it, beforeEach } from "@effect/vitest";
import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest";

import {
acquireBuildSlot,
Expand All @@ -13,6 +13,10 @@ describe("session-build-semaphore", () => {
resetBuildSlotsForTest();
});

afterEach(() => {
vi.useRealTimers();
});

it("grants up to the cap immediately, with no wait", async () => {
const results = await Promise.all([
acquireBuildSlot().promise,
Expand Down Expand Up @@ -214,6 +218,7 @@ describe("session-build-semaphore", () => {
});

it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => {
vi.useFakeTimers();
await Promise.all([
acquireBuildSlot().promise,
acquireBuildSlot().promise,
Expand All @@ -223,6 +228,10 @@ describe("session-build-semaphore", () => {
expect(currentActiveBuildsForTest()).toBe(4);

const timedOutHandle = acquireBuildSlot(10);
await vi.advanceTimersByTimeAsync(9);
expect(currentQueueLengthForTest()).toBe(1);
expect(currentActiveBuildsForTest()).toBe(4);
await vi.advanceTimersByTimeAsync(1);
const result = await timedOutHandle.promise;

expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true });
Expand Down
75 changes: 75 additions & 0 deletions e2e/selfhost/mcp-elicitation-deadline.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { randomBytes } from "node:crypto";
import { expect } from "@effect/vitest";
import { Effect } from "effect";
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { composePluginApi } from "@executor-js/api/server";
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
import { serveMcpServer } from "@executor-js/plugin-mcp/testing";
import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared";

import { scenario } from "../src/scenario";
import { Api, Mcp, Target } from "../src/services";

const api = composePluginApi([mcpHttpPlugin()] as const);

scenario(
"MCP · a human can approve after the active-work deadline without losing the tool call",
{ timeout: 180_000 },
Effect.scoped(
Effect.gen(function* () {
const target = yield* Target;
const mcp = yield* Mcp;
const { client: makeClient } = yield* Api;
const identity = yield* target.newIdentity();
const client = yield* makeClient(api, identity);
const slug = IntegrationSlug.make(`deadline_${randomBytes(4).toString("hex")}`);
const server = yield* serveMcpServer(() => {
const upstream = new McpServer({ name: "Human approval", version: "1" });
upstream.registerTool("approve", { inputSchema: {} }, async () => {
const reply = await upstream.server.elicitInput(
{
mode: "form",
message: "Approve the delayed call?",
requestedSchema: { type: "object", properties: {} },
},
{ timeout: 150_000 },
);
return { content: [{ type: "text", text: `decision:${reply.action}` }] };
});
return upstream;
});
yield* client.mcp.addServer({
payload: {
transport: "remote",
name: "Human approval",
endpoint: server.url,
slug,
remoteTransport: "streamable-http",
},
});
yield* Effect.gen(function* () {
yield* client.connections.create({
payload: {
owner: "org",
name: ConnectionName.make("main"),
integration: slug,
template: AuthTemplateSlug.make("none"),
value: "",
},
});
const session = mcp.session(identity, { elicitationMode: "model" });
yield* session.listTools();
const paused = yield* session.call("execute", {
code: `return await tools.${slug}.org.main.approve({});`,
});
expect(paused.text).toContain("executionId:");
// Cross the production 60-second active-work deadline. This is the
// behavior under test: a human waiting must consume none of that budget.
yield* Effect.sleep("65 seconds");
const completed = yield* session.approvePaused(paused.text);
expect(completed.ok).toBe(true);
expect(completed.text).toContain("decision:accept");
}).pipe(Effect.ensuring(client.mcp.removeServer({ params: { slug } }).pipe(Effect.orDie)));
}),
),
);
144 changes: 143 additions & 1 deletion packages/plugins/mcp/src/sdk/invoke.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { beforeAll, describe, expect, it } from "@effect/vitest";
import { Effect, Predicate } from "effect";
import { HttpServerResponse } from "effect/unstable/http";
// oxlint-disable-next-line executor/no-vitest-import -- boundary: fake-clock coverage for the active-work deadline
import { afterEach, vi } from "vitest";

import {
ProtocolError,
SdkErrorCode,
SdkHttpError,
type OAuthClientProvider,
type ClientContext,
} from "@modelcontextprotocol/client";
import { ElicitationResponse } from "@executor-js/sdk";
import { serveTestHttpApp } from "@executor-js/sdk/testing";
Expand All @@ -19,7 +22,7 @@ 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 } from "./invoke";
import { invokeMcpTool, makeActiveWorkDeadline, MCP_ACTIVE_WORK_TIMEOUT_MS } from "./invoke";

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

Expand Down Expand Up @@ -148,6 +151,145 @@ const invocationRejectionCases = [
];

describe("invokeMcpTool", () => {
afterEach(() => vi.useRealTimers());

it("pauses the active-work deadline across overlapping elicitations", () => {
vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"] });
const deadline = makeActiveWorkDeadline(100);

vi.advanceTimersByTime(40);
deadline.pause();
deadline.pause();
vi.advanceTimersByTime(1_000);
expect(deadline.signal.aborted).toBe(false);

deadline.resume();
vi.advanceTimersByTime(100);
expect(deadline.signal.aborted).toBe(false);

deadline.resume();
vi.advanceTimersByTime(59);
expect(deadline.signal.aborted).toBe(false);
vi.advanceTimersByTime(1);
expect(deadline.signal.aborted).toBe(true);
deadline.dispose();
});

it("uses the active signal for a tool call and excludes elicitation from its deadline", async () => {
vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout"] });

let requestHandler:
| ((request: { params: unknown }, context: ClientContext) => Promise<unknown>)
| undefined;
let callOptions: { signal: AbortSignal; timeout: number } | undefined;
let finishElicitation: (() => void) | undefined;
let resolveElicitationStarted: (() => void) | undefined;
const elicitationStarted = new Promise<void>((resolve) => {
resolveElicitationStarted = resolve;
});
const connectionAbort = new AbortController();

const client = {
setRequestHandler: (_method: string, handler: unknown) => {
requestHandler = handler as typeof requestHandler;
},
callTool: async (_request: unknown, options: { signal: AbortSignal; timeout: number }) => {
callOptions = options;
await requestHandler!(
{
params: { mode: "form", message: "Approve?", requestedSchema: {} },
},
{ mcpReq: { signal: connectionAbort.signal } } as ClientContext,
);
// oxlint-disable-next-line executor/no-promise-reject -- boundary: fake MCP client models SDK abort rejection
return await new Promise<never>((_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: "streamable-http",
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: () =>
Effect.callback((resume) => {
resolveElicitationStarted!();
finishElicitation = () =>
resume(Effect.succeed(ElicitationResponse.make({ action: "accept" })));
}),
}),
).then(
() => "completed" as const,
() => "failed" as const,
);

await elicitationStarted;
expect(callOptions?.timeout).toBeGreaterThan(MCP_ACTIVE_WORK_TIMEOUT_MS);
vi.advanceTimersByTime(MCP_ACTIVE_WORK_TIMEOUT_MS);
expect(callOptions?.signal.aborted).toBe(false);

finishElicitation!();
await Promise.resolve();
await Promise.resolve();
vi.advanceTimersByTime(MCP_ACTIVE_WORK_TIMEOUT_MS);
expect(callOptions?.signal.aborted).toBe(true);
expect(await invocation).toBe("failed");
});

it("interrupts an elicitation when the MCP connection closes", async () => {
let requestHandler:
| ((request: { params: unknown }, context: ClientContext) => Promise<unknown>)
| undefined;
const connectionAbort = new AbortController();
const client = {
setRequestHandler: (_method: string, handler: unknown) => {
requestHandler = handler as typeof requestHandler;
},
callTool: async () => {
await requestHandler!(
{
params: { mode: "form", message: "Approve?", requestedSchema: {} },
},
{ mcpReq: { signal: connectionAbort.signal } } as ClientContext,
);
return { content: [] };
},
};

const invocation = Effect.runPromise(
invokeMcpTool({
toolId: "closed",
toolName: "closed",
args: {},
transport: "streamable-http",
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: () => Effect.callback(() => undefined),
}),
).then(
() => "completed" as const,
() => "failed" as const,
);

await Promise.resolve();
connectionAbort.abort();
expect(await invocation).toBe("failed");
});

for (const testCase of invocationRejectionCases) {
it.effect(testCase.name, () =>
Effect.gen(function* () {
Expand Down
Loading
Loading