diff --git a/README.md b/README.md index 937a3037..e4f4196a 100644 --- a/README.md +++ b/README.md @@ -86,13 +86,14 @@ Use it when you want: - a review of your current uncommitted changes - a review of your branch compared to a base branch like `main` -Use `--base ` for branch review. It also supports `--wait` and `--background`. It is not steerable and does not take custom focus text. Use [`/codex:adversarial-review`](#codexadversarial-review) when you want to challenge a specific decision or risk area. +Use `--base ` for branch review. It also supports `--wait`, `--background`, `--model `, and `--effort `. It is not steerable and does not take custom focus text. Use [`/codex:adversarial-review`](#codexadversarial-review) when you want to challenge a specific decision or risk area. Examples: ```bash /codex:review /codex:review --base main +/codex:review --model spark --effort medium /codex:review --background ``` @@ -105,7 +106,7 @@ Runs a **steerable** review that questions the chosen implementation and design. It can be used to pressure-test assumptions, tradeoffs, failure modes, and whether a different approach would have been safer or simpler. 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 also supports `--wait`, `--background`, `--model `, and `--effort `. Unlike `/codex:review`, it can take extra focus text after the flags. Use it when you want: @@ -118,6 +119,7 @@ Examples: ```bash /codex:adversarial-review /codex:adversarial-review --base main challenge whether this was the right caching and retry design +/codex:adversarial-review --model gpt-5.4-mini --effort high look for race conditions and question the chosen approach /codex:adversarial-review --background look for race conditions and question the chosen approach ``` diff --git a/plugins/codex/commands/adversarial-review.md b/plugins/codex/commands/adversarial-review.md index da440ab4..867ac973 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 --- @@ -38,6 +38,7 @@ Argument handling: - Preserve the user's arguments exactly. - Do not strip `--wait` or `--background` yourself. - Do not weaken the adversarial framing or rewrite the user's focus text. +- `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `adversarial-review` call, but do not treat them as part of the review instructions. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. - `/codex:adversarial-review` uses the same review target selection as `/codex:review`. - It supports working-tree review, branch review, and `--base `. diff --git a/plugins/codex/commands/review.md b/plugins/codex/commands/review.md index fb70a487..665aae65 100644 --- a/plugins/codex/commands/review.md +++ b/plugins/codex/commands/review.md @@ -1,6 +1,6 @@ --- description: Run a Codex code review against local git state -argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- @@ -35,6 +35,7 @@ Argument handling: - Preserve the user's arguments exactly. - Do not strip `--wait` or `--background` yourself. - Do not add extra review instructions or rewrite the user's intent. +- `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `review` call, but do not treat them as part of the review instructions. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. - `/codex:review` is native-review only. It does not support staged-only review, unstaged-only review, or extra focus text. - If the user needs custom review instructions or more adversarial framing, they should use `/codex:adversarial-review`. diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468a..c3077c9f 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -370,6 +370,7 @@ async function executeReviewRun(request) { const result = await runAppServerReview(request.cwd, { target: reviewTarget, model: request.model, + effort: request.effort, onProgress: request.onProgress }); const payload = { @@ -411,6 +412,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 +713,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", "effort", "cwd"], booleanOptions: ["json", "background", "wait"], aliasMap: { m: "model" @@ -721,6 +723,8 @@ async function handleReviewCommand(argv, config) { const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); const focusText = positionals.join(" ").trim(); + const model = normalizeRequestedModel(options.model); + const effort = normalizeReasoningEffort(options.effort); const target = resolveReviewTarget(cwd, { base: options.base, scope: options.scope @@ -743,7 +747,8 @@ async function handleReviewCommand(argv, config) { cwd, base: options.base, scope: options.scope, - model: options.model, + model, + effort, focusText, reviewName: config.reviewName, onProgress: progress diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc..f167b9e4 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -1026,7 +1026,9 @@ export async function runAppServerReview(cwd, options = {}) { client.request("review/start", { threadId: sourceThreadId, delivery, - target: options.target + target: options.target, + model: options.model ?? null, + effort: options.effort ?? null }), { onProgress: options.onProgress, diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b0605..6075667d 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -25,6 +25,8 @@ test("review command uses AskUserQuestion and background Bash while staying revi assert.match(source, /run_in_background:\s*true/); assert.match(source, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" review "\$ARGUMENTS"`/); assert.match(source, /description:\s*"Codex review"/); + assert.match(source, /\[--model \] \[--effort \]/); + assert.match(source, /`--model` and `--effort` are runtime-selection flags/i); assert.match(source, /Do not call `BashOutput`/); assert.match(source, /Return the command stdout verbatim, exactly as-is/i); assert.match(source, /git status --short --untracked-files=all/); @@ -49,10 +51,12 @@ 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\].*\[focus \.\.\.\]/s); 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"/); + assert.match(source, /\[--model \] \[--effort \] \[focus \.\.\.\]/); + assert.match(source, /`--model` and `--effort` are runtime-selection flags/i); assert.match(source, /Do not call `BashOutput`/); assert.match(source, /Return the command stdout verbatim, exactly as-is/i); assert.match(source, /git status --short --untracked-files=all/); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0..75b23867 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -412,6 +412,14 @@ rl.on("line", (line) => { send({ method: "thread/started", params: { thread: { id: reviewThread.id } } }); } const turnId = nextTurnId(state); + state.lastReviewStart = { + threadId: message.params.threadId, + reviewThreadId: reviewThread.id, + model: message.params.model ?? null, + effort: message.params.effort ?? null, + target: message.params.target + }; + saveState(state); send({ id: message.id, result: { turn: buildTurn(turnId), reviewThreadId: reviewThread.id } }); emitTurnCompleted(reviewThread.id, turnId, [ { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835..4671276d 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -157,6 +157,29 @@ test("review renders a no-findings result from app-server review/start", () => { assert.match(result.stdout, /No material issues found/); }); +test("review forwards model selection and reasoning effort to 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")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 1;\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 = 2;\n"); + + const result = run("node", [SCRIPT, "review", "--model", "spark", "--effort", "high"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastReviewStart.model, "gpt-5.3-codex-spark"); + assert.equal(fakeState.lastReviewStart.effort, "high"); +}); + test("task runs when the active provider does not require OpenAI login", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -193,6 +216,33 @@ test("task runs without auth preflight so Codex can refresh an expired session", assert.match(result.stdout, /Handled the requested task/); }); +test("adversarial review forwards model selection and reasoning effort to app-server review/start", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "adversarial-clean"); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 1;\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 = 2;\n"); + + const result = run( + "node", + [SCRIPT, "adversarial-review", "--model", "gpt-5.4-mini", "--effort", "medium"], + { + cwd: repo, + env: buildEnv(binDir) + } + ); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.4-mini"); + assert.equal(fakeState.lastTurnStart.effort, "medium"); +}); + test("transfer delegates the current Claude session directly to native import", () => { const home = makeTempDir(); const repo = path.join(home, "repo");