From 938cc6df255505675b23cbdcf442cc4acf54b973 Mon Sep 17 00:00:00 2001 From: Naoyoshi Aikawa Date: Tue, 14 Apr 2026 18:17:12 +0900 Subject: [PATCH 1/2] feat(review): auto-decide wait vs background by default /codex:review now runs unattended by default: it sizes up the diff and picks foreground for tiny changes (~1-2 files) or background otherwise, instead of halting on AskUserQuestion. This unblocks multi-step plans that embed /codex:review. Users who want the old interactive prompt can pass --ask. --wait and --background still force a specific mode. --ask is stripped before the argument string is handed to codex-companion, which does not know about it. Closes #221 Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/commands/review.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/codex/commands/review.md b/plugins/codex/commands/review.md index fb70a487..0963de8c 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|--ask] [--base ] [--scope auto|working-tree|branch]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- @@ -16,23 +16,23 @@ Core constraint: - Your only job is to run the review and return Codex's output verbatim to the user. Execution mode rules: -- If the raw arguments include `--wait`, do not ask. Run the review in the foreground. -- If the raw arguments include `--background`, do not ask. Run the review in a Claude background task. -- Otherwise, estimate the review size before asking: +- If the raw arguments include `--wait`, run the review in the foreground. +- If the raw arguments include `--background`, run the review in a Claude background task. +- Estimate the review size in every remaining case: - For working-tree review, start with `git status --short --untracked-files=all`. - For working-tree review, also inspect both `git diff --shortstat --cached` and `git diff --shortstat`. - For base-branch review, use `git diff --shortstat ...HEAD`. - Treat untracked files or directories as reviewable work even when `git diff --shortstat` is empty. - Only conclude there is nothing to review when the relevant working-tree status is empty or the explicit branch diff is empty. - - Recommend waiting only when the review is clearly tiny, roughly 1-2 files total and no sign of a broader directory-sized change. - - In every other case, including unclear size, recommend background. + - A review is "tiny" when it is clearly 1-2 files total with no sign of a broader directory-sized change. Everything else, including unclear size, is "non-tiny". - When in doubt, run the review instead of declaring that there is nothing to review. -- Then use `AskUserQuestion` exactly once with two options, putting the recommended option first and suffixing its label with `(Recommended)`: +- If the raw arguments include `--ask`, use `AskUserQuestion` exactly once with two options, putting the recommended option first (tiny → `Wait for results`, non-tiny → `Run in background`) and suffixing its label with `(Recommended)`: - `Wait for results` - `Run in background` +- Otherwise (no `--wait` / `--background` / `--ask`), auto-decide without asking: tiny → foreground, non-tiny → background. This keeps `/codex:review` runnable unattended inside multi-step plans. Users who want the old prompt can pass `--ask`; users who want to force a mode can pass `--wait` or `--background`. Argument handling: -- Preserve the user's arguments exactly. +- Preserve the user's arguments exactly, with one exception: strip `--ask` before passing to the companion script. `--ask` is a Claude-side flag that only controls whether to prompt the user; the companion does not know about it and would reject it as unsupported focus text. - Do not strip `--wait` or `--background` yourself. - Do not add extra review instructions or rewrite the user's intent. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. @@ -40,7 +40,7 @@ Argument handling: - If the user needs custom review instructions or more adversarial framing, they should use `/codex:adversarial-review`. Foreground flow: -- Run: +- Run (with `--ask` removed from the argument string if present): ```bash node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review "$ARGUMENTS" ``` @@ -48,7 +48,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review "$ARGUMENTS" - Do not paraphrase, summarize, or add commentary before or after it. - Do not fix any issues mentioned in the review output. -Background flow: +Background flow (with `--ask` removed from the argument string if present): - Launch the review with `Bash` in the background: ```typescript Bash({ From 1113b5675be2b590a927ac1b21df98ba416afcd3 Mon Sep 17 00:00:00 2001 From: Naoyoshi Aikawa Date: Tue, 14 Apr 2026 18:41:29 +0900 Subject: [PATCH 2/2] fix(review): accept --ask in companion and update tests Address Codex review findings on the initial patch: 1. /codex:review --ask previously fell through to the companion as positional focus text and failed with "unsupported focus text". Add `ask` to handleReviewCommand's booleanOptions so it is silently accepted, and drop the now-unnecessary "strip --ask" instructions from review.md. The flag is purely consumed by Claude's prompt logic; the companion just ignores it. 2. tests/commands.test.mjs still asserted the removed prompt phrases ("Recommend waiting only...", "In every other case..."). Update the assertions to match the new auto-decide wording and add coverage for the --ask branch and the new argument hint. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/commands/review.md | 9 +++++---- plugins/codex/scripts/codex-companion.mjs | 2 +- tests/commands.test.mjs | 6 ++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/plugins/codex/commands/review.md b/plugins/codex/commands/review.md index 0963de8c..337a655b 100644 --- a/plugins/codex/commands/review.md +++ b/plugins/codex/commands/review.md @@ -32,15 +32,16 @@ Execution mode rules: - Otherwise (no `--wait` / `--background` / `--ask`), auto-decide without asking: tiny → foreground, non-tiny → background. This keeps `/codex:review` runnable unattended inside multi-step plans. Users who want the old prompt can pass `--ask`; users who want to force a mode can pass `--wait` or `--background`. Argument handling: -- Preserve the user's arguments exactly, with one exception: strip `--ask` before passing to the companion script. `--ask` is a Claude-side flag that only controls whether to prompt the user; the companion does not know about it and would reject it as unsupported focus text. -- Do not strip `--wait` or `--background` yourself. +- Preserve the user's arguments exactly. +- Do not strip `--wait`, `--background`, or `--ask` yourself. - Do not add extra review instructions or rewrite the user's intent. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. +- The companion script also accepts `--ask` as a no-op boolean so it can be safely forwarded in `$ARGUMENTS`. Only Claude's prompt logic above consumes it. - `/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`. Foreground flow: -- Run (with `--ask` removed from the argument string if present): +- Run: ```bash node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review "$ARGUMENTS" ``` @@ -48,7 +49,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review "$ARGUMENTS" - Do not paraphrase, summarize, or add commentary before or after it. - Do not fix any issues mentioned in the review output. -Background flow (with `--ask` removed from the argument string if present): +Background flow: - Launch the review with `Bash` in the background: ```typescript Bash({ diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 35222fd5..83ca3342 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -682,7 +682,7 @@ function enqueueBackgroundTask(cwd, job, request) { async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["base", "scope", "model", "cwd"], - booleanOptions: ["json", "background", "wait"], + booleanOptions: ["json", "background", "wait", "ask"], aliasMap: { m: "model" } diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index ef5adb09..25335be5 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -30,8 +30,10 @@ test("review command uses AskUserQuestion and background Bash while staying revi assert.match(source, /git status --short --untracked-files=all/); assert.match(source, /git diff --shortstat/); assert.match(source, /Treat untracked files or directories as reviewable work/i); - assert.match(source, /Recommend waiting only when the review is clearly tiny, roughly 1-2 files total/i); - assert.match(source, /In every other case, including unclear size, recommend background/i); + assert.match(source, /A review is "tiny" when it is clearly 1-2 files total/i); + assert.match(source, /auto-decide without asking: tiny → foreground, non-tiny → background/i); + assert.match(source, /If the raw arguments include `--ask`/); + assert.match(source, /\[--wait\|--background\|--ask\]/); assert.match(source, /The companion script parses `--wait` and `--background`/i); assert.match(source, /Claude Code's `Bash\(..., run_in_background: true\)` is what actually detaches the run/i); assert.match(source, /When in doubt, run the review/i);