Cross Model Review

Cross-Model Code Review

Cross-model validation: the authoring model writes code, a different model reviews it. Different architectures, different training distributions, no self-approval bias.

Core insight: Single-model self-review is systematically biased. The same blind spots that let bugs through during writing let them through during review. Cross-model review catches different bug classes because the reviewer has fundamentally different failure modes.

How to read this skill: patterns and decision trees below are guidelines. Pick what fits, blend when needed. The rules marked ⚠️ are different: they're real CLI behaviors (yield_time_ms, the -- separator, scope flags), not procedural ceremony. Audited sessions show 366+ orphaned claude -p processes per session and ~7 minutes wasted per spiral when ⚠️ rules are skipped. Treat them as facts about the tool, not opinions about workflow.

Direction & Pre-Flight

Identify the host first. The host runs the _other_ model's CLI as a subprocess.

Confirm the reviewer is reachable before the real call:

On Pi: prefer /xreview when the xreview extension is installed — it shells out to codex exec --sandbox read-only with stdin closed and injects Verdict, Findings, and Fix Queue back into the session. Scope explicitly: /xreview reviews the working tree, /xreview main reviews since a base ref; put any focused concern in the prompt before running it. Manual bash fallback follows the Claude-host rules below. Treat PASS as evidence only for the reviewed scope.

User defaults are authoritative. Both CLIs read configured defaults (~/.codex/config.toml, ~/.claude/settings.json). Never specify --model, -m, or -c model=. The only sanctioned override is reasoning effort, and only for spec review (see Effort Override Policy below).

---

⚠️ Codex → Claude: Four Non-Negotiable Rules

Rules 1–3 cause the overwhelming majority of cross-model review failures. They're not workflow preferences; they're how the claude -p shell tool behaves under Codex. Rule 4 doesn't break the review; it silently bills it to the wrong account. Get all four right on the first call.

Rule 1: yield_time_ms: 300000 on EVERY call

Codex's shell tool yields output back to the model after yield_time_ms elapses (default 1000 = 1 second). A real claude -p review takes 30 seconds to 5+ minutes. The default yields empty output + Process running with session ID NNNN before Claude has even started, and the model misreads this as failure.

The rule: every claude -p call uses yield_time_ms: 300000 (5 minutes). Initial call, every reaping call, every sanity ping beyond a one-line say ok. No exceptions.

{ "cmd": "claude -p --allowedTools \"Read,Glob,Grep,Bash(git *)\" -- \"PROMPT\"", "yield_time_ms": 300000 }

Common cognitive trap: "My prompt is short, I only need 30s." Wrong. Claude session setup, network, and model compute dominate; prompt length barely factors in. Always 300000.

Consistency rule: once you're at 300000, stay at 300000. Reverting to 1000 between calls in the same session creates a fresh wave of orphans on top of any still running.

Rule 2: Process running with session ID NNNN is NOT an error: REAP, never retry

When Codex returns Process running with session ID NNNN, the process is alive and computing in the background. The yield fired before completion. This is normal output, not failure.

digraph reap {
    rankdir=TB;
    node [shape=box];

    "Initial call" [style=filled, fillcolor="#e8e8ff"];
    "Process running ID 84814" [style=filled, fillcolor="#fff8e0"];
    "WRONG: re-invoke claude -p" [style=filled, fillcolor="#ffe8e8"];
    "RIGHT: reap session_id 84814" [style=filled, fillcolor="#e8ffe8"];
    "Process exited code 0" [style=filled, fillcolor="#e8ffe8"];
    "New orphan ID 84815" [style=filled, fillcolor="#ffe8e8"];

    "Initial call" -> "Process running ID 84814";
    "Process running ID 84814" -> "WRONG: re-invoke claude -p" [label="retry"];
    "Process running ID 84814" -> "RIGHT: reap session_id 84814" [label="reap"];
    "WRONG: re-invoke claude -p" -> "New orphan ID 84815" [label="spawns new process"];
    "RIGHT: reap session_id 84814" -> "Process exited code 0" [label="loop until exit"];
}

Wrong (each retry spawns a fresh process; original keeps running):

