Adversarial Code Review

adversarial-code-review

Multi-perspective adversarial review of a diff or codebase. Two independent reviewers (Architect + Inspector) run concurrently and each produce JSON findings, two cross-review passes (A reviews B's findings, B reviews A's findings) pressure-test them, and a synthesis rapporteur collapses everything into a single ranked report.

The review engine, subprocess runner, and personas live in the sibling adversarial-common skill — this skill only wires the review flow and the source-gathering modes.

Installation

Requires the adversarial-common sibling repo (shared engine). One-line install:

curl -fsSL https://raw.githubusercontent.com/chpomob/adversarial-code-review/main/scripts/install.sh | bash

or, from an existing checkout:

bash scripts/install.sh

Both place adversarial-code-review and adversarial-common side by side under ~/.hermes/skills (override the target with $1 or $HERMES_HOME).

When to use

• Before merging a feature branch (--diff-git).
• On a standalone patch file (--diff).
• On a whole directory or single file (--dir, --file).
• On an existing project in place (--project-dir).

Usage

python3 scripts/adversarial_review.py  [options]

The reviewer command defaults to the claude-tmux wrapper (no model pinned — the CLI picks its own best). Override per-run with --review-cmd or persistently with $ACR_REVIEW_CMD.

Sources (mutually exclusive)

Options

Env vars: ACR_A_CMD, ACR_B_CMD, ACR_CROSS_A_CMD, ACR_CROSS_B_CMD, ACR_SYNTH_CMD, ACR_ORCHESTRATOR_CMD, ACR_WORKER_CMD — each falls back to the resolved --review-cmd (or its env var ACR_REVIEW_CMD), except ACR_CROSS_A_CMD which falls back to ACR_A_CMD and ACR_CROSS_B_CMD which falls back to ACR_B_CMD.

Example: review a single file with Codex Architect + Claude Inspector

python3 scripts/adversarial_review.py \
  --file /path/to/target.py \
  --a-cmd "codex exec --skip-git-repo-check --sandbox read-only" \
  --b-cmd "python3 /path/to/claude-tmux.py --timeout 600 --hard-timeout 1200 --cwd /path/to/repo" \
  --synth-cmd "python3 /path/to/claude-tmux.py --timeout 600 --hard-timeout 1200 --cwd /path/to/repo" \
  --out /tmp/acr-review

Example: review the current branch against main

python3 scripts/adversarial_review.py --diff-git --base main --out .adversarial-review

Example: review a full project directory — Codex + Claude with mutual cross-review

python3 scripts/adversarial_review.py \
  --project-dir /path/to/repo \
  --a-cmd "codex exec --skip-git-repo-check --sandbox read-only" \
  --b-cmd "python3 /path/to/claude-tmux.py --timeout 600 --hard-timeout 1200 --cwd /path/to/repo" \
  --cross-a-cmd "codex exec --skip-git-repo-check --sandbox read-only" \
  --cross-b-cmd "python3 /path/to/claude-tmux.py --timeout 600 --hard-timeout 1200 --cwd /path/to/repo" \
  --synth-cmd "python3 /path/to/claude-tmux.py --timeout 600 --hard-timeout 1200 --cwd /path/to/repo" \
  --out .adversarial-review --html --show-costs

When --cross-a-cmd and --cross-b-cmd are omitted, they default to --a-cmd and --b-cmd respectively. The targets remain symmetric: cross-review A uses the Architect command to review the Inspector's findings, and cross-review B uses the Inspector command to review the Architect's findings. Override the cross commands only when those passes need different providers or settings.

Output

Artifacts land under --out (default .adversarial-review):

• 01_architect.txt, 02_inspector.txt — raw reviewer JSON
• 03_cross_1.txt — cross-review: A reviews B's (Inspector) findings
• 04_cross_2.txt — cross-review: B reviews A's (Architect) findings
• 05_synthesis.txt + review.md — the consolidated ranked report
• final.json — machine-readable verdict, complexity, parallel mode, and cost ledger for CI/cron
• report.html — optional self-contained report produced by --html

final.json shape:

{
  "verdict": "APPROVE|REQUEST_CHANGES|REJECT",
  "summary": "first lines of the synthesis report",
  "findings": {"blocker": 1, "major": 2, "minor": 4},
  "report": ".adversarial-review/review.md",
  "source_diff": true
}

Exit codes

Personas

Loaded from ../adversarial-common/personas/ — the single source of truth, now 100% generic (no embedded/hardware-specific references):

• architect.md — architecture, security, concurrency, design
• inspector.md — bugs, edge cases, error handling, quality
• cross_review.md — devil's advocate: VALIDATE / CHALLENGE / ADD
• synthesis.md — rapporteur: cross-validated / consensus / disputed

Model pairing rules

• Architect and Inspector MUST be different models (never the same model for both roles). The cross-reviews each default to one of the two (cross-A → A, cross-B → B), while their targets are the other reviewer's findings: A reviews B and B reviews A. The default is therefore a symmetric mutual cross-review.
• Never pin a specific Claude model (--model sonnet, --model best, etc.) unless the user explicitly asks for one — let the claude-tmux wrapper use its default.
• Preferred pairing: Codex (Architect) + Claude (Inspector + cross-B + Synthesis). Codex does the structural/design analysis; Claude produces reliable JSON output in the exact schema the pipeline expects.
• Alternative for Inspector: GLM-5.2 — works but may output different JSON keys (category instead of file, issue instead of summary). See GLM-5.2 pitfall below.
• Synthesis should use the same model as Inspector to avoid schema conflicts.

