renzrollon/interlock/review-code

Review a diff from several expert angles at once, then attack your own findings before reporting them.

The second half is the point. An unverified review reports everything it notices, so the reader learns to skim it. A review where every blocker survived two skeptics is a review worth reading line by line.

1. Scope the diff

Resolve the change and the file set:

interlock changes                        # active OpenSpec changes
git diff --name-only main...HEAD        # or the appropriate base; call it <base> below
interlock surface --changed <files> --json

--base <ref> names the base: when it is given, use it as <base> here and on every --base below instead of choosing one. --out <file> names a file the verdict is written to (§3). Neither changes what is reviewed or how it is judged.

--carry <file> names an earlier review record of the same pull request, forwarded verbatim to the interlock review line that carries --out (§3), and meaningful only together with --dimensions: interlock review appends to this review's surviving every finding of that record from a dimension this review did not run, marked carried, and exits 1 naming both flags when --dimensions is missing. The skill never invents it; the settle loop passes it on a scoped re-review.

--strict selects the strict review depth. Without it the depth is the CLI's published default, which this skill never names; interlock routing --json (§2) reports the depth that applies and what it runs. Do not add --strict because the diff looks large.

interlock surface tells you whether the devops dimension is warranted (needsDevopsReview). Use it — do not re-derive that judgement in prose.

If there are no changed files, stop and say so. A review of an empty diff is not a clean review.

Read the OpenSpec artifacts for the change before reviewing: proposal.md, design.md, tasks.md, specs/**/*.md. Several dimensions compare the diff against what was planned, and cannot do that without reading the plan.

Repository policy. If the repo has a root REVIEW.md, its prose reaches every reviewer inside the prompt interlock review brief prints (§2), quoted as advice that does not override the rubric or the evidence gate; this skill adds none of it by hand. Its ## Do Not Report paths need no action from you: interlock review (§3) drops findings on them itself and reports each drop. With --base, the path lists are read from the base ref, not from your working copy, so a change cannot edit the exclusions it is judged under. Its ## Always Human Review paths are not a review input at all — the merge verdict reads them. A copyable `REVIEW.template.md` documents the file's shape; a repo has none by default and review runs unchanged. interlock review-policy --base <base> --json prints the parsed file when you want to see it.

The graph must hold the diff. Before fanning out, check that the codebase graph covers the changed files, and update it when it does not:

git diff --name-only <base>...HEAD | interlock-graph covers --changed - --update

On exit 3 there is no graph: run no blast-radius reviewer, print the BLAST RADIUS SKIPPED line (§2), and build nothing. When it still exits 1 after its own update, print GRAPH STALE: <the files it lists>, run blast radius on the graph it has, and record blast-radius under degraded in the dimension record (§3) with those files as its reason. The retry is the CLI's; do not run the update again.

2. Fan out the dimensions

Domain context. Before fanning out, ask which living requirements the diff is judged against:

interlock domain-context <change> --changed <the changed files> --base <base> --json

The domain reviewer's prompt carries that text itself (below). If partial is set, run the reviewer and print a DOMAIN REVIEW PARTIAL: <reason> line; when omitted is non-empty, record domain under degraded in the dimension record (§3) with that reason, because the cap, not the reviewer, left those requirements out. If skipped is set, run no domain reviewer and print the DOMAIN REVIEW SKIPPED: <reason> line in your report. If nothing was selected and nothing skipped, run no domain reviewer and say DOMAIN NOT SELECTED with its reason. A domain reviewer given no requirement text is a reviewer making things up. When the output carries CHANGE READ FROM ARCHIVE:, print that line.

Routing. Before the first spawn, run interlock routing --json once — add --depth strict under --strict — and make no per-role routing call. Read from that one answer: roles.review-dimension is the model and effort of every dimension reviewer, roles.review-skeptic is the model and effort of every skeptic (§3), review.dimensions is the depth's always-on dimension set, and review.skeptics is how the skeptics are spawned. Pass the role's model and effort on every Agent call of that role; a null means omit that parameter, and the host's default applies. Print every line of banners verbatim before the fan-out. When the command exits non-zero, print ROUTING UNAVAILABLE: <first line of its output> and fall back to the strict shape at the session defaults: the strict depth's always-on dimensions, two skeptics per blocker and warning (§3), and no model or effort on any call. Routing is a cost hint, never a gate; a failed lookup does not stop the review and does not choose a cheaper one. Name no model and no effort level anywhere in this skill; the CLI publishes them.

