diff --git a/plugins/codex/agents/codex-rescue.md b/plugins/codex/agents/codex-rescue.md index 7009ec86..d8b06479 100644 --- a/plugins/codex/agents/codex-rescue.md +++ b/plugins/codex/agents/codex-rescue.md @@ -31,7 +31,7 @@ Forwarding rules: - If the user asks for `spark`, map that to `--model gpt-5.3-codex-spark`. - If the user asks for a concrete model name such as `gpt-5.4-mini`, pass it through with `--model`. - Treat `--effort ` and `--model ` as runtime controls and do not include them in the task text you pass through. -- Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. +- Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits; in those cases, add `--read-only` instead. - Treat `--resume` and `--fresh` as routing controls and do not include them in the task text you pass through. - `--resume` means add `--resume-last`. - `--fresh` means do not add `--resume-last`. diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555..920a586f 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,6 +1,6 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the Codex rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--read-only] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" allowed-tools: Bash(node:*), AskUserQuestion, Agent --- @@ -17,7 +17,7 @@ Execution mode: - If the request includes `--wait`, run the `codex:codex-rescue` subagent in the foreground. - If neither flag is present, default to foreground. - `--background` and `--wait` are execution flags for Claude Code. Do not forward them to `task`, and do not treat them as part of the natural-language task text. -- `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `task` call, but do not treat them as part of the natural-language task text. +- `--model` and `--effort` are runtime-selection flags. Preserve them and `--read-only` for the forwarded `task` call, but do not treat them as part of the natural-language task text. - If the request includes `--resume`, do not ask whether to continue. The user already chose. - If the request includes `--fresh`, do not ask whether to continue. The user already chose. - Otherwise, before starting Codex, check for a resumable rescue thread from this Claude session by running: diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468a..2ebfceea 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -79,7 +79,7 @@ function printUsage() { " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", - " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", + " node scripts/codex-companion.mjs task [--background] [--write] [--read-only] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", " node scripts/codex-companion.mjs transfer [--source ] [--json]", " node scripts/codex-companion.mjs status [job-id] [--all] [--json]", " node scripts/codex-companion.mjs result [job-id] [--json]", @@ -488,7 +488,7 @@ async function executeTaskRun(request) { defaultPrompt: resumeThreadId ? DEFAULT_CONTINUE_PROMPT : "", model: request.model, effort: request.effort, - sandbox: request.write ? "workspace-write" : "read-only", + sandbox: request.write ? "workspace-write" : request.readOnly ? "read-only" : null, onProgress: request.onProgress, persistThread: true, threadName: resumeThreadId ? null : buildPersistentTaskThreadName(request.prompt || DEFAULT_CONTINUE_PROMPT) @@ -601,13 +601,14 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { }); } -function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId }) { +function buildTaskRequest({ cwd, model, effort, prompt, write, readOnly, resumeLast, jobId }) { return { cwd, model, effort, prompt, write, + readOnly, resumeLast, jobId }; @@ -762,7 +763,7 @@ async function handleReview(argv) { async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["model", "effort", "cwd", "prompt-file"], - booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], + booleanOptions: ["json", "write", "read-only", "resume-last", "resume", "fresh", "background"], aliasMap: { m: "model" } @@ -780,6 +781,10 @@ async function handleTask(argv) { throw new Error("Choose either --resume/--resume-last or --fresh."); } const write = Boolean(options.write); + const readOnly = Boolean(options["read-only"]); + if (write && readOnly) { + throw new Error("Choose either --write or --read-only."); + } const taskMetadata = buildTaskRunMetadata({ prompt, resumeLast @@ -796,6 +801,7 @@ async function handleTask(argv) { effort, prompt, write, + readOnly, resumeLast, jobId: job.id }); @@ -814,6 +820,7 @@ async function handleTask(argv) { effort, prompt, write, + readOnly, resumeLast, jobId: job.id, onProgress: progress diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc..31e21449 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -65,7 +65,7 @@ function buildThreadParams(cwd, options = {}) { cwd, model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", - sandbox: options.sandbox ?? "read-only", + sandbox: options.sandbox ?? null, serviceName: SERVICE_NAME, ephemeral: options.ephemeral ?? true }; @@ -78,7 +78,7 @@ function buildResumeParams(threadId, cwd, options = {}) { cwd, model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", - sandbox: options.sandbox ?? "read-only" + sandbox: options.sandbox ?? null }; } diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf..a5737b58 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -102,7 +102,7 @@ function runStopReview(cwd, input = {}) { ...process.env, ...(input.session_id ? { [SESSION_ID_ENV]: input.session_id } : {}) }; - const result = spawnSync(process.execPath, [scriptPath, "task", "--json", prompt], { + const result = spawnSync(process.execPath, [scriptPath, "task", "--json", "--read-only", prompt], { cwd, env: childEnv, encoding: "utf8", diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb5..e1aa593d 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -21,7 +21,7 @@ Execution rules: - Leave `--effort` unset unless the user explicitly requests a specific effort. - Leave model unset by default. Add `--model` only when the user explicitly asks for one. - Map `spark` to `--model gpt-5.3-codex-spark`. -- Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. +- Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits; in those cases, add `--read-only` instead. Command selection: - Use exactly one `task` invocation per rescue handoff. diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0..1411b906 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -312,6 +312,7 @@ rl.on("line", (line) => { if (requiresExperimental("persistExtendedHistory", message, state) || requiresExperimental("persistFullHistory", message, state)) { throw new Error("thread/start.persistFullHistory requires experimentalApi capability"); } + state.lastThreadStart = { ...message.params }; 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 } }); send({ method: "thread/started", params: { thread: { id: thread.id } } }); @@ -344,6 +345,7 @@ rl.on("line", (line) => { if (requiresExperimental("persistExtendedHistory", message, state) || requiresExperimental("persistFullHistory", message, state)) { throw new Error("thread/resume.persistFullHistory requires experimentalApi capability"); } + state.lastThreadResume = { ...message.params }; const thread = ensureThread(state, message.params.threadId); thread.updatedAt = now(); saveState(state); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835..c9617cdf 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -139,6 +139,7 @@ test("setup reports not ready when app-server config read fails", () => { test("review renders a no-findings result from app-server review/start", () => { const repo = makeTempDir(); const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); installFakeCodex(binDir); initGitRepo(repo); fs.mkdirSync(path.join(repo, "src")); @@ -155,6 +156,8 @@ test("review renders a no-findings result from app-server review/start", () => { assert.equal(result.status, 0); assert.match(result.stdout, /Reviewed uncommitted changes/); assert.match(result.stdout, /No material issues found/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.sandbox, "read-only"); }); test("task runs when the active provider does not require OpenAI login", () => { @@ -369,6 +372,7 @@ test("review accepts the quoted raw argument style for built-in base-branch revi test("adversarial review renders structured findings over app-server turn/start", () => { const repo = makeTempDir(); const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); installFakeCodex(binDir); initGitRepo(repo); fs.mkdirSync(path.join(repo, "src")); @@ -384,6 +388,8 @@ test("adversarial review renders structured findings over app-server turn/start" assert.equal(result.status, 0); assert.match(result.stdout, /Missing empty-state guard/); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.sandbox, "read-only"); }); test("adversarial review accepts the same base-branch targeting as review", () => { @@ -482,6 +488,7 @@ test("review logs reasoning summaries and review output to the job log", () => { test("task --resume-last resumes the latest persisted task thread", () => { 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"); @@ -501,6 +508,8 @@ test("task --resume-last resumes the latest persisted task thread", () => { assert.equal(result.status, 0, result.stderr); assert.equal(result.stdout, "Resumed the prior run.\nFollow-up prompt accepted.\n"); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadResume.sandbox, null); }); test("task-resume-candidate returns the latest rescue thread from the current session", () => { @@ -701,6 +710,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 +724,8 @@ 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.sandbox, "workspace-write"); }); test("task --resume acts like --resume-last without leaking the flag into the prompt", () => { @@ -780,6 +792,7 @@ test("task forwards model selection and reasoning effort to app-server turn/star assert.equal(result.status, 0, result.stderr); const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.sandbox, null); assert.equal(fakeState.lastTurnStart.model, "gpt-5.3-codex-spark"); assert.equal(fakeState.lastTurnStart.effort, "low"); }); @@ -920,16 +933,17 @@ test("task using the shared broker still completes when Codex spawns subagents", assert.equal(result.stdout, "Handled the requested task.\nTask prompt accepted.\n"); }); -test("task --background enqueues a detached worker and exposes per-job status", async () => { +test("task --background preserves --read-only through the detached worker", async () => { const repo = makeTempDir(); const binDir = makeTempDir(); + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); installFakeCodex(binDir, "slow-task"); 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 launched = run("node", [SCRIPT, "task", "--background", "--json", "investigate the failing test"], { + const launched = run("node", [SCRIPT, "task", "--background", "--read-only", "--json", "investigate the failing test"], { cwd: repo, env: buildEnv(binDir) }); @@ -967,6 +981,63 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.equal(resultPayload.job.id, launchPayload.jobId); assert.equal(resultPayload.job.status, "completed"); assert.match(resultPayload.storedJob.rendered, /Handled the requested task/); + const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal(fakeState.lastThreadStart.sandbox, "read-only"); +}); + +test("task --read-only pins the app-server thread sandbox", () => { + 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", "--read-only", "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.sandbox, "read-only"); +}); + +test("task --read-only pins resumed app-server threads", () => { + 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", "--read-only", "--resume-last", "read-only 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.sandbox, "read-only"); +}); + +test("task rejects --write with --read-only", () => { + const result = run("node", [SCRIPT, "task", "--write", "--read-only", "inspect the failing test"], { + cwd: ROOT + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Choose either --write or --read-only\./); }); test("review rejects focus text because it is native-review only", () => { @@ -1967,6 +2038,7 @@ test("stop hook runs a stop-time review task and blocks on findings when the rev assert.match(fakeState.lastTurnStart.prompt, //i); assert.match(fakeState.lastTurnStart.prompt, /Only review the work from the previous Claude turn/i); assert.match(fakeState.lastTurnStart.prompt, /I completed the refactor and updated the retry logic\./); + assert.equal(fakeState.lastThreadStart.sandbox, "read-only"); const status = run("node", [SCRIPT, "status"], { cwd: repo,