Skip to content

Commit 6cc27eb

Browse files
NikolaiSviridovNikolaiSviridov
committed
fix: move pendingMcpApprovals to src/CodexElicitationHandler.ts
1 parent f7e743f commit 6cc27eb

4 files changed

Lines changed: 81 additions & 65 deletions

File tree

‎src/CodexAcpServer.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
import {CodexEventHandler} from "./CodexEventHandler";
99
import {CodexApprovalHandler} from "./CodexApprovalHandler";
1010
import {CodexElicitationHandler} from "./CodexElicitationHandler";
11-
import {PendingMcpApprovals} from "./PendingMcpApprovals";
1211
import {CodexAuthMethods, type CodexAuthRequest} from "./CodexAuthMethod";
1312
import {CodexAcpClient, type SessionMetadata, type SessionMetadataWithThread} from "./CodexAcpClient";
1413
import {ACPSessionConnection, type UpdateSessionEvent} from "./ACPSessionConnection";
@@ -707,12 +706,14 @@ export class CodexAcpServer implements acp.Agent {
707706
sessionState.lastTokenUsage = null;
708707

709708
try {
710-
const pendingMcpApprovals = new PendingMcpApprovals();
711-
const eventHandler = new CodexEventHandler(this.connection, sessionState, pendingMcpApprovals);
709+
const eventHandler = new CodexEventHandler(this.connection, sessionState);
712710
const approvalHandler = new CodexApprovalHandler(this.connection, sessionState);
713-
const elicitationHandler = new CodexElicitationHandler(this.connection, sessionState, pendingMcpApprovals);
711+
const elicitationHandler = new CodexElicitationHandler(this.connection, sessionState);
714712
await this.codexAcpClient.subscribeToSessionEvents(params.sessionId,
715-
(event) => eventHandler.handleNotification(event),
713+
(event) => {
714+
elicitationHandler.handleNotification(event);
715+
return eventHandler.handleNotification(event);
716+
},
716717
approvalHandler,
717718
elicitationHandler);
718719

‎src/CodexElicitationHandler.ts‎

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import * as acp from "@agentclientprotocol/sdk";
22
import type { SessionState } from "./CodexAcpServer";
33
import type { ElicitationHandler } from "./CodexAppServerClient";
4+
import type { ServerNotification } from "./app-server";
45
import type {
6+
ItemCompletedNotification,
7+
ItemStartedNotification,
58
McpServerElicitationRequestParams,
69
McpServerElicitationRequestResponse,
710
} from "./app-server/v2";
811
import { logger } from "./Logger";
9-
import type { PendingMcpApprovals } from "./PendingMcpApprovals";
1012

1113
// Standard elicitation options (non-tool-call approval).
1214
const ELICITATION_OPTIONS: acp.PermissionOption[] = [
@@ -70,16 +72,42 @@ function buildToolApprovalOptions(persistOptions: Set<PersistValue>): acp.Permis
7072
export class CodexElicitationHandler implements ElicitationHandler {
7173
private readonly connection: acp.AgentSideConnection;
7274
private readonly sessionState: SessionState;
73-
private readonly pendingMcpApprovals: PendingMcpApprovals | undefined;
75+
// In Rust, the MCP elicitation handler receives ElicitationRequestEvent directly from the MCP
76+
// protocol layer, where id is set to "mcp_tool_call_approval_<call_id>" — the call ID is extracted
77+
// by stripping that prefix.
78+
//
79+
// In TypeScript, Codex speaks the app-server JSON-RPC protocol (v2), where
80+
// McpServerElicitationRequestParams omits elicitationId for form mode, so the MCP-level ID never
81+
// reaches the client.
82+
//
83+
// Workaround: before requesting approval, Codex emits an item/started notification with an
84+
// mcpToolCall item carrying the call id and server name. We store (threadId, serverName) → callId
85+
// here so the elicitation request can correlate back to the already-rendered tool call item.
86+
//
87+
// Multiple calls are safe because Codex requests approval synchronously — it blocks on one tool
88+
// call's elicitation before starting the next, so there is at most one pending approval per
89+
// (threadId, serverName).
90+
private readonly pendingMcpApprovals = new Map<string, string>();
7491

75-
constructor(
76-
connection: acp.AgentSideConnection,
77-
sessionState: SessionState,
78-
pendingMcpApprovals?: PendingMcpApprovals
79-
) {
92+
constructor(connection: acp.AgentSideConnection, sessionState: SessionState) {
8093
this.connection = connection;
8194
this.sessionState = sessionState;
82-
this.pendingMcpApprovals = pendingMcpApprovals;
95+
}
96+
97+
handleNotification(notification: ServerNotification): void {
98+
switch (notification.method) {
99+
case "item/started":
100+
this.handleItemStarted(notification.params);
101+
return;
102+
case "item/completed":
103+
this.handleItemCompleted(notification.params);
104+
return;
105+
case "serverRequest/resolved":
106+
this.clearThread(notification.params.threadId);
107+
return;
108+
default:
109+
return;
110+
}
83111
}
84112

85113
async handleElicitation(
@@ -121,7 +149,7 @@ export class CodexElicitationHandler implements ElicitationHandler {
121149

122150
if (params.mode === "form") {
123151
const correlatedCallId = isToolApproval
124-
? this.pendingMcpApprovals?.pop(params.threadId, params.serverName)
152+
? this.popPendingApproval(params.threadId, params.serverName)
125153
: undefined;
126154
if (correlatedCallId !== undefined) {
127155
// The tool call item is already visible in the IDE conversation history because
@@ -193,4 +221,41 @@ export class CodexElicitationHandler implements ElicitationHandler {
193221
}
194222
return { action: "decline", content: null, _meta: null };
195223
}
224+
225+
private handleItemStarted(event: ItemStartedNotification): void {
226+
if (event.item.type !== "mcpToolCall") {
227+
return;
228+
}
229+
this.pendingMcpApprovals.set(this.key(event.threadId, event.item.server), event.item.id);
230+
}
231+
232+
private handleItemCompleted(event: ItemCompletedNotification): void {
233+
if (event.item.type !== "mcpToolCall") {
234+
return;
235+
}
236+
this.popPendingApproval(event.threadId, event.item.server);
237+
}
238+
239+
private popPendingApproval(threadId: string, serverName: string): string | undefined {
240+
const key = this.key(threadId, serverName);
241+
const callId = this.pendingMcpApprovals.get(key);
242+
this.pendingMcpApprovals.delete(key);
243+
return callId;
244+
}
245+
246+
private clearThread(threadId: string): void {
247+
for (const key of this.pendingMcpApprovals.keys()) {
248+
if (this.belongsToThread(key, threadId)) {
249+
this.pendingMcpApprovals.delete(key);
250+
}
251+
}
252+
}
253+
254+
private key(threadId: string, serverName: string): string {
255+
return `${threadId}:${serverName}`;
256+
}
257+
258+
private belongsToThread(key: string, threadId: string): boolean {
259+
return key.startsWith(`${threadId}:`);
260+
}
196261
}

‎src/CodexEventHandler.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import {
3434
fuzzyFileSearchToolCallId,
3535
} from "./CodexToolCallMapper";
3636
import { stripShellPrefix } from "./CommandUtils";
37-
import type { PendingMcpApprovals } from "./PendingMcpApprovals";
3837

3938
export { stripShellPrefix };
4039

@@ -44,12 +43,10 @@ export class CodexEventHandler {
4443
private readonly sessionState: SessionState;
4544
private failure: RequestError | null = null;
4645
private readonly activeFuzzyFileSearchSessions = new Set<string>();
47-
private readonly pendingMcpApprovals: PendingMcpApprovals | undefined;
4846

49-
constructor(connection: acp.AgentSideConnection, sessionState: SessionState, pendingMcpApprovals?: PendingMcpApprovals) {
47+
constructor(connection: acp.AgentSideConnection, sessionState: SessionState) {
5048
this.connection = connection;
5149
this.sessionState = sessionState;
52-
this.pendingMcpApprovals = pendingMcpApprovals;
5350
}
5451

5552
getFailure(): RequestError | null {
@@ -109,9 +106,7 @@ export class CodexEventHandler {
109106
case "account/updated":
110107
case "fs/changed":
111108
case "mcpServer/startupStatus/updated":
112-
return null;
113109
case "serverRequest/resolved":
114-
this.pendingMcpApprovals?.clearThread(notification.params.threadId);
115110
return null;
116111
case "item/mcpToolCall/progress":
117112
return this.createMcpToolProgressEvent(notification.params);
@@ -198,7 +193,6 @@ export class CodexEventHandler {
198193
case "commandExecution":
199194
return await createCommandExecutionUpdate(event.item);
200195
case "mcpToolCall":
201-
this.pendingMcpApprovals?.record(event.threadId, event.item.server, event.item.id);
202196
return await createMcpToolCallUpdate(event.item);
203197
case "dynamicToolCall":
204198
return await createDynamicToolCallUpdate(event.item);
@@ -228,7 +222,6 @@ export class CodexEventHandler {
228222
status: event.item.status === "completed" ? "completed" : "failed",
229223
}
230224
case "mcpToolCall":
231-
this.pendingMcpApprovals?.pop(event.threadId, event.item.server);
232225
return {
233226
sessionUpdate: "tool_call_update",
234227
toolCallId: event.item.id,

‎src/PendingMcpApprovals.ts‎

Lines changed: 0 additions & 43 deletions
This file was deleted.

0 commit comments

Comments
 (0)