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..f425b0d22 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" @@ -726,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({ @@ -743,7 +755,8 @@ 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, onProgress: progress @@ -1037,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/commands.test.mjs b/tests/commands.test.mjs index c34b06059..5089b2cb8 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,30 @@ 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"/); + // 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 /); + + const cmd = read("commands/adversarial-review.md"); + assert.match(cmd, /--effort /); +}); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..b309f5cc5 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(); @@ -989,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();