v2.18.6 to v2.19.0

1 added, 1 removed. Audit A to A.

---

name: codereview

metadata:

- version: 2.18.6

+ version: 2.19.0

description: Pre-PR review with severity grading and tiered model routing. Detects TOCTOU races, accessibility gaps, hardcoded secrets, silent-blinding sensors (swallowed errors, negative verdicts, gates aimed at the wrong file), docs drift, and dead code via a whole-repo sweep. Report carries an Overall Grade table + Recommended Actions. Stack-agnostic, TypeScript/React defaults. Triggers — code review, pre-PR, secrets scan, accessibility audit, dead code, silent failure, code health.

---

## User Input

```text

$ARGUMENTS

```

Read the user input before proceeding (if not empty). Valid inputs:

- Empty: full review of all changed files

- Focus area: `security`, `performance`, `types`, `bugs`, `tests`, `docs`, `a11y`, `race-conditions`, `dead-code`

- File path or glob: review only matching changed files

- Key-value overrides: `baseDir=app/ fileExtensions=ts,js`

- Focus in prose ("look at the separator") or a review handoff's risk list (Phase A): run `full`, prompts

unchanged; answer each named point in Phase C and say where it was checked

Defaults are `baseDir=src/`, `fileExtensions=ts,tsx`, `frameworkPatterns=react`, tests `**/*.{test,spec}.{ts,tsx}` and `**/test/**`, UI_LIB `src/components/ui/**`, `prisma/**`, `**/generated/**`, CONFIG `*.config.*`, `tsconfig*`, `.env*`, `package.json`.

## Goal

Perform a comprehensive, automated code review of all changes in the current branch compared to the base branch. Produce a structured Markdown report with severity-rated findings, test coverage assessment, and a final grade.

This skill is **stack-agnostic**. Defaults target TypeScript/React but all values are configurable.

## References

The agents read their own contract by absolute path (`{SKILL_DIR}/references/…`); the orchestrator

reads a file only at the step that needs it:

- `references/configuration.md` — only when `$ARGUMENTS` carries `key=value` overrides or names

more than one repo, the folder has no repo, the stack is not TypeScript/React or the focus

names code in a doc.

- `references/per-file-agent.md` — read it yourself only if Phase B runs inline (≤3 CODE files):

the Phase B agent contract (batch loading, scope, focus mapping, return format).

- `references/sweep-agent.md` — read it yourself when the sweep runs inline or the agent under-reports

and you redo the deepsearch: the Phase B2 contract (the two buckets, guardrails, return template).

- `references/detection-passes.md` — read it yourself when a phase runs inline, and in Phase C for the

recalibration rules and the pass 6.10 remediation block (the agents load it on their own): every

pass (Zen 5.x, 6.1–6.12) and its severity rules.

- `references/toctou-patterns.md` — the TOCTOU catalog with code examples. Agents load it only for

check-then-act shapes; read it in Phase C when a race spans files (check in one, act in another).

- `references/report-template.md` — read at Phase C step 10, before writing the report.

---

## Model Routing Strategy

|-------|------|---------|-----|

**≤3 CODE files** → skip routing and run Phase B inline too; the agent overhead isn't worth it.

**The `model` field makes the routing real.** An Agent call without it inherits the main model and bills the same per-file analysis at 2.5× to 5× Sonnet's rate. Pass `model: "sonnet"` on every Phase B and B2 call, and read the model back in the report's Cost footprint line.

---

## Operating Constraints

**Read-only.** This skill identifies issues and suggests fixes in the report; it does not apply them. Don't modify, create, or delete files, and don't run destructive commands — everything it runs (git, grep, the secrets script, dead-code tooling) is a pure read. The only output is the structured report in the conversation.

## Error Handling

- **Git command failures**: include the exact failing command and stop immediately. A folder with

no repo at all: configuration.md, *Folder without git*.

- **File read failures**: skip the file and record it as `Could not analyze: {filename} ({reason})`.

