From 508cb9665b530518293cd6eb1449a38cebb2b434 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 3 Jul 2026 23:06:52 -0700 Subject: [PATCH] fix: thread approvalPolicy through runAppServerTurn so --write is honored --- plugins/codex/scripts/codex-companion.mjs | 1 + plugins/codex/scripts/lib/codex.mjs | 2 + tests/fake-codex-fixture.mjs | 20 ++++++++- tests/runtime.test.mjs | 53 +++++++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468a..21977d47 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -488,6 +488,7 @@ async function executeTaskRun(request) { defaultPrompt: resumeThreadId ? DEFAULT_CONTINUE_PROMPT : "", model: request.model, effort: request.effort, + approvalPolicy: request.write ? "on-request" : "never", 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 fead00cc..bca24593 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1105,6 +1105,7 @@ 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, sandbox: options.sandbox, ephemeral: false }); @@ -1113,6 +1114,7 @@ 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, 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 f83c96a0..c8d285a2 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -313,7 +313,16 @@ 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, + 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", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); send({ method: "thread/started", params: { thread: { id: thread.id } } }); break; } @@ -346,8 +355,15 @@ 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, + 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", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); break; } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835..5d159e08 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,58 @@ 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.sandbox, "workspace-write"); +}); + +test("read-only task keeps never approval policy on app-server thread/start", () => { + 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.sandbox, "read-only"); +}); + +test("task --resume-last --write forwards write approval 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.sandbox, "workspace-write"); }); test("task --resume acts like --resume-last without leaking the flag into the prompt", () => {