From 5fdb450895321624b30c5c8bba9efe19b60ce4ce Mon Sep 17 00:00:00 2001 From: principalwater <67874880+principalwater@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:20:55 +0300 Subject: [PATCH 1/2] Accept MCP elicitation requests so headless MCP tool calls work Codex gates MCP tool calls behind an MCP elicitation (`mcpServer/elicitation/request`) rather than the exec/patch approval channel, so the `approvalPolicy: "never"` that every thread started by this client already declares does not cover them. `handleServerRequest` answered every server-initiated request with JSON-RPC -32601, which left the elicitation unresolved. Core maps a missing response to `ReviewDecision::Abort`, surfaced to the model as "user rejected MCP tool call". Because runs driven by this client are headless, nobody can accept interactively, so every MCP tool call in a `task` or `review` run failed. The job log only showed "Tool / failed.", which points the user at a networking or MCP-config problem that does not exist. Answer the elicitation with `action: "accept"` instead. The response logic moves into an exported `buildServerRequestResponse()` so it can be tested directly; `AppServerClientBase` is not exported. Fixes #640 Co-Authored-By: Claude Opus 5 --- plugins/codex/scripts/lib/app-server.mjs | 38 ++++++++++++++++-- tests/app-server.test.mjs | 49 ++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 tests/app-server.test.mjs diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index 72b30a76..f23b2f36 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -22,6 +22,39 @@ const PLUGIN_MANIFEST = JSON.parse(fs.readFileSync(PLUGIN_MANIFEST_URL, "utf8")) export const BROKER_ENDPOINT_ENV = "CODEX_COMPANION_APP_SERVER_ENDPOINT"; export const BROKER_BUSY_RPC_CODE = -32001; +/** + * Server-initiated request Codex uses to gate MCP tool calls. + * See `ServerRequest` in `codex-rs/app-server-protocol`. + */ +export const MCP_ELICITATION_REQUEST_METHOD = "mcpServer/elicitation/request"; + +/** + * Build the reply to a server-initiated request. + * + * Codex gates MCP tool calls behind an MCP elicitation rather than the + * exec/patch approval channel, so the `approvalPolicy: "never"` that every + * thread started by this client already declares does not cover them. + * Replying with an error leaves the elicitation unresolved, and core maps that + * to `ReviewDecision::Abort`, surfaced as "user rejected MCP tool call". Runs + * driven by this client are headless, so no one can accept interactively and + * every MCP tool call would fail. Accept explicitly instead. + * + * @param {{ id: unknown, method?: string }} message + */ +export function buildServerRequestResponse(message) { + if (message.method === MCP_ELICITATION_REQUEST_METHOD) { + return { + id: message.id, + result: { action: "accept", content: null, _meta: null } + }; + } + + return { + id: message.id, + error: buildJsonRpcError(-32601, `Unsupported server request: ${message.method}`) + }; +} + /** @type {ClientInfo} */ const DEFAULT_CLIENT_INFO = { title: "Codex Plugin", @@ -154,10 +187,7 @@ class AppServerClientBase { } handleServerRequest(message) { - this.sendMessage({ - id: message.id, - error: buildJsonRpcError(-32601, `Unsupported server request: ${message.method}`) - }); + this.sendMessage(buildServerRequestResponse(message)); } handleExit(error) { diff --git a/tests/app-server.test.mjs b/tests/app-server.test.mjs new file mode 100644 index 00000000..3c65198f --- /dev/null +++ b/tests/app-server.test.mjs @@ -0,0 +1,49 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + MCP_ELICITATION_REQUEST_METHOD, + buildServerRequestResponse +} from "../plugins/codex/scripts/lib/app-server.mjs"; + +test("accepts MCP elicitation requests so headless MCP tool calls are not denied", () => { + const response = buildServerRequestResponse({ + id: 12, + method: MCP_ELICITATION_REQUEST_METHOD, + params: { + threadId: "thread-1", + turnId: "turn-1", + serverName: "example", + mode: "form", + message: "Allow this tool call?", + requestedSchema: { type: "object", properties: {} } + } + }); + + assert.deepEqual(response, { + id: 12, + result: { action: "accept", content: null, _meta: null } + }); +}); + +test("still rejects server requests the client does not implement", () => { + const response = buildServerRequestResponse({ + id: "req-7", + method: "item/tool/requestUserInput" + }); + + assert.equal(response.id, "req-7"); + assert.equal(response.result, undefined); + assert.equal(response.error.code, -32601); + assert.match(response.error.message, /item\/tool\/requestUserInput/); +}); + +test("preserves the request id type for string ids", () => { + const response = buildServerRequestResponse({ + id: "elicitation-42", + method: MCP_ELICITATION_REQUEST_METHOD + }); + + assert.equal(response.id, "elicitation-42"); + assert.equal(response.result.action, "accept"); +}); From 2fb936efb9b6564736bbb7a3006d9fa19a5005ad Mon Sep 17 00:00:00 2001 From: principalwater <67874880+principalwater@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:30:23 +0300 Subject: [PATCH 2/2] Gate the auto-accept on the tool-approval discriminator `mcpServer/elicitation/request` also carries ordinary form and URL elicitations from configured MCP servers, not just Codex tool-approval prompts. Accepting those with empty content would hand the server bogus consent or invalid input. Accept only when `params._meta.codex_approval_kind` is `mcp_tool_call` (`APPROVAL_KIND_KEY` / `APPROVAL_KIND_MCP_TOOL_CALL` in codex-rs/protocol/src/mcp_approval_meta.rs) and decline everything else, which a headless client can neither render nor validate. Co-Authored-By: Claude Opus 5 --- plugins/codex/scripts/lib/app-server.mjs | 33 +++++++++++-- tests/app-server.test.mjs | 63 ++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index f23b2f36..d86ff2e7 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -23,11 +23,29 @@ export const BROKER_ENDPOINT_ENV = "CODEX_COMPANION_APP_SERVER_ENDPOINT"; export const BROKER_BUSY_RPC_CODE = -32001; /** - * Server-initiated request Codex uses to gate MCP tool calls. + * Server-initiated request Codex uses to gate MCP tool calls. The same method + * also carries ordinary form and URL elicitations from configured MCP servers. * See `ServerRequest` in `codex-rs/app-server-protocol`. */ export const MCP_ELICITATION_REQUEST_METHOD = "mcpServer/elicitation/request"; +/** + * Discriminator that marks a privileged Codex approval rather than a plain MCP + * elicitation. See `APPROVAL_KIND_KEY` / `APPROVAL_KIND_MCP_TOOL_CALL` in + * `codex-rs/protocol/src/mcp_approval_meta.rs`. + */ +const APPROVAL_KIND_KEY = "codex_approval_kind"; +const APPROVAL_KIND_MCP_TOOL_CALL = "mcp_tool_call"; + +/** @param {unknown} meta */ +function isMcpToolCallApproval(meta) { + return ( + typeof meta === "object" && + meta !== null && + meta[APPROVAL_KIND_KEY] === APPROVAL_KIND_MCP_TOOL_CALL + ); +} + /** * Build the reply to a server-initiated request. * @@ -37,15 +55,22 @@ export const MCP_ELICITATION_REQUEST_METHOD = "mcpServer/elicitation/request"; * Replying with an error leaves the elicitation unresolved, and core maps that * to `ReviewDecision::Abort`, surfaced as "user rejected MCP tool call". Runs * driven by this client are headless, so no one can accept interactively and - * every MCP tool call would fail. Accept explicitly instead. + * every MCP tool call would fail. Accept those explicitly. + * + * The same method also delivers ordinary form and URL elicitations from + * configured MCP servers. A headless client cannot render a form or complete a + * URL flow, so accepting one with empty content would hand the server bogus + * consent or invalid input. Decline those instead, which is a valid protocol + * answer and lets the server fail cleanly. * - * @param {{ id: unknown, method?: string }} message + * @param {{ id: unknown, method?: string, params?: { _meta?: unknown } }} message */ export function buildServerRequestResponse(message) { if (message.method === MCP_ELICITATION_REQUEST_METHOD) { + const action = isMcpToolCallApproval(message.params?._meta) ? "accept" : "decline"; return { id: message.id, - result: { action: "accept", content: null, _meta: null } + result: { action, content: null, _meta: null } }; } diff --git a/tests/app-server.test.mjs b/tests/app-server.test.mjs index 3c65198f..87ba778e 100644 --- a/tests/app-server.test.mjs +++ b/tests/app-server.test.mjs @@ -6,7 +6,7 @@ import { buildServerRequestResponse } from "../plugins/codex/scripts/lib/app-server.mjs"; -test("accepts MCP elicitation requests so headless MCP tool calls are not denied", () => { +test("accepts MCP tool-call approvals so headless MCP tool calls are not denied", () => { const response = buildServerRequestResponse({ id: 12, method: MCP_ELICITATION_REQUEST_METHOD, @@ -16,7 +16,8 @@ test("accepts MCP elicitation requests so headless MCP tool calls are not denied serverName: "example", mode: "form", message: "Allow this tool call?", - requestedSchema: { type: "object", properties: {} } + requestedSchema: { type: "object", properties: {} }, + _meta: { codex_approval_kind: "mcp_tool_call" } } }); @@ -26,6 +27,61 @@ test("accepts MCP elicitation requests so headless MCP tool calls are not denied }); }); +test("declines plain form elicitations this headless client cannot fill in", () => { + const response = buildServerRequestResponse({ + id: 13, + method: MCP_ELICITATION_REQUEST_METHOD, + params: { + threadId: "thread-1", + turnId: "turn-1", + serverName: "example", + mode: "form", + message: "Enter your project name", + requestedSchema: { + type: "object", + properties: { project: { type: "string" } }, + required: ["project"] + }, + _meta: null + } + }); + + assert.equal(response.result.action, "decline"); + assert.equal(response.result.content, null); +}); + +test("declines URL elicitations, e.g. an auth flow nobody can complete", () => { + const response = buildServerRequestResponse({ + id: 14, + method: MCP_ELICITATION_REQUEST_METHOD, + params: { + threadId: "thread-1", + serverName: "example", + mode: "url", + message: "Authorize this connector", + url: "https://example.com/oauth", + elicitationId: "elicit-1" + } + }); + + assert.equal(response.result.action, "decline"); +}); + +test("declines approvals of a kind this client does not implement", () => { + const response = buildServerRequestResponse({ + id: 15, + method: MCP_ELICITATION_REQUEST_METHOD, + params: { + serverName: "example", + mode: "form", + message: "Add this tool?", + _meta: { codex_approval_kind: "tool_suggestion" } + } + }); + + assert.equal(response.result.action, "decline"); +}); + test("still rejects server requests the client does not implement", () => { const response = buildServerRequestResponse({ id: "req-7", @@ -41,7 +97,8 @@ test("still rejects server requests the client does not implement", () => { test("preserves the request id type for string ids", () => { const response = buildServerRequestResponse({ id: "elicitation-42", - method: MCP_ELICITATION_REQUEST_METHOD + method: MCP_ELICITATION_REQUEST_METHOD, + params: { _meta: { codex_approval_kind: "mcp_tool_call" } } }); assert.equal(response.id, "elicitation-42");