From 8cccc1f6fa2cb7535240df2f42becc454eeb3676 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:09:56 +0200 Subject: [PATCH] fix(codex): answer each approval callback that shares one item (#5758) Codex sends several command approval requests for one item when it approves subcommands separately, telling them apart by approvalId. The permission id was derived from the item id alone, so the second request replaced the first and answering it left the first request unanswered. Key the permission on approvalId when Codex sends one. Closes #5615 Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com> --- .../providers/codex-app-server-agent.test.ts | 70 +++++++++++++++++++ .../agent/providers/codex-app-server-agent.ts | 4 +- .../codex/test-utils/fake-app-server.ts | 10 +-- 3 files changed, 79 insertions(+), 5 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index bb7d56d07a..ea52d982e8 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -10,6 +10,7 @@ import { fileURLToPath } from "node:url"; import type { AgentLaunchContext, + AgentPermissionRequest, AgentSession, AgentSessionConfig, AgentSlashCommand, @@ -1346,6 +1347,75 @@ describe("Codex app-server provider", () => { await session.close(); }); + test("answers each concurrent command approval callback that shares one item", async () => { + const appServer = createFakeCodexAppServer({ + initialize: () => ({}), + "collaborationMode/list": () => ({ data: [] }), + "skills/list": () => ({ data: [] }), + }); + const session = new CodexAppServerAgentSession( + createConfig({ cwd: "/workspace/project" }), + null, + createTestLogger(), + async () => appServer.child, + ); + const requested: AgentPermissionRequest[] = []; + session.subscribe((event) => { + if (event.type === "permission_requested") requested.push(event.request); + }); + + try { + await session.connect(); + + const firstPermission = waitForNextPermission(session); + appServer.requestCommandApproval({ + itemId: "shared-item", + approvalId: "callback-a", + threadId: "thread-1", + turnId: "turn-1", + command: "git add a.txt", + cwd: "/workspace/project", + reason: "subcommand a", + }); + await firstPermission; + const secondPermission = waitForNextPermission(session); + appServer.requestCommandApproval({ + itemId: "shared-item", + approvalId: "callback-b", + threadId: "thread-1", + turnId: "turn-1", + command: "git add b.txt", + cwd: "/workspace/project", + reason: "subcommand b", + }); + await secondPermission; + + expect(requested.map((request) => request.input?.command)).toEqual([ + "git add a.txt", + "git add b.txt", + ]); + expect(new Set(requested.map((request) => request.id)).size).toBe(2); + expect(session.getPendingPermissions()).toHaveLength(2); + + await session.respondToPermission(requested[0]!.id, { behavior: "allow" }); + await session.respondToPermission(requested[1]!.id, { + behavior: "deny", + message: "not b", + }); + + await expect(appServer.waitForCommandApprovalDecision("callback-a")).resolves.toEqual({ + decision: "accept", + }); + await expect(appServer.waitForCommandApprovalDecision("callback-b")).resolves.toEqual({ + decision: "decline", + }); + expect(session.getPendingPermissions()).toHaveLength(0); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }); + test("shows a successful shell command that produces no output", async () => { const appServer = createFakeCodexAppServer(); const session = new CodexAppServerAgentSession( diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 119be789b9..b1dad5332c 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -6933,6 +6933,8 @@ export class CodexAppServerAgentSession implements AgentSession { const parsed = z .object({ itemId: z.string(), + // Set when several approval callbacks belong to one item; it names the callback. + approvalId: z.string().nullable().optional(), threadId: z.string(), turnId: z.string(), command: z.string().nullable().optional(), @@ -6946,7 +6948,7 @@ export class CodexAppServerAgentSession implements AgentSession { cwd: parsed.cwd ?? this.config.cwd ?? null, running: true, }); - const requestId = `permission-${parsed.itemId}`; + const requestId = `permission-${parsed.approvalId ?? parsed.itemId}`; const title = parsed.command ? `Run command: ${parsed.command}` : "Run command"; const request: AgentPermissionRequest = { id: requestId, diff --git a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts index 31e231cdb1..713d744309 100644 --- a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts +++ b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts @@ -85,13 +85,15 @@ export interface FakeCodexAppServer { says(params: { threadId: string; itemId?: string; text: string; chunks?: string[] }): void; requestCommandApproval(params: { itemId: string; + approvalId?: string; threadId: string; turnId: string; command: string; cwd: string; reason: string; }): void; - waitForCommandApprovalDecision(itemId: string): Promise; + /** Keyed by `approvalId` when the request carried one, otherwise by `itemId`. */ + waitForCommandApprovalDecision(callbackId: string): Promise; requestFileChangeApproval(params: { itemId: string; threadId: string; @@ -518,7 +520,7 @@ export function createFakeCodexAppServer( requestCommandApproval(params) { const requestId = nextServerRequestId; nextServerRequestId += 1; - approvalRequestIds.set(params.itemId, requestId); + approvalRequestIds.set(params.approvalId ?? params.itemId, requestId); child.stdout.write( `${JSON.stringify({ jsonrpc: "2.0", @@ -528,8 +530,8 @@ export function createFakeCodexAppServer( })}\n`, ); }, - async waitForCommandApprovalDecision(itemId) { - return await this.waitForApprovalDecision(itemId); + async waitForCommandApprovalDecision(callbackId) { + return await this.waitForApprovalDecision(callbackId); }, requestFileChangeApproval(params) { const requestId = nextServerRequestId;