From 026d6c45a0a6ccb156dad0530cf6bfb437afe15c Mon Sep 17 00:00:00 2001 From: Yogev Date: Tue, 14 Jul 2026 01:59:16 +0300 Subject: [PATCH] fix(codex): auto-review write task escalations Keep read-only delegation fail-closed while allowing write-capable tasks to request Guardian-reviewed escalation on new and resumed threads. Builds on openai/codex-plugin-cc#426. Refs openai/codex-plugin-cc#445. --- plugins/codex/scripts/codex-companion.mjs | 2 + plugins/codex/scripts/lib/codex.mjs | 6 +++ tests/fake-codex-fixture.mjs | 22 ++++++++- tests/runtime.test.mjs | 56 +++++++++++++++++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..09cd1e1bc 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -488,6 +488,8 @@ async function executeTaskRun(request) { defaultPrompt: resumeThreadId ? DEFAULT_CONTINUE_PROMPT : "", model: request.model, effort: request.effort, + approvalPolicy: request.write ? "on-request" : "never", + approvalsReviewer: request.write ? "auto_review" : undefined, sandbox: request.write ? "workspace-write" : "read-only", onProgress: request.onProgress, persistThread: true, diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..32d1450b0 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -65,6 +65,7 @@ function buildThreadParams(cwd, options = {}) { cwd, model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", + approvalsReviewer: options.approvalsReviewer ?? null, sandbox: options.sandbox ?? "read-only", serviceName: SERVICE_NAME, ephemeral: options.ephemeral ?? true @@ -78,6 +79,7 @@ function buildResumeParams(threadId, cwd, options = {}) { cwd, model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", + approvalsReviewer: options.approvalsReviewer ?? null, sandbox: options.sandbox ?? "read-only" }; } @@ -1105,6 +1107,8 @@ export async function runAppServerTurn(cwd, options = {}) { emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); const response = await resumeThread(client, options.resumeThreadId, cwd, { model: options.model, + approvalPolicy: options.approvalPolicy, + approvalsReviewer: options.approvalsReviewer, sandbox: options.sandbox, ephemeral: false }); @@ -1113,6 +1117,8 @@ export async function runAppServerTurn(cwd, options = {}) { emitProgress(options.onProgress, "Starting Codex task thread.", "starting"); const response = await startThread(client, cwd, { model: options.model, + approvalPolicy: options.approvalPolicy, + approvalsReviewer: options.approvalsReviewer, sandbox: options.sandbox, ephemeral: options.persistThread ? false : true, threadName: options.persistThread ? options.threadName : options.threadName ?? null diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..bf840ccc5 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -313,7 +313,17 @@ rl.on("line", (line) => { throw new Error("thread/start.persistFullHistory requires experimentalApi capability"); } const thread = nextThread(state, message.params.cwd, message.params.ephemeral); - send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); + state.lastThreadStart = { + threadId: thread.id, + cwd: message.params.cwd ?? null, + model: message.params.model ?? null, + approvalPolicy: message.params.approvalPolicy ?? null, + approvalsReviewer: message.params.approvalsReviewer ?? null, + sandbox: message.params.sandbox ?? null, + ephemeral: message.params.ephemeral ?? null + }; + saveState(state); + send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: message.params.approvalPolicy || "never", approvalsReviewer: message.params.approvalsReviewer || "user", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); send({ method: "thread/started", params: { thread: { id: thread.id } } }); break; } @@ -346,8 +356,16 @@ rl.on("line", (line) => { } const thread = ensureThread(state, message.params.threadId); thread.updatedAt = now(); + state.lastThreadResume = { + threadId: message.params.threadId, + cwd: message.params.cwd ?? null, + model: message.params.model ?? null, + approvalPolicy: message.params.approvalPolicy ?? null, + approvalsReviewer: message.params.approvalsReviewer ?? null, + sandbox: message.params.sandbox ?? null + }; saveState(state); - send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); + send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: message.params.approvalPolicy || "never", approvalsReviewer: message.params.approvalsReviewer || "user", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); break; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..9887ace6f 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -701,6 +701,7 @@ test("session start hook exports the Claude session id, transcript path, and plu test("write task output focuses on the Codex result without generic follow-up hints", () => { const repo = makeTempDir(); const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); installFakeCodex(binDir); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); @@ -714,6 +715,61 @@ test("write task output focuses on the Codex result without generic follow-up hi assert.equal(result.status, 0, result.stderr); assert.equal(result.stdout, "Handled the requested task.\nTask prompt accepted.\n"); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.approvalPolicy, "on-request"); + assert.equal(fakeState.lastThreadStart.approvalsReviewer, "auto_review"); + assert.equal(fakeState.lastThreadStart.sandbox, "workspace-write"); +}); + +test("read-only task keeps never approval policy without auto-review", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "inspect the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.approvalPolicy, "never"); + assert.equal(fakeState.lastThreadStart.approvalsReviewer, null); + assert.equal(fakeState.lastThreadStart.sandbox, "read-only"); +}); + +test("task --resume-last --write forwards auto-review policy to app-server thread/resume", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const firstRun = run("node", [SCRIPT, "task", "initial task"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(firstRun.status, 0, firstRun.stderr); + + const result = run("node", [SCRIPT, "task", "--resume-last", "--write", "follow up"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadResume.threadId, "thr_1"); + assert.equal(fakeState.lastThreadResume.approvalPolicy, "on-request"); + assert.equal(fakeState.lastThreadResume.approvalsReviewer, "auto_review"); + assert.equal(fakeState.lastThreadResume.sandbox, "workspace-write"); }); test("task --resume acts like --resume-last without leaking the flag into the prompt", () => {