The depth line. Once the set is chosen — the always-on dimensions from the routing answer, plus every row of the table below whose trigger the diff met, or the names in --dimensions — and before the first dimension spawn, print one line: REVIEW DEPTH: <depth> (<n> dimensions: <a> always-on + <t> triggered). <depth> is the answer's depth, <a> the length of review.dimensions, <t> the triggered dimensions that will run, and <n> their sum. Under --dimensions print REVIEW DEPTH: <depth> (<n> dimensions: --dimensions) with <n> the names it lists. After ROUTING UNAVAILABLE print the same line with strict as the depth. Count the dimensions that will run: a skipped blast-radius or domain is not counted.

Spawn one agent per dimension, in parallel. The always-on set is the depth's review.dimensions from the routing answer; the other rows keep their own triggers at either depth. Each dimension's prompt is printed by the CLI: run, for each dimension,

interlock review brief --dimension <name> --base <base> --change <change> --changed <the changed files>

and spawn the agent with the printed prompt as its whole prompt. Add nothing to it and leave nothing out: it holds the bundled rubric, the scope with the changed line ranges, the repository's review policy and review context as quoted data, the domain text for domain, the run-and-measure and proof rules, and the shape to return. A skill's allowed-tools do not reach an agent it spawns; the printed prompt's write-nothing line is the bound.

Blast radius needs the graph. When interlock-graph covers (§1) exits 3, there is no graph: run no blast-radius reviewer and print BLAST RADIUS SKIPPED: no graph at .claude/graph/graph.json — run /interlock:graph. The other dimensions still run, and the skip is written to the dimension record under skipped with its reason (§3), so it reaches the verdict as a flag and not only as a line in this session.

--dimensions a,b,c overrides the selection. The names above are the rubric filenames, and they are the only names. A name that resolves to no brief file is an error: say which name did not resolve, list the nine, and stop. Do not silently run the dimensions that did match — a caller who asked for four dimensions and got three, with no message, has been told the review covered something it did not. A blast-radius named in --dimensions without a graph is not silently run and not silently dropped either: it is reported with the BLAST RADIUS SKIPPED line above and recorded under skipped, and the review names every dimension it did run.

(This used to bite: the fourth always-on dimension was called delivery here and in README.md while its brief file was technical-lead.md, so --dimensions delivery matched nothing at all and said nothing about it.)

A dimension whose brief the CLI cannot print (interlock review brief exits 1) is not dispatched with an empty rubric either. Report it as unavailable — a reviewer given a dimension name and no criteria produces findings that two skeptics then spend their budget verifying.

Every dimension agent returns findings in this shape:

{ "dimension": "security",
  "findings": [
    { "severity": "blocker|warning|suggestion",
      "file": "path/to/file.ts", "line": 42,
      "title": "short claim",
      "description": "what is wrong and why",
      "suggestion": "how to fix it",
      "requirement": "openspec/specs/<capability>/spec.md#<requirement heading>",
      "specGap": true } ],
  "coverage": {
    "checked": [ { "what": "what was examined", "at": "path/to/file.ts:40-58" } ],
    "unread": [ { "input": "what was not read", "reason": "why" } ] } }

severity, file, title and description are required; requirement is optional and only the domain dimension uses it — a domain blocker carries one, copied from the requirement text it was given. specGap is an optional boolean that only the domain and technical-lead dimensions set, and only when the root cause is a requirement no spec stated; absent means false. coverage is what proves the review looked: an answer without it is recorded unproven. A finding with no file is allowed — it lands in the unscoped group — but it is rarely a good finding.

This is the complete emit shape. Do not read lib/findings.mjs to rediscover it.

3. Adversarial verification

Every blocker and warning gets two independent skeptics. Suggestions pass through unverified — they are cheap to ignore and not worth the tokens.

Run both skeptics per finding, in parallel.

How the skeptics are spawned follows review.skeptics from the routing answer, and the CLI prints one prompt per agent for it:

interlock review brief --skeptics --findings <f> [--depth strict] --json

[--depth strict] is a placeholder: write --depth strict there under --strict, and nothing otherwise. Spawn one skeptic agent per entry of prompts, each with that entry's prompt as its whole prompt, and collect one verdict per id in its findingIds into the verdicts file. Add nothing to a prompt.

When it is per-review, the CLI prints two prompts for the whole review: each skeptic is given every blocker and warning, grouped by file, returns one verdict per finding, and is told that overlapping findings are one issue, judged once. Every finding still carries two verdicts, one from each agent.