fix(codex,review,ship): scope Codex review with --base — prompt-only review silently reviews the wrong diff

[fangearhq-boop]

/codex review has been reviewing the wrong diff

On Codex CLI 0.144.x, /codex review returns a confident, well-formatted review of changes that are not the ones you asked about. There is no error, no warning, and no way to tell from the output that anything is wrong.

The crash was the symptom. The fix for the crash was the bug.

Codex CLI 0.130.0 made the positional [PROMPT] mutually exclusive with the scope flags, so the old invocation started failing loudly:

error: the argument '[PROMPT]' cannot be used with '--base <BRANCH>'

That's #1428 and #1479. The resolution kept the prompt and dropped --base, then moved the scope into prompt text:

codex review "…Review the changes on this branch against the base branch <base>.
Run git diff origin/<base>...HEAD … to see the diff and review only those changes."

This parses cleanly, which is exactly why it survived. But codex review --help shows [PROMPT], --base, --commit, and --uncommitted are all scope selectors, and a prompt-only codex review falls back to the uncommitted working-tree scope: it runs git status --short; git diff and reviews that.

Prompt text asking the model to "run git diff origin/main...HEAD" does not change what the CLI feeds the reviewer. The model gets the working-tree diff and reviews it, faithfully. So:

• On a dirty tree, you get a real review of the wrong changes.
• On a clean tree, you get "no changes" on a branch full of them.

Neither case produces an error, so the failure is invisible. A review tool that silently reviews the wrong thing is worse than one that crashes: the crash told you to look.

The fix

Step 2A's default path now passes the scope flag and no prompt:

codex review --base <base> -c 'model_reasoning_effort="high"' --enable web_search_cached

Applied unconditionally, with no codex --version branch. [PROMPT] has always been optional, so the no-prompt form is valid on every version that supports --base — there is no version window this breaks.

Three supporting changes:

• Filesystem Boundary was scoped to the three codex exec paths. It claimed to govern Review mode's prompt argument, which no longer exists. Dropping it from the scoped path is acceptable: --base hands the model a pre-computed diff rather than turning it loose on the filesystem, so the rabbit-hole risk it guards against is much lower there.
• Custom instructions keep routing through codex exec. They cannot ride along with --base (that's the rejected combination) and cannot be smuggled in by dropping --base (that's the silent wrong scope). The rationale now says so explicitly.
• Error Handling documents both the argv error — carrying a "do not fix this by removing --base" warning, since that is the trap — and the silent-wrong-scope symptom. The second entry matters precisely because it has no error message anyone could search for.

Tests that pinned the bug

Two tests asserted the broken shape as desired behavior, including one named "codex review commands pass diff scope through prompt, not --base". Both worked by banning the substring:

expect(content).not.toContain('--base <base> -c \'model_reasoning_effort="high"\'');

The correct bare form contains that substring too, so the assertion could not distinguish a scoped call from an unscoped one — it just banned --base outright. They now assert the real invariant for codex/: no positional prompt may precede a scope flag. That's the same line-level check test/codex-hardening.test.ts already uses, and those existing guards pass unchanged.

A stale comment in the boundary test instructed future contributors that the call "must be through codex review "<prompt>" not bare codex review --base" — i.e. it told the next person to reintroduce this. Corrected; the assertion itself was already passing and is untouched.

/review and /ship had it too (second commit)

scripts/resolvers/review.ts:559 carried the identical prompt-only invocation, and that resolver feeds both /review and /ship's structured Codex pass — so those were reviewing the wrong diff as well, with the same absence of any error. Fixed the same way, with review/SKILL.md, ship/sections/adversarial.md, and the factory ship golden regenerated. The golden's only delta is this change.

The adversarial pass a few lines above is deliberately left as-is. It uses codex exec, which is agentic and genuinely runs the git command it's given, so scoping that one in prompt text is correct. The distinction is now written down next to both call sites, because the obvious "cleanup" is to unify them and that would reintroduce the bug on the wrong side.

With everything on the scoped form, both tests now assert the real invariant across all five files instead of pinning one shape per file: every codex review invocation carries a scope flag, and none puts a positional prompt in front of it. Prose mentions and codex exec calls are excluded.

Verification

• codex review --base HEAD~2 -c 'model_reasoning_effort="high"' --enable web_search_cached on a real repo returned a genuine review including a [P2] finding — the marker format Step 4's pass/fail gate parses, so the gate still works.
• codex review --base HEAD~1 correctly reported an empty diff.
• codex exec "<prompt>" still accepts a prompt, so Challenge (2B) and Consult (2C) are unaffected.
• codex review --help on 0.144.1 confirms [PROMPT] is optional and the scope flags are what set scope.
• Full free suite (bun test): failure set is identical before and after — 840 failures on both, all pre-existing on this Windows checkout (gstack-slug executable-bit and gtimeout resolution). Verified by running the suite on main and diffing the failure lists, not by assertion.
• One unrelated flake surfaced during the runs (context-save-hardening.test.ts → "50 .md files → only 20 returned"). It passed 3/3 in isolation and did not recur on the next full run; it touches nothing in this diff. Flagging it rather than quietly dropping it from the counts.
• bun run gen:skill-docs regenerated codex/SKILL.md against current main; regeneration is idempotent.

Tested on codex-cli 0.144.1.

Refs #1428, #1479

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here