From b8a3eefd47b29c97c91dbc133c6a6f3966fcc906 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 13 May 2026 13:58:15 +0800 Subject: [PATCH] feat(codex): implement Auto-review permission mode (#963) * feat(codex): plumb auto-review approvals reviewer * feat(codex): add Auto-review mode visuals * test(codex): cover Auto-review mode plumbing * test(codex): prove Auto-review approval routing --- .../app/src/components/agent-status-bar.tsx | 2 + .../src/server/agent/provider-manifest.ts | 10 +- .../providers/codex-app-server-agent.test.ts | 113 +++++- .../agent/providers/codex-app-server-agent.ts | 104 ++++- .../providers/codex/app-server-transport.ts | 31 +- .../codex-auto-review.real.e2e.test.ts | 382 ++++++++++++++++++ 6 files changed, 620 insertions(+), 22 deletions(-) create mode 100644 packages/server/src/server/daemon-e2e/codex-auto-review.real.e2e.test.ts diff --git a/packages/app/src/components/agent-status-bar.tsx b/packages/app/src/components/agent-status-bar.tsx index fe2485d44..5975b697e 100644 --- a/packages/app/src/components/agent-status-bar.tsx +++ b/packages/app/src/components/agent-status-bar.tsx @@ -27,6 +27,7 @@ import { ShieldAlert, ShieldCheck, ShieldOff, + ShieldQuestionMark, Zap, } from "lucide-react-native"; import { getProviderIcon } from "@/components/provider-icons"; @@ -180,6 +181,7 @@ const MODE_ICONS = { ShieldCheck, ShieldAlert, ShieldOff, + ShieldQuestionMark, } as const; function alwaysTrue() { diff --git a/packages/server/src/server/agent/provider-manifest.ts b/packages/server/src/server/agent/provider-manifest.ts index 66b218415..c89af500a 100644 --- a/packages/server/src/server/agent/provider-manifest.ts +++ b/packages/server/src/server/agent/provider-manifest.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import type { AgentMode } from "./agent-sdk-types.js"; export type AgentModeColorTier = "safe" | "moderate" | "dangerous" | "planning"; -export type AgentModeIcon = "ShieldCheck" | "ShieldAlert" | "ShieldOff"; +export type AgentModeIcon = "ShieldCheck" | "ShieldAlert" | "ShieldOff" | "ShieldQuestionMark"; export interface AgentModeVisuals { icon: AgentModeIcon; @@ -71,6 +71,14 @@ const CODEX_MODES: AgentProviderModeDefinition[] = [ icon: "ShieldAlert", colorTier: "moderate", }, + { + id: "auto-review", + label: "Auto-review", + description: + "Same workspace-write permissions as Default, but eligible `on-request` approvals are routed through the auto-reviewer subagent.", + icon: "ShieldQuestionMark", + colorTier: "moderate", + }, { id: "full-access", label: "Full Access", 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 8981cba94..1ea8a4fe5 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 @@ -73,7 +73,7 @@ function createConfig(overrides: Partial = {}): AgentSession function createSession( configOverrides: Partial = {}, - options: { goalsEnabled?: boolean } = {}, + options: { goalsEnabled?: boolean; autoReviewEnabled?: boolean } = {}, ): CodexTestSession { const session = new __codexAppServerInternals.CodexAppServerAgentSession( createConfig(configOverrides), @@ -85,6 +85,7 @@ function createSession( {}, false, options.goalsEnabled === true, + options.autoReviewEnabled === true, ) as CodexTestSession; session.connected = true; session.currentThreadId = "test-thread"; @@ -199,6 +200,116 @@ function capturedThreadStartConfig(records: CapturedFakeCodexRecord[]): unknown } describe("Codex app-server provider", () => { + test("getAvailableModes includes auto-review when the Codex version supports it", async () => { + const session = createSession({}, { autoReviewEnabled: true }); + + await expect(session.getAvailableModes()).resolves.toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "auto-review", + label: "Auto-review", + }), + ]), + ); + }); + + test("getAvailableModes excludes auto-review when the Codex version is too old", async () => { + const session = createSession({}, { autoReviewEnabled: false }); + + await expect(session.getAvailableModes()).resolves.not.toEqual( + expect.arrayContaining([expect.objectContaining({ id: "auto-review" })]), + ); + }); + + test("setMode auto-review sends approvalsReviewer to thread/start", async () => { + const requests: Array<{ method: string; params: unknown }> = []; + const session = createSession( + { modeId: "auto", thinkingOptionId: "medium" }, + { autoReviewEnabled: true }, + ); + session.currentThreadId = null; + session.activeForegroundTurnId = null; + session.client = { + request: vi.fn(async (method: string, params: unknown) => { + requests.push({ method, params }); + if (method === "thread/start") { + return { thread: { id: "auto-review-thread" } }; + } + if (method === "turn/start") { + return {}; + } + throw new Error(`Unexpected request: ${method}`); + }), + }; + + await session.setMode("auto-review"); + await session.startTurn("trigger thread creation"); + + const startCall = requests.find((req) => req.method === "thread/start"); + expect(startCall?.params).toMatchObject({ + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }); + }); + + test.each(["auto_review", "guardian_subagent"])( + "parses %s thread/start response as auto-review mode", + async (approvalsReviewer) => { + const session = createSession( + { modeId: "auto", thinkingOptionId: "medium" }, + { autoReviewEnabled: true }, + ); + session.currentThreadId = null; + session.activeForegroundTurnId = null; + session.client = { + request: vi.fn(async (method: string) => { + if (method === "thread/start") { + return { + thread: { id: "auto-review-thread" }, + approvalPolicy: "on-request", + sandbox: { type: "workspaceWrite", networkAccess: false }, + approvalsReviewer, + }; + } + if (method === "turn/start") { + return {}; + } + throw new Error(`Unexpected request: ${method}`); + }), + }; + + await session.startTurn("trigger thread creation"); + + await expect(session.getCurrentMode()).resolves.toBe("auto-review"); + }, + ); + + test("turn/start forwards approvalsReviewer while in auto-review mode", async () => { + const session = createSession({ modeId: "auto-review" }, { autoReviewEnabled: true }); + const request = vi.fn(async (method: string) => { + if (method === "thread/loaded/list") { + return { data: ["test-thread"] }; + } + if (method === "turn/start") { + return {}; + } + throw new Error(`Unexpected request: ${method}`); + }); + session.activeForegroundTurnId = null; + session.client = createStub({ request }); + + await session.startTurn("needs approval"); + + const turnStartCall = request.mock.calls.find(([method]) => method === "turn/start"); + expect(turnStartCall?.[1]).toEqual( + expect.objectContaining({ + approvalPolicy: "on-request", + approvalsReviewer: "auto_review", + }), + ); + }); + test("passes ephemeral: true to thread/start when constructed as ephemeral", async () => { const requests: Array<{ method: string; params: unknown }> = []; const fakeClient: CodexClientLike = { 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 3c30f581c..cf0d5c6a1 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 @@ -105,6 +105,7 @@ const CODEX_PLAN_IMPLEMENTATION_PROMPT_PREFIX = // `--enable goals` at launch, so we gate by version and silently skip the flag // (and the /goal slash command) when the binary is too old. const CODEX_GOALS_MIN_VERSION: readonly [number, number, number] = [0, 128, 0]; +const CODEX_AUTO_REVIEW_MIN_VERSION: readonly [number, number, number] = [0, 115, 0]; function parseCodexVersion(versionOutput: string): [number, number, number] | null { const match = versionOutput.match(/(\d+)\.(\d+)\.(\d+)/); @@ -161,6 +162,12 @@ const CODEX_MODES: AgentMode[] = [ label: "Default Permissions", description: "Edit files and run commands with Codex's default approval flow.", }, + { + id: "auto-review", + label: "Auto-review", + description: + "Same workspace-write permissions as Default, but eligible `on-request` approvals are routed through the auto-reviewer subagent.", + }, { id: "full-access", label: "Full Access", @@ -191,10 +198,14 @@ interface CodexAppServerAgentDeps { ) => CodexAppServerClientLike; } -const MODE_PRESETS: Record< - string, - { approvalPolicy: string; sandbox: string; networkAccess?: boolean } -> = { +interface CodexModePreset { + approvalPolicy: string; + sandbox: string; + networkAccess?: boolean; + approvalsReviewer?: "auto_review"; +} + +const MODE_PRESETS: Record = { "read-only": { approvalPolicy: "on-request", sandbox: "read-only", @@ -203,6 +214,11 @@ const MODE_PRESETS: Record< approvalPolicy: "on-request", sandbox: "workspace-write", }, + "auto-review": { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }, "full-access": { approvalPolicy: "never", sandbox: "danger-full-access", @@ -210,6 +226,31 @@ const MODE_PRESETS: Record< }, }; +function isAutoReviewReviewer(value: string | undefined): boolean { + return value === "auto_review" || value === "guardian_subagent"; +} + +function applyApprovalsReviewerParam( + params: Record, + preset: CodexModePreset, +): void { + if (preset.approvalsReviewer) { + params.approvalsReviewer = preset.approvalsReviewer; + } +} + +function shouldPromoteThreadResponseToAutoReview(params: { + approvalsReviewer: string | undefined; + approvalPolicy: string; + sandbox: string; +}): boolean { + return ( + isAutoReviewReviewer(params.approvalsReviewer) && + params.approvalPolicy === "on-request" && + params.sandbox === "workspace-write" + ); +} + function validateCodexMode(modeId: string): void { if (!(modeId in MODE_PRESETS)) { const validModes = Object.keys(MODE_PRESETS).join(", "); @@ -2658,6 +2699,7 @@ class CodexAppServerAgentSession implements AgentSession { private readonly deps: CodexAppServerAgentDeps = {}, private readonly ephemeral: boolean = false, private readonly goalsEnabled: boolean = false, + private readonly autoReviewEnabled: boolean = false, private readonly agentId?: string, ) { this.logger = logger.child({ @@ -3126,6 +3168,7 @@ class CodexAppServerAgentSession implements AgentSession { : preset.networkAccess, ), }; + applyApprovalsReviewerParam(params, preset); if (this.config.model) { params.model = this.config.model; @@ -3212,7 +3255,10 @@ class CodexAppServerAgentSession implements AgentSession { } async getAvailableModes(): Promise { - return CODEX_MODES; + if (this.autoReviewEnabled) { + return CODEX_MODES; + } + return CODEX_MODES.filter((mode) => mode.id !== "auto-review"); } async getCurrentMode(): Promise { @@ -3642,7 +3688,7 @@ class CodexAppServerAgentSession implements AgentSession { const approvalPolicy = this.config.approvalPolicy ?? preset.approvalPolicy; const sandbox = this.config.sandboxMode ?? preset.sandbox; const innerConfig = this.buildCodexInnerConfig(); - const rawResponse = await this.client.request("thread/start", { + const params: Record = { model, cwd: this.config.cwd ?? null, approvalPolicy, @@ -3652,13 +3698,27 @@ class CodexAppServerAgentSession implements AgentSession { : {}), ...(innerConfig ? { config: innerConfig } : {}), ...(this.ephemeral ? { ephemeral: true } : {}), - }); + }; + applyApprovalsReviewerParam(params, preset); + const rawResponse = await this.client.request("thread/start", params); const response = toObjectRecord(rawResponse); const threadRecord = toObjectRecord(response?.thread); const threadId = typeof threadRecord?.id === "string" ? threadRecord.id : undefined; if (!threadId) { throw new Error("Codex app-server did not return thread id"); } + const responseApprovalsReviewer = + typeof response?.approvalsReviewer === "string" ? response.approvalsReviewer : undefined; + if ( + shouldPromoteThreadResponseToAutoReview({ + approvalsReviewer: responseApprovalsReviewer, + approvalPolicy, + sandbox, + }) + ) { + this.currentMode = "auto-review"; + this.cachedRuntimeInfo = null; + } this.currentThreadId = threadId; } @@ -4651,6 +4711,7 @@ export class CodexAppServerAgentClient implements AgentClient { readonly provider = CODEX_PROVIDER; readonly capabilities = CODEX_APP_SERVER_CAPABILITIES; private goalsEnabledPromise: Promise | null = null; + private autoReviewEnabledPromise: Promise | null = null; constructor( private readonly logger: Logger, @@ -4693,6 +4754,31 @@ export class CodexAppServerAgentClient implements AgentClient { return this.goalsEnabledPromise; } + private resolveAutoReviewEnabled(): Promise { + if (!this.autoReviewEnabledPromise) { + this.autoReviewEnabledPromise = (async () => { + try { + const launchPrefix = await resolveCodexLaunchPrefix(this.runtimeSettings); + const versionOutput = await resolveBinaryVersion(launchPrefix.command); + const enabled = codexVersionAtLeast(versionOutput, CODEX_AUTO_REVIEW_MIN_VERSION); + this.logger.trace( + { + provider: CODEX_PROVIDER, + versionOutput, + enabled, + }, + "provider.codex.config.auto_review_resolved", + ); + return enabled; + } catch (error) { + this.logger.warn({ err: error }, "Failed to probe codex version for auto-review gate"); + return false; + } + })(); + } + return this.autoReviewEnabledPromise; + } + private async spawnAppServer( launchEnv?: Record, options?: { goalsEnabled?: boolean; agentId?: string }, @@ -4737,6 +4823,7 @@ export class CodexAppServerAgentClient implements AgentClient { } const sessionConfig: AgentSessionConfig = { ...config, provider: CODEX_PROVIDER }; const goalsEnabled = await this.resolveGoalsEnabled(); + const autoReviewEnabled = await this.resolveAutoReviewEnabled(); const session = new CodexAppServerAgentSession( sessionConfig, null, @@ -4746,6 +4833,7 @@ export class CodexAppServerAgentClient implements AgentClient { this.sessionDeps(), options?.persistSession === false, goalsEnabled, + autoReviewEnabled, launchContext?.agentId, ); await session.connect(); @@ -4765,6 +4853,7 @@ export class CodexAppServerAgentClient implements AgentClient { cwd: overrides?.cwd ?? storedConfig.cwd ?? process.cwd(), }; const goalsEnabled = await this.resolveGoalsEnabled(); + const autoReviewEnabled = await this.resolveAutoReviewEnabled(); const session = new CodexAppServerAgentSession( merged, handle, @@ -4774,6 +4863,7 @@ export class CodexAppServerAgentClient implements AgentClient { this.sessionDeps(), false, goalsEnabled, + autoReviewEnabled, launchContext?.agentId, ); await session.connect(); diff --git a/packages/server/src/server/agent/providers/codex/app-server-transport.ts b/packages/server/src/server/agent/providers/codex/app-server-transport.ts index d6d89fd28..410fc9c2d 100644 --- a/packages/server/src/server/agent/providers/codex/app-server-transport.ts +++ b/packages/server/src/server/agent/providers/codex/app-server-transport.ts @@ -234,6 +234,7 @@ export class CodexAppServerClient { if (isJsonRpcRequest(raw)) { const request = raw; + this.traceRawEvent(request); const handler = this.requestHandlers.get(request.method); try { const result = handler ? await handler(request.params) : {}; @@ -249,20 +250,24 @@ export class CodexAppServerClient { } if (isJsonRpcNotification(raw)) { - const traceContext = this.getTraceContext(); - this.logger.trace( - { - provider: "codex", - agentId: traceContext.agentId, - sessionId: traceContext.sessionId ?? readProviderSessionId(raw.params), - turnId: traceContext.turnId ?? readProviderTurnId(raw.params), - method: raw.method, - params: raw.params, - rawEvent: raw, - }, - "provider.codex.raw_event", - ); + this.traceRawEvent(raw); this.notificationHandler?.(raw.method, raw.params); } } + + private traceRawEvent(raw: JsonRpcRequest | JsonRpcNotification): void { + const traceContext = this.getTraceContext(); + this.logger.trace( + { + provider: "codex", + agentId: traceContext.agentId, + sessionId: traceContext.sessionId ?? readProviderSessionId(raw.params), + turnId: traceContext.turnId ?? readProviderTurnId(raw.params), + method: raw.method, + params: raw.params, + rawEvent: raw, + }, + "provider.codex.raw_event", + ); + } } diff --git a/packages/server/src/server/daemon-e2e/codex-auto-review.real.e2e.test.ts b/packages/server/src/server/daemon-e2e/codex-auto-review.real.e2e.test.ts new file mode 100644 index 000000000..c0e9bc615 --- /dev/null +++ b/packages/server/src/server/daemon-e2e/codex-auto-review.real.e2e.test.ts @@ -0,0 +1,382 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { appendFile, writeFile } from "node:fs/promises"; +import { createServer } from "node:http"; +import os from "node:os"; +import path from "node:path"; +import { Writable } from "node:stream"; +import { once } from "node:events"; + +import pino from "pino"; +import { describe, expect, test } from "vitest"; + +import { CodexAppServerAgentClient } from "../agent/providers/codex-app-server-agent.js"; +import { createMessageCollector } from "../test-utils/message-collector.js"; +import { createTestPaseoDaemon } from "../test-utils/paseo-daemon.js"; +import { DaemonClient } from "../test-utils/daemon-client.js"; + +const QA_REPORT_PATH = "/tmp/codex-auto-review-qa.md"; + +function parseCodexVersion(): [number, number, number] | null { + try { + const output = execFileSync("codex", ["--version"], { encoding: "utf8" }); + const match = output.match(/(\d+)\.(\d+)\.(\d+)/); + if (!match) { + return null; + } + return [Number(match[1]), Number(match[2]), Number(match[3])]; + } catch { + return null; + } +} + +function codexVersionAtLeast(version: [number, number, number] | null): boolean { + if (!version) { + return false; + } + const min = [0, 115, 0] as const; + for (let index = 0; index < min.length; index += 1) { + if (version[index] > min[index]) return true; + if (version[index] < min[index]) return false; + } + return true; +} + +function sse(events: unknown[]): string { + return events + .map((event) => { + const type = + typeof event === "object" && + event !== null && + "type" in event && + typeof (event as { type?: unknown }).type === "string" + ? (event as { type: string }).type + : "message"; + return `event: ${type}\ndata: ${JSON.stringify(event)}\n\n`; + }) + .join(""); +} + +function responseCreated(id: string): Record { + return { type: "response.created", response: { id } }; +} + +function responseCompleted(id: string): Record { + return { + type: "response.completed", + response: { + id, + usage: { + input_tokens: 0, + input_tokens_details: null, + output_tokens: 0, + output_tokens_details: null, + total_tokens: 0, + }, + }, + }; +} + +function functionCallEvent( + callId: string, + name: string, + argumentsJson: string, +): Record { + return { + type: "response.output_item.done", + item: { + type: "function_call", + call_id: callId, + name, + arguments: argumentsJson, + }, + }; +} + +function assistantMessageEvent(id: string, text: string): Record { + return { + type: "response.output_item.done", + item: { + type: "message", + role: "assistant", + id, + content: [{ type: "output_text", text }], + }, + }; +} + +function approvalToolCallSse(): string { + return sse([ + responseCreated("resp-main-1"), + functionCallEvent( + "call-write-outside", + "exec_command", + JSON.stringify({ + cmd: "python3 -c \"import urllib.request; urllib.request.urlopen('https://example.com', timeout=1)\"", + yield_time_ms: 1000, + sandbox_permissions: "require_escalated", + justification: "Probe auto-review approval routing.", + }), + ), + responseCompleted("resp-main-1"), + ]); +} + +function guardianApprovalSse(): string { + return sse([ + responseCreated("resp-guardian-1"), + assistantMessageEvent( + "msg-guardian-1", + JSON.stringify({ + outcome: "allow", + risk_level: "low", + user_authorization: "high", + rationale: "Test harness approves the isolated write.", + }), + ), + responseCompleted("resp-guardian-1"), + ]); +} + +function finalAnswerSse(): string { + return sse([ + responseCreated("resp-main-2"), + assistantMessageEvent("msg-main-2", "done"), + responseCompleted("resp-main-2"), + ]); +} + +async function startMockResponsesServer(sequence: string[]): Promise<{ + url: string; + close: () => Promise; + requestBodies: unknown[]; +}> { + const requestBodies: unknown[] = []; + let index = 0; + const server = createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (chunk) => chunks.push(Buffer.from(chunk))); + req.on("end", () => { + const rawBody = Buffer.concat(chunks).toString("utf8"); + requestBodies.push(JSON.parse(rawBody)); + if (req.method !== "POST" || req.url !== "/v1/responses") { + res.statusCode = 404; + res.end("not found"); + return; + } + const body = sequence[index] ?? finalAnswerSse(); + index += 1; + res.statusCode = 200; + res.setHeader("content-type", "text/event-stream"); + res.setHeader("cache-control", "no-cache"); + res.end(body); + }); + }); + + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected TCP address for mock responses server"); + } + return { + url: `http://127.0.0.1:${address.port}`, + requestBodies, + close: () => + new Promise((resolve, reject) => { + server.close((error) => { + if (error) reject(error); + else resolve(); + }); + }), + }; +} + +function writeMockCodexConfig(codexHome: string, serverUrl: string): void { + writeFileSync( + path.join(codexHome, "config.toml"), + ` +model = "mock-model" +model_provider = "mock_provider" +approval_policy = "never" +sandbox_mode = "read-only" + +[features] +guardian_approval = true + +[model_providers.mock_provider] +name = "Mock provider" +base_url = "${serverUrl}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +supports_websockets = false +`, + ); +} + +function createTraceLogger() { + const records: Array> = []; + const stream = new Writable({ + write(chunk, _encoding, callback) { + try { + records.push(JSON.parse(chunk.toString()) as Record); + } catch { + // Ignore non-JSON logger output. + } + callback(); + }, + }); + return { + logger: pino({ level: "trace" }, stream), + records, + }; +} + +function rawCodexEvents(records: Array>): Array> { + return records + .filter((record) => record.msg === "provider.codex.raw_event") + .map((record) => ({ + method: record.method, + params: record.params, + rawEvent: record.rawEvent, + })); +} + +function hasClientPermissionRequest(messages: unknown[]): boolean { + return messages.some( + (message) => + typeof message === "object" && + message !== null && + "type" in message && + (message as { type?: unknown }).type === "agent_permission_request", + ); +} + +async function appendQaSection(title: string, payload: unknown): Promise { + await appendFile( + QA_REPORT_PATH, + `\n## ${title}\n\n\`\`\`json\n${JSON.stringify(payload, null, 2)}\n\`\`\`\n`, + ); +} + +async function runScenario(modeId: "auto" | "auto-review") { + const cwd = mkdtempSync(path.join(os.tmpdir(), `codex-auto-review-${modeId}-cwd-`)); + const codexHome = mkdtempSync(path.join(os.tmpdir(), `codex-auto-review-${modeId}-home-`)); + const mockServer = await startMockResponsesServer( + modeId === "auto-review" + ? [approvalToolCallSse(), guardianApprovalSse(), finalAnswerSse()] + : [approvalToolCallSse(), finalAnswerSse()], + ); + const { logger, records } = createTraceLogger(); + const daemon = await createTestPaseoDaemon({ + agentClients: { + codex: new CodexAppServerAgentClient(logger, { + env: { + CODEX_HOME: codexHome, + }, + }), + }, + logger, + }); + const client = new DaemonClient({ + url: `ws://127.0.0.1:${daemon.port}/ws`, + appVersion: "0.1.75", + }); + const collector = createMessageCollector(client); + + try { + writeMockCodexConfig(codexHome, mockServer.url); + await client.connect(); + await client.fetchAgents({ subscribe: { subscriptionId: `codex-auto-review-${modeId}` } }); + + const agent = await client.createAgent({ + provider: "codex", + cwd, + title: `codex-auto-review-${modeId}`, + modeId, + model: "mock-model", + thinkingOptionId: "medium", + }); + + collector.clear(); + await client.sendAgentMessage(agent.id, "Run the scripted command."); + + if (modeId === "auto") { + const permissionState = await client.waitForFinish(agent.id, 30_000); + expect(permissionState.status).toBe("permission"); + const permission = permissionState.final?.pendingPermissions?.[0]; + expect(permission?.kind).toBe("tool"); + await client.respondToPermission(agent.id, permission!.id, { behavior: "deny" }); + } + + const finish = await client.waitForFinish(agent.id, 60_000); + expect(finish.status).toBe("idle"); + + return { + modeId, + websocketMessages: [...collector.messages], + rawCodexEvents: rawCodexEvents(records), + mockResponsesRequests: mockServer.requestBodies, + }; + } finally { + collector.unsubscribe(); + await client.close(); + await daemon.close(); + await mockServer.close(); + rmSync(cwd, { recursive: true, force: true }); + rmSync(codexHome, { recursive: true, force: true }); + } +} + +const codexVersion = parseCodexVersion(); +const canRun = codexVersionAtLeast(codexVersion); + +describe.skipIf(!canRun)("daemon E2E (real codex) - Auto-review mode", () => { + test("Auto-review routes on-request approvals through the Codex auto-reviewer subagent", async () => { + await writeFile( + QA_REPORT_PATH, + `# Codex Auto-review QA\n\nE2E started with codex version ${JSON.stringify(codexVersion)}.\n`, + ); + + const result = await runScenario("auto-review"); + const methods = result.rawCodexEvents.map((event) => event.method); + + expect(methods).toContain("item/autoApprovalReview/started"); + expect(methods).toContain("item/autoApprovalReview/completed"); + expect(methods).not.toContain("item/commandExecution/requestApproval"); + expect(hasClientPermissionRequest(result.websocketMessages)).toBe(false); + + await appendQaSection("E2E Auto-review Wire Payloads", { + rawCodexEvents: result.rawCodexEvents.filter( + (event) => + event.method === "item/autoApprovalReview/started" || + event.method === "item/autoApprovalReview/completed", + ), + requestApprovalEvents: result.rawCodexEvents.filter( + (event) => event.method === "item/commandExecution/requestApproval", + ), + }); + }, 90_000); + + test("Default Permissions surfaces the same approval as a client permission request", async () => { + const result = await runScenario("auto"); + const methods = result.rawCodexEvents.map((event) => event.method); + + expect(methods).toContain("item/commandExecution/requestApproval"); + expect(methods).not.toContain("item/autoApprovalReview/started"); + expect(methods).not.toContain("item/autoApprovalReview/completed"); + expect(hasClientPermissionRequest(result.websocketMessages)).toBe(true); + + await appendQaSection("E2E Default Permissions Wire Payloads", { + rawCodexEvents: result.rawCodexEvents.filter( + (event) => event.method === "item/commandExecution/requestApproval", + ), + autoApprovalReviewEvents: result.rawCodexEvents.filter( + (event) => + event.method === "item/autoApprovalReview/started" || + event.method === "item/autoApprovalReview/completed", + ), + }); + }, 90_000); +});