{"cmd": "claude -p --allowedTools '...' -- 'PROMPT'", "yield_time_ms": 300000}
→ "Process running with session ID 84814"
{"cmd": "claude -p --allowedTools '...' -- 'PROMPT'", "yield_time_ms": 300000}
→ "Process running with session ID 84815"   // 84814 still alive, orphaned
{"cmd": "claude -p --allowedTools '...' -- 'PROMPT'", "yield_time_ms": 300000}
→ "Process running with session ID 84816"   // 84814 + 84815 both still alive
... 8 more retries ... 11+ orphans, ~7 minutes wall time

Right (reap the existing session by ID until exit):

{"cmd": "claude -p --allowedTools '...' -- 'PROMPT'", "yield_time_ms": 300000}
→ "Process running with session ID 84814"
{"session_id": 84814, "yield_time_ms": 300000}        // reap, do NOT re-invoke claude -p
→ "Process running with session ID 84814"             // still computing
{"session_id": 84814, "yield_time_ms": 300000}        // keep reaping
→ "Process exited with code 0"                        // done, parse the output

Reaping rules:

• Do NOT re-invoke claude -p (creates a new process)
• Do NOT change flags, prompts, or tools (reaping is a different operation entirely)
• DO call {"session_id": NNNN, "yield_time_ms": 300000} repeatedly
• Stop only when Process exited with code X appears

Rule 3: Variadic flags require the -- separator

The claude CLI has flags that take <value...> and greedily consume every following argument until the next flag. If your prompt follows one of these without a -- separator, the prompt gets swallowed as a flag value, the prompt arg goes missing, and Claude errors with Input must be provided either through stdin or as a prompt argument when using --print or hangs waiting on stdin.

Variadic flags: --allowedTools / --allowed-tools, --disallowedTools / --disallowed-tools, --tools, --add-dir, --betas, --file, --mcp-config, --plugin-dir.

Required form (default to this, works regardless of flag order):

claude -p --allowedTools "Read,Glob,Grep,Bash(git *)" -- "PROMPT"

