Base: <baseRefName> ← <headRefName> Additions/Deletions: +<additions> -<deletions> Description: <body, first 500 chars> ## Changed Files <files list> ## PR Diff <full diff> ## Existing Review Comments (inline) <inline comments from /pulls/N/comments, if any> ## Existing PR Discussion (issue comments) <discussion comments from /issues/N/comments, if any> ``` Skip either comments section if empty. The point of collecting both is to avoid duplicating findings already raised by humans — whether inline or in the discussion thread. ### Working-tree mode ``` git status --short git diff --cached --no-ext-diff --submodule=diff git diff --no-ext-diff --submodule=diff git ls-files --others --exclude-standard git log --oneline -10 ``` For each untracked file: skip binary, skip >24KB, otherwise read and include content. ### Branch mode ``` git merge-base HEAD <base-ref> git log --oneline --decorate <merge-base>..HEAD git diff --stat <merge-base>..HEAD git diff --no-ext-diff --submodule=diff <merge-base>..HEAD ``` If `git merge-base` fails (common in shallow clones / CI), emit the error output with error code `shallow_clone_no_base` and instruct: "Run `git fetch --unshallow` or use `--scope working-tree` / explicit `--base <ref>`." ### Empty diff handling If the resolved diff is empty (no staged, unstaged, untracked, or branch-divergent changes), emit the error output with error code `empty_diff` and verdict `null`. Do NOT return `approve` — an empty review is not an approval. --- ## Step 3b — Patch-chain scan After collecting the diff but before the large-diff guard, scan recent commit history for patterns that indicate iterative patching on the same surface. Multi-round defensive commits on the same file set are a signal that candidate findings in this review may be artifacts of prior rounds' guards rather than organic defects — and the correct next step is then a structural refactor, not round N+1 of guard-chasing. The interpretation rule and severity implications live in the "Patch-chain detection" section of `output-schema.md` (loaded at Step 7); this step is the data-collection side, and the theme-vs-root guard below is all the interpretation needed before emit. ### Collect the commit history Run: ``` git log -<N> --oneline -- <changed-files> ``` Where `<N>` is `5` for working-tree and branch modes, `10` for PR mode (PRs accumulate more commits than typical local changes). `<changed-files>` is the set of files already identified in Step 3's diff. ### Signals — at least one must fire to populate `patch_chain_risk` 1. **Fix-prefix cluster.** Among the last 4 commits that touch any reviewed file, ≥50% (i.e., ≥2 of 4) have messages prefixed with any of: `fix:`, `guard:`, `prevent:`, `patch:`, `workaround:`, `hotfix:` (case-insensitive; conventional-commits scope suffix like `fix(auth):` still counts). The cluster is "same surface, repeatedly defensive". 2. **Same-file hotspot.** A single reviewed file appears in ≥3 of the last 5 commits (working-tree / branch mode) or ≥5 of the last 10 commits (PR mode). File frequency alone is not enough without a defensive-prefix cluster, but combined with signal 1 it strengthens the signal — record both when both fire. 3. **Prior-review overlap** (auto-detected — no flag required). Compute the **target slug** for the current review (see Step 8 for the slug rules) and resolve the prior-review path to `.claude/devil-review/${CLAUDE_SESSION_ID}/<target-slug>.md`. Session and target scoping ensure that stale reviews from unrelated sessions or different targets never bleed in. If the file exists, load it; extract its findings array and `considered_not_promoted` array via its JSON fence (treat the load as absent if no `schema_version` field is present, the file is malformed, or the file does not exist — emit the corresponding status per the observability rule below). If ≥50% of the current review's candidate findings reference file locations that also appeared in the prior review's findings or `considered_not_promoted`, the signal fires. Also cross-reference each current finding's `file:line` against the prior review's entries and annotate any overlaps in the finding body: "This location also appeared in the prior review as finding #N" — this annotation is body-only, not a new schema field. **Observability requirement.** The skill **must always** emit one `scenarios_considered` line of the form `context: prior=<status> rejections=<status> rules=<n>` on every non-error run (schema v2.0 — this single line replaces the separate `prior-review ingestion:` and `rejection memory:` lines). `prior=` is exactly one of `loaded`, `absent` (file does not exist — fresh run), `rejected-no-schema-version`, or `rejected-malformed-json`; `rejections=` is exactly one of `loaded`, `absent`, or `rejected-malformed-json` (per `rejection-memory.md`); `rules=` is the count of project rule files loaded in Step 5.2b. This line makes the auto-detect outcomes visible; silent drops are not permitted. ### Prior-relation classification (schema v1.11+) When Step 3b's `<status>` is `loaded`, the loaded prior review's findings feed three **emit-time obligations**: per-finding `prior_relation` attribution (three categories per the plugin v1.15.1 correction — `resolved` is not a finding-level value), the `trace_log.prior_review_summary` roll-up, and severity dampening for `carries-over` findings. Rules live in the "Prior-relation classification" and "Severity dampening for carries-over findings" sections of `output-schema.md`; its Pre-emit checklist enforces them at Step 7. When `<status>` is `absent` or any `rejected-*` value, omit `prior_relation` on all findings and omit `trace_log.prior_review_summary` entirely. ### Rejection memory load (schema v1.14+) Rejection memory lets the reviewer avoid silently re-raising findings the user has already dismissed via `--reject`. The full mechanics — hash normalization (authoritative), `--reject` recording, file load, suppression vs. re-raise, and the chain-of-rejections verdict override — live in **`rejection-memory.md`** (sibling file in this skill directory). Load it now. Execute its **Phase A** (substeps 1–3) at this step: record any `--reject <CSV>` entries from Step 1 into `.claude/devil-review/${CLAUDE_SESSION_ID}/rejections.json`, then load the file into `trace_log.rejections_loaded` (present only when the file exists — schema v2.0). Its **Phase B** (substeps 4–6: per-candidate suppression check, suppress-vs-re-raise judgment, chain-of-rejections override) runs later — after the Claim verification pass and before emit — NOT at this step; there are no candidate findings to match yet. The load outcome feeds the `rejections=` slot of the `context:` observability line above. ### Theme-vs-root guard (reviewer-gated) Before emitting `patch_chain_risk.detected: true`, answer one sanity-check sentence: *"do the prior defensive commits address the same underlying root cause, or different root causes on the same file set?"* - **Same root** → the patch chain is real. The same invariant has been violated repeatedly; each round has added a guard on top. Emit the signal, and it satisfies clause (a) of verdict derivation rule 3 (`refactor-recommended`) in `output-schema.md` — prefer refactor over further guard iteration even if individual current findings are only medium severity. - **Different roots** → a legitimate hotfix-heavy file (e.g., a known-flaky integration test harness that genuinely receives independent hotfixes) has tripped the frequency/prefix signals without the underlying patch-chain dynamic. Do **not** emit `detected: true`; set `detected: false` with a note in `theme_assessment` explaining why. This guard exists because the deterministic signals alone over-fire on legitimate hotspots, and `refactor-recommended` is wrong for a file where every fix addresses a different invariant. Record the theme-vs-root assessment in `patch_chain_risk.theme_assessment` — this field is mandatory whenever any of signals 1–3 fired, regardless of whether `detected` ends up `true` or `false`. The purpose is auditability: downstream consumers should see that the reviewer considered the guard and chose one way or the other. ### Threshold rationale The specific thresholds above (`N` commits scanned, the 50% cluster ratio, the 4-commit window, the 3-of-5 same-file hotspot) are acknowledged uncalibrated starting values — authoritative discipline in `methodology.md` §Calibration rules → Threshold discipline. --- ## Step 4 — Large diff guard Count total lines changed. **The counting method depends on the active mode** — do not use `git diff --stat` blindly; in PR mode it counts local working tree state unrelated to the PR. - **Working-tree mode**: total = lines from `git diff --stat` + `git diff --cached --stat` + total byte count of included untracked files. - **Branch mode**: total = lines from `git diff --stat <merge-base>..HEAD`. - **PR mode**: total = `additions + deletions` from the `gh pr view --json additions,deletions` call already made in Step 3. If that field is unavailable, fall back to counting lines of the captured `gh pr diff` output. Then apply the thresholds: - **> 1500 lines**: split review. Group files by directory/module, review each group, maintain a running list of findings. In output, note: `split review (N files across G groups)`. - **Single file > 800 lines**: focus on public API, error handling, state mutations. Mark affected findings `[partial-review]`. - **> 5000 lines**: warn upfront: "This diff is very large. Review will focus on high-risk areas. Consider splitting the change." Prioritize error handling, state management, concurrency, auth, data persistence. Skip test files and generated files unless they are the focus. The findings cap still applies per group (see `methodology.md`). --- ## Step 5 — Load context and run mandatory traces **Read these files before reviewing the diff.** They are not optional. They define the review itself. 1. **`methodology.md`** (sibling file in this skill directory) — operating stance, attack surface, tracing disciplines, severity + block test, hunt-side calibration rules (hard cap, lift hierarchy, generalization test, threshold discipline), finding bar, grounding rules, claim verification, final check. Load it now. (Emit-time rules — classification axes, verdict/decision derivation, rejection Phase B — live in `output-schema.md` and load at Step 7, not now.) 2. **Pre-review context** (in this order, skip if absent): - **CLAUDE.md** (repo root) — read the "Architectural Decisions" section or equivalent. These are intentional choices. Findings that contradict them must be marked `[spec-accepted]` or dropped. - **Active specs / RFCs** — look in `docs/`, `specs/`, `rfcs/`, `.claude/rfcs/`, task board files. Same rule. 2b. **Project review rules** (cite, don't drop). Pre-review context in 5.2 is used to **drop** findings that contradict intentional architectural decisions. Project review rules are the opposite direction: the project's own rule files authorize findings to cite a specific rule as the grounding, making the finding more actionable than prose advice. A finding that says "violates `.claude/rules/no-patches.md`: enforce at the writer" is materially more useful than "this is a patch on a patch". **Glob for project rule candidates**, load the ones that exist (skip gracefully if nothing matches): - `.claude/rules/*.md` - `code-review.md`, `CODE_REVIEW.md`, `REVIEW.md` (at repo root) - `docs/review-rules.md`, `docs/contributing.md`, `CONTRIBUTING.md` - `**/rules/*.md` at repo root or one level deep (e.g., `apps/*/rules/*.md`) **Load caps** — to prevent context bloat on projects with long rule corpora: - At most **10 files** loaded. When more candidates exist, prefer `.claude/rules/*.md` first (explicit rule files), then root-level review/contributing docs, then deeper matches. - At most **30 KB** total content across all loaded rule files combined. If a single file blows the budget, truncate at the end of the last complete top-level section (markdown `##` heading) before the cap. - Skip any file under `node_modules/`, `vendor/`, `.git/`, build output directories, or test fixtures. `domains/*.md` inside the devil-review plugin itself is **not** a project rule file — it ships with the skill. **Record what was loaded** in `trace_log.project_rules_loaded` as entries of `{path, bytes}` whenever at least one rule file loaded; when none matched, omit the field — the attempt stays visible via the `rules=<n>` slot of the `context:` observability line (schema v2.0). **During finding generation** (Step 6), for each finding, attempt to cite applicable rule(s) from the loaded corpus. Each citation lives on the finding as an entry in `findings[].rule_refs` with three fields: - `source` — the path to the rule file - `rule` — a short identifier (heading name, numbered rule, or one-sentence paraphrase if the rule has no heading) - `quote` — **a verbatim 1–2 line quote from the rule file** that directly supports the finding's framing The verbatim-quote requirement is the anti-hallucination gate. Findings whose `rule_refs[].quote` strings do not appear **literally** in the cited file are schema-invalid — downstream consumers are entitled to reject them. If you cannot produce a verbatim quote, you cannot cite the rule; either rewrite the finding without the citation or drop the citation. Paraphrased "quotes" are the common failure mode to avoid. Empty `rule_refs: []` on a finding is always valid. Citation is opportunistic: a finding that does not correspond to any loaded project rule simply has no citation, not a forced one. 3. **Domain checklists** — classify the changed files and load every matching checklist. A single diff can match more than one domain (e.g., a React Native component touches both UI and mobile; an Electron renderer touches both UI and desktop; a backend handler that writes SQL touches both API and data). Load all that apply. | Domain | File / marker | Checklist | |---|---|---| | **Web UI / view layer** | `.vue`, `.tsx`, `.jsx`, `.svelte`, `.html`, layout CSS files (files with `display:`, `position:`, `z-index:`, `grid`, `flex`); composables, hooks, and store files whose output drives templates (`useXxx.ts`, `stores/*.ts`, `composables/*.ts`) | `domains/ui.md` | | **Mobile app** | iOS: `.swift`, `.m`, `.mm`, `.h`, `*.xcodeproj/`, `Info.plist`, `Podfile`, `.entitlements`. Android: `.kt`, `.kotlin`, `.java` under `android/`/`app/`, `AndroidManifest.xml`, `build.gradle`. React Native: any `.tsx`/`.jsx` in a project whose `package.json` depends on `react-native`. Flutter: `.dart`, `pubspec.yaml`, platform channels. Capacitor/Cordova: `capacitor.config.*`, `config.xml`, plugin code | `domains/mobile.md` | | **Desktop app** | Electron: `main.ts/.js`, `preload.ts/.js`, references to `BrowserWindow`/`ipcMain`/`ipcRenderer`/`app.on`. Tauri: anything under `src-tauri/`, `tauri.conf.json`, `#[tauri::command]`. Native: macOS Cocoa/AppKit `.swift`/`.m` outside `ios/`; Windows Win32/WinUI `.cs`/`.cpp` with MFC/WPF/WinRT; Linux Gtk/Qt sources. Packaging: `electron-builder.yml`, `forge.config.*`, `.wxs`, `.iss`, notarization scripts | `domains/desktop.md` | | **Backend API / server** | route handlers, controllers, middleware; request/response DTOs; schema files `openapi.*`, `.proto`, GraphQL SDL; framework signals: Express/Koa/Fastify/NestJS route files, Rails `app/controllers/`, Django `views.py`/`urls.py`, FastAPI route files, ASP.NET `*Controller.cs`, Spring `@RestController`; directory hints: `routes/`, `controllers/`, `handlers/`, `api/`, `rpc/`, `endpoints/`; background job handlers, queue consumers, webhook receivers (same trust-boundary concerns as HTTP handlers) | `domains/api.md` | | **Library / SDK** | changes to `package.json` `main`/`module`/`exports`/`types`; `src/index.*`, `src/lib.*`, `lib/*`; `Cargo.toml` with `[lib]`; `pyproject.toml` / `setup.py` in a published package; `.d.ts` / `.pyi` declaration files; any file whose project publishes to a registry (npm, PyPI, crates.io, Maven, NuGet) | `domains/library.md` | | **Data / persistence / migrations** | `.sql` files; migration directories: `migrations/`, `db/migrate/`, `prisma/migrations/`, `alembic/versions/`, `schema/`; ORM schemas: `schema.prisma`, Drizzle `schema.ts`, Ecto migrations, SQLAlchemy models, TypeORM entities, Rails migrations, Django migrations; stored procedures, triggers, views; cache key shapes and cache layer code; queue payload schemas; blob storage keys and object storage wrappers | `domains/data.md` | | **CLI tool** | `bin/`, `cmd/` entry points; files with `#!/usr/bin/env` shebangs; `main()` in a project whose manifest declares a binary/script target (`package.json` `bin` field, `Cargo.toml` `[[bin]]`, `pyproject.toml` `[project.scripts]`); argument parsing libraries (`commander`, `yargs`, `clap`, `argparse`, `cobra`, `click`); signal handling, subprocess spawning, TTY detection | `domains/cli.md` | | **Crypto / security-critical** | calls to cryptographic libraries (`crypto`, `subtle`, `libsodium`, `openssl`, `ring`, `cryptography`, `bcrypt`, `argon2`, `scrypt`, `hashlib`, `secrets`); JWT / token signing & verification; password hashing; key generation, derivation, storage, rotation; nonce / IV / salt handling; TLS / certificate verification; session management; webhook signature verification; authentication and authorization flows | `domains/crypto.md` | Match inclusively — when in doubt, load the checklist. The cost of loading an extra domain file is a few KB of context; the cost of missing one is a shipped bug. Under-matching is the failure mode to avoid. **Classification must be recorded.** Fill in `trace_log.domains_loaded` with every domain you loaded. For any genuinely ambiguous call, add a `scenarios_considered` line (e.g., `classification: .tsx — loaded ui.md not mobile.md, no react-native dependency`); straightforward loads need no line (schema v2.0 removed the dedicated `domains_considered_dropped` / `classification_notes` fields). See `output-schema.md`. If **no** domain matches, set `domains_loaded: []` and add a scenario `"generic attack surface only — no domain matched"`. Proceed with only the generic attack surface from `methodology.md`. Future domains live alongside (e.g., `domains/iac.md`, `domains/graphql.md`) — when added, extend this table. 4. **Changed symbols & consumers tracing** — for every added or modified symbol in the diff, **use the Grep tool** (not shell `grep`) to find its usages, and use the Read tool for the calling sites. Shell `grep` is not in `allowed-tools` and triggers a permission prompt per call; the Grep tool is in `allowed-tools` and runs without prompting. When searching a specific directory, pass `path` to the Grep tool — do **not** `cd` in a Bash call to change directories, as the compound-command pattern `cd X && grep Y` triggers Claude Code's path-resolution security guard and requires manual approval every time. The methodology file defines what counts as a "symbol" and what to trace. Every symbol you inspect must appear in the Trace Log in the final output. **Also run the failure-mode audit**: when the diff introduces a new caller chain that reaches an unchanged function, lifecycle, or handler, read the callee's existing failure-handling paths (auto-clear, auto-retry, default fallback, error suppression, timeout retries) and check each against the new caller's semantics — auto-recovery written for implicit/best-effort callers is often wrong for explicit-user-intent callers. Record findings under `trace_log.symbols_inspected[].failure_modes_considered`. See the "Failure-mode audit on existing callees with new callers" subsection in `methodology.md`. 5. **Mutated record fanout tracing** — for every record (struct, store entity, DB row, IPC/API/queue payload) whose fields are written in the diff, enumerate all sibling fields on the same record and check each for stale references, lifecycle leakage, or silently broken invariants. This follows the data model, not the call graph, and catches bugs that symbol tracing cannot. See the "Mutated record fanout" section in `methodology.md`. Every record you inspect must appear in `trace_log.mutated_records_inspected`. **Also run the reader-path fanout audit** for each sibling classified as "preserved": if the diff introduces a new writer→reader code path that reaches an existing reader of the preserved field, check whether the reader's implicit invariants still hold on the new path. Record findings under `trace_log.mutated_records_inspected[].new_reader_paths`. See the "Reader-path fanout" subsection in `methodology.md`. 6. **Runtime contract verification** — for every type in the diff that crosses a trust or language boundary (IPC, API response, DB row, queue payload, FFI), read the producer in its native source rather than trusting the consumer-side type signature. Tests that mock the payload from the consumer's perspective do not count as verification. See the "Runtime contract verification" section in `methodology.md`. 7. **LLM/agent output validation** — if the diff consumes structured data emitted by a language model, agent, ML pipeline, rule engine, or any other non-deterministic automation, audit every consumed field for consumer-side validation. Unvalidated fields that reach persistent state or user-visible action are findings; per the LLM-compliance severity floor in calibration rules, they start at **high** by default. Prompt-side constraints ("the prompt asks for backlog-only") are not consumer-side validation. See the "LLM/agent output validation" section in `methodology.md`. Record one line per consumed field under `scenarios_considered` in the form `llm-field: <name> — <validated|unvalidated|partial>`. 8. **Acceptance criteria crosswalk** — if the pre-review context step (5.2) loaded a spec, RFC, task file, or any document with **structured acceptance criteria** (bulleted "must" statements, numbered requirements, definition-of-done checklist), walk the AC list top to bottom. For every AC, write down the specific file:line that implements it. Flag ACs that are unimplemented, ambiguously mapped, or contradicted — these are findings at **high** by default. Record the complete crosswalk (passing and failing ACs) in `trace_log.acceptance_criteria_crosswalk`. If the spec is prose-only with no structured ACs, skip this step and note it as a `scenarios_considered` line. See the "Acceptance criteria crosswalk" section in `methodology.md`. 9. **Test-trace** — every finding you plan to report must carry a test_coverage answer explaining why existing tests did not catch the bug, chosen from `no-test`, `mock-bypass`, or `missing-assertion`. If no answer is possible, the finding is invalid — re-read the tests or drop it. See the "Test-trace" section in `methodology.md`. --- ## Step 6 — Review Apply the methodology from `methodology.md` plus any loaded domain checklists to the collected diff. Keep the calibration rules in mind continuously — every finding you consider keeping must pass the ship-blocker question and the block test before it earns a slot under the hard cap. ### Focus text routing If `FOCUS_TEXT` parsed in Step 1 is non-empty: 1. Treat it as an explicit weighting on the attack surface. Findings that match the focus area are prioritized over unrelated findings of equal severity when applying the hard cap. 2. Include `FOCUS_TEXT` verbatim in the `focus` field of the output (both markdown and JSON). 3. Record at least one scenario under `scenarios_considered` that directly targets the focus area, prefixed as `focus: <text>`. 4. If after applying the methodology you find **no** material issue in the focus area, say so explicitly in the summary — "focus area (<text>) reviewed, no material findings" — rather than staying silent. The user asked; answer. If `FOCUS_TEXT` is empty, set `focus` to `null` in the JSON and omit the markdown `Focus:` line. ### Pre-output checklist (hunt side) Do not proceed to Step 7 until you have: - answered the ship-blocker question (the answer is recorded in the Trace Log at emit) - traced consumers for every changed symbol - routed `FOCUS_TEXT` if present - run the **Claim verification pass** (six steps — step 5 is the evidence gate for cross-boundary external claims, step 6 the event-source upstream trace) on every candidate finding per `methodology.md` - applied the final_check to every candidate finding - dropped weak findings to fit the hard cap The emit-side checklist — per-finding classification axes, conditional trace_log blocks, rejection memory Phase B, the `decision` block, observability lines, and the required-field backstop — lives in `output-schema.md` ("Pre-emit checklist") and is completed after loading that file in Step 7. Do not load `output-schema.md` before the hunt is done; keeping the output contract out of hunt context is deliberate. --- ## Step 7 — Emit output Read **`output-schema.md`** (sibling file in this skill directory) — now, not earlier — complete its **Pre-emit checklist**, and produce output in **exactly** the format it specifies: markdown section followed by a JSON fence. Both parts are mandatory on every non-error run. The Trace Log is non-negotiable. If you reported findings without a populated trace log, you skipped the grounding step — go back, trace, and try again. If the review cannot run (not a repo, `gh` missing, empty diff, shallow clone without base), emit the error output format from `output-schema.md` instead. Do not fabricate a review. ## Step 8 — Auto-save for future runs After emitting the output in Step 7, use the Write tool to write the **complete emitted output** (markdown section + JSON fence, verbatim) to: ``` .claude/devil-review/${CLAUDE_SESSION_ID}/<target-slug>.md ``` `${CLAUDE_SESSION_ID}` is substituted by the runtime. Create the directory tree if it does not exist. **Target slug** (deterministic from Step 2's resolved target): - Working-tree mode → `working-tree` - Branch mode → `branch-<base-ref>` with forward slashes and other non-`[A-Za-z0-9._-]` chars replaced by hyphens. Example: `feature/auth` → `branch-feature-auth`. - PR mode → `pr-<number>`. Example: `pr-42`. Overwrite unconditionally — each `(session, target)` pair holds one file. Different targets never collide within a session; different sessions never collide at all. Step 3b's auto-detect reads this same path on the next run. **Skip** when the review ended in an error output (verdict `null`). No scenarios_considered line is emitted for the write — the read side (Step 3b ingestion status) already carries the observability. `.gitignore` setup is covered in the plugin README.

