Skip to content

Commit 40b2f2e

Browse files
SunkenInTimemikemikimikeRhysSullivan
authored
Carry an approval's persistence choice through elicitation (#1976)
* fix(mcp): pause active timeout during elicitation * Carry an approval's persistence choice through elicitation Codex Computer Use offers `persist: ["session", "always"]` in the terms of its "Allow Computer Use to use X?" prompt and remembers the app only when the answer names one. Executor lost the offer on the way in — the terms projection kept strings only — and the choice on the way out, because every adapter rebuilt the reply from `action` and `content`. So each accept was a one-time approval and the same app prompted on every call. - `ElicitationResponse.meta.persist` carries the choice; the vocabulary is closed so no host can grant more than the prompt offered. - `approvalTerms` keeps string lists, so the offered scopes reach the host. - The MCP plugin, the app-server bridge, and the MCP host (native mode) pass `_meta` through in both directions. - The model-mode `resume` tool takes `persist`; the pause output names the offered scopes and says a bare accept is one-time. - The HTTP resume API takes `persist`, and the browser approval page offers the scopes in a select. Nothing is chosen automatically. Fixes #1962 * Preserve approval lifetime through browser and cloud resume paths * Test approval waits beyond the MCP active-work deadline * Test queue timeout with a controlled clock * Test queue timeout with a controlled clock --------- Co-authored-by: mikemikimike <13286568797@163.com> Co-authored-by: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com>
1 parent b5d53cb commit 40b2f2e

27 files changed

Lines changed: 635 additions & 43 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@executor-js/sdk": patch
3+
"@executor-js/execution": patch
4+
"@executor-js/plugin-mcp": patch
5+
"@executor-js/api": patch
6+
"@executor-js/react": patch
7+
"@executor-js/host-mcp": patch
8+
"@executor-js/cloudflare": patch
9+
"executor": patch
10+
---
11+
12+
Carry an approval's persistence choice through elicitation, so Codex Computer Use stops asking to use the same app on every call.
13+
14+
Computer Use offers `persist: ["session", "always"]` in the prompt's terms and remembers the app only when the answer names one. Executor dropped the offer on the way in (the terms projection kept strings only) and the choice on the way out (every adapter rebuilt the reply from `action` and `content`), so each accept was one-time. `ElicitationResponse` now has `meta.persist`; the MCP plugin, the app-server bridge, and the MCP host pass it through; the model-mode `resume` tool and the browser approval page let the approver pick from the offered scopes. Nothing is chosen automatically: a bare accept still approves once.

