Skip to content

Commit 7f8d7ca

Browse files
fix: pop() stale entries mcpToolCall
1 parent 7c54ae3 commit 7f8d7ca

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

src/CodexEventHandler.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ export class CodexEventHandler {
226226
status: event.item.status === "completed" ? "completed" : "failed",
227227
}
228228
case "mcpToolCall":
229+
this.pendingMcpApprovals?.pop(event.threadId, event.item.server);
229230
return {
230231
sessionUpdate: "tool_call_update",
231232
toolCallId: event.item.id,

src/__tests__/CodexACPAgent/elicitation-events.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { McpServerElicitationRequestParams } from '../../app-server/v2';
33
import { createCodexMockTestFixture, createTestSessionState, type CodexMockTestFixture } from '../acp-test-utils';
44
import type { SessionState } from '../../CodexAcpServer';
55
import { AgentMode } from "../../AgentMode";
6+
import type { ServerNotification } from "../../app-server";
67

78
describe('Elicitation Events', () => {
89
let fixture: CodexMockTestFixture;
@@ -242,6 +243,69 @@ describe('Elicitation Events', () => {
242243
completeTurn();
243244
await promptPromise;
244245
});
246+
247+
it('should not reuse a completed auto-approved call id for a later approval request', async () => {
248+
const { promptPromise, completeTurn } = setupSessionWithPendingPrompt();
249+
fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } });
250+
251+
const startedNotification: ServerNotification = {
252+
method: 'item/started',
253+
params: {
254+
threadId: sessionId,
255+
turnId: 'turn-1',
256+
item: {
257+
type: "mcpToolCall",
258+
id: "completed-call-id",
259+
server: "tool-server",
260+
tool: "tool-name",
261+
status: "inProgress",
262+
arguments: { argument: "example" },
263+
result: null,
264+
error: null,
265+
durationMs: null,
266+
},
267+
},
268+
};
269+
const completedNotification: ServerNotification = {
270+
method: 'item/completed',
271+
params: {
272+
threadId: sessionId,
273+
turnId: 'turn-1',
274+
item: {
275+
type: "mcpToolCall",
276+
id: "completed-call-id",
277+
server: "tool-server",
278+
tool: "tool-name",
279+
status: "completed",
280+
arguments: { argument: "example" },
281+
result: { content: [], structuredContent: null, _meta: null },
282+
error: null,
283+
durationMs: 15,
284+
},
285+
},
286+
};
287+
288+
fixture.sendServerNotification(startedNotification);
289+
fixture.sendServerNotification(completedNotification);
290+
fixture.clearAcpConnectionDump();
291+
292+
const params: McpServerElicitationRequestParams = {
293+
threadId: sessionId, turnId: 'turn-2', serverName: 'tool-server',
294+
mode: 'form',
295+
_meta: { codex_approval_kind: 'mcp_tool_call', persist: ['session', 'always'] },
296+
message: 'Allow tool call?',
297+
requestedSchema: { type: 'object', properties: {} },
298+
};
299+
300+
await fixture.sendServerRequest('mcpServer/elicitation/request', params);
301+
302+
const [requestPermissionEvent] = fixture.getAcpConnectionEvents(['_meta']);
303+
expect(requestPermissionEvent?.method).toBe('requestPermission');
304+
expect(requestPermissionEvent?.args[0].toolCall.toolCallId).toBe('elicitation-tool-server');
305+
306+
completeTurn();
307+
await promptPromise;
308+
});
245309
});
246310

247311
describe('URL mode elicitation', () => {

0 commit comments

Comments
 (0)