- **Context / token exhaustion**: finish analyzing files already processed, note truncation, proceed to report.

- **Timeout**: prioritize CRITICAL/HIGH checks on remaining files, skip MEDIUM/LOW.

Regardless of failures, always produce a final report listing all files analyzed and all failures.

---

## Execution Phases

### Phase A: Git Context, File Classification & Secrets Pre-Scan (inline, main session)

Every step is a fixed command with one right answer, so it runs inline, not in an agent: an agent only adds variation, latency and the chance of a silently dropped field — and without the secrets pre-scan JSON the F-grade gate goes blind. Outputs are small.

Apply any `$ARGUMENTS` overrides before classifying, and keep the raw outputs. Three Bash turns cover steps 1–8 — (1) steps 1–3, base-branch detection as one fallback chain, plus this skill's version for the Cost footprint (`sed -n 's/^ version: //p' {SKILL_DIR}/SKILL.md`), `git ls-files '*handoff*'` and, from a plugin cache, the `installPath` in `~/.claude/plugins/installed_plugins.json`: a `{SKILL_DIR}` outside it is a stale copy if `diff -rq` differs (ask for `/reload-plugins`); (2) step 4; (3) steps 5–8 as parallel calls in one message — because every extra orchestrator turn is a main-model round-trip over the whole session context:

1. Verify git repo: `git rev-parse --is-inside-work-tree`

2. Detect base branch (try: origin HEAD symbolic-ref, then main, then master)

3. Current branch: `git rev-parse --abbrev-ref HEAD`

4. Merge base: `git merge-base {BASE_BRANCH} HEAD`, or the base commit the user names (`base=<sha>`, even in prose); an end other than `HEAD` is `head=<sha>` (configuration.md)

5. Changed files: `git diff {MERGE_BASE}...HEAD --name-only`

6. Diff stats: `git diff {MERGE_BASE}...HEAD --stat`

7. Commit log: `git log {MERGE_BASE}..HEAD --oneline --no-decorate`

8. Secrets pre-scan — runs on every review, whatever its size or focus:

`git diff {MERGE_BASE}...HEAD --unified=0 | bash {SKILL_DIR}/scripts/scan_secrets.sh`

