Skip to content

Commit 18a2bc3

Browse files
igerberclaude
andcommitted
Forward --backend through skill Step-5 template + document codex surface area
R0 review on PR #421 caught the exact "incomplete parameter propagation" anti-pattern: the script accepted --backend via argparse, but the skill's Step-5 command template never forwarded it. Effect: `/ai-review-local --backend codex` (or `--backend api`) was silently ignored, and the script's default `auto` always won. Fix: - Step 5 command template now includes `--backend "$backend"` - Step 5 bullet list documents that --backend always passes through - New test `test_skill_step5_command_template_forwards_backend` pins the template (asserts both the literal `--backend ` token and the `$backend` shell variable are present) Also documents codex backend's surface area in the skill doc: Under codex backend, Codex can read any file under the repo root (via `--cd`), not just the staged diff. Same surface as direct `codex` use, but the Step-3b pre-upload secret scan only covers diff content, NOT files Codex may load on its own. Recommend `--backend api` (or a sanitized worktree) when the repo contains sensitive files outside the diff. This is documentation, not enforcement — sandboxing reads to a temp checkout would defeat the agentic value (Codex needs the full repo to do cross-surface audits). Users opting into codex are already running `codex` directly as part of `codex login` setup. 197 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7840885 commit 18a2bc3

2 files changed

Lines changed: 36 additions & 1 deletion

File tree

.claude/commands/ai-review-local.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ Notes:
3636
cleaned up automatically.
3737
- `--context` and `--token-budget` are ignored under the codex backend (Codex
3838
chooses what to load on its own); the script warns if you pass them.
39+
- **Surface area**: under the codex backend, Codex can read any file under the
40+
repo root via `--cd`, not just the staged diff. This is the same surface that
41+
any direct invocation of `codex` itself would have, but worth knowing: the
42+
Step 3b pre-upload secret scan only covers diff content, NOT files Codex may
43+
load on its own. If your repo contains sensitive files outside the diff
44+
(e.g. `.env`, local credentials), prefer `--backend api` or run the codex
45+
backend from a sanitized worktree.
3946

4047
## Arguments
4148

@@ -356,10 +363,11 @@ review via `--previous-review`.
356363
### Step 5: Run the Review Script
357364

358365
Build and run the command. Include optional arguments only when their conditions are met:
366+
- `--backend`: pass through from parsed arguments (default `auto`); the script auto-detects
359367
- `--previous-review`: only if `.claude/reviews/local-review-previous.md` exists AND `--force-fresh` was NOT set
360368
- `--delta-diff` and `--delta-changed-files`: only if delta files were generated in Step 4
361369
- `--review-state`, `--commit-sha`, `--base-ref`: always include (even with `--force-fresh`, to seed a new baseline)
362-
- `--context`, `--include-files`, `--token-budget`: pass through from parsed arguments
370+
- `--context`, `--include-files`, `--token-budget`: pass through from parsed arguments (only meaningful for `--backend api`; ignored under codex)
363371

364372
```bash
365373
python3 .claude/scripts/openai_review.py \
@@ -370,6 +378,7 @@ python3 .claude/scripts/openai_review.py \
370378
--output .claude/reviews/local-review-latest.md \
371379
--branch-info "$branch_name" \
372380
--repo-root "$(pwd)" \
381+
--backend "$backend" \
373382
--context "$context_level" \
374383
--review-state .claude/reviews/review-state.json \
375384
--commit-sha "$(git rev-parse HEAD)" \
@@ -385,6 +394,12 @@ python3 .claude/scripts/openai_review.py \
385394
[--dry-run]
386395
```
387396

397+
Always pass `--backend "$backend"` (where `$backend` is the parsed value, defaulting
398+
to `auto`). The script handles auto-detection internally; forwarding the flag means
399+
explicit `/ai-review-local --backend codex` and `/ai-review-local --backend api`
400+
choices are honored end-to-end. Without forwarding, the user's `--backend` selection
401+
would be silently ignored.
402+
388403
Note: `--force-fresh` is a skill-only flag — it controls whether delta diffs are
389404
generated in Step 4 and is NOT passed to the script.
390405

tests/test_openai_review.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,6 +2045,26 @@ def test_skill_doc_mentions_codex_install(self):
20452045
assert "brew install --cask codex" in text or "@openai/codex" in text
20462046
assert "codex login" in text
20472047

2048+
def test_skill_step5_command_template_forwards_backend(self):
2049+
"""Regression: the Step-5 invocation MUST pass --backend through to
2050+
the script. Without this, /ai-review-local --backend codex (or api)
2051+
is silently ignored — the script's parsed --backend always defaults
2052+
to 'auto'. This is the exact 'incomplete parameter propagation'
2053+
anti-pattern; pin the template."""
2054+
assert _SCRIPT_PATH is not None
2055+
repo_root = _SCRIPT_PATH.parent.parent.parent
2056+
doc = repo_root / ".claude" / "commands" / "ai-review-local.md"
2057+
if not doc.exists():
2058+
pytest.skip("ai-review-local.md not found")
2059+
text = doc.read_text()
2060+
# The Step 5 command template must contain the --backend flag,
2061+
# forwarded as a shell variable substitution.
2062+
assert "--backend " in text and "$backend" in text, (
2063+
"Step 5 command template must forward --backend to the script "
2064+
"(use `--backend \"$backend\"`); otherwise users' explicit "
2065+
"--backend selection is dropped."
2066+
)
2067+
20482068

20492069
class TestExtractResponseText:
20502070
def test_prefers_output_text_field(self, review_mod):

0 commit comments

Comments
 (0)