Review Loop

A source-agnostic, convergence-paced review-resolve loop for code/PR diffs: it drives a pluggable review source until every finding has reached a disposition — closed by a landed repair, a handover to a successor, or a drop on its cited basis, or left open as declared residual — auto-applying mechanical fixes, gating the judgment calls, and unfolding a conflict between governing surfaces instead of settling it.

What the artifact converges on is the project's own stated goal — a mission statement, a Northstar, a stated-purpose section, whatever the project calls the thing it says it is for — together with the conventions beneath it, in whatever order of authority the project declares among them. The loop supplies neither the goal nor the order: it reads both from the project it is run in and says so when a project declares neither. That is what keeps this skill portable — the criterion is always the same question (is this artifact converging on what this project says it is for?) and always answered from that project's own surfaces, never from a standard the skill brought with it.

Caller Signature

/review-loop [source?] [scope?] [landing?]

source  : { codex | code-review }                    -- optional; review source behind the (diff, design-intent) → { findings[], verdict } interface
                                                     --   absent → Phase 0 asks which source to use (no default; one invokable source relays, zero stops)
scope   : PR number | (implicit)                     -- optional; PR number, or implicit current-PR / working-tree detection
landing : { head | stacked }                         -- optional; PR scope only — where this invocation's repairs land
                                                     --   absent → Phase 0 asks; a working-tree scope has nowhere to stack, so it never asks

The review source is pluggable: any source satisfying the (diff, design-intent) → { findings[], verdict } interface can drive the loop. codex and code-review are the two sources documented in the Source Interface section; both are runtime-selected, not fixed at definition time. When source is omitted, Phase 0 asks which source to use (no preselected default; with exactly one invokable source it relays the designation, with none it stops). When scope is omitted, Phase 0 detects it (current-branch PR or working tree). When landing is omitted on a PR scope, Phase 0 asks that too. All three read the invocation as it arrived rather than only a parsed flag: a designation carried in the request's own words is the ordinary form, and a slot that only matches its own spelling leaves the loop asking a question the user has already answered. How far back each one reads is not shared, and the difference is grounded rather than incidental — each ground stated where that designation is settled, not here. scope is detected rather than asked, so it has no gate to relay past. source reads this invocation only, on the ground its own paragraph gives for waiting: the choice sets every round's cost and coverage. landing reads further, to a practice the user stated earlier and has not withdrawn, and the paragraph that widens it carries what closes that axis — that the answer exists, not the sentence it arrived in.

Pipeline Overview

