From bae057fd771dcb2c137e3e71b37a5b69617a7561 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 23 Apr 2026 02:31:52 +0100 Subject: [PATCH] fix: accept Codex MCP approval elicitations (#68807) --- CHANGELOG.md | 1 + docs/plugins/codex-harness.md | 5 + .../src/app-server/elicitation-bridge.test.ts | 117 ++++++++++++++- .../src/app-server/elicitation-bridge.ts | 133 +++++++++++++++--- .../codex/src/app-server/run-attempt.test.ts | 81 +++++++++++ .../gateway-codex-harness.live-helpers.ts | 1 + 6 files changed, 316 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c99244c9da4..200c190eb7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Codex harness: route Codex-tagged MCP tool approval elicitations through OpenClaw plugin approvals, including current empty-schema app-server requests, while leaving generic user-input prompts fail-closed. (#68807) Thanks @kesslerio. - Providers/OpenAI: harden Voice Call realtime transcription against OpenAI Realtime session-update drift, forward language and prompt hints, and add live coverage for realtime STT. - Providers/Moonshot: stop strict-sanitizing Kimi's native tool_call IDs (shaped like `functions.:`) on the OpenAI-compatible transport, so multi-turn agentic flows through Kimi K2.6 no longer break after 2-3 tool-calling rounds when the serving layer fails to match mangled IDs against the original tool definitions. Adds a `sanitizeToolCallIds` opt-out to the shared `openai-compatible` replay family helper and wires Moonshot to it. Fixes #62319. (#70030) Thanks @LeoDu0314. - Dependencies/security: override transitive `uuid` to `14.0.0`, clearing the runtime advisory across dependencies. diff --git a/docs/plugins/codex-harness.md b/docs/plugins/codex-harness.md index 3ce12bff8b9..4300a9239cb 100644 --- a/docs/plugins/codex-harness.md +++ b/docs/plugins/codex-harness.md @@ -509,6 +509,11 @@ OpenClaw still builds the tool list and receives dynamic tool results from the harness. Text, images, video, music, TTS, approvals, and messaging-tool output continue through the normal OpenClaw delivery path. +Codex MCP tool approval elicitations are routed through OpenClaw's plugin +approval flow when Codex marks `_meta.codex_approval_kind` as +`"mcp_tool_call"`; other elicitation and free-form input requests still fail +closed. + When the selected model uses the Codex harness, native thread compaction is delegated to Codex app-server. OpenClaw keeps a transcript mirror for channel history, search, `/new`, `/reset`, and future model or harness switching. The diff --git a/extensions/codex/src/app-server/elicitation-bridge.test.ts b/extensions/codex/src/app-server/elicitation-bridge.test.ts index 8cafa1347ba..0ca491d47eb 100644 --- a/extensions/codex/src/app-server/elicitation-bridge.test.ts +++ b/extensions/codex/src/app-server/elicitation-bridge.test.ts @@ -53,6 +53,26 @@ function buildApprovalElicitation() { }; } +function buildCurrentCodexApprovalElicitation() { + return { + ...buildApprovalElicitation(), + _meta: { + codex_approval_kind: "mcp_tool_call", + persist: ["session", "always"], + connector_name: "GitHub", + tool_title: "Create pull request", + tool_description: "Creates a pull request in the selected repository.", + tool_params_display: [ + { name: "repo", display_name: "Repository", value: "openclaw/openclaw" }, + ], + }, + requestedSchema: { + type: "object", + properties: {}, + }, + }; +} + describe("Codex app-server elicitation bridge", () => { beforeEach(() => { mockCallGatewayTool.mockReset(); @@ -84,7 +104,75 @@ describe("Codex app-server elicitation bridge", () => { ]); }); - it("maps allow-always decisions onto session-scoped persistence when offered", async () => { + it("accepts current Codex MCP approval elicitations with an empty form schema", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-current", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-current", decision: "allow-once" }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildCurrentCodexApprovalElicitation(), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + }); + + expect(result).toEqual({ + action: "accept", + content: null, + _meta: null, + }); + expect(mockCallGatewayTool).toHaveBeenCalledWith( + "plugin.approval.request", + expect.any(Object), + expect.objectContaining({ + description: expect.stringContaining("App: GitHub"), + }), + { expectFinal: false }, + ); + const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as { + description: string; + }; + expect(approvalRequest.description).toContain("Tool: Create pull request"); + expect(approvalRequest.description).toContain("Repository: openclaw/openclaw"); + }); + + it("accepts approval elicitations with a null turn id when the thread matches", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-null-turn", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-null-turn", decision: "allow-once" }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: { + ...buildCurrentCodexApprovalElicitation(), + turnId: null, + }, + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + }); + + expect(result).toEqual({ + action: "accept", + content: null, + _meta: null, + }); + }); + + it("ignores unscoped approval elicitations without the active thread id", async () => { + const { turnId, serverName, mode, message, _meta, requestedSchema } = + buildCurrentCodexApprovalElicitation(); + const result = await handleCodexAppServerElicitationRequest({ + requestParams: { turnId, serverName, mode, message, _meta, requestedSchema }, + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + }); + + expect(result).toBeUndefined(); + expect(mockCallGatewayTool).not.toHaveBeenCalled(); + }); + + it("maps allow-always decisions onto persistent approval metadata when offered", async () => { mockCallGatewayTool .mockResolvedValueOnce({ id: "plugin:approval-2", status: "accepted" }) .mockResolvedValueOnce({ id: "plugin:approval-2", decision: "allow-always" }); @@ -100,9 +188,32 @@ describe("Codex app-server elicitation bridge", () => { action: "accept", content: { approve: true, - persist: "session", + persist: "always", + }, + _meta: { + persist: "always", + }, + }); + }); + + it("maps allow-always decisions onto metadata for current empty-schema approvals", async () => { + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-current-always", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-current-always", decision: "allow-always" }); + + const result = await handleCodexAppServerElicitationRequest({ + requestParams: buildCurrentCodexApprovalElicitation(), + paramsForRun: createParams(), + threadId: "thread-1", + turnId: "turn-1", + }); + + expect(result).toEqual({ + action: "accept", + content: null, + _meta: { + persist: "always", }, - _meta: null, }); }); diff --git a/extensions/codex/src/app-server/elicitation-bridge.ts b/extensions/codex/src/app-server/elicitation-bridge.ts index 714d88a6d5b..c22775edfb7 100644 --- a/extensions/codex/src/app-server/elicitation-bridge.ts +++ b/extensions/codex/src/app-server/elicitation-bridge.ts @@ -20,6 +20,14 @@ type BridgeableApprovalElicitation = { meta: JsonObject; }; +const MCP_TOOL_APPROVAL_KIND = "mcp_tool_call"; +const MCP_TOOL_APPROVAL_KIND_KEY = "codex_approval_kind"; +const MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY = "connector_name"; +const MCP_TOOL_APPROVAL_TOOL_TITLE_KEY = "tool_title"; +const MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY = "tool_description"; +const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY = "tool_params_display"; +const MAX_DISPLAY_PARAM_VALUE_LENGTH = 120; + export async function handleCodexAppServerElicitationRequest(params: { requestParams: JsonValue | undefined; paramsForRun: EmbeddedRunAttemptParams; @@ -71,7 +79,7 @@ function readBridgeableApprovalElicitation( !requestParams || readString(requestParams, "mode") !== "form" || !isJsonObject(requestParams._meta) || - requestParams._meta.codex_approval_kind !== "mcp_tool_call" || + requestParams._meta[MCP_TOOL_APPROVAL_KIND_KEY] !== MCP_TOOL_APPROVAL_KIND || !isJsonObject(requestParams.requestedSchema) ) { return undefined; @@ -80,14 +88,54 @@ function readBridgeableApprovalElicitation( const requestedSchema = requestParams.requestedSchema; if ( readString(requestedSchema, "type") !== "object" || - !isJsonObject(requestedSchema.properties) || - Object.keys(requestedSchema.properties).length === 0 + !isJsonObject(requestedSchema.properties) ) { return undefined; } const title = readString(requestParams, "message") ?? "Codex MCP tool approval"; - const propertyLines = Object.entries(requestedSchema.properties) + return { + title, + description: buildApprovalDescription({ + title, + meta: requestParams._meta, + requestedSchema, + serverName: readString(requestParams, "serverName"), + }), + requestedSchema, + meta: requestParams._meta, + }; +} + +function buildApprovalDescription(params: { + title: string; + meta: JsonObject; + requestedSchema: JsonObject; + serverName: string | undefined; +}): string { + const summaryLines = [ + readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY) && + `App: ${readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY)}`, + readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY) && + `Tool: ${readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY)}`, + params.serverName && `MCP server: ${params.serverName}`, + readString(params.meta, MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY), + ].filter((line): line is string => Boolean(line)); + const paramLines = readDisplayParamLines(params.meta); + const propertyLines = readPropertyDescriptionLines(params.requestedSchema); + return [ + params.title, + summaryLines.join("\n"), + paramLines.length > 0 ? ["Parameters:", ...paramLines].join("\n") : "", + propertyLines.length > 0 ? ["Fields:", ...propertyLines].join("\n") : "", + ] + .filter(Boolean) + .join("\n\n"); +} + +function readPropertyDescriptionLines(requestedSchema: JsonObject): string[] { + const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : {}; + return Object.entries(properties) .map(([name, value]) => { const schema = isJsonObject(value) ? value : undefined; if (!schema) { @@ -98,15 +146,33 @@ function readBridgeableApprovalElicitation( return description ? `- ${propTitle}: ${description}` : `- ${propTitle}`; }) .filter((line): line is string => Boolean(line)); +} - return { - title, - description: [title, propertyLines.length > 0 ? ["Fields:", ...propertyLines].join("\n") : ""] - .filter(Boolean) - .join("\n\n"), - requestedSchema, - meta: requestParams._meta, - }; +function readDisplayParamLines(meta: JsonObject): string[] { + const displayParams = meta[MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY]; + if (!Array.isArray(displayParams)) { + return []; + } + return displayParams + .map((entry) => { + const param = isJsonObject(entry) ? entry : undefined; + if (!param) { + return undefined; + } + const name = readString(param, "display_name") ?? readString(param, "name"); + if (!name) { + return undefined; + } + return `- ${name}: ${formatDisplayParamValue(param.value)}`; + }) + .filter((line): line is string => Boolean(line)); +} + +function formatDisplayParamValue(value: JsonValue | undefined): string { + const formatted = typeof value === "string" ? value : JSON.stringify(value ?? null); + return formatted.length <= MAX_DISPLAY_PARAM_VALUE_LENGTH + ? formatted + : `${formatted.slice(0, MAX_DISPLAY_PARAM_VALUE_LENGTH - 3)}...`; } async function requestPluginApprovalOutcome(params: { @@ -152,14 +218,21 @@ function buildElicitationResponse( const content = buildAcceptedContent(requestedSchema, meta, outcome); if (!content) { + if (hasNoSchemaProperties(requestedSchema)) { + return { + action: "accept", + content: null, + _meta: buildAcceptedMeta(meta, outcome), + }; + } embeddedAgentLog.warn("codex MCP approval elicitation approved without a mappable response", { - approvalKind: meta.codex_approval_kind, + approvalKind: meta[MCP_TOOL_APPROVAL_KIND_KEY], fields: Object.keys(requestedSchema.properties ?? {}), outcome, }); return { action: "decline", content: null, _meta: null }; } - return { action: "accept", content, _meta: null }; + return { action: "accept", content, _meta: buildAcceptedMeta(meta, outcome) }; } function buildAcceptedContent( @@ -248,15 +321,14 @@ function readPersistFieldValue( if (options.length === 0) { return undefined; } - for (const preferred of persistHints) { + const preferred = choosePersistHint(persistHints); + if (preferred) { const match = options.find( (option) => option.value === preferred || option.label === preferred, ); - if (match) { - return match.value; - } + return match?.value; } - return options.find((option) => option.value === "session" || option.label === "session")?.value; + return undefined; } function readDefaultValue(schema: JsonObject): JsonValue | undefined { @@ -304,6 +376,29 @@ function readPersistHints(meta: JsonObject): string[] { return ["session", "always"]; } +function buildAcceptedMeta(meta: JsonObject, outcome: AppServerApprovalOutcome): JsonObject | null { + if (outcome !== "approved-session") { + return null; + } + const persist = choosePersistHint(readPersistHints(meta)); + return persist ? { persist } : null; +} + +function choosePersistHint(persistHints: string[]): "always" | "session" | undefined { + if (persistHints.includes("always")) { + return "always"; + } + if (persistHints.includes("session")) { + return "session"; + } + return undefined; +} + +function hasNoSchemaProperties(requestedSchema: JsonObject): boolean { + const properties = isJsonObject(requestedSchema.properties) ? requestedSchema.properties : {}; + return Object.keys(properties).length === 0; +} + function readEnumOptions(schema: JsonObject): Array<{ value: string; label: string }> { if (Array.isArray(schema.enum)) { const values = schema.enum.filter((entry): entry is string => typeof entry === "string"); diff --git a/extensions/codex/src/app-server/run-attempt.test.ts b/extensions/codex/src/app-server/run-attempt.test.ts index 7c36ee48e53..f67f8d136dd 100644 --- a/extensions/codex/src/app-server/run-attempt.test.ts +++ b/extensions/codex/src/app-server/run-attempt.test.ts @@ -533,6 +533,87 @@ describe("runCodexAppServerAttempt", () => { ); }); + it("routes MCP approval elicitations through the native bridge", async () => { + let notify: (notification: CodexServerNotification) => Promise = async () => undefined; + let handleRequest: + | ((request: { id: string; method: string; params?: unknown }) => Promise) + | undefined; + const bridgeSpy = vi + .spyOn(elicitationBridge, "handleCodexAppServerElicitationRequest") + .mockResolvedValue({ + action: "accept", + content: null, + _meta: null, + }); + const request = vi.fn(async (method: string) => { + if (method === "thread/start") { + return threadStartResult(); + } + if (method === "turn/start") { + return turnStartResult(); + } + return {}; + }); + __testing.setCodexAppServerClientFactoryForTests( + async () => + ({ + request, + addNotificationHandler: (handler: typeof notify) => { + notify = handler; + return () => undefined; + }, + addRequestHandler: ( + handler: (request: { + id: string; + method: string; + params?: unknown; + }) => Promise, + ) => { + handleRequest = handler; + return () => undefined; + }, + }) as never, + ); + + const run = runCodexAppServerAttempt( + createParams(path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace")), + ); + await vi.waitFor(() => expect(handleRequest).toBeTypeOf("function"), { interval: 1 }); + + const result = await handleRequest?.({ + id: "request-elicitation-1", + method: "mcpServer/elicitation/request", + params: { + threadId: "thread-1", + turnId: "turn-1", + serverName: "codex_apps__github", + mode: "form", + }, + }); + + expect(result).toEqual({ + action: "accept", + content: null, + _meta: null, + }); + expect(bridgeSpy).toHaveBeenCalledWith( + expect.objectContaining({ + threadId: "thread-1", + turnId: "turn-1", + }), + ); + + await notify({ + method: "turn/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + turn: { id: "turn-1", status: "completed" }, + }, + }); + await run; + }); + it("does not leak unhandled rejections when shutdown closes before interrupt", async () => { const unhandledRejections: unknown[] = []; const onUnhandledRejection = (reason: unknown) => { diff --git a/src/gateway/gateway-codex-harness.live-helpers.ts b/src/gateway/gateway-codex-harness.live-helpers.ts index 24612dce581..bc8a338fbb8 100644 --- a/src/gateway/gateway-codex-harness.live-helpers.ts +++ b/src/gateway/gateway-codex-harness.live-helpers.ts @@ -1,6 +1,7 @@ export const EXPECTED_CODEX_MODELS_COMMAND_TEXT = [ "Codex models:", "Available Codex models", + "Available models, local cache:", "Available agent target:", "Available agent targets:", "opened an interactive trust prompt",