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;