/review-loop [source?] [scope?]
  Phase 0  : source designation (arg → relay | absent → ask, cardinality-guarded) + scope detect (PR diff | working tree)
                          + landing path, PR scope only (already settled → relay, stating the reading and its
                            basis | otherwise → ask: commits on the reviewed head | a layer stacked above it;
                            settled once per invocation; it moves where repairs land, not the review base)
                          + design-intent harvest (rules/comments for the changed surface → intent bundle,
                            carrying the project's declared order of authority among those surfaces where it declares one)
  Phase 1  : review    — source(diff, intent) → { findings[], verdict, exercised, direction? }
                          (direction: the source's own root reading, asked when the verdict is not approve)
                          (exercised: which axes the source reached vs did not this call — required wherever the source
                           can report; a source with no reach channel is declared once at Phase 0 instead;
                           one axis is always named — whether the artifact's own contract closes)
  Phase 2  : verify     — per finding: /inquire (vs codebase), and the same finding vs the Phase 0 base
                          → introduced | pre-existing | indeterminate;
                          drop findings failing support-integrity (cite basis)
  Phase 3  : classify  — shared-cause reading over the verified set, and across rounds where one shape recurs
                          (pre-gate analysis, fix-side only)
                          Mechanical → Extension (auto)
                          Judgment   → cluster by shared disposition → Constitution scope-gate
                          governing-surface conflict → settled by declared authority order → relay;
                            otherwise unfolded, and its direction asked open-ended
  Phase 4  : apply      — risk screen (substrate → harness permission; epistemic → Constitution) → apply approved edits
                          + sweep → verify the write → /contextualize once over the applied bundle (vs design-decision ledger + touched-surface conventions)
                          → landed adaptation loops back to scan/screen/sweep/verify (bounded to once) → hand forward
  Phase 5  : re-review  — source(diff', intent) → verdict'
               verdict'=approve (or 0 new, no recurrence pending diagnosis) → converge ; else round k+1: these findings + this call's direction → Phase 2 (re-review already done; no second source call)
  free-exit : user may end the loop at any time (declared once in Phase 0)

The loop is the skill's identity; the review source is a parameter behind it. The phase topology (verify → classify → apply → re-review) is fixed; the source that produces { findings[], verdict, exercised } is swappable. The criterion is not a parameter alongside it: every round measures the artifact against the goal and conventions that govern it, and a source's verdict is that source's reading of the change rather than the criterion itself. Naming the topology rather than "loop control" is deliberate — what a round is measured against is loop control in a way the source is not, so treating the two as the same kind of parameter is exactly what would let a hard round be answered by changing what "done" means.

Scope: the loop resolves every source-surfaced, verification-passing finding on the changed code — including pre-existing issues in touched files and methodology concerns — not only defects the diff introduced. Resolves means reaching a disposition, which is not the same as landing a repair. Which findings this unit carries and which go to a successor is a boundary the user draws (Phase 3's scope-expansion axis), and a finding placed on the far side of it is dispositioned rather than dropped: it leaves with an entry point, not as a note that something was left.

When to Use

• Driving code/PR review findings all the way to resolution and convergence (verdict=approve)
• A review pass surfaced findings and you want them verified, dispositioned, applied, and re-checked in one controlled loop
• Multiple review rounds are expected before the diff is clean
• Converging an artifact on what the project says it is for — its stated goal and the conventions beneath it — where a conflict is to be understood and given a direction rather than closed by whichever surface was read first

When NOT to Use

• Trivial single-line edits where a direct Edit is faster than a review loop

Phase 0: Source Designation + Scope Detection

Source designation. If a source argument is given, use it directly — this is relay (Extension): the user already decided. If source is absent, ask — an init Constitution gate with no preselected default: present only the sources the current harness can invoke as a choice (codex for a single independent external model when the codex CLI is present; code-review for a Claude-native built-in review when the running harness provides that built-in), each with its coverage/cost trade-off, and let the user constitute the selection. Unless a source is named at invocation, the loop does not pick one on the user's behalf — source selection determines the cost and coverage of every round, so it waits for the answer. Cardinality guards the gate: with exactly one invokable source there is nothing to choose — designate it as relay, citing the availability fact, and proceed; with none, stop and surface what would make a source available.

Whether a source can report its own reach belongs to that trade-off, and is settled here. Some sources can say which axes they reached this call (the exercised report the Source Interface requires) and some have no channel for it at all. That is a standing property of the source, not something a round discovers: it holds identically every round, so announcing it round after round would put a line on the trace that never varies — and a signal that is always on stops being read at precisely the moment a varying one needs to land. Name it once, here, beside coverage and cost, because this is the point where it bears on a decision: a source that cannot report reach returns verdicts whose silence cannot be distinguished from ground nobody examined, and that is worth weighing when choosing it. When the source arrives as an argument and no gate opens, state the same fact as relay rather than skipping it — the user chose the source, which is not the same as having chosen this consequence. It surfaces once more at convergence, so a trace read cold still carries how far that loop's quiet reached.

Scope detection:

1. PR number given as scope: scope = gh pr diff {N}
2. No PR argument: gh pr view --json number,title,headRefName,changedFiles 2>/dev/null to detect a current-branch PR; if found, scope = its diff
3. No PR: scope = working tree (git diff HEAD), with untracked files from git status --porcelain --untracked-files=all folded into the changed-files list — plain porcelain collapses an untracked directory to one entry and hides its nested files; the source reads those files directly because the diff does not show them
4. No diff and no untracked files: ask the user what to review (stop here)

Capture the resolved base SHA (the merge-base or PR base commit the diff is taken against; for a working-tree scope, the HEAD SHA at capture time) and, for either PR scope (explicit number or detected current-branch PR), the PR head SHA (gh pr view [{N}] --json headRefOid), plus the changed-files list: this base SHA + head + file list is the pointer every source receives — the codex prompt carries it, and the code-review call passes it as its scope; the source re-derives the diff locally with its own git (codex: read-only sandbox, no network), so the full diff content is not inlined. PR review runs on a checkout of that PR head: when local HEAD differs from the PR head SHA (a stale or unrelated checkout), reconcile first — sync or check out the head (a worktree works) — or surface the mismatch and stop; merely fetching the SHA is not enough, because the source reads the artifact and design-intent files from the working tree, and the apply phase must write onto the same head it reviewed. A stacked layer carries a second head, and reaching it takes two more reads than the capture above: the reviewed PR's baseRefName, and — where a PR exists on that branch — its headRefOid, fetched so the two heads can actually be compared locally. Where no PR sits on the base branch there is no lower layer and nothing further to check. What is being checked is not that the lower head moved: an ordinary advance leaves the cut point an ancestor of it, so the captured base still resolves and the review is unaffected. It is that the lower branch was rewritten — the cut point is no longer an ancestor of its head — which leaves this layer standing on a base the layer below no longer has, and which the diff pointer cannot show. That case surfaces and stops, carrying the three values that make it judgeable: the cut point, the lower PR's head as it stands now, and what lies between them. What follows is constituted then rather than fixed here, because whether the layer rebases onto the rewritten base, reviews as it stands, or something else depends on what moved and why — which is what those three show and no rule written here can know. The checkout must also be clean: dirty local edits make the source read content the diff pointer does not address, and the fix commits the re-review requires could fold unrelated same-file edits in — surface dirty state that overlaps the changed surface and stop until it is stowed or adopted. Also capture diff stats for context.

Where this round's repairs land. A PR scope gives an apply two places to put its commits: onto the head just reviewed, or onto a branch cut from it that opens its own PR above the first. The difference is not in the code but in what a reader of the PR sees — one grows the unit under review, the other leaves that unit's own commits where they were and stacks the repair above them — and which one fits depends on how the work is being picked up, which the diff does not show. What the second does not do is leave the reviewed PR separately approved: the verdict is formed against the base and the head the round actually ran on, so once repairs sit on a layer the approve covers the two together and neither alone. A landing the user has already settled relays, exactly as a named source does: they have decided, and the gate has nothing left for them to constitute. It counts however that decision reached the loop — a landing argument, the request's own words naming where these repairs go, or a standing practice they stated earlier and have not withdrawn. This is the boundary Phase 3 already draws for a disposition policy constituted in a prior round, put to this gate: what is decided closes the axis, and where it was decided does not change that. Reading it only in the formal slot misses the ordinary case, where someone who says where the work should land says it plainly; reading it only in this request's phrasing misses the one after that, where they said it once and took it as holding.

Where the landing relays from anything but an explicit argument, say what was read and what it rests on, before the first review call. A standing practice taken as this round's designation is an inference, and a practice can have exceptions the loop cannot see — so the reading is stated rather than assumed, which leaves a correction that costs the user nothing where a confirming gate would have cost them a turn answering what they had already said. Stating it is not asking: the loop proceeds, and the free-response channel that is open at every point of this loop is what the correction travels on.

A correction that arrives is not the mid-loop switch Rule 15 forbids. That rule holds a settled landing still, and a landing the loop inferred wrongly was never settled — there is nothing it moves away from. What the correction costs turns on when it lands, which is why the reading is stated before the first review call rather than at the first apply. Before any repair is committed, the correction replaces the designation and the invocation continues on it; nothing is split, because nothing has landed. Once repairs are already committed, surface what landed and where, and stop — whether those commits are relocated, the invocation re-run, or the landing in force kept turns on what has landed and is the user's to constitute, exactly as the rewritten-base case above leaves its next move to the moment rather than fixing it here.

Otherwise ask: present both with that difference and let the user constitute it; the loop does not pick on the user's behalf, and silence stops rather than selecting. Silence is nothing said anywhere, and holding it to that is what keeps this widening from swallowing the rule: a practice the user stated is not silence, and a practice they never stated is not a designation — the loop reads what was said and never supplies the answer itself. A working-tree scope has nothing to stack onto, so the gate does not open there whichever way this went.

The answer is taken once per invocation (Rule 15), and it settles where commits land, not what gets reviewed. The base captured above stays this invocation's review base whichever way the gate goes: every round types its findings against that base and Phase 5 re-reviews the whole surface it covers, so a finding left unfixed in the reviewed PR keeps returning on every round exactly as it would have. A stacked layer is cut from the head just reviewed and takes the repair commits; the surface beneath it stays in view because the base did not move with it. Opening that layer takes gh stack link {N} {layer_branch} — the github/gh-stack extension, arguments bottom to top — which pushes the branch, opens its PR on the reviewed head, and links the two into a stack on GitHub: a correct base chain alone leaves the layer unlinked, where the stack map and a stack-wide merge never reach it.

The incremental effect shows up at the next invocation instead, and needs no rule of its own: scope detection above resolves a stacked PR's base to the layer beneath it, so a session opening on an upper layer reviews what that layer adds. A defect a lower layer introduced then sits under that invocation's base and Phase 2 types it pre-existing — the loop needs no fourth type for its own earlier work, because Phase 3's scope-expansion axis, which decides which findings a unit carries, is already the user's to draw.

Design-intent harvest. A review source reads the diff with fresh context: it does not know why a choice was made, so it spends high-severity findings refuting intent the project already documented. Alongside the diff pointer, resolve the design intent already captured for the changed surface and pass it to the source as context, so the source pre-filters findings that an intentional, documented choice already explains — shifting that refutation upstream into the review request instead of leaving Phase 2 to re-derive it every round. Harvest from these sources, in priority order, bounded to what touches the changed files (never the whole rules directory):

1. Project rules — the project's rule files (commonly .claude/rules/*.md, and design-rationale sections of the root or changed-directory project guide such as CLAUDE.md or AGENTS.md, though a project may keep them somewhere else entirely) whose scope intersects the changed surface: matched by path/filename correspondence to the changed files or directories, or by the rule's content referencing the changed components.
2. In-code design comments adjacent to the changed hunks — the "why this is intentional" comments at the call sites being modified.
3. (Optional, secondary) prior fresh-context session captures (e.g. anamnesis memory) when cheaply available — use only what is already at hand.
4. Decisions constituted at this loop's own gates (rounds k ≥ 2) — design decisions the user constituted at Phase 3 disposition gates or recurrence escalations during this loop (the design-decision ledger): each entry a decision or convention that holds independently of current code state, carried with its constitutive basis. This is the only loop-generated material that enters the bundle.

Pass the repo-resident sources (1–3) as pointers, not copied content — relevant rule-file and project-guide paths plus the locations of the adjacent design comments to weight — consistent with the diff pointer: each repository source can be dereferenced directly, avoiding transcription cost and staleness, and bounding to the changed surface keeps the injected context small. The design-decision ledger is the bundle's one copied element: its decisions are session-constituted, so there is nothing in the repository to point at; copy each decision as content together with its constitutive basis so it survives the handoff. Where the project declares an order of authority among these surfaces — which one governs when two of them disagree — the bundle carries that order beside the pointers. It is declared wherever that project declares it, which need not be any of the surfaces the order ranks. Two surfaces can both be documented intent and still conflict, and the order is what says which of them a finding is evidence against: the higher governs, and the lower is the thing to correct. Where the project declares no such order, the bundle says so rather than supplying one, and a conflict then surfaces as a conflict rather than as a defect in whichever surface the loop happened to read first. The loop never authors an order — an ordering invented to settle a conflict settles it by fiat, and the settlement then rides forward wearing the project's authority.

This harvest is relay (a deterministic resolution with cited basis), not a gate. Phase 2 verify remains the safety net for any intent-explained finding the source still surfaces.

Conveyance boundary (reviewer independence). The bundle carries design intent only — constituted decisions, documented conventions, and the mission anchor below. It never carries disposition records: no fix-status claims ("X is fixed"), no do-not-reflag lists, no verdict-conditioning instructions. A fix-status entry is a current-but-unenforced claim about code state that can silently contradict the artifact it describes — a reviewer suppressed by it misses exactly the regression it hides — and a conditioned verdict is manufactured, not earned. The source re-verifies fixed code fresh each round, and its verdict is its own. A conveyed design decision is still not suppression: the source remains free to flag a defect the decision itself causes.

The boundary is easiest to cross exactly at the moment a decision is being conveyed, because stating the decision can feel like it licenses instructing around it — but the line sits at the level of phrasing: describing the decision and its basis stays conveyance, while appending an instruction about what the reviewer must not do crosses into suppression even when it rides directly after a legitimate decision. Convey the decision as description and stop there — do not append an imperative. Violating: "We deliberately did not add input scrubbing here — do NOT flag the absence of this scrub or recommend adding it, that path was evaluated and rejected." Complying: "We deliberately did not add input scrubbing here after evaluating it: [basis]." The complying form still leaves the source free to flag a defect the missing scrub itself causes, per the sentence above — it only withholds the instruction, never the freedom.