# Devil Review > The devil is in the details — adversarial review of working-tree, branch, or PR diffs that finds what's hiding in them - Skill: `mralabs/devil-review` (Agent Skill, multi-file: 12 files) - Install (CLI): `npx skillmds@latest add mralabs/devil-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/mralabs/devil-review/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: mralabs (https://skillmd.com/u/mralabs) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/mralabs/devil-review --- You are performing an adversarial code review. Your job is to break confidence in the change, not to validate it. Do not fix issues. Review only. Raw slash-command arguments: `$ARGUMENTS` This file is the **orchestrator**. It parses arguments, collects the diff, and points you at the files that define the methodology and output format. Do not attempt to review the diff until you have loaded those files. --- ## Step 1 — Parse arguments Parse the raw arguments: - `--scope ` — review target scope (default: `auto`) - `--base ` — explicit base ref for branch diff - `--pr ` — GitHub PR number to review (implies `--scope pr`) - `--reject ` — record rejections of findings from the prior snapshot **of the resolved review target** (the Step 8 target slug — the same snapshot Step 3b auto-detects; not simply the most recently written snapshot, which may belong to a different target) before running this review. `` is a comma-separated list of 1-based finding indices (e.g. `--reject 2,5,7`). Rejections are persisted to `.claude/devil-review/${CLAUDE_SESSION_ID}/rejections.json` and consulted on subsequent runs per `rejection-memory.md`. - Everything else after flags → `FOCUS_TEXT` **No `--prior` flag.** Prior-review auto-detection is handled inside Step 3b — the skill always looks for a snapshot at `.claude/devil-review/${CLAUDE_SESSION_ID}/.md` (session-scoped, target-scoped — see Step 8 for slug rules) and uses it for patch-chain detection when present. Absent prior files produce a fresh review. Users do not control this via a flag; the behavior is zero-config. To force a fresh review on a target that already has a snapshot, delete the corresponding file. **`--reject` semantics.** The flag both *records* rejections and *runs* a new review — single code path. Rejections are applied at the start of Step 3b (before candidate-finding generation), so the new review sees the freshly-added rejections and suppresses or re-raises candidates matching them per the Rejection memory load rule. If `--reject` is passed but no prior snapshot exists for the resolved target (fresh first run), emit the error output with code `reject_without_prior` and a message pointing at the missing snapshot path. Rationale for rejections recorded via this flag is `null` — users who want rationales attached must edit `rejections.json` directly after recording. --- ## Step 2 — Resolve review target 1. **Sanity check**: run `git rev-parse --is-inside-work-tree`. If it fails, emit the error output per `output-schema.md` with error code `not_a_repo` and stop. 2. If `--pr ` is given or `--scope pr` → **PR mode** 3. If `--base ` is given → **branch mode** against that ref 4. If `--scope working-tree` → **working-tree mode** 5. If `--scope branch` → **branch mode**, detect default branch: - Try `git symbolic-ref refs/remotes/origin/HEAD` - Fall back to checking `main`, `master`, `trunk` (local then remote) 6. If `--scope auto` (default): - Run `git status --short`, `git diff --shortstat`, `git diff --cached --shortstat` - If working tree is dirty (staged, unstaged, or untracked) → **working-tree mode** - If clean → **branch mode** against detected default branch --- ## Step 3 — Collect review context ### PR mode Requires `gh` CLI. Run `gh --version` first. If it fails, emit the error output with error code `gh_missing` and stop. Do not fall back silently. Collect the PR metadata, diff, and **both comment streams** — inline review comments and PR discussion comments — because GitHub models a PR as both a pull and an issue: ``` gh pr view --json title,body,baseRefName,headRefName,additions,deletions,commits,files gh pr diff gh api repos/{owner}/{repo}/pulls//comments --jq '.[].body' gh api repos/{owner}/{repo}/issues//comments --jq '.[].body' ``` The `{owner}` / `{repo}` placeholders in `gh api` are expanded automatically by `gh` when run inside a cloned repository with a GitHub remote. If the current directory is not such a repository, fall through to explicit resolution via `gh repo view --json nameWithOwner`. Assemble: ``` ## PR Info Title: Base: <baseRefName> ← <headRefName> Additions/Deletions: +<additions> -<deletions> Description: <body, first 500 chars> ## Changed Files <files list> ## PR Diff <full diff> ## Existing Review Comments (inline) <inline comments from /pulls/N/comments, if any> ## Existing PR Discussion (issue comments) <discussion comments from /issues/N/comments, if any> ``` Skip either comments section if empty. The point of collecting both is to avoid duplicating findings already raised by humans — whether inline or in the discussion thread. ### Working-tree mode ``` git status --short git diff --cached --no-ext-diff --submodule=diff git diff --no-ext-diff --submodule=diff git ls-files --others --exclude-standard git log --oneline -10 ``` For each untracked file: skip binary, skip >24KB, otherwise read and include content. ### Branch mode ``` git merge-base HEAD <base-ref> git log --oneline --decorate <merge-base>..HEAD git diff --stat <merge-base>..HEAD git diff --no-ext-diff --submodule=diff <merge-base>..HEAD ``` If `git merge-base` fails (common in shallow clones / CI), emit the error output with error code `shallow_clone_no_base` and instruct: "Run `git fetch --unshallow` or use `--scope working-tree` / explicit `--base <ref>`." ### Empty diff handling If the resolved diff is empty (no staged, unstaged, untracked, or branch-divergent changes), emit the error output with error code `empty_diff` and verdict `null`. Do NOT return `approve` — an empty review is not an approval. --- ## Step 3b — Patch-chain scan After collecting the diff but before the large-diff guard, scan recent commit history for patterns that indicate iterative patching on the same surface. Multi-round defensive commits on the same file set are a signal that candidate findings in this review may be artifacts of prior rounds' guards rather than organic defects — and the correct next step is then a structural refactor, not round N+1 of guard-chasing. The interpretation rule and severity implications live in the "Patch-chain detection" section of `output-schema.md` (loaded at Step 7); this step is the data-collection side, and the theme-vs-root guard below is all the interpretation needed before emit. ### Collect the commit history Run: ``` git log -<N> --oneline -- <changed-files> ``` Where `<N>` is `5` for working-tree and branch modes, `10` for PR mode (PRs accumulate more commits than typical local changes). `<changed-files>` is the set of files already identified in Step 3's diff. ### Signals — at least one must fire to populate `patch_chain_risk` 1. **Fix-prefix cluster.** Among the last 4 commits that touch any reviewed file, ≥50% (i.e., ≥2 of 4) have messages prefixed with any of: `fix:`, `guard:`, `prevent:`, `patch:`, `workaround:`, `hotfix:` (case-insensitive; conventional-commits scope suffix like `fix(auth):` still counts). The cluster is "same surface, repeatedly defensive". 2. **Same-file hotspot.** A single reviewed file appears in ≥3 of the last 5 commits (working-tree / branch mode) or ≥5 of the last 10 commits (PR mode). File frequency alone is not enough without a defensive-prefix cluster, but combined with signal 1 it strengthens the signal — record both when both fire. 3. **Prior-review overlap** (auto-detected — no flag required). Compute the **target slug** for the current review (see Step 8 for the slug rules) and resolve the prior-review path to `.claude/devil-review/${CLAUDE_SESSION_ID}/<target-slug>.md`. Session and target scoping ensure that stale reviews from unrelated sessions or different targets never bleed in. If the file exists, load it; extract its findings array and `considered_not_promoted` array via its JSON fence (treat the load as absent if no `schema_version` field is present, the file is malformed, or the file does not exist — emit the corresponding status per the observability rule below). If ≥50% of the current review's candidate findings reference file locations that also appeared in the prior review's findings or `considered_not_promoted`, the signal fires. Also cross-reference each current finding's `file:line` against the prior review's entries and annotate any overlaps in the finding body: "This location also appeared in the prior review as finding #N" — this annotation is body-only, not a new schema field. **Observability requirement.** The skill **must always** emit one `scenarios_considered` line of the form `context: prior=<status> rejections=<status> rules=<n>` on every non-error run (schema v2.0 — this single line replaces the separate `prior-review ingestion:` and `rejection memory:` lines). `prior=` is exactly one of `loaded`, `absent` (file does not exist — fresh run), `rejected-no-schema-version`, or `rejected-malformed-json`; `rejections=` is exactly one of `loaded`, `absent`, or `rejected-malformed-json` (per `rejection-memory.md`); `rules=` is the count of project rule files loaded in Step 5.2b. This line makes the auto-detect outcomes visible; silent drops are not permitted. ### Prior-relation classification (schema v1.11+) When Step 3b's `<status>` is `loaded`, the loaded prior review's findings feed three **emit-time obligations**: per-finding `prior_relation` attribution (three categories per the plugin v1.15.1 correction — `resolved` is not a finding-level value), the `trace_log.prior_review_summary` roll-up, and severity dampening for `carries-over` findings. Rules live in the "Prior-relation classification" and "Severity dampening for carries-over findings" sections of `output-schema.md`; its Pre-emit checklist enforces them at Step 7. When `<status>` is `absent` or any `rejected-*` value, omit `prior_relation` on all findings and omit `trace_log.prior_review_summary` entirely. ### Rejection memory load (schema v1.14+) Rejection memory lets the reviewer avoid silently re-raising findings the user has already dismissed via `--reject`. The full mechanics — hash normalization (authoritative), `--reject` recording, file load, suppression vs. re-raise, and the chain-of-rejections verdict override — live in **`rejection-memory.md`** (sibling file in this skill directory). Load it now. Execute its **Phase A** (substeps 1–3) at this step: record any `--reject <CSV>` entries from Step 1 into `.claude/devil-review/${CLAUDE_SESSION_ID}/rejections.json`, then load the file into `trace_log.rejections_loaded` (present only when the file exists — schema v2.0). Its **Phase B** (substeps 4–6: per-candidate suppression check, suppress-vs-re-raise judgment, chain-of-rejections override) runs later — after the Claim verification pass and before emit — NOT at this step; there are no candidate findings to match yet. The load outcome feeds the `rejections=` slot of the `context:` observability line above. ### Theme-vs-root guard (reviewer-gated) Before emitting `patch_chain_risk.detected: true`, answer one sanity-check sentence: *"do the prior defensive commits address the same underlying root cause, or different root causes on the same file set?"* - **Same root** → the patch chain is real. The same invariant has been violated repeatedly; each round has added a guard on top. Emit the signal, and it satisfies clause (a) of verdict derivation rule 3 (`refactor-recommended`) in `output-schema.md` — prefer refactor over further guard iteration even if individual current findings are only medium severity. - **Different roots** → a legitimate hotfix-heavy file (e.g., a known-flaky integration test harness that genuinely receives independent hotfixes) has tripped the frequency/prefix signals without the underlying patch-chain dynamic. Do **not** emit `detected: true`; set `detected: false` with a note in `theme_assessment` explaining why. This guard exists because the deterministic signals alone over-fire on legitimate hotspots, and `refactor-recommended` is wrong for a file where every fix addresses a different invariant. Record the theme-vs-root assessment in `patch_chain_risk.theme_assessment` — this field is mandatory whenever any of signals 1–3 fired, regardless of whether `detected` ends up `true` or `false`. The purpose is auditability: downstream consumers should see that the reviewer considered the guard and chose one way or the other. ### Threshold rationale The specific thresholds above (`N` commits scanned, the 50% cluster ratio, the 4-commit window, the 3-of-5 same-file hotspot) are acknowledged uncalibrated starting values — authoritative discipline in `methodology.md` §Calibration rules → Threshold discipline. --- ## Step 4 — Large diff guard Count total lines changed. **The counting method depends on the active mode** — do not use `git diff --stat` blindly; in PR mode it counts local working tree state unrelated to the PR. - **Working-tree mode**: total = lines from `git diff --stat` + `git diff --cached --stat` + total byte count of included untracked files. - **Branch mode**: total = lines from `git diff --stat <merge-base>..HEAD`. - **PR mode**: total = `additions + deletions` from the `gh pr view --json additions,deletions` call already made in Step 3. If that field is unavailable, fall back to counting lines of the captured `gh pr diff` output. Then apply the thresholds: - **> 1500 lines**: split review. Group files by directory/module, review each group, maintain a running list of findings. In output, note: `split review (N files across G groups)`. - **Single file > 800 lines**: focus on public API, error handling, state mutations. Mark affected findings `[partial-review]`. - **> 5000 lines**: warn upfront: "This diff is very large. Review will focus on high-risk areas. Consider splitting the change." Prioritize error handling, state management, concurrency, auth, data persistence. Skip test files and generated files unless they are the focus. The findings cap still applies per group (see `methodology.md`). --- ## Step 5 — Load context and run mandatory traces **Read these files before reviewing the diff.** They are not optional. They define the review itself. 1. **`methodology.md`** (sibling file in this skill directory) — operating stance, attack surface, tracing disciplines, severity + block test, hunt-side calibration rules (hard cap, lift hierarchy, generalization test, threshold discipline), finding bar, grounding rules, claim verification, final check. Load it now. (Emit-time rules — classification axes, verdict/decision derivation, rejection Phase B — live in `output-schema.md` and load at Step 7, not now.) 2. **Pre-review context** (in this order, skip if absent): - **CLAUDE.md** (repo root) — read the "Architectural Decisions" section or equivalent. These are intentional choices. Findings that contradict them must be marked `[spec-accepted]` or dropped. - **Active specs / RFCs** — look in `docs/`, `specs/`, `rfcs/`, `.claude/rfcs/`, task board files. Same rule. 2b. **Project review rules** (cite, don't drop). Pre-review context in 5.2 is used to **drop** findings that contradict intentional architectural decisions. Project review rules are the opposite direction: the project's own rule files authorize findings to cite a specific rule as the grounding, making the finding more actionable than prose advice. A finding that says "violates `.claude/rules/no-patches.md`: enforce at the writer" is materially more useful than "this is a patch on a patch". **Glob for project rule candidates**, load the ones that exist (skip gracefully if nothing matches): - `.claude/rules/*.md` - `code-review.md`, `CODE_REVIEW.md`, `REVIEW.md` (at repo root) - `docs/review-rules.md`, `docs/contributing.md`, `CONTRIBUTING.md` - `**/rules/*.md` at repo root or one level deep (e.g., `apps/*/rules/*.md`) **Load caps** — to prevent context bloat on projects with long rule corpora: - At most **10 files** loaded. When more candidates exist, prefer `.claude/rules/*.md` first (explicit rule files), then root-level review/contributing docs, then deeper matches. - At most **30 KB** total content across all loaded rule files combined. If a single file blows the budget, truncate at the end of the last complete top-level section (markdown `##` heading) before the cap. - Skip any file under `node_modules/`, `vendor/`, `.git/`, build output directories, or test fixtures. `domains/*.md` inside the devil-review plugin itself is **not** a project rule file — it ships with the skill. **Record what was loaded** in `trace_log.project_rules_loaded` as entries of `{path, bytes}` whenever at least one rule file loaded; when none matched, omit the field — the attempt stays visible via the `rules=<n>` slot of the `context:` observability line (schema v2.0). **During finding generation** (Step 6), for each finding, attempt to cite applicable rule(s) from the loaded corpus. Each citation lives on the finding as an entry in `findings[].rule_refs` with three fields: - `source` — the path to the rule file - `rule` — a short identifier (heading name, numbered rule, or one-sentence paraphrase if the rule has no heading) - `quote` — **a verbatim 1–2 line quote from the rule file** that directly supports the finding's framing The verbatim-quote requirement is the anti-hallucination gate. Findings whose `rule_refs[].quote` strings do not appear **literally** in the cited file are schema-invalid — downstream consumers are entitled to reject them. If you cannot produce a verbatim quote, you cannot cite the rule; either rewrite the finding without the citation or drop the citation. Paraphrased "quotes" are the common failure mode to avoid. Empty `rule_refs: []` on a finding is always valid. Citation is opportunistic: a finding that does not correspond to any loaded project rule simply has no citation, not a forced one. 3. **Domain checklists** — classify the changed files and load every matching checklist. A single diff can match more than one domain (e.g., a React Native component touches both UI and mobile; an Electron renderer touches both UI and desktop; a backend handler that writes SQL touches both API and data). Load all that apply. | Domain | File / marker | Checklist | |---|---|---| | **Web UI / view layer** | `.vue`, `.tsx`, `.jsx`, `.svelte`, `.html`, layout CSS files (files with `display:`, `position:`, `z-index:`, `grid`, `flex`); composables, hooks, and store files whose output drives templates (`useXxx.ts`, `stores/*.ts`, `composables/*.ts`) | `domains/ui.md` | | **Mobile app** | iOS: `.swift`, `.m`, `.mm`, `.h`, `*.xcodeproj/`, `Info.plist`, `Podfile`, `.entitlements`. Android: `.kt`, `.kotlin`, `.java` under `android/`/`app/`, `AndroidManifest.xml`, `build.gradle`. React Native: any `.tsx`/`.jsx` in a project whose `package.json` depends on `react-native`. Flutter: `.dart`, `pubspec.yaml`, platform channels. Capacitor/Cordova: `capacitor.config.*`, `config.xml`, plugin code | `domains/mobile.md` | | **Desktop app** | Electron: `main.ts/.js`, `preload.ts/.js`, references to `BrowserWindow`/`ipcMain`/`ipcRenderer`/`app.on`. Tauri: anything under `src-tauri/`, `tauri.conf.json`, `#[tauri::command]`. Native: macOS Cocoa/AppKit `.swift`/`.m` outside `ios/`; Windows Win32/WinUI `.cs`/`.cpp` with MFC/WPF/WinRT; Linux Gtk/Qt sources. Packaging: `electron-builder.yml`, `forge.config.*`, `.wxs`, `.iss`, notarization scripts | `domains/desktop.md` | | **Backend API / server** | route handlers, controllers, middleware; request/response DTOs; schema files `openapi.*`, `.proto`, GraphQL SDL; framework signals: Express/Koa/Fastify/NestJS route files, Rails `app/controllers/`, Django `views.py`/`urls.py`, FastAPI route files, ASP.NET `*Controller.cs`, Spring `@RestController`; directory hints: `routes/`, `controllers/`, `handlers/`, `api/`, `rpc/`, `endpoints/`; background job handlers, queue consumers, webhook receivers (same trust-boundary concerns as HTTP handlers) | `domains/api.md` | | **Library / SDK** | changes to `package.json` `main`/`module`/`exports`/`types`; `src/index.*`, `src/lib.*`, `lib/*`; `Cargo.toml` with `[lib]`; `pyproject.toml` / `setup.py` in a published package; `.d.ts` / `.pyi` declaration files; any file whose project publishes to a registry (npm, PyPI, crates.io, Maven, NuGet) | `domains/library.md` | | **Data / persistence / migrations** | `.sql` files; migration directories: `migrations/`, `db/migrate/`, `prisma/migrations/`, `alembic/versions/`, `schema/`; ORM schemas: `schema.prisma`, Drizzle `schema.ts`, Ecto migrations, SQLAlchemy models, TypeORM entities, Rails migrations, Django migrations; stored procedures, triggers, views; cache key shapes and cache layer code; queue payload schemas; blob storage keys and object storage wrappers | `domains/data.md` | | **CLI tool** | `bin/`, `cmd/` entry points; files with `#!/usr/bin/env` shebangs; `main()` in a project whose manifest declares a binary/script target (`package.json` `bin` field, `Cargo.toml` `[[bin]]`, `pyproject.toml` `[project.scripts]`); argument parsing libraries (`commander`, `yargs`, `clap`, `argparse`, `cobra`, `click`); signal handling, subprocess spawning, TTY detection | `domains/cli.md` | | **Crypto / security-critical** | calls to cryptographic libraries (`crypto`, `subtle`, `libsodium`, `openssl`, `ring`, `cryptography`, `bcrypt`, `argon2`, `scrypt`, `hashlib`, `secrets`); JWT / token signing & verification; password hashing; key generation, derivation, storage, rotation; nonce / IV / salt handling; TLS / certificate verification; session management; webhook signature verification; authentication and authorization flows | `domains/crypto.md` | Match inclusively — when in doubt, load the checklist. The cost of loading an extra domain file is a few KB of context; the cost of missing one is a shipped bug. Under-matching is the failure mode to avoid. **Classification must be recorded.** Fill in `trace_log.domains_loaded` with every domain you loaded. For any genuinely ambiguous call, add a `scenarios_considered` line (e.g., `classification: .tsx — loaded ui.md not mobile.md, no react-native dependency`); straightforward loads need no line (schema v2.0 removed the dedicated `domains_considered_dropped` / `classification_notes` fields). See `output-schema.md`. If **no** domain matches, set `domains_loaded: []` and add a scenario `"generic attack surface only — no domain matched"`. Proceed with only the generic attack surface from `methodology.md`. Future domains live alongside (e.g., `domains/iac.md`, `domains/graphql.md`) — when added, extend this table. 4. **Changed symbols & consumers tracing** — for every added or modified symbol in the diff, **use the Grep tool** (not shell `grep`) to find its usages, and use the Read tool for the calling sites. Shell `grep` is not in `allowed-tools` and triggers a permission prompt per call; the Grep tool is in `allowed-tools` and runs without prompting. When searching a specific directory, pass `path` to the Grep tool — do **not** `cd` in a Bash call to change directories, as the compound-command pattern `cd X && grep Y` triggers Claude Code's path-resolution security guard and requires manual approval every time. The methodology file defines what counts as a "symbol" and what to trace. Every symbol you inspect must appear in the Trace Log in the final output. **Also run the failure-mode audit**: when the diff introduces a new caller chain that reaches an unchanged function, lifecycle, or handler, read the callee's existing failure-handling paths (auto-clear, auto-retry, default fallback, error suppression, timeout retries) and check each against the new caller's semantics — auto-recovery written for implicit/best-effort callers is often wrong for explicit-user-intent callers. Record findings under `trace_log.symbols_inspected[].failure_modes_considered`. See the "Failure-mode audit on existing callees with new callers" subsection in `methodology.md`. 5. **Mutated record fanout tracing** — for every record (struct, store entity, DB row, IPC/API/queue payload) whose fields are written in the diff, enumerate all sibling fields on the same record and check each for stale references, lifecycle leakage, or silently broken invariants. This follows the data model, not the call graph, and catches bugs that symbol tracing cannot. See the "Mutated record fanout" section in `methodology.md`. Every record you inspect must appear in `trace_log.mutated_records_inspected`. **Also run the reader-path fanout audit** for each sibling classified as "preserved": if the diff introduces a new writer→reader code path that reaches an existing reader of the preserved field, check whether the reader's implicit invariants still hold on the new path. Record findings under `trace_log.mutated_records_inspected[].new_reader_paths`. See the "Reader-path fanout" subsection in `methodology.md`. 6. **Runtime contract verification** — for every type in the diff that crosses a trust or language boundary (IPC, API response, DB row, queue payload, FFI), read the producer in its native source rather than trusting the consumer-side type signature. Tests that mock the payload from the consumer's perspective do not count as verification. See the "Runtime contract verification" section in `methodology.md`. 7. **LLM/agent output validation** — if the diff consumes structured data emitted by a language model, agent, ML pipeline, rule engine, or any other non-deterministic automation, audit every consumed field for consumer-side validation. Unvalidated fields that reach persistent state or user-visible action are findings; per the LLM-compliance severity floor in calibration rules, they start at **high** by default. Prompt-side constraints ("the prompt asks for backlog-only") are not consumer-side validation. See the "LLM/agent output validation" section in `methodology.md`. Record one line per consumed field under `scenarios_considered` in the form `llm-field: <name> — <validated|unvalidated|partial>`. 8. **Acceptance criteria crosswalk** — if the pre-review context step (5.2) loaded a spec, RFC, task file, or any document with **structured acceptance criteria** (bulleted "must" statements, numbered requirements, definition-of-done checklist), walk the AC list top to bottom. For every AC, write down the specific file:line that implements it. Flag ACs that are unimplemented, ambiguously mapped, or contradicted — these are findings at **high** by default. Record the complete crosswalk (passing and failing ACs) in `trace_log.acceptance_criteria_crosswalk`. If the spec is prose-only with no structured ACs, skip this step and note it as a `scenarios_considered` line. See the "Acceptance criteria crosswalk" section in `methodology.md`. 9. **Test-trace** — every finding you plan to report must carry a test_coverage answer explaining why existing tests did not catch the bug, chosen from `no-test`, `mock-bypass`, or `missing-assertion`. If no answer is possible, the finding is invalid — re-read the tests or drop it. See the "Test-trace" section in `methodology.md`. --- ## Step 6 — Review Apply the methodology from `methodology.md` plus any loaded domain checklists to the collected diff. Keep the calibration rules in mind continuously — every finding you consider keeping must pass the ship-blocker question and the block test before it earns a slot under the hard cap. ### Focus text routing If `FOCUS_TEXT` parsed in Step 1 is non-empty: 1. Treat it as an explicit weighting on the attack surface. Findings that match the focus area are prioritized over unrelated findings of equal severity when applying the hard cap. 2. Include `FOCUS_TEXT` verbatim in the `focus` field of the output (both markdown and JSON). 3. Record at least one scenario under `scenarios_considered` that directly targets the focus area, prefixed as `focus: <text>`. 4. If after applying the methodology you find **no** material issue in the focus area, say so explicitly in the summary — "focus area (<text>) reviewed, no material findings" — rather than staying silent. The user asked; answer. If `FOCUS_TEXT` is empty, set `focus` to `null` in the JSON and omit the markdown `Focus:` line. ### Pre-output checklist (hunt side) Do not proceed to Step 7 until you have: - answered the ship-blocker question (the answer is recorded in the Trace Log at emit) - traced consumers for every changed symbol - routed `FOCUS_TEXT` if present - run the **Claim verification pass** (six steps — step 5 is the evidence gate for cross-boundary external claims, step 6 the event-source upstream trace) on every candidate finding per `methodology.md` - applied the final_check to every candidate finding - dropped weak findings to fit the hard cap The emit-side checklist — per-finding classification axes, conditional trace_log blocks, rejection memory Phase B, the `decision` block, observability lines, and the required-field backstop — lives in `output-schema.md` ("Pre-emit checklist") and is completed after loading that file in Step 7. Do not load `output-schema.md` before the hunt is done; keeping the output contract out of hunt context is deliberate. --- ## Step 7 — Emit output Read **`output-schema.md`** (sibling file in this skill directory) — now, not earlier — complete its **Pre-emit checklist**, and produce output in **exactly** the format it specifies: markdown section followed by a JSON fence. Both parts are mandatory on every non-error run. The Trace Log is non-negotiable. If you reported findings without a populated trace log, you skipped the grounding step — go back, trace, and try again. If the review cannot run (not a repo, `gh` missing, empty diff, shallow clone without base), emit the error output format from `output-schema.md` instead. Do not fabricate a review. ## Step 8 — Auto-save for future runs After emitting the output in Step 7, use the Write tool to write the **complete emitted output** (markdown section + JSON fence, verbatim) to: ``` .claude/devil-review/${CLAUDE_SESSION_ID}/<target-slug>.md ``` `${CLAUDE_SESSION_ID}` is substituted by the runtime. Create the directory tree if it does not exist. **Target slug** (deterministic from Step 2's resolved target): - Working-tree mode → `working-tree` - Branch mode → `branch-<base-ref>` with forward slashes and other non-`[A-Za-z0-9._-]` chars replaced by hyphens. Example: `feature/auth` → `branch-feature-auth`. - PR mode → `pr-<number>`. Example: `pr-42`. Overwrite unconditionally — each `(session, target)` pair holds one file. Different targets never collide within a session; different sessions never collide at all. Step 3b's auto-detect reads this same path on the next run. **Skip** when the review ended in an error output (verdict `null`). No scenarios_considered line is emitted for the write — the read side (Step 3b ingestion status) already carries the observability. `.gitignore` setup is covered in the plugin README.