From 225f6c3a69a92695edb78b3444cf9df5e47b4480 Mon Sep 17 00:00:00 2001 From: Christopher-Steigerwald_vail Date: Sun, 23 Aug 2026 21:37:01 +0000 Subject: [PATCH 1/3] Support --effort on adversarial-review `/codex:task` accepts `--effort`, but `/codex:adversarial-review` does not: `handleReviewCommand` omits it from `valueOptions`, so `lib/args.mjs` pushes the flag into positionals and `handleReviewCommand` joins positionals into the review's focus text. The flag is therefore not ignored -- it is injected as literal prose into the review prompt -- while `executeReviewRun` calls `runAppServerTurn` without an effort key and `turn/start` receives `effort: null`. The result is that reasoning effort is settable for tasks but not for reviews, where it falls back to `model_reasoning_effort` in config.toml. That file is global, so on a machine running several Claude Code sessions there is no way to raise effort for one review without changing every other session's reviews for the duration. This adds `"effort"` to the review path's `valueOptions`, normalizes it with the same `normalizeReasoningEffort` the task path uses, and threads it to the `turn/start` call that already accepts an `effort` parameter. Three object keys; no new machinery. Also documents the flag in the usage string, the command's argument-hint and the README, and adds a test asserting the parse, the threading, and the documentation stay in sync. Verified against `npm test`: 89 pass, 3 fail, with the same three failures present on an unmodified checkout (they concern `status` and `result` job bookkeeping and are unrelated). Note those runs need CLAUDE_PLUGIN_DATA pointed at a scratch directory -- the suite otherwise writes fixture registries into the developer's live plugin state, which is what a fourth apparent failure turned out to be. --- README.md | 3 +++ plugins/codex/commands/adversarial-review.md | 2 +- plugins/codex/scripts/codex-companion.mjs | 6 ++++-- tests/commands.test.mjs | 22 +++++++++++++++++++- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 937a3037b..442d1b921 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,8 @@ It can be used to pressure-test assumptions, tradeoffs, failure modes, and wheth It uses the same review target selection as `/codex:review`, including `--base ` for branch review. It also supports `--wait` and `--background`. Unlike `/codex:review`, it can take extra focus text after the flags. +It accepts `--model` and `--effort` as runtime-selection flags, the same as `/codex:task`. Leave them unset unless you explicitly want a specific model or reasoning effort. + Use it when you want: - a review before shipping that challenges the direction, not just the code details @@ -119,6 +121,7 @@ Examples: /codex:adversarial-review /codex:adversarial-review --base main challenge whether this was the right caching and retry design /codex:adversarial-review --background look for race conditions and question the chosen approach +/codex:adversarial-review --effort xhigh --base main scrutinise the auth changes ``` This command is read-only. It does not fix code. diff --git a/plugins/codex/commands/adversarial-review.md b/plugins/codex/commands/adversarial-review.md index da440ab4d..461a7f4c5 100644 --- a/plugins/codex/commands/adversarial-review.md +++ b/plugins/codex/commands/adversarial-review.md @@ -1,6 +1,6 @@ --- description: Run a Codex review that challenges the implementation approach and design choices -argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [focus ...]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ] [focus ...]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..67f60c98c 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -78,7 +78,7 @@ function printUsage() { "Usage:", " 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 adversarial-review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [focus text]", " node scripts/codex-companion.mjs task [--background] [--write] [--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]", @@ -411,6 +411,7 @@ async function executeReviewRun(request) { const result = await runAppServerTurn(context.repoRoot, { prompt, model: request.model, + effort: request.effort, sandbox: "read-only", outputSchema: readOutputSchema(REVIEW_SCHEMA), onProgress: request.onProgress @@ -711,7 +712,7 @@ function enqueueBackgroundTask(cwd, job, request) { async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["base", "scope", "model", "cwd"], + valueOptions: ["base", "scope", "model", "cwd", "effort"], booleanOptions: ["json", "background", "wait"], aliasMap: { m: "model" @@ -744,6 +745,7 @@ async function handleReviewCommand(argv, config) { base: options.base, scope: options.scope, model: options.model, + effort: normalizeReasoningEffort(options.effort), focusText, reviewName: config.reviewName, onProgress: progress diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..0b2cf4cbd 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -49,7 +49,8 @@ test("adversarial review command uses AskUserQuestion and background Bash while assert.match(source, /```bash/); assert.match(source, /```typescript/); assert.match(source, /adversarial-review "\$ARGUMENTS"/); - assert.match(source, /\[--scope auto\|working-tree\|branch\] \[focus \.\.\.\]/); + assert.match(source, /\[--scope auto\|working-tree\|branch\]/); + assert.match(source, /\[--effort \] \[focus \.\.\.\]/); assert.match(source, /run_in_background:\s*true/); assert.match(source, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" adversarial-review "\$ARGUMENTS"`/); assert.match(source, /description:\s*"Codex adversarial review"/); @@ -223,3 +224,22 @@ test("setup command can offer Codex install and still points users to codex logi assert.match(readme, /\/codex:setup --enable-review-gate/); assert.match(readme, /\/codex:setup --disable-review-gate/); }); + +test("adversarial-review documents and parses --model and --effort", () => { + const companion = read("scripts/codex-companion.mjs"); + // The review path must parse both runtime-selection flags, not just --model. + assert.match( + companion, + /valueOptions: \["base", "scope", "model", "cwd", "effort"\]/, + "handleReviewCommand must accept --effort" + ); + // ...and thread effort through to the turn, or parsing it changes nothing. + assert.match(companion, /effort: normalizeReasoningEffort\(options\.effort\)/); + assert.match(companion, /effort: request\.effort,\n\s*sandbox: "read-only"/); + + const usage = companion.match(/adversarial-review \[[^\n"]*/)?.[0] ?? ""; + assert.match(usage, /--effort /); + + const cmd = read("commands/adversarial-review.md"); + assert.match(cmd, /--effort /); +}); From d0b7a0949610bc02c80f449f784c476f7da4e499 Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 11:46:07 +0000 Subject: [PATCH 2/3] fix(review): resolve the spark model alias on the adversarial-review path Addresses the review finding on #677. This PR advertises --model in the adversarial-review usage, but handleReviewCommand forwarded options.model unchanged, so turn/start received the literal "spark" instead of gpt-5.3-codex-spark. normalizeRequestedModel had exactly one call site, in handleTask, which is why the task path resolved the alias and the review path did not -- the same object literal already normalized effort but not model. The review path now calls normalizeRequestedModel the same way. This introduces no undefined-vs-null change: normalizeRequestedModel returns null for unset input, and runAppServerTurn already sends model: options.model ?? null. Tests: adds a behavioural test mirroring the existing task-path spark test, asserting turn/start receives gpt-5.3-codex-spark and effort "low" for adversarial-review --model spark --effort low. Verified non-vacuous against the unpatched tree, where it fails with actual 'spark'. That test also gives the --effort threading this PR adds its first behavioural coverage rather than source assertions alone. --- plugins/codex/scripts/codex-companion.mjs | 2 +- tests/commands.test.mjs | 8 +++++++ tests/runtime.test.mjs | 26 +++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 67f60c98c..2eb6f4b86 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -744,7 +744,7 @@ async function handleReviewCommand(argv, config) { cwd, base: options.base, scope: options.scope, - model: options.model, + model: normalizeRequestedModel(options.model), effort: normalizeReasoningEffort(options.effort), focusText, reviewName: config.reviewName, diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index 0b2cf4cbd..5089b2cb8 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -236,6 +236,14 @@ test("adversarial-review documents and parses --model and --effort", () => { // ...and thread effort through to the turn, or parsing it changes nothing. assert.match(companion, /effort: normalizeReasoningEffort\(options\.effort\)/); assert.match(companion, /effort: request\.effort,\n\s*sandbox: "read-only"/); + // The usage advertises --model , so the review path must resolve the alias + // the same way the task path does. Forwarding options.model raw sends the literal + // "spark" to turn/start instead of gpt-5.3-codex-spark. + assert.match( + companion, + /model: normalizeRequestedModel\(options\.model\),\n\s*effort: normalizeReasoningEffort\(options\.effort\)/, + "handleReviewCommand must normalize --model, not forward it raw" + ); const usage = companion.match(/adversarial-review \[[^\n"]*/)?.[0] ?? ""; assert.match(usage, /--effort /); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..67b88d53d 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -784,6 +784,32 @@ test("task forwards model selection and reasoning effort to app-server turn/star assert.equal(fakeState.lastTurnStart.effort, "low"); }); +test("adversarial review forwards model selection and reasoning effort to 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")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0];\n"); + run("git", ["add", "src/app.js"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0].id;\n"); + + const result = run("node", [SCRIPT, "adversarial-review", "--model", "spark", "--effort", "low"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + // The usage advertises --model . Forwarding options.model unchanged sends + // the literal "spark" to turn/start instead of the resolved alias, the way the task + // path already resolves it. + assert.equal(fakeState.lastTurnStart.model, "gpt-5.3-codex-spark"); + assert.equal(fakeState.lastTurnStart.effort, "low"); +}); + test("task logs reasoning summaries and assistant messages to the job log", () => { const repo = makeTempDir(); const binDir = makeTempDir(); From 3750b7a914e4046eb342c1afc8497145fd96b2de Mon Sep 17 00:00:00 2001 From: Christopher Steigerwald Date: Mon, 24 Aug 2026 12:34:06 +0000 Subject: [PATCH 3/3] fix(review): reject --effort on the native review path instead of ignoring it Addresses the second review finding on #677. handleReviewCommand is shared by the native `review` subcommand and `adversarial-review`. Adding "effort" to its valueOptions therefore made `review --effort high` parse the flag as valid, and the native branch calls runAppServerReview without request.effort -- so the review silently ran at the configured default. Before this PR the unsupported flag stayed in focusText and validateNativeReviewRequest rejected it, so this was a regression that turned a loud failure into a silent one. The flag is still parsed, so the error can name it precisely, but a caller must now opt in with supportsEffort. Failing closed rather than keying off the review name means a future caller of this handler cannot inherit the silent drop by omission: if (options.effort !== undefined && !config.supportsEffort) throw ... The adversarial-review call site sets supportsEffort: true; the native one does not, and now reports: `/codex:review` maps directly to the built-in reviewer and does not support `--effort`. Retry with `/codex:adversarial-review --effort high` to choose a reasoning effort. Test mirrors the existing focus-text and staged-scope rejection tests. Verified non-vacuous against d0b7a09, where the flag is accepted and silently dropped. --- plugins/codex/scripts/codex-companion.mjs | 14 +++++++++++++- tests/runtime.test.mjs | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 2eb6f4b86..f425b0d22 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -727,6 +727,17 @@ async function handleReviewCommand(argv, config) { scope: options.scope }); + // Parsing --effort here made it a valid flag for the native `review` subcommand too, + // which shares this handler. The native branch never forwards effort, so the request + // was being silently ignored where it used to fall into focusText and be rejected. + // Fail closed: a caller must opt in, so a future one cannot inherit the silent drop. + if (options.effort !== undefined && !config.supportsEffort) { + throw new Error( + "`/codex:review` maps directly to the built-in reviewer and does not support `--effort`. Retry with `/codex:adversarial-review --effort " + + `${String(options.effort)}\` to choose a reasoning effort.` + ); + } + config.validateRequest?.(target, focusText); const metadata = buildReviewJobMetadata(config.reviewName, target); const job = createCompanionJob({ @@ -1039,7 +1050,8 @@ async function main() { break; case "adversarial-review": await handleReviewCommand(argv, { - reviewName: "Adversarial Review" + reviewName: "Adversarial Review", + supportsEffort: true }); break; case "task": diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 67b88d53d..b309f5cc5 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -1015,6 +1015,29 @@ test("review rejects focus text because it is native-review only", () => { assert.match(result.stderr, /\/codex:adversarial-review focus on auth/i); }); +test("review rejects --effort because the native reviewer cannot honour it", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + 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 }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + // handleReviewCommand is shared with the native `review` subcommand, which never + // forwards effort. Parsing the flag without rejecting it would silently run the review + // at the configured default instead of the requested effort. + const result = run("node", [SCRIPT, "review", "--effort", "high"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status > 0, true, result.stdout); + assert.match(result.stderr, /does not support `--effort`/i); + assert.match(result.stderr, /\/codex:adversarial-review --effort high/i); +}); + test("review rejects staged-only scope because it is native-review only", () => { const repo = makeTempDir(); const binDir = makeTempDir();