Two fallback shapes (use only if -- won't work in your context):

The codex CLI does not have this issue, its flags are non-variadic.

Rule 4: Strip ANTHROPIC_API_KEY so the review bills to your subscription

Codex — and most shells that touch the Anthropic API — export ANTHROPIC_API_KEY into the environment. Child claude -p calls inherit it, and Claude Code's auth precedence ranks the API key above your Pro/Max subscription OAuth. Interactive claude prompts once before using a stray key and remembers your choice; -p (non-interactive) mode uses the key silently, on every call. The review still works — it just bills per-token against the API instead of drawing from your plan. Nothing surfaces it until the invoice does, which is why interactive sessions can read "Max" in /status while the headless review path quietly meters to API.

The rule: prefix every spawning claude -p call with env -u ANTHROPIC_API_KEY. That strips the variable for just that call, so Claude falls through to the subscription credentials stored by /login.

{
  "cmd": "env -u ANTHROPIC_API_KEY claude -p --allowedTools \"Read,Glob,Grep,Bash(git *)\" -- \"PROMPT\"",
  "yield_time_ms": 300000
}

The prefix goes on the spawning call only — reaping calls (Rule 2) are bare session_id polls with no command, so there is nothing to strip.

Precedence trap: CLAUDE_CODE_OAUTH_TOKEN ranks _below_ ANTHROPIC_API_KEY, so exporting an OAuth token does not rescue you while the key is present — stripping is mandatory either way. The fallback only lands on the plan if a prior interactive /login (Pro/Max) wrote ~/.claude/.credentials.json; without those creds, claude -p has nothing to fall through to.

---

⚠️ Claude → Codex: One Non-Negotiable Rule

Always pass a scope flag to codex review

A bare codex review (no scope) is the #1 cause of Claude → Codex failures: it hangs or produces 100KB+ blob output. Always specify exactly one scope flag:

For anything outside this trio (spec docs, single files, custom scopes, personas), use codex exec "PROMPT" with explicit scope in the prompt, never bare codex review.

If codex review output exceeds ~100KB, the diff is too large for one pass. Split: codex review --commit <SHA1>, codex review --commit <SHA2>, or use codex exec with a narrowed prompt ("Review error handling only").

---

⚠️ Both Directions: Capture Output to a File

Never pipe a review to | tail -N or | head -N. Three reasons it fails:

1. The pipe buffers until EOF. tail (and head) read the entire upstream stream before producing output. The agent gets _nothing_ until the review process exits or times out, no progress signal, no early verdict, no way to tell if the call is alive. With claude -p, this compounds the yield_time_ms problem: the wrapping shell call holds output until claude exits, then tail finally runs. 2. Reviews don't put the verdict at the end. Findings are typically ordered by severity (BLOCKER first), with the summary/verdict near the top. tail -300 discards exactly the part you need. 3. A file lets a human watch progress live. tail -f /tmp/review.txt in another terminal shows the review streaming in real time, completely independent of the agent's call. The pipe pattern hides everything until exit.

Right pattern: pick a non-colliding filename, redirect to it, then read it back.

# Use mktemp so parallel/repeat reviews don't clobber each other.
# Bake the scope into the slug so the file is self-describing when you tail -f it.
out=$(mktemp -t codex-review-pre-pr.XXXXXX) && echo "$out"

# Claude → Codex
codex review --base main > "$out" 2>&1
codex exec --sandbox read-only "PROMPT" > "$out" 2>&1

# Codex → Claude (yield_time_ms: 300000 + env -u ANTHROPIC_API_KEY still required — Rules 1 & 4)
env -u ANTHROPIC_API_KEY claude -p --allowedTools "Read,Glob,Grep,Bash(git *)" -- "PROMPT" > "$out" 2>&1
git diff main...HEAD | env -u ANTHROPIC_API_KEY claude -p "PROMPT" > "$out" 2>&1

If mktemp isn't handy, use a PID + timestamp slug: out=/tmp/codex-review-$$-$(date +%s).txt.

Echo the path before the redirect so the agent (and a human running tail -f) knows where to look. After the command exits, Read (or cat) the file. It persists across turns, re-read instead of re-running.

---

Review Modes Matrix

Match the row to what you're actually reviewing. The current skill historically documented 5 patterns; real usage covers many more.

Billing: every claude -p in the Codex → Claude column assumes the env -u ANTHROPIC_API_KEY prefix from Rule 4 — the cells omit it for width. Drop the prefix and the review silently meters to the API instead of your subscription.

Common scope mistakes:

• Using --base main when you only want one commit (review noise from unrelated commits) → use --commit <SHA>
• Using git diff when you meant git diff --staged → reviewer sees WIP and produces noisy findings on incomplete code
• Using piped diff for architecture review → diff lacks surrounding context; use --allowedTools tool-access mode instead

---

Sandbox & Permission Flags

Both CLIs scope what the reviewer can read, write, and execute. Default to the most restrictive that does the job.

Codex sandbox modes

codex exec and codex review accept --sandbox <mode>:

Codex working-directory and ergonomics flags

Claude permission flags (claude -p)

The default toolset for Codex → Claude is --allowedTools "Read,Glob,Grep,Bash(git *)". Add Bash(rg:*) if the reviewer needs grep across files. Resist write tools unless the review explicitly applies fixes.

---

Effort Override Policy

Code review defers to user config. Spec review overrides higher.

Why split: specs are higher-stakes than diffs, a subtle architectural mistake compounds across the eventual implementation. Code diffs are smaller scope and the user's configured effort is fine.

---

Piped Diff vs Tool Access (Codex → Claude)

For Codex-hosted sessions, choose based on depth:

Tool access costs more tokens but catches bugs that need surrounding context (signatures defined elsewhere, downstream consumers, similar patterns). Both shapes take the env -u ANTHROPIC_API_KEY prefix (Rule 4) so the cost lands on your subscription, not the API.

---

Multi-Pass Strategy

Thorough reviews benefit from multiple focused passes rather than one vague pass. Single passes dilute attention and produce shallow findings on each dimension. Each pass gets a specific persona and concern domain.

Run passes sequentially. Fix critical findings between passes to avoid noise compounding. Three review iterations is the practical ceiling; past that, returns diminish and you start re-litigating findings rather than fixing real bugs.

---

Prompt Engineering Heuristics

These apply to both directions; prompts are model-agnostic and reliably improve review signal:

1. Assign a persona. "Senior security engineer" beats "review for security" 2. Specify what to skip. "Skip formatting, naming style, minor docs gaps" prevents bikeshedding 3. Require confidence scores and act only on findings ≥ 0.7 4. Demand file:line citations. Vague findings without location aren't actionable 5. Ask for concrete fixes. "Suggest a specific fix" 6. One domain per pass. Security-only, architecture-only 7. Demand a verdict. "Verdict: patch is correct / incorrect" or "go / no-go"

Ready-to-use prompt templates for security, architecture, performance, error handling, and concurrency are in references/prompts.md.

---

Anti-Patterns

---

What This Skill is NOT

• Not a replacement for human review, can't evaluate product direction or UX
• Not a linter, use linters for formatting and style
• Not infallible, 5–15% false positive rate is normal; triage findings
• Not for self-approval, the entire point is cross-model validation

References

For ready-to-use prompt templates (security, architecture, performance, error handling, concurrency), see references/prompts.md.

Cross-Model Review Prompt Templates

Ready-to-use prompts for each review pass. These are model-agnostic — they work with any reviewer CLI.

Usage

Pass prompts as the final argument to the reviewer CLI:

# Claude-hosted session (Codex reviews)
codex exec "PROMPT_TEXT_HERE"

# Codex-hosted session (Claude reviews) — basic
# The `env -u ANTHROPIC_API_KEY` prefix is required: Codex exports the key, and
# `claude -p` would otherwise bill per-token to the API instead of your subscription.
env -u ANTHROPIC_API_KEY claude -p "PROMPT_TEXT_HERE"

# Codex-hosted session (Claude reviews) — with read-only tool access
# The `--` is required: --allowedTools is variadic and will swallow the prompt without it.
env -u ANTHROPIC_API_KEY claude -p --allowedTools "Read,Glob,Grep,Bash(git *)" -- "PROMPT_TEXT_HERE"

# Or pipe a diff into either
git diff main...HEAD | codex exec "PROMPT_TEXT_HERE"
git diff main...HEAD | env -u ANTHROPIC_API_KEY claude -p "PROMPT_TEXT_HERE"

For Codex's structured codex review command, prompts aren't needed — it has its own review format.

Claude CLI gotcha: Variadic flags (--allowedTools, --allowed-tools, --disallowedTools, --tools, --add-dir, --betas, --file, --mcp-config, --plugin-dir) greedily consume every following argument until the next flag. Always either put the prompt before the flag, separate it with --, or feed it via stdin.

Codex sandbox gotcha: When Codex is the host, run claude -p with yield_time_ms: 300000. The default 1000ms yield returns empty output and Process running with session ID NNNN while claude is still working — do not retry, reap session_id: NNNN until it exits. See SKILL.md for details.

Billing gotcha: Codex exports ANTHROPIC_API_KEY, which outranks subscription OAuth in Claude Code's auth precedence. In -p mode the key is used silently, so the review bills per-token to the API instead of your Pro/Max plan. Prefix every spawning claude -p call with env -u ANTHROPIC_API_KEY. See SKILL.md Rule 4.

General Review

Best as the first pass. Broad coverage across all dimensions.

Review the changes between main and HEAD with extreme thoroughness. Prioritize:
1. Correctness — logic errors, edge cases, null handling, race conditions
2. Security — injection, auth gaps, secrets exposure, OWASP Top 10:2025
3. Performance — algorithmic complexity, N+1 queries, memory leaks
4. Maintainability — coupling, abstraction leaks, API consistency

For each finding:
- Cite exact file and line range
- Explain the bug/risk concretely (not "this could be improved")
- Suggest a specific fix
- Rate confidence 0.0-1.0

Skip: formatting, naming style, minor documentation gaps.
Overall verdict: "patch is correct" or "patch is incorrect" with justification.

Security Deep-Dive

You are a senior application security engineer reviewing a code change.

Analyze the diff between the current branch and main for:
1. Injection vulnerabilities (SQL, XSS, command, LDAP, template)
2. Authentication & authorization flaws
3. Secrets / credential exposure (hardcoded keys, tokens in logs)
4. Insecure deserialization or data handling
5. SSRF, path traversal, open redirects
6. Cryptographic misuse (weak algorithms, improper randomness)
7. Dependency risks (known CVEs, typosquatting)
8. Error handling that leaks internal state

For each finding, provide:
- Severity: critical / high / medium / low
- Attack vector description
- Affected file and line range
- Concrete remediation with code example
- Confidence: 0.0-1.0

If no security issues found, state that explicitly with your confidence level.
Do NOT flag style or non-security concerns.

Architecture Review

You are a principal software architect reviewing a code change for design quality.

Evaluate the diff between current branch and main:
1. Does this change respect existing architectural boundaries?
2. Are abstractions at the right level — not too leaky, not over-engineered?
3. Does coupling increase or decrease? Quantify if possible.
4. Is the API surface consistent with existing patterns in the codebase?
5. Does this change make the system harder to test, extend, or maintain?
6. Are there backwards compatibility concerns?
7. Would a different design achieve the same goal more cleanly?

For each concern:
- Reference specific files and patterns
- Explain the architectural principle being violated
- Suggest a concrete alternative approach
- Rate impact: blocks-merge / should-fix / nice-to-have
- Confidence: 0.0-1.0

Skip: implementation details, performance micro-optimizations, style.

Performance Review

You are a performance engineer reviewing a code change for efficiency.

Analyze the diff between current branch and main:
1. Algorithmic complexity — is there O(n^2) where O(n) or O(n log n) suffices?
2. Database queries — N+1 patterns, missing indexes, unnecessary JOINs
3. Memory — leaks, unnecessary copies, unbounded growth
4. I/O — blocking calls on hot paths, missing async/streaming
5. Caching — missed opportunities, cache invalidation bugs
6. Bundle/binary size — unnecessary dependencies, tree-shaking failures
7. Concurrency — lock contention, thread-safety, deadlock potential

For each finding:
- Estimated impact magnitude (minor / moderate / severe)
- Affected hot path or user-facing scenario
- Concrete optimization with before/after code
- Whether a benchmark is warranted
- Confidence: 0.0-1.0

Skip: premature optimization, style preferences, sub-millisecond concerns in cold paths.

Error Handling Review

You are reviewing a code change specifically for error handling correctness.

Analyze the diff between current branch and main:
1. Are all error paths handled? Check every function that can fail.
2. Do errors propagate correctly to callers? No silent swallowing.
3. Are error messages meaningful to the user/operator?
4. Are resources cleaned up in error paths (connections, file handles, locks)?
5. Are retries safe? Is the operation idempotent?
6. Are error types/codes consistent with the rest of the codebase?
7. Could any error cause cascading failures?

For each finding:
- The specific error path that's mishandled
- What happens when this error occurs (user impact)
- Concrete fix with code
- Confidence: 0.0-1.0

Concurrency Review

You are reviewing a code change for concurrency correctness.

Analyze the diff between current branch and main:
1. Shared mutable state — is it properly synchronized?
2. Race conditions — could interleaving produce incorrect results?
3. Deadlock potential — are locks acquired in consistent order?
4. Atomicity — are compound operations atomic when they need to be?
5. Async correctness — are promises/futures properly awaited? Error handled?
6. Thread safety — are data structures safe for concurrent access?
7. Resource lifecycle — are connections/handles properly scoped?

For each finding:
- The specific interleaving or scenario that causes the bug
- Affected file and line range
- Concrete fix
- Confidence: 0.0-1.0

Skip: single-threaded code paths, non-concurrent modules.

Custom Template Skeleton

For domain-specific reviews, use this skeleton:

You are a [specific role] reviewing a code change for [specific domain].

Analyze the diff between current branch and main:
1. [Specific check 1]
2. [Specific check 2]
3. [Specific check 3]
...

For each finding:
- [Required output field 1]
- [Required output field 2]
- Affected file and line range
- Concrete fix with code example
- Confidence: 0.0-1.0

Skip: [explicitly list what to ignore].