apps/cloud/src/auth/api.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ const McpSessionExecutionParams = {
115115
const ResumeMcpExecutionBody = Schema.Struct({
116116
action: Schema.Literals(["accept", "decline", "cancel"]),
117117
content: Schema.optional(Schema.Unknown),
118+
persist: Schema.optional(Schema.String),
118119
});
119120

120121
const McpPausedExecutionResponse = Schema.Struct({

apps/cloud/src/auth/handlers.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,9 @@ export const CloudSessionAuthHandlers = HttpApiBuilder.group(
704704
{
705705
action: payload.action,
706706
content: payload.content as Record<string, unknown> | undefined,
707+
...(payload.action === "accept" && payload.persist !== undefined
708+
? { meta: { persist: payload.persist } }
709+
: {}),
707710
},
708711
),
709712
);

apps/cloud/src/routes/app/resume.$executionId.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,17 @@ function CloudMcpResumeApproval(props: { executionId: string; mcpSessionId: stri
3737
executionId: string,
3838
action: "accept" | "decline" | "cancel",
3939
content?: Record<string, unknown>,
40+
persist?: string,
4041
) =>
4142
doResume({
4243
params: {
4344
mcpSessionId: props.mcpSessionId,
4445
executionId,
4546
},
46-
payload: action === "accept" ? { action, content: content ?? {} } : { action },
47+
payload:
48+
action === "accept"
49+
? { action, content: content ?? {}, ...(persist === undefined ? {} : { persist }) }
50+
: { action },
4751
}),
4852
[doResume, props.mcpSessionId],
4953
);
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { randomBytes } from "node:crypto";
2+
import { expect } from "@effect/vitest";
3+
import { Effect } from "effect";
4+
import { composePluginApi } from "@executor-js/api/server";
5+
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
6+
import { makeElicitationMcpServer, serveMcpServer } from "@executor-js/plugin-mcp/testing";
7+
import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared";
8+
9+
import { scenario } from "../src/scenario";
10+
import { Api, Browser, Mcp, Target } from "../src/services";
11+
import { parseBrowserApproval } from "../src/surfaces/mcp";
12+
import { visit } from "../src/surfaces/browser";
13+
14+
const api = composePluginApi([mcpHttpPlugin()] as const);
15+
16+
scenario(
17+
"MCP · browser approval preserves the chosen lifetime and defaults to once",
18+
{ timeout: 180_000 },
19+
Effect.scoped(
20+
Effect.gen(function* () {
21+
const target = yield* Target;
22+
const browser = yield* Browser;
23+
const mcp = yield* Mcp;
24+
const { client: makeClient } = yield* Api;
25+
const identity = yield* target.newIdentity();
26+
const client = yield* makeClient(api, identity);
27+
const slug = IntegrationSlug.make(`approval_terms_${randomBytes(4).toString("hex")}`);
28+
const server = yield* serveMcpServer(makeElicitationMcpServer);
29+
yield* client.mcp.addServer({
30+
payload: {
31+
transport: "remote",
32+
name: "Approval terms",
33+
endpoint: server.url,
34+
slug,
35+
remoteTransport: "streamable-http",
36+
},
37+
});
38+
yield* Effect.gen(function* () {
39+
yield* client.connections.create({
40+
payload: {
41+
owner: "org",
42+
name: ConnectionName.make("main"),
43+
integration: slug,
44+
template: AuthTemplateSlug.make("none"),
45+
value: "",
46+
},
47+
});
48+
const session = mcp.session(identity, { elicitationMode: "browser" });
49+
yield* session.listTools();
50+
for (const scope of ["session", "always", ""] as const) {
51+
const paused = yield* session.call("execute", {
52+
code: `return await tools.${slug}.org.main.remembered_echo({value:"browser"});`,
53+
});
54+
const approval = parseBrowserApproval(paused);
55+
const [resumed] = yield* Effect.all(
56+
[
57+
session.awaitResume(approval.executionId),
58+
browser.session(identity, async ({ page, step }) => {
59+
await step(`Approve ${scope || "once"} through the console`, async () => {
60+
await visit(page, approval.approvalUrl);
61+
const choice = page.getByLabel("Remember this approval");
62+
await choice.waitFor();
63+
expect(await choice.inputValue(), "every approval starts as one-time").toBe("");
64+
if (scope !== "") await choice.selectOption(scope);
65+
await page.getByRole("button", { name: "Approve", exact: true }).click();
66+
await page.getByText("Approve sent").waitFor();
67+
});
68+
}),
69+
],
70+
{ concurrency: "unbounded" },
71+
);
72+
expect(resumed.ok).toBe(true);
73+
expect(resumed.text, "the chosen lifetime reaches the upstream MCP server").toContain(
74+
`approved:browser:${scope || "once"}`,
75+
);
76+
}
77+
}).pipe(Effect.ensuring(client.mcp.removeServer({ params: { slug } }).pipe(Effect.orDie)));
78+
}),
79+
),
80+
);

e2e/selfhost/mcp-elicitation-deadline.test.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { randomBytes } from "node:crypto";
22
import { expect } from "@effect/vitest";
3-
import { Effect } from "effect";
3+
import { Effect, Schema } from "effect";
44
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
55
import { composePluginApi } from "@executor-js/api/server";
66
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
@@ -11,9 +11,10 @@ import { scenario } from "../src/scenario";
1111
import { Api, Mcp, Target } from "../src/services";
1212

1313
const api = composePluginApi([mcpHttpPlugin()] as const);
14+
const decodeExecutionId = Schema.decodeUnknownSync(Schema.String);
1415

1516
scenario(
16-
"MCP · a human can approve after the active-work deadline without losing the tool call",
17+
"MCP · delayed approval preserves the chosen lifetime beyond the active-work deadline",
1718
{ timeout: 180_000 },
1819
Effect.scoped(
1920
Effect.gen(function* () {
@@ -31,10 +32,15 @@ scenario(
3132
mode: "form",
3233
message: "Approve the delayed call?",
3334
requestedSchema: { type: "object", properties: {} },
35+
_meta: { persist: ["session", "always"] },
3436
},
3537
{ timeout: 150_000 },
3638
);
37-
return { content: [{ type: "text", text: `decision:${reply.action}` }] };
39+
return {
40+
content: [
41+
{ type: "text", text: `decision:${reply.action}:${reply._meta?.persist ?? "once"}` },
42+
],
43+
};
3844
});
3945
return upstream;
4046
});
@@ -66,9 +72,14 @@ scenario(
6672
// Cross the production 60-second active-work deadline. This is the
6773
// behavior under test: a human waiting must consume none of that budget.
6874
yield* Effect.sleep("65 seconds");
69-
const completed = yield* session.approvePaused(paused.text);
75+
const executionId = decodeExecutionId(/\bexecutionId:\s*(\S+)/.exec(paused.text)?.[1]);
76+
const completed = yield* session.call("resume", {
77+
executionId,
78+
action: "accept",
79+
persist: "session",
80+
});
7081
expect(completed.ok).toBe(true);
71-
expect(completed.text).toContain("decision:accept");
82+
expect(completed.text).toContain("decision:accept:session");
7283
}).pipe(Effect.ensuring(client.mcp.removeServer({ params: { slug } }).pipe(Effect.orDie)));
7384
}),
7485
),

