review-policy

What this skill does

# Policy Review

Review a code change against the project's accepted ADRs. An ADR is policy: drivers, chosen option, rejected alternatives, and invariants the decision asked future code to respect. This skill reads the change, identifies which ADRs apply, and reports where the change **violates**, **erodes**, or **drifts toward rejected options** — and, secondarily, where a driver may have **shifted** enough that the ADR itself should be revisited.

This skill does **not** grade code quality, test coverage, or style — those belong to `review-code`, `review-tests`, and `review-docs`. It judges the change *against recorded decisions*.

> [!IMPORTANT]
> Consult [REFERENCE.md](REFERENCE.md) for the expected output format and level of detail.

## User Input

```text
$ARGUMENTS
```

You **MUST** consider the user input before proceeding (if not empty).

## Argument Parsing

Parse `$ARGUMENTS` for:
- **Change scope** (optional): a git ref range (`main..HEAD`), a PR URL/number, a path, or empty. Default: working diff against the base branch (`main` or `master`, whichever the repo uses).
- **ADR filter** (optional): a specific ADR id like `ADR-0007`, or a tag like `storage`. Default: all accepted ADRs.

If the scope is ambiguous, ask once, then proceed.

## Prerequisites

- The repo has an ADR directory — check `docs/adr/`, `docs/decisions/`, `docs/adrs/` in that order. If none exists, stop and tell the user there are no ADRs to review against.
- The repo is a git repo (the skill reads the diff via git). If not, ask the user to point at a specific path and skip diff-based analysis.

## Philosophy — what counts as a finding

ADRs record decisions. A finding must trace back to a specific claim in a specific accepted ADR — not to the reviewer's taste. If you cannot cite the ADR id and the section of the ADR that the change contradicts, omit the finding.

There are four categories of finding, and they are **not** interchangeable:

1. **Violation** — the change does the thing an ADR explicitly rejected, or fails the invariant the ADR required. The strongest claim; use only when the mismatch is textual (the ADR said X, the change does not-X).
2. **Erosion** — the change weakens an invariant the ADR required without breaking it outright. The ADR said "every service must emit a `request_id`"; the change adds a new handler that does not. The decision still stands, but this change digs a hole under it.
3. **Drift** — the change moves the codebase toward a rejected option without crossing the line. Often a P3 — the author may not realize the path they're on. Example: ADR chose Postgres for the event store and rejected DynamoDB; the change introduces a new DynamoDB-backed store "just for this feature".
4. **Driver shift** — the change reveals that one of the ADR's decision drivers no longer holds. Not a complaint about the change; a signal that the *ADR* is a candidate for revisit. Example: ADR-0003 cited "team has no Kafka experience" as a driver; the change is by a team member who just shipped a Kafka integration. The ADR's premise has eroded.

Only **accepted** ADRs bind. `proposed` ADRs are not yet policy; `superseded` ones are history. Ignore both unless the user explicitly asks.

Be honest about ambiguity. If the ADR is vague on the point the change touches, say so — "ADR-0007 does not speak to this case; flagging as a judgement call, not a violation." Hallucinated ADR intent is worse than no finding.

## Workflow

### Step 1 — Resolve the change scope

Turn `$ARGUMENTS` into a concrete diff and a file list:

- Empty → `git diff <base>...HEAD` where `<base>` is the default branch
- Git ref range (`a..b`, `a...b`) → `git diff <range>`
- PR URL / number → `gh pr diff <n>` and `gh pr view <n> --json files`
- Path → treat as current-state review of that path; note in the report that this is not a diff review
- Bare commit sha → `git show <sha>`

Record:
- The diff itself (file + line ranges + hunk content)
- The list of changed files
- The base ref the diff is relative to

If the diff is empty, stop and tell the user there's nothing to review.

### Step 2 — Load the ADR corpus

Read every ADR file in the ADR directory. For each, extract:

- `id`, `title`, `status`
- `tags`
- `supersedes`, `superseded-by`
- The Decision Outcome (chosen option) and Considered Options (rejected ones)
- The Decision Drivers
- The Consequences (especially "Negative" — these are the invariants the ADR accepted and expects code to respect)

Filter to ADRs with `status: accepted`. Do not bind against `proposed` or `superseded`. If the user passed an ADR filter (specific id or tag), apply it now.

### Step 3 — Score ADR relevance to the change

Not every ADR applies to every change. Spending budget analyzing irrelevant ADRs produces false positives and wastes tokens. Score each accepted ADR against the change using a cheap signal pass:

- **Path overlap** — does the ADR's text (title, problem statement, implementation notes) name directories, modules, services, or filenames that appear in the changed file list?
- **Tag alignment** — does the change touch a domain the ADR's tags cover (e.g., tag `storage` and the diff touches `db/`, `repository/`, `*migrations*`)?
- **Keyword alignment** — does the diff introduce names/APIs/imports the ADR explicitly discussed (libraries, services, config keys)?

Classify each ADR as:
- **Applicable** — at least one strong signal. Review in Step 4.
- **Possibly applicable** — weak signal only. Review in Step 4 but note the weak link.
- **Out of scope** — no signal. Skip; mention count in the report summary.

Record the applicable/possibly-applicable set and *why* each made the list. Include this in the final report's header so the user can sanity-check that nothing relevant was skipped.

### Step 4 — Choose execution strategy

- **1–2 applicable ADRs → Direct mode**: evaluate the change against each ADR inline, then proceed to Synthesis.
- **3+ applicable ADRs → Parallel mode**: spawn one subagent per ADR (or batch two ADRs per subagent if there are many), collect results, merge.

### Parallel Review Mode

Use this mode when 3 or more applicable ADRs are discovered.

#### Spawn subagents

Use `Agent(subagent_type="general-purpose")`. **Spawn all subagents in a single message** so they run in parallel.

Each subagent prompt MUST include:

1. The ADR file path(s) it is reviewing against — instruct it to read them in full
2. The diff scope (either the actual diff, or paths + a note to obtain the diff via `git diff <base>...HEAD`)
3. The **Finding Categories** section from this skill — copy it verbatim
4. The **Severity** section from this skill — copy it verbatim
5. The explicit instruction: **"You are reviewing the change only against the ADR(s) you were given. Do not flag issues unrelated to these ADRs. Do not grade code quality, tests, or style."**
6. The explicit instruction: **"Every finding must cite: (a) the ADR id, (b) the specific section or claim in the ADR the finding traces to, and (c) the file:line in the change. If you cannot supply all three, omit the finding."**
7. The explicit instruction: **"If the ADR is silent on the point the change touches, say so in the report summary rather than invent a rule."**

Instruct each subagent to return findings in this exact delimited format:

```
---FINDING---
priority: P<1|2|3>
category: <violation|erosion|drift|driver-shift>
adr: <ADR-NNNN>
adr_section: <which section of the ADR this traces to — e.g., "Decision Outcome", "Consequences > Negative", "Decision Drivers > #3">
location: <file:line(s) in the change>
title: <short title>
explanation: <what the change does, what the ADR asked for, why they conflict>
fix: <concrete prescription — revise the change, amend the ADR, or escalate for revisit>
done_when: <verifiable criterion>
---END---
```

If no findings, the subagent returns `---NO-FINDINGS---` plus a one-line note on why (either the ADR is not actually engaged by the change, or the change is on-policy)