(`{SKILL_DIR}` = absolute path of this SKILL.md's directory). It prints JSON `{findings:[...], scanners:[...], errors:[...]}`; keep it verbatim as `SECRETS_PRESCAN`, the authoritative source for Phase C's Secrets Detection table and F-grade gate. A crash or non-JSON output means the scan did not run: warn the user and re-run it — an absent payload is never "scan returned clean".

Classify each changed file:

- EXCLUDED: lock files, node_modules, dist, build, .next, min files, binaries, .claude/ (unless the user names it; rules/ is DOCS)

- CODE: source files matching {fileExtensions} in {baseDir}, excluding tests and generated; an extensionless executable counts when its shebang runs one of those languages (`bin/tool`, `#!/usr/bin/env bash`)

- UI_LIB: files in {generatedDirs}

- TESTS: files matching {testFilePatterns}

- CONFIG: files matching {configFilePatterns}

- DOCS: *.md, *.txt

- STYLES: CSS/SCSS/LESS

For each CODE file, check test coverage by probing candidate test file paths — same dir (`{Base}.test.{ext}`, `{Base}.spec.{ext}`), a `__tests__` sibling, then the project test root, then a changed test that imports it — and record it as WITH_TESTS / STALE_TESTS / NO_TESTS. Probe all CODE files in one shell loop (one Bash call that prints `path|status` per file), not one call per file, in bash with `nullglob`: zsh aborts an unmatched glob.

Phase A hands Phases B and C: BASE_BRANCH, BRANCH_NAME, MERGE_BASE, DIFF_STAT, COMMIT_LOG, the FILES list (path, category, test_status), COUNTS per category, and SECRETS_PRESCAN.

If CHANGED_FILES empty, dirty `git status --porcelain` → `worktree` mode (`references/configuration.md`). Both empty → output "No changes detected between this branch and `{BASE_BRANCH}`." and stop — unless the user scoped paths: then empty is a pathspec error (read configuration.md §Path-scoped reviews).

More than 15 CODE files: order the groups by change size (diff stat lines); every file still gets an agent, none is cut.

### Phase B: Per-File Analysis (sonnet agents, parallel)

For each CODE file (or group of 2-3 small files sharing imports) — and each changed TESTS file on a test-quality focus (6.12) — **spawn a sonnet agent** to analyze it. Launch all agents **in parallel**, in one message.

Each agent reads its instructions itself, when it starts, from `{SKILL_DIR}/references/per-file-agent.md`, so every agent gets the same contract. Emit only the launch prompt below, placeholders filled — nothing added (no themes, framing or reproduction requests: those come back marked `needs reproduction`), nothing removed. `model: "sonnet"` on every call.

```

Agent(model: "sonnet", prompt: "

Start with ONE batch of parallel tool calls: Read {SKILL_DIR}/references/per-file-agent.md,

Read {SKILL_DIR}/references/detection-passes.md, run `git diff {MERGE_BASE}...HEAD -- {FILE_PATH}`,

and Read {FILE_PATH}. Then follow per-file-agent.md exactly — it carries your instructions,

scope and output format.

- Repository: {REPO}

- Branch: {BRANCH_NAME} → {BASE_BRANCH}

- Merge base: {MERGE_BASE}

- Framework: {frameworkPatterns}

- File: {FILE_PATH} (category: {CATEGORY}) — one line per file in the group

- Focus area: {FOCUS or 'full'}

- Skill dir: {SKILL_DIR}

- Hard rules: {CLAUDE.md prohibitions, verbatim, or none}

```

The agent returns a numbered findings list (`N. [SEVERITY] {category} — {file}:{line} — {title}`, with Description and Suggestion), or `No findings for {FILE_PATH}`, and ends with `Tool calls: … | Files read in full: …` and `END_OF_FILE_REVIEW`.

**Grouping**: files that import from each other share an agent when possible (max 3), which catches intra-group issues without the main model. A **TOCTOU** race spanning files (service reads, controller acts) is flagged single-file with "cross-file verification needed"; Phase C judges it.

### Phase B2: Dead Code Sweep (sonnet agent, parallel)

Spawn **one dedicated agent** for pass 6.9 (Dead Code & Unused Symbols), **in the same parallel batch** as the Phase B agents. It is separate because dead code is a **whole-repo reference-graph** question: a per-file agent sees one file and cannot tell whether an exported symbol is referenced elsewhere. This agent gets the changed-file list, the diff, and grep/tooling access to the whole repo.

**When to run it:**

- **Full review** (empty `$ARGUMENTS`) → run it, with Bucket A only — what this PR introduced or orphaned.

- **Bucket B is opt-in**: the repo-wide pass over pre-existing dead code (`knip`/`ts-prune`/`vulture`…, capped) runs only on focus `dead-code` or `sweep=full`. It was the expensive part of the sweep and none of it belongs to the PR; the report says in one line how to get it.

- Focus `dead-code` → run it and skip the per-file passes (the only analysis). Focus `bugs` → run it (dead code often masks or accompanies bugs).

- Narrow focuses (`security`, `a11y`, `types`, `performance`, `docs`, `tests`, `race-conditions`) → **skip it.** Unlike 6.10, dead code is hygiene, not a gate; in a focused security review it is noise.

- **≤3 CODE files** (routing skipped) → run the sweep **inline in the main model**.

**Output discipline**: the orchestrator sees only the agent's **final assistant message**, never its grep/tool outputs — so that message is the filled return template from `sweep-agent.md` (ending in `END_OF_DEAD_CODE_SWEEP`), not "done". Launch prompt, placeholders filled:

```

Agent(model: "sonnet", prompt: "

Start with ONE batch of parallel tool calls: Read {SKILL_DIR}/references/sweep-agent.md,

Read {SKILL_DIR}/references/detection-passes.md (pass 6.9), and run `git diff {MERGE_BASE}...HEAD`.

Then follow sweep-agent.md exactly — it carries the two buckets, the guardrails and the

return template.

- Repository root: {REPO}

- Branch: {BRANCH_NAME} → {BASE_BRANCH}

- Merge base: {MERGE_BASE}

- Framework: {frameworkPatterns}

- Changed CODE/CONFIG files: {LIST_OF_CHANGED_FILES}

- Focus area: {FOCUS or 'full'}

- Sweep: {full | pr} — `full` only when `$ARGUMENTS` carries `sweep=full`

- Skill dir: {SKILL_DIR}

- Hard rules: {CLAUDE.md prohibitions, verbatim, or none}

```

Under-report (no `END_OF_DEAD_CODE_SWEEP`, or a bare status sentence) → re-run the grep deepsearch inline for the changed files; unlike the secrets gate, a missing dead-code result is **non-blocking** — note "dead-code sweep incomplete" and proceed.

### Phase C: Cross-File Review & Final Report (main model)

After all sonnet agents return, the main model:

1. **Collects all findings** from sonnet agents into a unified list. A per-file message without its `END_OF_FILE_REVIEW` line is partial: keep what it reports and mark the file `partially analyzed` in the report

2. **Merges the Phase A secrets pre-scan with pass-6.10 findings from the per-file agents.**

- `secrets_prescan.findings` is **authoritative** — every entry is real (regex matched + exception filter applied) and goes straight into the Secrets Detection table.

- Agent 6.10 findings are **supplemental** (context the regex missed, e.g. a custom DSL with a non-standard keyword). One NOT already in `secrets_prescan` (by `{file, line, kind}`) enters the table only if (a) it shows a concrete literal credential, not a category like "potential leak", AND (b) it matches a 6.10 category or a clear equivalent; otherwise drop it as low-signal speculation.

- Dedup remaining entries by `{file, line, kind}`; on collision, keep the higher severity and prefer `source=ggshield` > `gitleaks` > `regex` > `sonnet` for provenance.

3. **Cross-file analysis** — only the main model has the whole picture: races spanning files (check in controller, act in service), schema consistency across related endpoints, import-chain coherence (producer and consumer types match). Code that already runs (cron, service): read its latest outputs (configuration.md §Runtime evidence). Add what you find to the list.

4. **Severity recalibration** — review each finding's severity:

- Per-file agents may over-flag memoization issues (React.memo, useCallback) — downgrade per the rules in detection-passes.md

- Ambiguous TOCTOU patterns in single-user contexts — downgrade to LOW

- Patterns that are actually project conventions (check CLAUDE.md) — remove or downgrade

- **Pass 6.11 (Silent-Blinding Sensors) findings never rise above HIGH and never touch the grade gate.** Downgrade to LOW, or drop, anything where the swallowed error has observable fallback behavior — the pass is about blind spots, not about `catch` syntax. A per-file claim that a future drift would blind a site was made without the tests: find the test that pins it before keeping the severity (detection-passes.md 6.11).

- **Pass 6.10 (Secrets) findings are NEVER downgraded to MEDIUM/LOW and NEVER removed.** The only allowed recalibration is CRITICAL ↔ HIGH per the test-file nuance in detection-passes.md (inline test literals are HIGH; prod code is CRITICAL; env-var lookups are not flagged at all).

5. **Deduplication** — remove findings that overlap or repeat the same root cause (does not apply to pass 6.10 — each occurrence is reported, then aggregated if ≥3 in one file or ≥5 across PR).

6. **Test coverage summary** — compile from Phase A results

7. **Documentation sync check** — verify docs files in CHANGED_FILES per 6.5.2 rules

8. **Secrets gate** — if the merged Secrets Detection list has **≥1 entry from `secrets_prescan` OR ≥1 entry from sonnet that survived the supplemental filter in step 2**:

- Set overall grade to **F** regardless of any other signal.

- Prepend a BLOCKED banner to the report (see report template).

- Add an entry under "Must Fix (CRITICAL)" per file with the remediation block from detection-passes.md pass 6.10.

- If `secrets_prescan.errors` is non-empty (script crashed, ggshield timed out), also surface a warning to the user — the gate may have under-reported.

9. **Merge dead-code findings** from B2 (Bucket A = introduced/orphaned by this PR; Bucket B = pre-existing, capped, only on `dead-code` focus or `sweep=full`):

- **MEDIUM/LOW only** — never HIGH/CRITICAL, never the secrets gate, never a forced F.

- Honor the per-finding **Confidence**: drop or footnote Low-confidence items the guardrails couldn't clear (an unreferenced library export — external consumers are invisible to repo grep).

- Bucket B stays a **capped summary** labeled "pre-existing (not introduced by this PR)", so it doesn't drown Bucket A; `BUCKET_B: skipped` → the template's one-line note instead of a table.

- Sweep skipped (narrow focus) or under-reported → say so in the Dead Code section, don't omit it.

10. **Produce the final report** — before writing, read `references/report-template.md`; order:

BLOCKED banner (only if step 8 triggered) · **Secrets Detection table** (always; "Status: PASS" with

0 rows when clean) · findings table (CRITICAL > HIGH > MEDIUM > LOW, grouped by file) · Zen

Principles summary · Bug/Security/Performance/Types summary · test coverage table · documentation

sync table · **🧹 Dead Code & Cleanup** (when the sweep ran — Bucket A primary, Bucket B as a capped

pre-existing summary when on; dead-code findings also feed Recommended Actions → Consider Fixing

and the Code Quality rationale) · **Overall Grade table** · **Recommended Actions** (every bucket,

`_None._` when empty) · **Cost footprint** line, last.

**The Overall Grade table, the Recommended Actions block and the Cost footprint close every report**,

in full even with little to say: the template's fallbacks, never prose in place of the table.

### Special Cases

- **Zero findings**: Output a congratulatory report. Grade A. Still show header, test coverage, and grade.

- **Focus area specified**: Only the matching detection passes were applied by sonnet agents. Mark non-analyzed sections as "Not analyzed (focused review on {area})".

- **File path/glob specified**: Only matching files were analyzed. Report shows only those files.

- **UI_LIB files**: Sonnet agents only flagged CRITICAL/HIGH. Note "(UI_LIB — reduced rigor)" in findings.

- **More than 50 findings**: Show all CRITICAL/HIGH/MEDIUM first, then LOW up to 50 total; the rest one line each below it (report-template.md).

---

## Operating Principles and Gotchas

- **Context efficiency**: agents hold file content and diffs, so the main model sees only findings.

- **Measure, don't guess** — compare the report's Cost footprint line (report-template.md) with

`/cost` (or the harness's per-session cost log) before and after any change to this skill; a change

without that pair of numbers is a guess.

- **Re-review the fix, not the branch** — after fixing a report, the next round runs with

`base=<head the last round reviewed>` and stops at the first round that finds nothing; a fix written

as prose is the likeliest home of the next finding (measured 11 → 5 → 3 → 0).

- **Line numbers come from the diff or file actually read** — a line the reader can't find discredits

the whole report.

- **A clean report is a valid outcome** — if the code is clean, say so rather than inventing findings.

- **Never whitelist a secret finding to reduce noise** — test-file passwords count like production

ones; GitGuardian agrees. A false-positive re-read costs far less than a leaked credential.

- **Ground findings in evidence** — quote the problematic snippet when helpful; for secrets, mask the

value with `***` (the report itself is copied into chat history).