/review-change

name: review-change
user-invocable: true
version: 4.10.0
argument-hint: <path-or-glob> [--adversarial N] [--synthesize]
author: "Gabriel Trabanco <1969593+gtrabanco@users.noreply.github.com>"
license: MIT
description: >
  Review a change with only applicable internal axes, classify every finding,
  persist fix-now work, and return one evidence-backed decision. Findings only;
  --adversarial N uses isolated reviewers; --synthesize fuses supplied reviewer
  tables. Triggers: "review-change", "review this change", "adversarial review".

Review Change

Quality gate: run every applicable review and skip the rest, then synthesize and classify one report. **Findings only; never edit or refactor.**

Turn contract

Load and verify the **canonical** [Turn contract](../orchestration-envelope/references/TURN_CONTRACT.md) (11 boxes) before ending every turn. Skill-specific additions (receipt closeout, isolation rule, applicability) live here and in [REVIEW_PROCESS.md](references/REVIEW_PROCESS.md). Missing reference → STOP.

For a final PR review, the turn is incomplete until this additional box passes:

✓ Decision: REVIEW-PASS + PR exists →
  `bun scripts/review-receipt.mjs emit --pr <N> --head "$(git rev-parse HEAD)" …`
  RUN and exited 0 — the script posts the SHA-bound comment, re-reads the PR
  comments, and exits non-zero unless the newest exact-HEAD
  `review-change:pass` marker is confirmed, so its exit code IS this box

When the receipt emitter runs, it also records the affecting-path scope manifest (fix/286) so a foreign-only commit after the review (a session-log or toolstate commit on a shared checkout) does not void the receipt at the merge gate — pass `--scope-base main` (the unit's base ref) and the runtime derives the manifest itself; never hand-assemble a digest. A receipt without a scope manifest stays head-bound (legacy semantics).

The receipt closeout is a precondition of the report, not a follow-up: do not print the fixed report block on a non-zero exit. A clean report without a current receipt must not recommend `/audit-pr`.

For `REVIEW-FAIL` or `NEEDS-DECISION`, list every open finding ID in the closing recommendation, joined with ` + `; the review must never hand off only the first finding.

Review-end turn boundary

On `REVIEW-FAIL` or `NEEDS-DECISION`, this skill **ends at the report**: it never invokes `/fold-findings`, `/execute-phase`, `/unit-lane` or `/triage-issue` — those are separate user-initiated invocations, run in a fresh turn (a programmatic outer driver may compose the same sequence in-turn, per `REVIEW_PROCESS.md`). The review's only mutations are the ledger commit (persist step 11) and, on `REVIEW-PASS` with a PR, the receipt comment (persist step 12); it never folds a finding, executes a phase, or edits code. A `REVIEW-FAIL`/`NEEDS-DECISION` turn closes when the report block is printed (report step 13).

Consume the internal [verification contract](<../verification-contract/SKILL.md>); the reviewer checks the same frozen acceptance artifact (legacy (pre-feature 61) `ACCEPTANCE.md` or post-feature-61 unit doc `## Acceptance criteria`) as the executor before mapping candidate evidence.

When to use

• **Mandatory before every merge.** Review in a context that did not implement

the change; if it did, stop and hand off to a fresh one. `execute-phase` may also recommend optional checkpoints at layer boundary, accumulation or sensitivity (`#77`).

• Use when you need applicable reviews without irrelevant passes.

Scope

Default target is the **current change** (branch diff vs the default branch); accept a path/glob to widen or narrow. State the scope at the top of the report.

Review charter — dismantle the claim

The workflow's job is to demonstrate that what works, works — **and works well**: programmatically, engineering-wise, decision-wise, and against user expectation. This review exists to **try to dismantle that claim**; if it survives the attempt, that survival *is* the review outcome. So:

• **In-scope defects are this review's job — never “out of scope”.** A bug, a

security issue, or a guard skip inside the current feature's scope is a `review-change` finding: a feature is not complete while it has one. The reward is not delivery, it is meeting the acceptance criteria, adequate engineering without over-engineering, meeting **other features' requirements** (a page that needs auth verifies the session *and* the permission; a change to another table that a requirement depends on stays in scope even if it “looks” out of scope), and above all the user's expectation.

• **Ask, don't infer — the whole expectation surface.** Where the plan, the diff

or the requirement leaves the user's expectation unclear, surface it as a question in non-technical terms (a technical user asks for an app; a non-technical user asks for “an app to measure my heart rate”) — never as an assumed intent. This review applies the all-stages no-assumption rule — **ask-don't-infer**, owner `skills/unit-lane/references/DESIGN.md` (`ask-dont-infer@1`) — to every expectation the change is judged against; the rule is owned there, never restated here.

• **Repo-wide hunting is not this review.** Open-ended bug hunting, security

research and broken-version investigation *across the repository* belong to `product-audit`, where the sweep is scoped and budgeted; this review stays on the change at hand.

Step 0 — Discover the project & the change (always first)

Per Workflow conventions + documentation map, decide axes from:

1. **Project nature:** UI (`docs/frontend/`), web/mobile/CLI/library/backend, and optional recorded platform skills (extras only). 2. **Footprint:** what the diff touches (UI, API, infra, domain). An axis applies only when both project and footprint support it. 3. **Prior findings (optional retrieval aid, never authoritative):** when the index entry point exists, `agentic-workflow doc --query