# Ce Code Review > Structured code review for bugs, regressions, tests, and standards. Use before PRs or when asked for review; interactive mode can fix locally, while mode:agent reports only for pipeline callers. - Skill: `livelybug/ce-code-review` (Agent Skill, multi-file: 26 files) - Install (CLI): `npx skillmds@latest add livelybug/ce-code-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/livelybug/ce-code-review/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: DevOps & Infra - Author: livelybug (https://skillmd.com/u/livelybug) - Updated: 2026-09-08 - Page: https://skillmd.com/skills/livelybug/ce-code-review --- # Code Review Reviews code changes using dynamically selected reviewer personas. Spawns parallel sub-agents that return structured JSON, then merges and deduplicates findings into a single report. ## When to Use - Before creating a PR - After completing a task during iterative implementation - When feedback is needed on any code changes - Can be invoked standalone - Can run inside larger workflows; use `mode:agent` when the caller needs JSON instead of markdown tables ## Argument Parsing Parse `$ARGUMENTS` for optional tokens. Strip each recognized token before interpreting the remainder as a PR number, GitHub URL, or branch name. | Token | Example | Effect | |-------|---------|--------| | `mode:agent` | `mode:agent` | **Report-only**: return **JSON** instead of markdown tables and skip the Stage 5c apply (the caller applies). Does not change reviewer selection, merge logic, or scope rules (see Output format) | | `mode:headless` | `mode:headless` | **Deprecated alias** for `mode:agent` | | `mode:report-only` | `mode:report-only` | **Deprecated — ignored.** Former no-artifacts mode; default behavior is review-only without checkout | | `base:` | `base:abc1234` or `base:origin/main` | Diff base on the **current checkout** (explicit; skips auto base detection) | | `plan:` | `plan:docs/plans/2026-03-25-001-feat-foo-plan.md` | Plan file for requirements verification (explicit). Supports markdown and HTML unified plans. | | `depth:full` | `depth:full` | **Force the full reviewer roster** — skip the Stage 3c small-diff lite path so every always-on persona runs regardless of diff size. Use when a deep/thorough review is explicitly requested (the one escalation signal Stage 3c cannot infer from the diff). Does not change conditional selection, merge, or scope. | | `depth:auto` | `depth:auto` | **Default** — self-right-size via Stage 3c (lite roster for trivial, low-risk, code-only diffs; full roster otherwise). | | `grouping:auto` | `grouping:auto` | **Default** — build thematic triage groups when findings span distinct concerns (Stage 5 step 9b) | | `grouping:off` | `grouping:off` | Suppress triage groups: no Triage Groups section, empty `triage_groups` in JSON | | `grouping:always` | `grouping:always` | Always build triage groups, even for small reviews | **Grouping is presentation, not a mode.** The `grouping:` tokens change how the finding set is organized for triage — never reviewer selection, merge logic, scope rules, or the Stage 5c apply decision. **Mode alias:** `mode:headless` normalizes to `mode:agent`. `mode:agent` + `mode:headless` is not a conflict. **Conflicting arguments:** Stop without dispatching reviewers when: - Multiple incompatible scope selectors appear together (e.g. `base:` **and** a PR number/branch target — `base:` means "review the current checkout against this base") - Multiple distinct `mode:` tokens other than the `mode:agent`/`mode:headless` alias pair - Multiple distinct `grouping:` tokens (e.g. `grouping:off` **and** `grouping:always`) Deprecated `mode:autofix` is **not** a conflict — ignore the token and proceed with the normal flow (see below). Emit a one-line failure reason. In `mode:agent`, return JSON: `{"status":"failed","reason":"..."}`. ## Operating principles Same pipeline for default and `mode:agent`: - **Apply locally; never push.** Never push, open PRs, or file tickets in any mode — push is the outward step the user owns. In **default (interactive)** mode the review applies safe, verified fixes and commits them when the pre-review tree was clean (Stage 5c owns the full rule). In **`mode:agent`** it never mutates the tree — it reports and the caller applies. - **No blocking prompts.** Never use `AskUserQuestion`, `request_user_input`, `ask_user`, or other blocking question tools. Infer intent, plan, and scope from explicit tokens, git state, PR metadata, and conversation. Note uncertainty in Coverage or the verdict — do not stop to ask. - **Explicit mutations only.** Never run `gh pr checkout`, `git checkout`, `git switch`, or similar branch-switch commands. Passing a PR number, URL, or branch name selects **review scope**, not permission to mutate the working tree. To review local uncommitted work on a feature branch, check out that branch yourself (or stay on it) and pass `base:` or no target. - **Smart defaults.** Untracked files: review tracked changes only and list excluded paths in Coverage. Plan: use `plan:` when passed; otherwise discover conservatively from PR body or branch keywords. Weak advisory P2/P3 from testing/maintainability alone: demote to `testing_gaps` / `residual_risks` per Stage 5. - **Report outcomes, not machinery.** What you show the user is about the review: what's being examined (the PR/branch), which coverage is included and the one-line reason for each conditional lens, the independent cross-model pass and which model runs it, and the findings. Keep the skill's internals out of user-facing text — model-tier assignments, raw scope-mode codenames (`local-aligned`/`pr-remote`), staging the diff to disk, loading persona files, parallel-dispatch bookkeeping, and step-by-step narration of your own setup. Name what the user would recognize (a PR number, a reviewer's concern, a peer model), not the plumbing. This governs *what* you surface and suppress; it does not script the wording — use your own voice. ## Output format | Invocation | Deliverable | |------------|-------------| | **Default** | Markdown report (pipe-delimited finding tables) + Actionable Findings summary | | **`mode:agent`** | One JSON object (see ### JSON output format below) + the same `/tmp/.../ce-code-review//` artifacts | `mode:agent` is **report-only**: it skips the Stage 5c apply (the caller applies) and serializes findings as JSON instead of markdown. It does not change reviewer selection, merge logic, or scope rules — the JSON is the deterministic contract for programmatic and cross-harness callers (Codex, Gemini, etc.). The default markdown is the human view; keep it ASCII-safe (pipe tables, `->` not middot `·`, no box-drawing) so it degrades gracefully across terminals. ## Quick Review Short-Circuit If `$ARGUMENTS` indicates the user wants a quick, fast, or light code review — and **`mode:agent` is not active** — do not dispatch the multi-agent flow. **Announce the chosen path** before any other work (Quick review vs Multi-agent review). Skip this announcement when `mode:agent` is active. Sequence: 1. **Run the harness's built-in code review.** Forward any review target after stripping tokens. Then stop — do not dispatch the multi-agent pipeline. 2. **Exemption:** If no built-in review exists, continue into the full multi-agent review. 3. **`mode:agent` bypasses this short-circuit** — always run the full multi-agent review and return JSON. **Deprecated:** `mode:autofix` is no longer supported — there is no apply *mode*. If passed, ignore the token and proceed with the normal flow (default applies safe fixes via Stage 5c; `mode:agent` reports and the caller applies). ## Severity Scale All reviewers use P0-P3: | Level | Meaning | Action | |-------|---------|--------| | **P0** | Critical breakage, exploitable vulnerability, data loss/corruption | Must fix before merge | | **P1** | High-impact defect likely hit in normal usage, breaking contract | Should fix | | **P2** | Moderate issue with meaningful downside (edge case, perf regression, maintainability trap) | Fix if straightforward | | **P3** | Low-impact, narrow scope, minor improvement | User's discretion | ## Action Routing Severity answers **urgency**. `autofix_class` and `owner` are **signal** describing follow-up shape for callers — **not apply permission or an apply gate.** The apply decision is judgment (Stage 5c), not a function of `autofix_class`: default mode applies; in `mode:agent` this skill does not mutate the checkout — the caller applies. See `references/action-class-rubric.md` for persona guidance. | `autofix_class` | Default owner | Meaning | |-----------------|---------------|---------| | `gated_auto` | `downstream-resolver` or `human` | Concrete `suggested_fix` proposed; caller applies after judgment | | `manual` | `downstream-resolver` or `human` | Actionable work needing design input or handoff | | `advisory` | `human` or `release` | Report-only — learnings, rollout notes, residual risk | Routing rules: - **Synthesis owns the final route.** Persona-provided routing metadata is input, not the last word. - **Choose the more conservative route on disagreement.** A merged finding may move from `gated_auto` to `manual`, but never widen without stronger evidence. - **Reject `safe_auto` and `review-fixer` if present** — drop the finding or remap to `gated_auto` / `downstream-resolver` during synthesis. - **`requires_verification: true` means any caller-applied fix needs targeted tests or follow-up validation.** ## Reviewers 14 reviewer personas in layered conditionals, plus CE local prompt assets. Quick roster with one-line triggers below; the persona catalog included at the bottom has the full per-persona selection criteria and spawn gates. Each selected reviewer is a generic subagent seeded with a local prompt file from `references/personas/`; do not dispatch standalone agents by type/name. **Always-on (full review):** local prompt assets `correctness-reviewer`, `testing-reviewer`, `maintainability-reviewer`, `project-standards-reviewer`, plus CE local prompt assets `agent-native-reviewer` and `learnings-researcher`. (Stage 3c may reduce this set to a lite roster for trivial, low-risk diffs.) **Cross-cutting conditional (per diff):** - `security-reviewer` — auth, public endpoints, user input, permissions - `performance-reviewer` — DB queries, data transforms, caching, async - `api-contract-reviewer` — routes, serializers, type signatures, versioning - `data-migration-reviewer` — migration files / schema dumps / backfills (see spawn gate in Stage 3) - `reliability-reviewer` — error handling, retries, timeouts, background jobs - `adversarial-reviewer` — >=50 changed code lines, or auth / payments / data mutations / external APIs. When selected, a **cross-model adversarial pass** (Stage 4) additionally runs the same brief through a different model family via a peer CLI — additive, non-blocking - `previous-comments-reviewer` — PR with existing review comments (PR-only, comment-gated) **Stack-specific conditional (per diff):** `julik-frontend-races-reviewer` (Stimulus/Turbo, DOM events, async UI) and `swift-ios-reviewer` (Swift/SwiftUI/UIKit, entitlements, Core Data, `.pbxproj`). **CE conditional (migration-specific):** local prompt asset `deployment-verification-agent` — deployment checklist + rollback when the migration gate applies and the change is risky. ## Review Scope A full review spawns generic subagents for all 4 always-on personas plus the 2 CE always-on local prompt assets, then adds whichever cross-cutting and stack-specific conditionals fit the diff (Stage 3c can collapse this to a lite roster for trivial, low-risk diffs). The model naturally right-sizes: a small config change triggers 0 conditionals = 6 reviewers. A Rails auth feature might trigger security + reliability + adversarial = 9 reviewers. ## Protected Artifacts The following paths are compound-engineering pipeline artifacts and must never be flagged for deletion, removal, or gitignore by any reviewer: - `docs/brainstorms/*` -- legacy requirements documents created by older ce-brainstorm versions - `docs/plans/*.{md,html}` -- unified plan artifacts created by ce-brainstorm or ce-plan (decision artifacts; execution progress is derived from git, not stored in plan bodies) - `docs/solutions/*.md` -- solution documents created during the pipeline If a reviewer flags any file in these directories for cleanup or removal, discard that finding during synthesis. ## Plan Requirements Completeness When a plan is provided via `plan:` or discovered from PR/branch context, classify readiness before checking completeness: - Unified artifact: metadata includes `artifact_contract: ce-unified-plan/v1`. - `artifact_readiness: requirements-only` can inform product intent, but it must not trigger implementation-unit completeness findings. Report that the artifact was not implementation-ready if the diff appears to implement it. - `artifact_readiness: implementation-ready` is eligible for full requirements and U-ID completeness checks. - Invalid progress-like readiness values (`active`, `in_progress`, `completed`, `done`) are contract errors. - Legacy plan: use the existing completeness checks. Extract requirements from these shapes, in order: 1. Unified `Product Contract` -> `### Requirements` 2. Legacy top-level `## Requirements` 3. Legacy `## Requirements Trace` For unified implementation-ready plans, also extract U-IDs from `## Implementation Units` and compare against PR body/branch context when available. Do not require every Product Contract R-ID to map one-to-one to a single U-ID; verify that implemented U-IDs cite the relevant R/F/AE/KTD IDs and that no claimed U-ID is missing from the plan. ## How to Run ### Stage 1: Determine scope Compute the diff range, file list, and diff. Minimize permission prompts by combining into as few commands as possible. **If `base:` argument is provided (fast path):** The caller already knows the diff base. Skip all base-branch detection, remote resolution, and merge-base computation. Use the provided value directly: ``` BASE_ARG="{base_arg}" BASE=$(git merge-base HEAD "$BASE_ARG" 2>/dev/null) || BASE="$BASE_ARG" ``` Then produce the same output as the other paths: ``` echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard ``` This path works with any ref — a SHA, `origin/main`, a branch name. Callers reviewing the current checkout should pass explicit `base:` when auto-detection is unnecessary. **Do not combine `base:` with a PR number or branch target.** If both are present, stop with an error: "Cannot use `base:` with a PR number or branch target — `base:` implies the current checkout is already the correct branch. Pass `base:` alone, or pass the target alone and let scope detection resolve the base." **If a PR number or GitHub URL is provided as an argument:** Do **not** check out the PR branch. Scope comes from GitHub read APIs plus optional local alignment when HEAD already matches the PR head branch. **Skip-condition pre-check.** Before scope detection, run a PR-state probe: ``` gh pr view --json state,title,body,files ``` Apply skip rules in order: - `state` is `CLOSED` or `MERGED` -> stop with reason `PR is closed/merged; not reviewing.` - **Trivial-PR judgment**: spawn a lightweight sub-agent on the platform's cheapest capable model when a known override exists; otherwise omit the model override and inherit. Give it the PR title, body, and changed file paths. The agent's task: "Is this an automated or trivial PR that does not warrant a code review? Consider: dependency lock-file or manifest-only bumps, automated release commits, chore version increments with no substantive code changes. When in doubt, answer no — false negatives (skipped reviews that should have run) are more costly than false positives (unnecessary reviews)." If the judgment returns yes: stop with reason `PR appears to be a trivial automated PR; not reviewing. Run without a PR argument to review the current branch, or pass base: if review is intended.` When any skip rule fires, stop without dispatching reviewers. **Default mode:** emit the reason as plain text. **`mode:agent`:** emit JSON only — `{"status":"skipped","reason":""}` — so programmatic callers can parse the outcome. **Standalone**, **`base:`**, and **branch-remote** paths are unaffected. **Draft PRs are reviewed normally.** If no skip rule fires, fetch PR metadata **without checkout**: ``` gh pr view --json title,body,baseRefName,headRefName,headRefOid,isCrossRepository,url,files,reviews,comments --jq '{title, body, baseRefName, headRefName, headRefOid, isCrossRepository, url, files: [.files[].path], hasPriorComments: ((.reviews | map(select(.state != "APPROVED" or .body != "")) | length) > 0 or (.comments | length) > 0)}' ``` Set `BASE:` to `pr:` (logical marker — not a git SHA). Set `UNTRACKED:` from `git ls-files --others --exclude-standard` on the **current** checkout (usually empty during PR-remote review). **PR scope mode.** Classify as **`local-aligned`** only when **all** of these hold; otherwise use **`pr-remote`**. A matching branch name alone is not enough — a fork PR or a stale local branch can share a name with the PR head while pointing at unrelated code, and trusting the name would diff and inspect the wrong tree. 1. `git rev-parse --abbrev-ref HEAD` equals `headRefName`. 2. The PR is **not** cross-repository (`isCrossRepository` is false). 3. The PR head commit is contained in the local checkout: `git merge-base --is-ancestor HEAD` exits 0. This confirms the working tree actually carries the PR head (allowing unpushed local fixes layered on top) rather than an unrelated same-named branch. - **`local-aligned`** — all three checks pass. Local Read/Grep/git blame against workspace files are valid for PR changed paths. - **`pr-remote`** — any check fails. The working tree is **not** the PR head; workspace file contents for changed paths may be stale or unrelated. **Diff by scope mode** (do not mix remote and local diffs — contradictory hunks cause false positives): - **`local-aligned`:** Resolve `` from `baseRefName` (fetch if needed). Compute `BASE=$(git merge-base HEAD )`, then set `FILES:` from `git diff --name-only $BASE` and `DIFF:` from `git diff -U10 $BASE` (includes committed, staged, and unstaged changes on the PR branch). Do **not** call `gh pr diff` or append remote hunks — when unpushed fixes exist, the local tree is canonical. Note in Coverage: `scope: local-aligned (PR; local tree diff)`. - **`pr-remote`:** Set `FILES:` from the PR `files` array. Set `DIFF:` from `gh pr diff --color=never`. If `gh pr diff` fails, stop with an actionable error — do not fall back to checkout. When **`pr-remote`**, before Stage 4: 1. Best-effort fetch PR head without checkout: `git fetch --no-tags origin :refs/review/pr--head` (substitute PR number from metadata). 2. When fetch succeeds, set `PR_HEAD_REF=refs/review/pr--head` for reviewers and validators. When fetch fails, omit `PR_HEAD_REF` and note in Coverage — reviewers must rely on diff hunks only. 3. Best-effort fetch the PR base without checkout: `git fetch --no-tags origin `. When it succeeds, resolve a concrete ref with `git rev-parse FETCH_HEAD` and set `PR_BASE_REF` to that SHA — a **real git base ref** reviewers and validators use for file-level git diffs (e.g. `data-migration-reviewer` runs `git diff -- db/schema.rb`/`structure.sql`). The `pr:` logical marker in `BASE:` stays the scope marker; `PR_BASE_REF` is the diffable base. When the fetch fails, omit `PR_BASE_REF` and note in Coverage — schema-drift and other git-diff checks fall back to diff hunks only and must **not** assume `main`. 4. Include `pr-remote` and, when set, `...` and `...` in the Stage 4 review context bundle. Reviewers and Stage 5b validators in **`pr-remote`** mode must **not** Read/Grep workspace paths for files in `FILES:`. Inspect via `git show :` when `PR_HEAD_REF` is set, otherwise use only the provided diff hunks. **`local-aligned`** uses normal workspace inspection. **If a branch name is provided as an argument:** Substitute the provided branch name as ``. Do **not** check out ``. If `git rev-parse --abbrev-ref HEAD` equals ``, use the **standalone (current branch)** path below — same tree, explicit branch name; do not use remote-only diff. Otherwise diff the remote/local ref **without checkout**: 1. Try `gh pr view --json baseRefName,url,headRefName` — if a PR exists, prefer the **PR number/URL path** above (same remote diff rules). 2. Else resolve `` as `origin/` or `` after `git fetch --no-tags origin ` when needed. 3. Resolve default base branch (same logic as standalone). Compute `BASE=$(git merge-base )` and `git diff -U10 $BASE `. 4. If `` cannot be resolved locally, stop: "Cannot diff branch `` without checkout. Check out that branch, pass its open PR URL/number, or review the current branch with `base:`." On success for remote branch diff, set **branch-remote scope**. The working tree is **not** ``. Include `branch-remote` and `` in the Stage 4 review context bundle. Reviewers and Stage 5b validators must **not** Read/Grep workspace paths for files in `FILES:`. Inspect via `git show :` or diff hunks only. Produce: ``` echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard ``` **If no argument (standalone on current branch):** Apply the same base-detection logic as branch mode above, using the current branch (i.e., `gh pr view --json baseRefName,url` with no argument defaults to the current branch). If no base can be resolved, **stop**. Do not fall back to `git diff HEAD` — a standalone review without the base would only show uncommitted changes and silently miss all committed work on the branch. On success, produce the diff: ``` echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard ``` Using `git diff $BASE` (without `..HEAD`) diffs the merge-base against the working tree, which includes committed, staged, and unstaged changes together. **Untracked file handling:** Always inspect `UNTRACKED:`. Untracked paths are out of scope unless staged. When non-empty, list excluded files in Coverage and continue on tracked changes only — never stop or prompt. ### Stage 1b: Compute scope signals (cheap, deterministic) Derive deterministic signals from the resolved diff once, so reviewer selection (Stage 3) and the small-diff fast path (Stage 3c) do not each re-reason over the whole diff. **These signals only ever shrink the roster via Stage 3c, and that gate fails closed (Stage 3c) — so any failure here (unresolved base, count failure, an uncounted file type) must surface as `UNKNOWN`/non-zero `UNCOUNTED_FILES`, never as a silent `0` that reads as "trivial."** **Set `DIFF_A`/`DIFF_B` to the two endpoints to diff, by Stage 1 scope mode:** - **`local-aligned` / standalone / `base:`** — `DIFF_A="$BASE"` (a real SHA/ref), `DIFF_B` empty (diffs base vs working tree). - **`pr-remote` / `branch-remote`** — `DIFF_A=`, `DIFF_B=` (or ``) — the **fetched** refs from Stage 1. Do **not** model-count from hunks (it drifts per host/model). If either ref was not fetched, skip the block and emit `EXEC_LINES:UNKNOWN` + `UNCOUNTED_FILES:1` so Stage 3c forces the full roster. ``` # Fail closed: an unresolved/invalid endpoint must NOT become a silent EXEC_LINES:0. # Validate BOTH endpoints — a set-but-unfetched DIFF_B (pr-remote/branch-remote head ref) # would otherwise make `git diff` fail, awk print 0, and the lite gate clear on the wrong tree. if [ -z "${DIFF_A:-}" ] || ! git rev-parse --verify --quiet "${DIFF_A}^{commit}" >/dev/null 2>&1 \ || { [ -n "${DIFF_B:-}" ] && ! git rev-parse --verify --quiet "${DIFF_B}^{commit}" >/dev/null 2>&1; }; then echo "EXEC_LINES:UNKNOWN"; echo "UNCOUNTED_FILES:1"; echo "SIGNALS:" else # EXEC_LINES via --numstat (per-file added/deleted counts) summed in awk — robust across # shells, immune to diff-content edge cases. Do NOT grep raw +/- lines: a content line that # itself starts with + or - (markdown bullets, unary minus, `++i`) is miscounted. Keep the # globs single-quoted and inline so the shell passes them to git literally (an unquoted or # variable-held glob would be expanded against the CWD before git ever sees it). EXEC_LINES=$(git diff --numstat "$DIFF_A" $DIFF_B -- '*.rb' '*.py' '*.js' '*.jsx' '*.ts' '*.tsx' '*.go' '*.rs' '*.java' '*.swift' '*.kt' '*.c' '*.cc' '*.cpp' '*.cs' '*.php' '*.ex' '*.exs' '*.scala' 2>/dev/null | awk '{s+=$1+$2} END{print s+0}') echo "EXEC_LINES:$EXEC_LINES" FILES=$(git diff --name-only "$DIFF_A" $DIFF_B 2>/dev/null) # UNCOUNTED_FILES = changed files NOT in the counted code set (skill prose, schemas, configs, # scripts, lockfiles, unknown extensions). Any >0 disqualifies the lite path (fail closed). UNCOUNTED=$(printf '%s\n' "$FILES" | awk 'NF && $0 !~ /\.(rb|py|js|jsx|ts|tsx|go|rs|java|swift|kt|c|cc|cpp|cs|php|ex|exs|scala)$/ {n++} END{print n+0}') echo "UNCOUNTED_FILES:$UNCOUNTED" echo "SIGNALS:" # here-strings (no pipe) — a piped `grep -q` can drop a true match under `set -o pipefail` via SIGPIPE. grep -qiE 'db/migrate/|schema\.(rb|sql)|/migrations?/|alembic|flyway|liquibase' <<< "$FILES" && echo " migrations" || true grep -qiE '\.(tsx|jsx|vue|svelte|css|scss|html|erb|haml)$|/components?/|stimulus|turbo' <<< "$FILES" && echo " frontend" || true grep -qiE '/(routes?|controllers?|api|serializers?|graphql)/|\.proto$|openapi|swagger' <<< "$FILES" && echo " api" || true grep -qiE '\.(swift|kt|pbxproj|xcconfig|entitlements)$' <<< "$FILES" && echo " swift-ios" || true fi ``` `EXEC_LINES` counts changed executable lines (added + removed, counted code extensions only — so a modified line counts as 2; the Stage 3c `<40` threshold is in add+delete units). `EXEC_LINES:UNKNOWN` means the base was unresolved — treat as non-trivial. `UNCOUNTED_FILES` is the count of changed files outside the code set (skill `.md`, JSON schemas, `.sh`, config, CI, lockfiles, unknown extensions) — **this plugin's own product surface is mostly uncounted, which is exactly why Stage 3c must fail closed on it.** The `SIGNALS` list is **path heuristics, not selection decisions**: Stage 3 still applies judgment and adds the matching conditional persona only when the runtime concern is real. Content-based risk (auth, payments, data mutation) is **not** path-derivable — read it from the diff in Stage 3 as before; it also disqualifies the Stage 3c fast path regardless of line count. ### Stage 2: Intent discovery Understand what the change is trying to accomplish. The source of intent depends on which Stage 1 path was taken: **PR/URL mode:** Use the PR title, body, and linked issues from `gh pr view` metadata. Supplement with commit messages from the PR if the body is sparse. **Branch mode:** Run `git log --oneline ${BASE}..` using the resolved merge-base and resolved branch ref from Stage 1. Use `` (the resolved `origin/` or fetched ref), not the raw `` argument — a remote-only branch has no matching local ref, so the raw name would fail or read a stale same-named local branch. **Standalone (current branch):** Run: ``` echo "BRANCH:" && git rev-parse --abbrev-ref HEAD && echo "COMMITS:" && git log --oneline ${BASE}..HEAD ``` Combined with conversation context (plan section summary, PR description), write a 2-3 line intent summary: ``` Intent: Simplify tax calculation by replacing the multi-tier rate lookup with a flat-rate computation. Must not regress edge cases in tax-exempt handling. ``` Pass this to every reviewer in their spawn prompt. Intent shapes *how hard each reviewer looks*, not which reviewers are selected. **When intent is ambiguous:** Infer from branch name, commits, PR title/body, diff, `plan:`, and conversation. Write the best-effort intent summary and note uncertainty in Coverage — never block on a clarifying question. ### Stage 2b: Plan discovery (requirements verification) Locate the plan document so Stage 6 can verify requirements completeness. Check these sources in priority order — stop at the first hit: 1. **`plan:` argument.** If the caller passed a plan path, use it directly. Read the file to confirm it exists. 2. **PR body.** If PR metadata was fetched in Stage 1, scan the body for paths matching `docs/plans/*.{md,html}` (unified plans may be markdown or HTML). If exactly one match is found and the file exists, use it as `plan_source: explicit`. If multiple plan paths appear, treat as ambiguous — demote to `plan_source: inferred` for the most recent match that exists on disk, or skip if none exist or none clearly relate to the PR title/intent. Always verify the selected file exists before using it — stale or copied plan links in PR descriptions are common. 3. **Auto-discover.** Extract 2-3 keywords from the branch name (e.g., `feat/onboarding-skill` -> `onboarding`, `skill`). Glob `docs/plans/*` and filter filenames containing those keywords. If exactly one match, use it. If multiple matches or the match looks ambiguous (e.g., generic keywords like `review`, `fix`, `update` that could hit many plans), **skip auto-discovery** — a wrong plan is worse than no plan. If zero matches, skip. **Confidence tagging:** Record how the plan was found: - `plan:` argument -> `plan_source: explicit` (high confidence) - Single unambiguous PR body match -> `plan_source: explicit` (high confidence) - Multiple/ambiguous PR body matches -> `plan_source: inferred` (lower confidence) - Auto-discover with single unambiguous match -> `plan_source: inferred` (lower confidence) If a plan is found, classify readiness before extraction (see "Plan Requirements Completeness" above): for a unified plan read the metadata/header first, and treat a requirements-only artifact as product intent only — it must not drive implementation-unit completeness findings. Then read its **Requirements** in this order — unified `Product Contract` -> `### Requirements`, then legacy top-level `## Requirements`, then legacy `## Requirements Trace` — and the R-IDs (R1, R2, etc.) listed there, plus **Implementation Units** (current numeric subsections such as `### U1.`, `### U2.`, or `### Unit 1:` under `## Implementation Units`; legacy bullet or checkbox unit entries under that section also count). For HTML unified plans the same section names and R-/U-IDs appear as visible headings/anchors — match on the section name, ignoring HTML wrapper tags. Store the extracted requirements list and `plan_source` for Stage 6. Do not block the review if no plan is found — requirements verification is additive, not required. ### Stage 3: Select reviewers Read the diff and file list from Stage 1, and the `SIGNALS` / `EXEC_LINES` from Stage 1b. The 4 always-on personas and 2 CE always-on agents are automatic. For each cross-cutting and stack-specific conditional persona in the persona catalog included below, decide whether the diff warrants it. This is agent judgment, not keyword matching — a `SIGNALS` hit (`migrations`, `frontend`, `api`, `swift-ios`) is a *prompt* to consider the matching persona, not an instruction to spawn it; confirm the runtime concern is real in the diff before adding it, and add content-gated personas (`security`, `reliability`, `adversarial`) from the diff as before since those are not path-derivable. **File-type awareness for conditional selection:** Instruction-prose files (Markdown skill definitions, JSON schemas, config files) are product code but do not benefit from runtime-focused reviewers. The adversarial reviewer's techniques (race conditions, cascade failures, abuse cases) target executable code behavior. For diffs that only change instruction-prose files, skip adversarial unless the prose describes auth, payment, or data-mutation behavior. Count only executable code lines toward line-count thresholds. **`previous-comments` is PR-only AND comment-gated.** Only select this persona when both conditions hold: 1. Stage 1 gathered PR metadata (PR number or URL was provided as an argument, or `gh pr view` returned metadata for the current branch). 2. `hasPriorComments` from Stage 1 is true (the PR has at least one review submission or issue comment). Skip it for standalone branch reviews with no associated PR, and skip it for PRs with no prior feedback yet -- there is nothing for the persona to verify, and a spawned subagent that returns empty findings still costs the full subagent startup overhead (persona spec, diff, schema, plus its own gh calls). Stack-specific personas are additive when runtime behavior warrants them. A Hotwire UI change may warrant `julik-frontend-races`; a TypeScript API diff may warrant `api-contract` and `reliability`. **`data-migration` spawn gate.** Select `data-migration-reviewer` only when the diff includes at least one migration or schema artifact: `db/migrate/*`, `db/schema.rb`, `db/structure.sql`, Alembic/Flyway/Liquibase migration paths, or explicit backfill/data-transform scripts (rake tasks, one-off data migration classes). **Do not spawn** for model-only changes, query-only refactors, serializers/controllers that reference columns without a migration or schema dump in the diff, or migration tests alone. For `deployment-verification-agent`, use the same migration-artifact gate when the change is risky (destructive DDL, backfills, NOT NULL without default, column renames/drops). Announce the team before spawning, as a user-facing summary: name the always-on reviewers plainly, and for each conditional reviewer give the one-line reason it was added (the real concern, not the keyword that matched). Do **not** put model-tier labels (`[session model]`/`[mid-tier]`) or scope-mode codenames in this announce — those are internal. Still *decide* each reviewer's tier here and keep it in your own working notes (Stage 4 applies it at dispatch as a correctness guarantee); just keep it out of this user-facing summary. If the cross-model adversarial pass will run (adversarial selected + `local-aligned`/standalone scope), resolve its peer now via the cross-model reference's host/preflight steps and surface it **as its own prominent line that names the peer** (the peer CLI, plus its model if cheaply known) — the headline is that a second, genuinely independent model is also reviewing. Keep it with the team, not buried after it, and honor that reference's host gating (interactive hosts only). If it won't run, omit it. Illustrative shape only — match your own voice, do not copy verbatim: ``` Reviewing PR #1234 (against main) Also running an independent adversarial pass via a different model: Codex. Reviewers: - correctness, testing, maintainability, project-standards, agent-native, learnings (always) - security — new endpoint accepts a user-provided redirect URL - data-migration — adds migration 20260303_add_index_to_orders ``` This is progress reporting, not a blocking confirmation. ### Stage 3b: Discover project standards paths Before spawning sub-agents, find the file paths (not contents) of all relevant standards files for the `project-standards` persona. Use the native file-search/glob tool to locate: 1. Use the native file-search tool (e.g., Glob in Claude Code) to find all `**/CLAUDE.md` and `**/AGENTS.md` in the repo. 2. Filter to those whose directory is an ancestor of at least one changed file. A standards file governs all files below it (e.g., `AGENTS.md` at the repo root applies to the whole checkout, while `skills/AGENTS.md` would apply to everything under `skills/`). Pass the resulting path list to the `project-standards` persona inside a `` block in its review context (see Stage 4). The persona reads the files itself, targeting only the sections relevant to the changed file types. This keeps the orchestrator's work cheap (path discovery only) and avoids bloating the subagent prompt with content the reviewer may not fully need. ### Stage 3c: Small-diff fast path (reduce the roster for trivial, low-risk diffs) **`depth:full` hard-disables this gate** — when that token was passed, skip Stage 3c entirely and run the full roster (the caller explicitly asked for a deep review; size no longer matters). **This gate fails closed: it only ever fires for a positive count of low-risk application code, and every uncertainty resolves to the full roster.** Collapse to a lite roster only when **all** of these hold: - `EXEC_LINES` from Stage 1b is a **number** (not `UNKNOWN`) and **between 1 and 39**, AND - `UNCOUNTED_FILES` from Stage 1b is **0** — i.e. *every* changed file is counted application code. Any uncounted file (skill `.md`, JSON schema, `.sh`/CI/config, lockfile, unknown extension) disqualifies the lite path. This guard is load-bearing on **mixed** diffs the line count alone would pass: 15 exec lines of application code plus two `.md` skill files reads `EXEC_LINES:15` with `UNCOUNTED_FILES:2`, and the uncounted files force the **full** roster. (This plugin's own product surface — `skills/**`, `SKILL.md`, `references/**`, schemas — is uncounted.) AND - Stage 1b `SIGNALS` is **empty** (no `migrations`, `frontend`, `api`, or `swift-ios`), AND - No content-based risk read from the diff in Stage 3 (auth, payments, data mutation, external API, secrets/permissions, deserialization, crypto, concurrency/background jobs, filesystem/process execution), AND - No conditional persona was selected in Stage 3. `EXEC_LINES:UNKNOWN` (unresolved base) or `UNCOUNTED_FILES > 0` are **hard disqualifiers** — never lite. A pure code diff that also touches one `.md` runs the full roster; that conservatism is the point. **Lite roster:** the inline fast pass (Stage 4) plus `correctness-reviewer` and `project-standards-reviewer` only — skip `testing`, `maintainability`, `agent-native`, and `learnings`. Announce the reduction plainly (e.g. "Small diff (28 exec lines, code-only, no risk signals) — running a lite review: fast pass + correctness + standards.") and note it in Coverage. **Do not collapse** when any gate condition fails — the gate keys on risk, not size alone (a 12-line auth change still needs the full roster). When in doubt (signals ambiguous, risk unclear, count `UNKNOWN`), run the full roster. ### Stage 4: Spawn sub-agents #### Inline fast pass (emit before dispatch) To surface findings in seconds, **in the same turn that fires the parallel dispatch** the orchestrator does a quick first-principles scan of the diff it already holds — emit the fast-pass block as text, then make the Agent calls, so it adds no wall-clock to the fan-out. Scan only for **high-signal, obvious** issues a careful first read catches: data/SQL safety, injection (shell/SQL/LLM-output trust boundary), broken control flow, a missing `await`/unhandled promise, a swapped argument or off-by-one, an enum/status added without updating its sibling switch, a null deref on a value the diff makes reachable. Do **not** do deep analysis, read beyond the diff (except a quick Grep for enum completeness), or chase subtle concerns. Quote the verbatim motivating line for each, same bar as a persona finding. Present these under a clearly preliminary header (e.g. `### Fast pass (preliminary — deep review in progress)`) as a short list of `severity — file:line — what`, with one line stating they are unverified and will be deduplicated into the final report. Do **not** assign stable `#` numbers here. The fast pass enters Stage 5 as a pseudo-reviewer named `fast-pass`, with two hard constraints because it is the orchestrator's own read, **not** an independent reviewer (it s …(truncated)