LLM-27035 [Codex] Add missing elicitation, LLM-27036 [Codex] Improve mcp block detection

src/CodexAcpClient.ts

@@ -2,7 +2,7 @@ import {isCodexAuthRequest} from "./CodexAuthMethod";
 import type {EmbeddedResourceResource} from "@agentclientprotocol/sdk";
 import * as acp from "@agentclientprotocol/sdk";
 import {type McpServer, RequestError} from "@agentclientprotocol/sdk";
-import type {ApprovalHandler, CodexAppServerClient} from "./CodexAppServerClient";
+import type {ApprovalHandler, CodexAppServerClient, ElicitationHandler} from "./CodexAppServerClient";
 import open from "open";
 import type {Disposable} from "vscode-jsonrpc";
 import type {
@@ -360,10 +360,12 @@ export class CodexAcpClient {
     async subscribeToSessionEvents(
         sessionId: string,
         eventHandler: (result: ServerNotification) => void,
-        approvalHandler: ApprovalHandler
+        approvalHandler: ApprovalHandler,
+        elicitationHandler: ElicitationHandler
     ) {
         this.codexClient.onServerNotification(sessionId, eventHandler);
         this.codexClient.onApprovalRequest(sessionId, approvalHandler);
+        this.codexClient.onElicitationRequest(sessionId, elicitationHandler);
     }
 
     async sendPrompt(

src/CodexAcpServer.ts

@@ -7,6 +7,7 @@ import {
 } from "@agentclientprotocol/sdk";
 import {CodexEventHandler} from "./CodexEventHandler";
 import {CodexApprovalHandler} from "./CodexApprovalHandler";
+import {CodexElicitationHandler} from "./CodexElicitationHandler";
 import {CodexAuthMethods, type CodexAuthRequest} from "./CodexAuthMethod";
 import {CodexAcpClient, type SessionMetadata, type SessionMetadataWithThread} from "./CodexAcpClient";
 import {ACPSessionConnection, type UpdateSessionEvent} from "./ACPSessionConnection";
@@ -707,9 +708,14 @@ export class CodexAcpServer implements acp.Agent {
         try {
             const eventHandler = new CodexEventHandler(this.connection, sessionState);
             const approvalHandler = new CodexApprovalHandler(this.connection, sessionState);
+            const elicitationHandler = new CodexElicitationHandler(this.connection, sessionState);
             await this.codexAcpClient.subscribeToSessionEvents(params.sessionId,
-                (event) => eventHandler.handleNotification(event),
-                approvalHandler);
+                (event) => {
+                    elicitationHandler.handleNotification(event);
+                    return eventHandler.handleNotification(event);
+                },
+                approvalHandler,
+                elicitationHandler);
 
             if (await this.availableCommands.tryHandle(params.prompt, sessionState)) {
                 logger.log("Prompt handled by a command");

src/CodexAppServerClient.ts

@@ -28,6 +28,8 @@ import type {
     CommandExecutionRequestApprovalResponse,
     FileChangeRequestApprovalParams,
     FileChangeRequestApprovalResponse,
+    McpServerElicitationRequestParams,
+    McpServerElicitationRequestResponse,
     ThreadResumeParams,
     ThreadResumeResponse,
     SkillsListParams,
@@ -41,6 +43,10 @@ export interface ApprovalHandler {
     handleFileChange(params: FileChangeRequestApprovalParams): Promise<FileChangeRequestApprovalResponse>;
 }
 
+export interface ElicitationHandler {
+    handleElicitation(params: McpServerElicitationRequestParams): Promise<McpServerElicitationRequestResponse>;
+}
+
 const CommandExecutionApprovalRequest = new RequestType<
     CommandExecutionRequestApprovalParams,
     CommandExecutionRequestApprovalResponse,
@@ -53,13 +59,20 @@ const FileChangeApprovalRequest = new RequestType<
     void
 >('item/fileChange/requestApproval');
 
+const McpServerElicitationRequest = new RequestType<
+    McpServerElicitationRequestParams,
+    McpServerElicitationRequestResponse,
+    void
+>('mcpServer/elicitation/request');
+
 /**
  * A type-safe client over the Codex App Server's JSON-RPC API.
  * Maps each request to its expected response and exposes clear, typed methods for supported JSON-RPC operations.
  */
 export class CodexAppServerClient {
     readonly connection: MessageConnection;
     private approvalHandlers = new Map<string, ApprovalHandler>();
+    private elicitationHandlers = new Map<string, ElicitationHandler>();
     private mcpStartupCompleteVersion = 0;
     private lastMcpStartupComplete: McpStartupCompleteEvent | null = null;
     private readonly mcpStartupCompleteResolvers: Array<SignalResolver<McpStartupCompleteEvent>> = [];
@@ -98,12 +111,24 @@ export class CodexAppServerClient {
             }
             return await handler.handleFileChange(params);
         });
+
+        this.connection.onRequest(McpServerElicitationRequest, async (params) => {
+            const handler = this.elicitationHandlers.get(params.threadId);
+            if (!handler) {
+                return { action: "cancel", content: null, _meta: null };
+            }
+            return await handler.handleElicitation(params);
+        });
     }
 
     onApprovalRequest(threadId: string, handler: ApprovalHandler): void {
         this.approvalHandlers.set(threadId, handler);
     }
 
+    onElicitationRequest(threadId: string, handler: ElicitationHandler): void {
+        this.elicitationHandlers.set(threadId, handler);
+    }
+
     async initialize(params: InitializeParams): Promise<InitializeResponse> {
         return await this.sendRequest({ method: "initialize", params: params });
     }

src/CodexElicitationHandler.ts

@@ -0,0 +1,266 @@
+import * as acp from "@agentclientprotocol/sdk";
+import type { SessionState } from "./CodexAcpServer";
+import type { ElicitationHandler } from "./CodexAppServerClient";
+import type { ServerNotification } from "./app-server";
+import type {
+    ItemCompletedNotification,
+    ItemStartedNotification,
+    McpServerElicitationRequestParams,
+    McpServerElicitationRequestResponse,
+} from "./app-server/v2";
+import { logger } from "./Logger";
+
+// Standard elicitation options (non-tool-call approval).
+const ELICITATION_OPTIONS: acp.PermissionOption[] = [
+    { optionId: "accept", name: "Accept", kind: "allow_once" },
+    { optionId: "decline", name: "Decline", kind: "reject_once" },
+];
+
+// Option IDs used for MCP tool call approval persist choices.
+const OPTION_ALLOW_ONCE = "allow_once";
+const OPTION_ALLOW_SESSION = "allow_session";
+const OPTION_ALLOW_ALWAYS = "allow_always";
+
+type PersistValue = "session" | "always";
+
+/**
+ * Parses the `persist` field from the elicitation request `_meta`.
+ * Codex advertises which persistence options the client should show.
+ * Returns a set of supported persist values.
+ */
+function parsePersistOptions(meta: unknown): Set<PersistValue> {
+    const result = new Set<PersistValue>();
+    if (!meta || typeof meta !== "object") return result;
+    const persist = (meta as Record<string, unknown>)["persist"];
+    if (persist === "session") {
+        result.add("session");
+    } else if (persist === "always") {
+        result.add("always");
+    } else if (Array.isArray(persist)) {
+        if (persist.includes("session")) result.add("session");
+        if (persist.includes("always")) result.add("always");
+    }
+    return result;
+}
+
+function isMcpToolCallApproval(meta: unknown): boolean {
+    return (
+        meta !== null &&
+        typeof meta === "object" &&
+        (meta as Record<string, unknown>)["codex_approval_kind"] === "mcp_tool_call"
+    );
+}
+
+/**
+ * Builds the ACP permission options for an MCP tool call approval elicitation.
+ * Always includes "Allow Once"; adds session/always persist options when advertised.
+ */
+function buildToolApprovalOptions(persistOptions: Set<PersistValue>): acp.PermissionOption[] {
+    const options: acp.PermissionOption[] = [
+        { optionId: OPTION_ALLOW_ONCE, name: "Allow", kind: "allow_once" },
+    ];
+    if (persistOptions.has("session")) {
+        options.push({ optionId: OPTION_ALLOW_SESSION, name: "Allow for This Session", kind: "allow_always" });
+    }
+    if (persistOptions.has("always")) {
+        options.push({ optionId: OPTION_ALLOW_ALWAYS, name: "Allow and Don't Ask Again", kind: "allow_always" });
+    }
+    options.push({ optionId: "decline", name: "Decline", kind: "reject_once" });
+    return options;
+}
+
+export class CodexElicitationHandler implements ElicitationHandler {
+    private readonly connection: acp.AgentSideConnection;
+    private readonly sessionState: SessionState;
+    // In Rust, the MCP elicitation handler receives ElicitationRequestEvent directly from the MCP
+    // protocol layer, where id is set to "mcp_tool_call_approval_<call_id>" — the call ID is extracted
+    // by stripping that prefix.
+    //
+    // In TypeScript, Codex speaks the app-server JSON-RPC protocol (v2), where
+    // McpServerElicitationRequestParams omits elicitationId for form mode, so the MCP-level ID never
+    // reaches the client.
+    //
+    // Workaround: before requesting approval, Codex emits an item/started notification with an
+    // mcpToolCall item carrying the call id and server name. We store (threadId, serverName) → callId
+    // here so the elicitation request can correlate back to the already-rendered tool call item.
+    //
+    // Multiple calls are safe because Codex requests approval synchronously — it blocks on one tool
+    // call's elicitation before starting the next, so there is at most one pending approval per
+    // (threadId, serverName).
+    private readonly pendingMcpApprovals = new Map<string, string>();
+
+    constructor(connection: acp.AgentSideConnection, sessionState: SessionState) {
+        this.connection = connection;
+        this.sessionState = sessionState;
+    }
+
+    handleNotification(notification: ServerNotification): void {
+        switch (notification.method) {
+            case "item/started":
+                this.handleItemStarted(notification.params);
+                return;
+            case "item/completed":
+                this.handleItemCompleted(notification.params);
+                return;
+            case "serverRequest/resolved":
+                this.clearThread(notification.params.threadId);
+                return;
+            default:
+                return;
+        }
+    }
+
+    async handleElicitation(
+        params: McpServerElicitationRequestParams
+    ): Promise<McpServerElicitationRequestResponse> {
+        try {
+            const { request, correlatedCallId } = this.buildPermissionRequest(params);
+            const response = await this.connection.requestPermission(request);
+            if (correlatedCallId !== undefined && response.outcome.outcome !== "cancelled") {
+                const optionId = response.outcome.optionId;
+                if (optionId !== "decline") {
+                    await this.connection.sessionUpdate({
+                        sessionId: this.sessionState.sessionId,
+                        update: { sessionUpdate: "tool_call_update", toolCallId: correlatedCallId, status: "in_progress" },
+                    });
+                }
+            }
+            return this.convertResponse(response);
+        } catch (error) {
+            logger.error("Error handling MCP elicitation request", error);
+            return { action: "cancel", content: null, _meta: null };
+        }
+    }
+
+    private buildPermissionRequest(
+        params: McpServerElicitationRequestParams
+    ): { request: acp.RequestPermissionRequest; correlatedCallId: string | undefined } {
+        const sessionId = this.sessionState.sessionId;
+        const messageContent: acp.ToolCallContent = {
+            type: "content",
+            content: { type: "text", text: params.message },
+        };
+
+        const meta = params._meta;
+        const isToolApproval = isMcpToolCallApproval(meta);
+        const options = isToolApproval
+            ? buildToolApprovalOptions(parsePersistOptions(meta))
+            : ELICITATION_OPTIONS;
+
+        if (params.mode === "form") {
+            const correlatedCallId = isToolApproval
+                ? this.popPendingApproval(params.threadId, params.serverName)
+                : undefined;
+            if (correlatedCallId !== undefined) {
+                // The tool call item is already visible in the IDE conversation history because
+                // item/started was emitted before the elicitation request. Sending content or
+                // rawInput here would duplicate that information in the approval widget.
+                return {
+                    request: {
+                        sessionId,
+                        toolCall: {
+                            toolCallId: correlatedCallId,
+                            kind: "execute",
+                            status: "pending",
+                            // content: [messageContent],   — omitted: already rendered via item/started
+                            // rawInput: { ... }            — omitted: same reason
+                        },
+                        _meta: { is_mcp_tool_approval: true },
+                        options,
+                    },
+                    correlatedCallId,
+                };
+            }
+            return {
+                request: {
+                    sessionId,
+                    toolCall: {
+                        toolCallId: `elicitation-${params.serverName}`,
+                        kind: isToolApproval ? "execute" : "other",
+                        status: "pending",
+                        content: [messageContent],
+                        rawInput: { serverName: params.serverName, schema: params.requestedSchema },
+                    },
+                    ...(isToolApproval ? { _meta: { is_mcp_tool_approval: true } } : {}),
+                    options,
+                },
+                correlatedCallId: undefined,
+            };
+        } else {
+            return {
+                request: {
+                    sessionId,
+                    toolCall: {
+                        toolCallId: `elicitation-${params.elicitationId}`,
+                        kind: "fetch",
+                        status: "pending",
+                        content: [messageContent],
+                        rawInput: { serverName: params.serverName, url: params.url },
+                    },
+                    options,
+                },
+                correlatedCallId: undefined,
+            };
+        }
+    }
+
+    private convertResponse(
+        response: acp.RequestPermissionResponse
+    ): McpServerElicitationRequestResponse {
+        if (response.outcome.outcome === "cancelled") {
+            return { action: "cancel", content: null, _meta: null };
+        }
+
+        const optionId = response.outcome.optionId;
+        if (optionId === OPTION_ALLOW_SESSION) {
+            return { action: "accept", content: null, _meta: { persist: "session" } };
+        }
+        if (optionId === OPTION_ALLOW_ALWAYS) {
+            return { action: "accept", content: null, _meta: { persist: "always" } };
+        }
+        if (optionId === OPTION_ALLOW_ONCE || optionId === "accept") {
+            return { action: "accept", content: null, _meta: null };
+        }
+        return { action: "decline", content: null, _meta: null };
+    }
+
+    private handleItemStarted(event: ItemStartedNotification): void {
+        if (event.item.type !== "mcpToolCall") {
+            return;
+        }
+        this.pendingMcpApprovals.set(this.key(event.threadId, event.item.server), event.item.id);
+    }
+
+    private handleItemCompleted(event: ItemCompletedNotification): void {
+        if (event.item.type !== "mcpToolCall") {
+            return;
+        }
+        // This may run after the elicitation path already consumed the same entry.
+        // That double-pop is intentional: approvals pop on request correlation, while
+        // auto-approved or interrupted calls need completion-side cleanup.
+        this.popPendingApproval(event.threadId, event.item.server);
+    }
+
+    private popPendingApproval(threadId: string, serverName: string): string | undefined {
+        const key = this.key(threadId, serverName);
+        const callId = this.pendingMcpApprovals.get(key);
+        this.pendingMcpApprovals.delete(key);
+        return callId;
+    }
+
+    private clearThread(threadId: string): void {
+        for (const key of this.pendingMcpApprovals.keys()) {
+            if (this.belongsToThread(key, threadId)) {
+                this.pendingMcpApprovals.delete(key);
+            }
+        }
+    }
+
+    private key(threadId: string, serverName: string): string {
+        return `${threadId}:${serverName}`;
+    }
+
+    private belongsToThread(key: string, threadId: string): boolean {
+        return key.startsWith(`${threadId}:`);
+    }
+}

src/CodexEventHandler.ts

@@ -103,10 +103,10 @@ export class CodexEventHandler {
             case "turn/diff/updated":
             case "item/commandExecution/terminalInteraction":
             case "item/fileChange/outputDelta":
-            case "serverRequest/resolved":
             case "account/updated":
             case "fs/changed":
             case "mcpServer/startupStatus/updated":
+            case "serverRequest/resolved":
                 return null;
             case "item/mcpToolCall/progress":
                 return this.createMcpToolProgressEvent(notification.params);