e2e/vitest.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export default defineConfig({
4848
project("cloudflare", {
4949
include: [
5050
"scenarios/browser-approval.test.ts",
51+
"scenarios/mcp-approval-persistence.test.ts",
5152
"scenarios/microsoft-graph-full.test.ts",
5253
"scenarios/toolkits-mcp.test.ts",
5354
"cloudflare/**/*.test.ts",

packages/core/api/src/executions/api.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ const ExecuteResponse = Schema.Union([CompletedResult, PausedResult]);
4444
const ResumeRequest = Schema.Struct({
4545
action: Schema.Literals(["accept", "decline", "cancel"]),
4646
content: Schema.optional(Schema.Unknown),
47+
/** How long an accepted approval lasts, when the paused interaction's
48+
* terms offer a choice (`interaction.meta.persist` lists the scopes).
49+
* Omitted, the approval is for this call only. */
50+
persist: Schema.optional(Schema.String),
4751
});
4852

4953
const ResumeResponse = Schema.Union([CompletedResult, PausedResult]);

packages/core/api/src/handlers/executions.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ export const ExecutionsHandlers = HttpApiBuilder.group(ExecutorApi, "executions"
251251
engine.resume(path.executionId, {
252252
action: payload.action,
253253
content: payload.content as Record<string, unknown> | undefined,
254+
...(payload.persist === undefined ? {} : { meta: { persist: payload.persist } }),
254255
}),
255256
);
256257

packages/core/execution/src/engine.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,34 @@ describe("formatPausedExecution approval terms", () => {
279279
});
280280
});
281281

282+
it("says how to answer when the terms leave the approval's lifetime to the caller", () => {
283+
// Computer Use's app approval: a bare accept is one-time and the same
284+
// prompt returns on the next call, so the caller has to be told the
285+
// scopes on offer and how to pick one.
286+
const result = formatPausedExecution(
287+
paused(
288+
FormElicitation.make({
289+
message: 'Allow Computer Use to use "Finder"?',
290+
requestedSchema: {},
291+
meta: { persist: ["session", "always"], connector_name: "Computer Use" },
292+
}),
293+
),
294+
);
295+
296+
const interaction = result.structured["interaction"] as {
297+
readonly meta?: unknown;
298+
readonly instructions: string;
299+
};
300+
expect(interaction.meta).toEqual({
301+
persist: ["session", "always"],
302+
connector_name: "Computer Use",
303+
});
304+
expect(interaction.instructions).toContain(
305+
'pass persist as one of "session", "always"; without it the approval is for this call only',
306+
);
307+
expect(result.text).toContain(interaction.instructions);
308+
});
309+
282310
it("says nothing about terms when the upstream attached none", () => {
283311
const result = formatPausedExecution(
284312
paused(FormElicitation.make({ message: "Proceed?", requestedSchema: {} })),

0 commit comments

Comments
 (0)