Skip to content

fix: auto-review write-capable task escalations #494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Yogitmeister wants to merge 1 commit into
base: main
Choose a base branch
from
+84 −2
Open

fix: auto-review write-capable task escalations #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions plugins/codex/scripts/codex-companion.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment on lines +491 to +492

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify auto-review support before enabling approvals

When --write is used with a Codex app-server version that does not honor approvalsReviewer (or rejects the new field), this still switches the run to approvalPolicy: "on-request". This client has no fallback approval UI—plugins/codex/scripts/lib/app-server.mjs responds to every server-initiated request with -32601—so any escalation such as blocked network or an out-of-workspace write will fail instead of being auto-reviewed. Please gate this on a supported start/resume response or fail early with an upgrade message before selecting on-request.

Useful? React with 👍 / 👎.

sandbox: request.write ? "workspace-write" : "read-only",
onProgress: request.onProgress,
persistThread: true,
Expand Down
6 changes: 6 additions & 0 deletions plugins/codex/scripts/lib/codex.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
};
}
Expand Down Expand Up @@ -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
});
Expand All @@ -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
Expand Down
22 changes: 20 additions & 2 deletions tests/fake-codex-fixture.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down
56 changes: 56 additions & 0 deletions tests/runtime.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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", () => {
Expand Down