History

Let Codex plugin approval prompts reach the client

Rhys Sullivan committed 2 months ago

Commit 8fa7ed81 parent 1631dad3 files · +58 −1Browse files

Unified · Split

3 files
  1. packages/plugins/mcp/src/sdk/appserver-connector.test.ts +24 −0
  2. packages/plugins/mcp/src/sdk/appserver-connector.ts +14 −1
  3. packages/plugins/mcp/src/sdk/appserver-test-server.ts +20 −0
packages/plugins/mcp/src/sdk/appserver-connector.test.ts+24−0
@@ -78,6 +78,30 @@ describe("codex app-server bridge", () => { · 77 unmodified lines
7878 ),
7979 );
8080
81+ it.effect("starts the thread with an approval policy that lets prompts through", () =>
82+ Effect.scoped(
83+ Effect.gen(function* () {
84+ // Codex declines MCP elicitations ITSELF on a thread whose approval
85+ // policy does not allow them — the prompt never reaches the client and
86+ // the tool just reports "access was not approved". The fixture only
87+ // elicits when the bridge asked for a permitting policy, so reaching
88+ // the handler at all is the assertion.
89+ const connection = yield* withConnection(appServerInput("messages"));
90+ let prompted = false;
91+ connection.client.setRequestHandler("elicitation/create", () => {
92+ prompted = true;
93+ return Promise.resolve({ action: "accept" as const, content: {} });
94+ });
95+
96+ const result = yield* Effect.promise(() =>
97+ connection.client.callTool({ name: "needs_approval", arguments: {} }),
98+ );
99+ expect(prompted, "the plugin's own approval prompt reached the client").toBe(true);
100+ expect(result.isError).toBeFalsy();
101+ }),
102+ ),
103+ );
104+
81105 it.effect("a declined elicitation reaches the app-server as a decline, not an approval", () =>
82106 Effect.scoped(
83107 Effect.gen(function* () {
packages/plugins/mcp/src/sdk/appserver-connector.ts+14−1
@@ -17,6 +17,8 @@ · 16 unmodified lines
1717 //
1818 // MCP upstream app-server downstream
1919 // initialize → initialize → initialized → thread/start
20+// (with an approval policy that lets the
21+// plugin's own prompts reach the client)
2022 // tools/list → mcpServerStatus/list (one server's tools)
2123 // tools/call → mcpServer/tool/call
2224 // elicitation/create (to client) ← mcpServer/elicitation/request
@@ -277,7 +279,18 @@ class AppServerClientTransport implements Transport { · 254 unmodified lines
277279 return;
278280 }
279281 this.#sendDownstream({ jsonrpc: "2.0", method: "initialized" });
280− const started = await this.#request("thread/start", { sessionStartSource: "startup" });
282+ // `approvalPolicy` is load-bearing, not a default worth inheriting: on a
283+ // thread whose policy is `never` (or a granular one without
284+ // `mcpElicitations`) Codex DECLINES every MCP elicitation itself and never
285+ // forwards it, which surfaces as an unexplained "access was not approved"
286+ // on any tool that asks — `read_messages` above all. `on-request` is the
287+ // policy that routes the plugin's own approval prompt to the client, where
288+ // executor's elicitation bridge answers it. Nothing else can escalate here:
289+ // this thread runs no turns, so there is no shell or exec approval to ask.
290+ const started = await this.#request("thread/start", {
291+ sessionStartSource: "startup",
292+ approvalPolicy: "on-request",
293+ });
281294 if (!started.ok) {
282295 this.#fail(message.id, started.error);
283296 return;
packages/plugins/mcp/src/sdk/appserver-test-server.ts+20−0
@@ -29,6 +29,10 @@ const decodeInitializeParams = Schema.decodeUnknownOption( · 28 unmodified lines
2929 Schema.Struct({ clientInfo: Schema.Struct({ name: Schema.String }) }),
3030 );
3131
32+const decodeThreadStartParams = Schema.decodeUnknownOption(
33+ Schema.Struct({ approvalPolicy: Schema.optional(Schema.String) }),
34+);
35+
3236 const decodeThreadParams = Schema.decodeUnknownOption(
3337 Schema.Struct({
3438 threadId: Schema.optional(Schema.String),
@@ -77,6 +81,7 @@ const replyError = (id: number | string, code: number, message: string): void => · 42 unmodified lines
7781 };
7882
7983 let initializedSeen = false;
84+let elicitationsAllowed = false;
8085 let nextServerRequestId = 1000;
8186 /** Elicitation request id → the pending tool call's request id. */
8287 const pendingApprovals = new Map<number | string, number | string>();
@@ -134,6 +139,15 @@ const handleToolCall = (id: number | string, params: unknown): void => { · 51 unmodified lines
134139 return;
135140 }
136141 if (call.tool === "needs_approval") {
142+ if (!elicitationsAllowed) {
143+ // Exactly what Codex returns when it declines the elicitation for the
144+ // client: an error result, no prompt, no explanation.
145+ reply(id, {
146+ content: [{ type: "text", text: "access was not approved" }],
147+ isError: true,
148+ });
149+ return;
150+ }
137151 const elicitationId = nextServerRequestId++;
138152 pendingApprovals.set(elicitationId, id);
139153 write({
@@ -200,6 +214,12 @@ readline.createInterface({ input: process.stdin }).on("line", (line) => { · 60 unmodified lines
200214 replyError(message.id, -32600, "thread/start before the initialized notification");
201215 return;
202216 }
217+ // Codex DECLINES every MCP elicitation itself on a thread whose approval
218+ // policy does not allow them, so a thread started without one can never
219+ // receive an approval prompt. The fixture models that: it only elicits
220+ // when the bridge asked for a policy that permits elicitations.
221+ const params = Option.getOrUndefined(decodeThreadStartParams(message.params));
222+ elicitationsAllowed = params?.approvalPolicy === "on-request";
203223 reply(message.id, { thread: { id: THREAD_ID } });
204224 return;
205225 }