Pitfalls

• GLM-5.2 inspector may output a different JSON schema than expected in --file mode. The pipeline expects findings with keys {id, severity, file, line, summary, evidence} plus a top-level verdict. GLM-5.2 may write prose with different keys like {id, severity, category, location, issue, fix} — a structural schema mismatch that strip_json_wrapper cannot fix. Symptom: 02_inspector.txt exists but Phase 'inspector' failed (exit 1) with invalid reviewer JSON: expected findings with id, severity, file, line, summary, and evidence. Diagnosis: check 02_inspector.txt — if the JSON keys don't match the pipeline schema, it's a schema mismatch, not a formatting issue. Fix: either (a) add the missing keys to the persona prompt in personas/inspector.md, or (b) switch the inspector to a model that reliably outputs the exact schema (Codex works; DeepSeek V4 Pro usually works). Validated 2026-07-14 on claude-tmux.py review.

• _valid_line() now accepts free-form string markers, not just integers. Models sometimes emit non-numeric line markers like "(review request)" or "(global)" for findings that don't map to a specific line. Previously _valid_line() required isinstance(line, int) or line.isdigit(), which rejected these strings and caused the entire phase to fail with invalid reviewer JSON. Fixed 2026-07-15: _valid_line() now returns True for any non-empty string, preserving the original intent (integer preferred) while tolerating model-generated location markers. Validation still rejects empty strings and None. See git log -1 -- scripts/adversarial_review.py for the commit change.

• Full-project reviews (--project-dir, --dir) exceed the foreground timeout cap. The 5-phase pipeline (Architect + Inspector + 2 cross-reviews + Synthesis) on a multi-file codebase takes 5–30 minutes depending on model speed and file count. On Hermes CLI, the foreground terminal timeout caps at 600s. Always run --project-dir or --dir reviews in background mode with notify_on_complete=true. See example above.

• Synthesis phase times out when Claude quota is exhausted.

• Cross-review is symmetric even when the cross command flags are omitted. Cross-review 1 runs the Architect command on the Inspector's findings; cross-review 2 runs the Inspector command on the Architect's findings and receives round 1 as additional context. The flags select commands, not review targets.

• The claude-tmux wrapper rejects --yolo. Do not add that option to reviewer, cross-review, or synthesis commands.

• ~ in --a-cmd/--b-cmd/--synth-cmd mid-command breaks resolve_role_cmd. providers.resolve_role_cmd() only calls os.path.expanduser() when the entire command starts with ~. A command like python3 ~/.hermes/skills/... (tilde mid-string) never gets expanded, so the subprocess runner receives a literal ~ and fails with Command not found. Fix (applied 2026-07-14): split the command per-token with shlex.split(), expand each token, and re-join with shlex.join() before returning. This ensures ~ is resolved regardless of position in the command string. The fix lives in adversarial_common/adversarial_common/providers.py:resolve_role_cmd.

• Pre-publication reviews need a cleanup sweep, not just code defects. Before publishing any Hermes skill, run the full checklist in references/pre-publication-cleanup.md: privacy scan, tracking audit (French files, pipeline artifacts, backup copies, personal notes, OAuth bypass docs), .gitignore hygiene, and SKILL.md reference de-dangling. The adversarial review finds code defects but does NOT check for leaked config, language-mismatched content, or missing metadata — the orchestrator must run those separately. Validated 2026-07-16: adversarial-code-loop had 9 French-language files and 52 personal workflow references committed; adversarial-plan had pipeline artifacts from 2 separate loop runs. All were git rm --cached + push-removed.

• Personas historically contained hardcoded hardware references (ESP32-S3, CC1101 at 433 MHz, BLE) that biased reviews of pure-software projects. This was fixed 2026-07-17: all 4 persona files in ../adversarial-common/personas/ were rewritten to be generic. The old architect.md asked about DSP on ESP32-S3, noise floor, antenna gain, IRAM usage; the old inspector.md asked about CC1101 RSSI quantization, SPI bus speed, and BLE spectral scans. If you encounter any remaining hardware-specific language in the personas, patch ../adversarial-common/personas/.md to remove it.

• --diff-git needs git ≥ 2.5 (worktree support). gitops.ensure_git_available() guards git presence; older hosts should use --diff or --project-dir.

• Worktrees are created under /tmp/review-- and force-removed in a try/finally, even when the applied patch leaves them dirty. A crash mid-review can leave one behind — git worktree prune cleans stale metadata.

• A dirty working tree is auto-stashed and restored. If git stash pop hits a conflict (rare — the review does not touch the main workdir), the stash is kept and a warning is printed; resolve and git stash pop manually.

• Base resolution is a fallback chain, not strict: --base that does not resolve keeps trying $ACR_BASE → main → master. Set ACR_BASE in CI to make the base explicit and stable.

• An empty or insufficient diff exits 5 (EXIT_CONTEXT_BLOCKED), not 0 — configure CI to handle a blocked preflight explicitly.

• Worktree creation failure exits 2 by default (no silent fallback to the live workdir, which could review the wrong tree). Pass --allow-fallback to instead review the current working directory with a prominent stderr warning.

• --diff-git never moves the main workdir's branch — the worktree is a separate checkout at the merge-base. The original branch is restored defensively in cleanup.

• The reviewer CLIs are invoked through adversarial_common.runner.run_cli (temp-file IO, start_new_session, killpg on timeout) — a hung sandbox grandchild cannot deadlock the pipeline.