Skip to content

Commit f524f46

Browse files
mvanhornclaude
andcommitted
fix(review): address Codex P1 findings in codex-reviewer agent
- Remove error suppression (2>/dev/null) from `which codex` check - Split chained `git symbolic-ref | sed` into separate steps per AGENTS.md shell rule (no chaining, no error suppression) - Add fail-closed guard: do not fall back to git rev-parse against local branch names that may track a different remote - Clarify that ce:review orchestrator provides base branch context Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 70476b8 commit f524f46

1 file changed

Lines changed: 10 additions & 7 deletions

File tree

plugins/compound-engineering/agents/review/codex-reviewer.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ If either `CODEX_SANDBOX` or `CODEX_SESSION_ID` is set, return this JSON and sto
3232
## Step 2: Verify codex CLI availability
3333

3434
```bash
35-
which codex 2>/dev/null
35+
which codex
3636
```
3737

3838
If codex is not found, return this JSON and stop:
@@ -48,15 +48,18 @@ If codex is not found, return this JSON and stop:
4848

4949
## Step 3: Determine the diff target
5050

51-
Extract the base branch from the review context passed by ce:review.
51+
Extract the base branch from the review context passed by ce:review. The orchestrator passes the base branch as part of the subagent dispatch context.
5252

53-
Fallback resolution order:
54-
1. Base branch from PR metadata (if reviewing a PR)
55-
2. Detect from remote HEAD:
53+
Resolution order (stop at the first success):
54+
1. Base branch from the review context (ce:review always provides this for PR reviews)
55+
2. If no context is available (standalone invocation), detect from remote HEAD:
5656
```bash
57-
git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@'
57+
git symbolic-ref refs/remotes/origin/HEAD
5858
```
59-
3. Fall back to `main`
59+
Then strip the `refs/remotes/origin/` prefix from the result.
60+
3. If the above command fails (no remote HEAD configured), default to `main`
61+
62+
Do not fall back to `git rev-parse --verify` against local branch names. A local branch with the same name as the PR base may track a different remote or point at a different lineage, producing misleading diffs. Fail closed instead of guessing.
6063

6164
Store the resolved branch in `BASE_BRANCH`.
6265

0 commit comments

Comments
 (0)