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>
This commit is contained in:
paseo-bot[bot]
2026-09-30 14:09:56 +02:00
committed by GitHub
parent 3a9304607c
commit 8cccc1f6fa
3 changed files with 79 additions and 5 deletions
@@ -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(
@@ -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,
@@ -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<unknown>;
/** Keyed by `approvalId` when the request carried one, otherwise by `itemId`. */
waitForCommandApprovalDecision(callbackId: string): Promise<unknown>;
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;