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 9986357cf4..9b5263e474 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 @@ -227,6 +227,57 @@ function deferred() { return { promise, resolve }; } +describe("Codex file-change approvals", () => { + test("file-change approval includes the matching patch when notifications are still queued", async () => { + const appServer = createFakeCodexAppServer(); + const { session } = await startPublicSteeringSession(appServer); + try { + for (const [id, filePath, diff] of [ + ["file-alpha", "/workspace/project/alpha.txt", "@@ -1 +1 @@\n-blue\n+green\n"], + ["file-beta", "/workspace/project/beta.txt", "@@ -1 +1 @@\n-small\n+large\n"], + ]) { + appServer.startsFileChange({ + threadId: "thread-1", + turnId: "native-A", + callId: id, + path: filePath, + diff, + }); + } + const permission = waitForNextPermission(session); + appServer.requestFileChangeApproval({ + itemId: "file-alpha", + threadId: "thread-1", + turnId: "native-A", + reason: "Apply the patch", + }); + + expect((await permission).request).toEqual({ + id: "permission-file-alpha", + provider: "codex", + name: "CodexFileChange", + kind: "tool", + title: "Apply file changes: alpha.txt", + description: "Apply the patch", + detail: { + type: "edit", + filePath: "alpha.txt", + unifiedDiff: "@@ -1 +1 @@\n-blue\n+green\n", + }, + metadata: { itemId: "file-alpha", threadId: "thread-1", turnId: "native-A" }, + }); + + await session.respondToPermission("permission-file-alpha", { behavior: "allow" }); + await expect(appServer.waitForApprovalDecision("file-alpha")).resolves.toEqual({ + decision: "accept", + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }); +}); + describe("Codex active-turn steering admission", () => { test("steers an accepted turn whose native id arrives after startTurn returns", async () => { const appServer = createFakeCodexAppServer({ 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 cc667b2494..f349978996 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 @@ -3483,6 +3483,7 @@ export class CodexAppServerAgentSession implements AgentSession { private pendingReasoning = new Map(); private pendingCommandOutputDeltas = new Map(); private pendingFileChangeOutputDeltas = new Map(); + private pendingToolCallDetails = new Map(); private pendingAssistantMessageBoundary = false; private terminalCommandByProcessId = new Map(); // The root thread's still-running command process ids, keyed by the native turn that @@ -5251,6 +5252,7 @@ export class CodexAppServerAgentSession implements AgentSession { async close(): Promise { this.closed = true; this.clearPendingPermissions(); + this.pendingToolCallDetails.clear(); this.pendingSubAgentNotificationsByThreadId.clear(); this.subscribers.clear(); this.startupEvents = null; @@ -6416,6 +6418,7 @@ export class CodexAppServerAgentSession implements AgentSession { this.pendingReasoning.clear(); this.pendingCommandOutputDeltas.clear(); this.pendingFileChangeOutputDeltas.clear(); + this.pendingToolCallDetails.clear(); this.pendingAssistantMessageBoundary = false; this.warnedIncompleteEditToolCallIds.clear(); this.pendingRootCompactionItemIds.clear(); @@ -6789,6 +6792,9 @@ export class CodexAppServerAgentSession implements AgentSession { if (shouldIgnoreMirroredLifecycleItem(parsed.source, parsed.item)) { return; } + if (parsed.item.id) { + this.pendingToolCallDetails.delete(parsed.item.id); + } this.receiveAsyncQuestion(parsed.threadId, parsed.item); if (this.isUserMessageItem(parsed.item)) { await this.handleUserMessageItem(parsed); @@ -6836,10 +6842,8 @@ export class CodexAppServerAgentSession implements AgentSession { if (timelineItem.type === "assistant_message") { this.pendingAssistantMessageBoundary = true; } - if (itemId) { - this.emittedItemCompletedIds.add(itemId); - this.emittedItemStartedIds.delete(itemId); - } + this.emittedItemCompletedIds.add(itemId); + this.emittedItemStartedIds.delete(itemId); await this.replayPendingSubAgentNotifications(registeredChildThreadIds); return; } @@ -6875,7 +6879,7 @@ export class CodexAppServerAgentSession implements AgentSession { private consumeStreamedTextCompletion( timelineItem: AgentTimelineItem, itemId: string | null | undefined, - ): boolean { + ): itemId is string { if (!itemId) { return false; } @@ -6984,6 +6988,7 @@ export class CodexAppServerAgentSession implements AgentSession { if (!timelineItem || timelineItem.type !== "tool_call") { return; } + this.pendingToolCallDetails.set(timelineItem.callId, timelineItem.detail); const registeredChildThreadIds = this.registerSubAgentToolCall({ timelineItem, rawItem: parsed.item, @@ -7253,7 +7258,8 @@ export class CodexAppServerAgentSession implements AgentSession { }); } - private handleFileChangeApprovalRequest(params: unknown): Promise { + private async handleFileChangeApprovalRequest(params: unknown): Promise { + await this.pendingNotifications; const parsed = z .object({ itemId: z.string(), @@ -7263,20 +7269,25 @@ export class CodexAppServerAgentSession implements AgentSession { }) .parse(params); const requestId = `permission-${parsed.itemId}`; + const detail: ToolCallTimelineItem["detail"] = this.pendingToolCallDetails.get( + parsed.itemId, + ) ?? { + type: "unknown", + input: { reason: parsed.reason ?? null }, + output: null, + }; + const title = + detail.type === "edit" && detail.filePath + ? `Apply file changes: ${detail.filePath}` + : "Apply file changes"; const request: AgentPermissionRequest = { id: requestId, provider: CODEX_PROVIDER, name: "CodexFileChange", kind: "tool", - title: "Apply file changes", + title, description: parsed.reason ?? undefined, - detail: { - type: "unknown", - input: { - reason: parsed.reason ?? null, - }, - output: null, - }, + detail, metadata: { itemId: parsed.itemId, threadId: parsed.threadId, 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 99e6a73050..ec14a773e4 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 @@ -43,6 +43,13 @@ interface FakeTerminalInput { processId: string; text: string; } +interface FakeFileChange { + threadId: string; + turnId: string; + callId: string; + path: string; + diff: string; +} interface FakeLegacyPatch { threadId: string; callId: string; @@ -88,6 +95,7 @@ export interface FakeCodexAppServer { completesSubAgentActivity(params: FakeSubAgentActivity): void; completesCompaction(params: { threadId: string; itemId: string }): void; runsLegacyCommand(params: FakeLegacyCommand): void; + startsFileChange(params: FakeFileChange): void; appliesLegacyPatch(params: FakeLegacyPatch): void; completesCommand(params: FakeLegacyCommand): void; completesSilentCommand(params: FakeSilentCommand): void; @@ -467,6 +475,20 @@ export function createFakeCodexAppServer( success: true, }); }, + startsFileChange(params) { + writeNotification("item/started", { + threadId: params.threadId, + turnId: params.turnId, + item: { + type: "fileChange", + id: params.callId, + status: "inProgress", + changes: [ + { path: params.path, kind: { type: "update", move_path: null }, diff: params.diff }, + ], + }, + }); + }, appliesLegacyPatch(params) { const changes = [ {