codex-code-review

Files (12)

• SKILL.md
• codex-invocation-contract.md · references/
• codex-prompt-branch.md · references/
• codex-prompt-fast.md · references/
• codex-prompt-full.md · references/
• codex-research-instructions.md · references/
• codex-transport.md · references/
• loop-diagnostics.md · references/
• review-common.md · references/
• scope-contract.md · references/
• review_rubric.md
• review_output.md · templates/

SKILL.md (first lines)

# Codex Code Review

<!-- Security note: Bash(bash:*) is broader than ideal; cannot scope to specific
     script paths until Claude Code #9354 resolves ${CLAUDE_PLUGIN_ROOT} in
     command frontmatter. Only invoke bash for project scripts (scripts/*). -->

## Trigger

- Keywords: review, PR, code review, second opinion, audit, check

## When NOT to Use

- Document review (use `doc-review`)
- Security-specific review (use `security-review`)
- Test coverage review (use `test-review`)
- Just want to understand code (use `code-explore`)

## Variants

| Variant | Command | Scope | Pre-checks |
|---------|---------|-------|------------|
| Fast    | `/codex-review-fast` | Diff only | None |
| Full    | `/codex-review` | Diff + local checks | lint:fix + build |
| Branch  | `/codex-review-branch` | Full branch | None |

## Shared Workflow

```
Resolve adapter → [Pre-checks if Full] → Collect changes & freeze baseline → Codex Review → Gate: derive sentinel × gate_reason (Step 4.5)
  → Ready × NONE → next gate | Blocked × IN_SCOPE_BLOCKING × untriggered → fix loop | other Blocked outcomes → E1/E2
```

Dual dispatch adds a second reviewer, and is opt-in:

```
--dual:  … → Codex + Task in parallel → Merge findings in conversation (field-level)
  → Gate: derive sentinel × gate_reason (Step 4.5)
  → Ready × NONE → next gate | Blocked × IN_SCOPE_BLOCKING × untriggered → fix loop | other Blocked outcomes → E1/E2
```

### Step 0: Reviewer Mode

**Default: Codex alone.** Do not launch a secondary reviewer. One reviewer, one verdict, noted in Step 4.5 — there is no mode field, no aggregate plane and no state machine behind this choice: which reviewers ran is a fact of the conversation, not of a store (hook-lightweighting § 3.3).

**`--dual` (Branch variant only):** adds a second reviewer **in parallel**; on the Codex-healthy path the merge is yours to perform in conversation (Step 4). A second opinion for releases, security-sensitive changes and public API surfaces — nothing persists it, nothing blocks on it, and the next invocation starts single again unless the flag is passed again. When Codex is out, there is no merge: the validated fallback report carries the gate alone (Step 3.5 Codex-failure path).

| Variant | `--dual` accepted? |
|---------|--------------------|
| Fast (`/codex-review-fast`) | No — single only |
| Full (`/codex-review`) | No — single only |
| Branch (`/codex-review-branch`) | Yes, off unless passed |

See `@rules/auto-loop.md § Review Dispatch` for why single is the default.

### Step 0.5: Resolve the adapter locator (before any snapshot)

Resolve the adapter through `references/codex-transport.md` § Locator **now**, and let any
auto-install that section prescribes happen here — before Step 1 freezes anything.

The ordering is the transport contract's, not a preference: in a consuming repository whose first
review predates the installed adapter, § Locator's second step *writes* it into the tree. Resolve it
at dispatch time instead and that write lands after Step 1 froze the changed-file set and the scope
baseline, so the new untracked file is a tree change no baseline contains and no reviewer prompt
lists — a review whose own snapshot went stale while it ran.

A `setup-required` locator outcome (no adapter at any step) stops here and is surfaced to the
operator. It is **not** `codex_fail`: nothing was dispatched, so no fallback reviewer runs and no
verdict is noted.

### Step 0.7: Pre-checks (Full variant only)

```bash
{LINT_FIX_COMMAND}
{BUILD_COMMAND}
```

These placeholders are resolved from the host project's `CLAUDE.md` or `package.json` scripts. Record
results as `LOCAL_CHECKS`.

**It is numbered before Step 1 for the same reason Step 0.5 is.** `{LINT_FIX_COMMAND}` writes — a
project-wide lint fix edits files, and a build can regenerate them. Run it after Step 1 and those
edits land outside the frozen changed-file set and scope baseline: delivered changes every reviewer
dispatch then misses or misclassifies as out-of-scope. Settle the tree first — adapter, then
pre-checks — and freeze once, over the tree that will actually be reviewed. It was numbered Step 2
until 2026-09-04, which put it after the freeze.

If anything writes to the tree *after* Step 1 has run, Step 1 is redone in full. The baseline is
frozen once per review session, and a baseline computed over a tree that has since changed is not
the one this review is judging.

### Step 1: Collect Change Metadata

Collect **metadata only** — Codex reads the actual diffs and file contents itself via sandbox access.

`TASK_DESCRIPTION` is the original task in one or two sentences, captured here and **frozen for
the whole review session** — every first, fallback, and rotated dispatch carries the same value,
and it is never rewritten from review findings (`rules/codex-invocation.md`, full contract in
`skills/codex-code-review/references/codex-invocation-contract.md` — read it before the first dispatch; the three-part
dispatch shape).

`CHANGED_FILES` is the **frozen baseline set itself**, not a narrower query — the two are computed
from the same expression below, because a manifest that is a subset of the baseline hands the
reviewer a shorter change than the one it is told to judge. The change that added this paragraph
proves it: the transport adapter, its reference and their tests were all untracked, so
`git diff --name-only HEAD` alone omitted every one of them.

| Variant | Collection Method |
|---------|-------------------|
| Fast    | `CHANGED_FILES`: `git diff --name-only HEAD` ∪ `git ls-files --others --exclude-standard` + `DIFF_STAT`: `git diff --stat HEAD`, plus a line count for each untracked file (`wc -l`), which no diff stat covers |
| Full    | Same as Fast |
| Branch  | Resolve `MERGE_BASE` **once**, per § Resolving the Branch base below, then use only that object id: `CHANGED_FILES`: `git diff --name-only $MERGE_BASE` ∪ the same uncommitted and untracked sets + `DIFF_STAT`: `git diff --stat $MERGE_BASE` + `CURRENT_BRANCH` + `BASE_BRANCH` + `COMMIT_COUNT` |

Codex reads the diffs and file contents itself, and **which command shows them depends on the
variant and on whether the file is tracked** — one blanket `git diff HEAD -- <file>` is wrong for two
of the three cases:

| What | How Codex reads it |
|------|--------------------|
| Fast / Full, tracked file | `git diff HEAD -- <file>` |

From sd0xdev/sd0x-harness, MIT.

More from sd0xdev/sd0x-harness