--- name: review-the-pr description: "Reviews a pull request in three documentation-grounded passes (business correctness, regression safety, coding standards) plus a docs-sync check, and emits an evidence-dense, verdict-first report. Use when reviewing a PR or diff in any repo that keeps CLAUDE.md plus uc-*/domain-* skills and .claude/rules docs." disable-model-invocation: true --- # Review the PR ## Usage ``` /make-it-work:review-the-pr [--autopilot] ``` - **No argument** — interactive. Every checkpoint below pauses for a human, exactly as documented in Steps 0–1. - **`--autopilot`** — unattended. At every interactive point below, apply the Autopilot Mode policy instead of pausing. ## Autopilot Mode This skill uses no `AskUserQuestion` calls today — every interactive point is a plain-text prompt. Autopilot answers each one in place rather than introducing a tool call where none exists. **Resolution table** (one row per interactive site, in the order they appear): | Step | Site | `kind` | Autopilot resolution | |---|---|---|---| | Step 0 | "ask the user for the pull-request link" | `open_text` | If the invocation already includes a PR link, use it (log it as already provided — this is not a guess). Otherwise resolve to the skill's own documented **"No PR yet"** branch: proceed without one, note it, and cite files by commit hash only. Never invent or look up a PR URL — Step 0's "never substitute your own guess" rule is about not picking *which* PR the user means among several candidates; it does not forbid this fixed, non-destructive fallback the skill already defines for "no PR yet." | | Step 1 (Requirements) | "Is there a requirements file or ticket text I should review against?" | `open_text` | If requirements were already supplied in the invocation, use them (log as already provided). Otherwise answer "none provided" and let the skill's own documented fallback chain run unattended: attempt the ticket fetch (extract the ticket key from the branch name, fetch via whatever MCP issue-tracker tool is available in this environment); if that yields nothing, fall through to the branch-name/commit-message inference. Either way, the Coverage line's requirements-source field must say so, exactly as the interactive path already requires. | | Step 1 (Base) | Source/destination branch confirmation | `checkpoint` | Auto-confirm a deterministic guess — current branch as source, the repo's default branch (`main`/`master`, or whatever `git branch -r`/PR metadata points to) as destination — exactly as if a human had accepted both suggested branches. If the two resolve to the same branch (e.g. running `--autopilot` directly on the default branch with no feature branch checked out), this is not a confirmable pair — there is no diff to review. Stop and require a human rather than auto-confirming a vacuous empty-diff review. Otherwise, record the confirmed pair in the Coverage line as usual. | **Hard-stop exception:** no destructive action is flagged for this skill — autopilot never deletes, force-pushes, or discards anything. One caveat worth naming rather than silently assuming away: a re-run for the same ticket overwrites `make-it-work/{TICKET}/{TICKET}-review.md` from a prior run, in **both** interactive and autopilot mode — the interactive path has no overwrite prompt either (Step 7's Output format section just says "Save to..."). Autopilot therefore removes no human gate that existed before it. This file is a regenerable analysis artifact, not hand-curated source of truth (unlike, say, a plan file), so this is not treated as the hard-stop destructive-action exception — just documented here so it isn't mistaken for an oversight. One missing-input stop remains unconditional under `--autopilot`: the Step 1 (Base) branch pair resolving to the same branch on both sides, since there is no diff to review in that case — see the resolution table below. **Decision log:** write `make-it-work//review-the-pr-autopilot-log.jsonl` at repo root, overwritten fresh at the start of each autopilot run. Field shape follows the shared schema in `docs/autopilot-log-schema.md` — this skill never reaches `"askUserQuestion"`, so it omits `multiSelect`/`question`/`options` entirely; `kind` is only ever `"open_text"` or `"checkpoint"`, and `chosen` is therefore only ever the free-text answer (`open_text`), a short string describing what was confirmed (`checkpoint`), or `null` for a site that stopped rather than resolving. Every line also still includes `rationale` (one sentence), per the shared schema's core fields. `phase` examples: `"Step 0"`, `"Step 1"`. `site` examples: `"pr-link"`, `"requirements-source"`, `"branch-pair"`. When Bash is permitted, use the shared writer in `docs/autopilot-log-schema.md` with `--ticket ""` for this log, invoking each writer command in its own Bash tool call; preserve the start-of-run timing and never use it to bypass a denied log write. Resolve the reviewed repository's absolute root once at the start of the run (for example, `git rev-parse --show-toplevel`, after resolving Step 0). Keep that same absolute path as `` for `init`, every `append` or `write`, and final verification. Do not substitute `$PWD` or the current working directory in later writer commands: reviewing or editing the report may change directories, which would write a second log under the wrong `make-it-work/` directory. Log a line for every site above on every run, including a site skipped because its input was already supplied in the invocation — mark `chosen` accordingly (e.g. `"already provided in the invocation"`) so the log stays a complete, auditable record of the run rather than only the sites that needed a real decision. At the end of an autopilot run, verify that `/make-it-work//review-the-pr-autopilot-log.jsonl` contains valid JSONL entries for `pr-link`, `requirements-source`, and `branch-pair`, then print a short human-readable summary of those resolutions and that absolute log path. An empty log or a log found only under another directory is incomplete; do not claim the autopilot log was saved. Before that summary, verify that the required Tier 1 review file was actually saved at the path specified in Output format. A chat-only review is incomplete, including when the verdict is "Request changes" or no code edit is needed. If a write is denied, say that the durable review was not saved; do not claim the review deliverable is complete or bypass the denial. ## Overview Review changes in three passes, grounded in **this repo's documentation** — never from generic intuition alone: 1. **Business correctness** — validate the changed logic against the end-to-end use-case flows (`uc-*` skills), the Domain Validation Rules in `.claude/rules/product.md`, and the ticket's stated scope. 2. **Regression safety** — cross-reference the touched components against the functional domains (`domain-*` skills), the Architectural Constraints in `.claude/rules/architecture.md`, and every *other* use case that flows through the same code. 3. **Coding standards** — the Guidelines section of `CLAUDE.md` (already in context). A finding is reportable only if you can state a **concrete failure scenario** backed by evidence from the code (a specific input/state that produces a wrong output, a lost record, a wrong response sent to an external system, a misleading log). "This could be cleaner" is not a finding unless a `CLAUDE.md` guideline names it. This is production code that moves real money or data on behalf of real customers. Review with that blast radius in mind. ## Step 0 — Ask for the PR link first Before doing anything else — before fetching requirements, before running any git command — **ask the user for the pull-request link** unless the request already includes one. The PR link is the findings anchor: it pins the exact head commit and gives the output its Bitbucket/GitHub/GitLab source URL for inline citations. One question, then wait (autopilot: see Autopilot Mode): > "Please share the PR link for this review (or tell me there's no PR yet)." - **PR link provided** → extract the head commit hash and carry the link into the output. Branch verification happens in Step 1 regardless. - **No PR yet** → note it; the output will link to files by commit hash only. Move to Step 1. - Never substitute your own guess of "the PR they probably mean" for asking. ## Step 1 — Establish the diff 1. **Requirements** — the review's definition of *intended scope*, resolved in this order: 1. **User-provided (preferred)**: the user may hand you the requirements directly — a pasted ticket description, a ticket key, or a path to a requirements/refined-ticket markdown file. If the request doesn't include one, ask once: "Is there a requirements file or ticket text I should review against?" — then proceed with whatever the answer is (autopilot: see Autopilot Mode). 2. **Ticket fetch**: otherwise extract the ticket key from the branch name (e.g. `feature/TICKET-123-...` → `TICKET-123`) and fetch it using whatever MCP tool is available for this repo's issue tracker (Jira, Linear, GitHub Issues, etc.). 3. **Fallback**: if neither is available, derive intent from the branch name and commit messages, and state in the output that scope was inferred, not confirmed. Whichever source is used, record it in the Coverage line. Scope-creep findings (Step 3) are only as strong as this source — with user-provided requirements they are authoritative; with inferred scope, phrase them as questions to the author rather than verdicts. 2. **Base — always verify the branch pair before diffing**: the review is only as valid as the source/destination pair it diffs; a wrong base reports other people's merged work as this PR's changes and buries the real diff. **Always ask** — even when a PR link is provided — before reading any code (autopilot: see Autopilot Mode): > **Source (origin) branch**: `` / other (type below) > **Destination branch**: `` / other (type below) Derive the best guess from the PR metadata, branch name, and `git branch -r` output. Wait for confirmation before running `git diff ...HEAD`. 3. **Read whole files, not hunks.** `git diff --name-only ...HEAD`, then read each changed file in full at the reviewed commit. Findings frequently live in *unchanged* sibling code — a neighboring method that guards a case the new code misses, an interface with two similar fields, a writer that already handles what the new code re-handles. ## Step 2 — Load the skills that match the diff The `CLAUDE.md` skill-loading gate applies to reviews. Before judging any changed file, load **every** matching skill via the `Skill` tool. **How to find matching skills:** 1. Open `CLAUDE.md` and locate the skill-loading gate section — it contains a mapping table of code areas → skills. Match each changed file path against that table and load every skill listed for it. 2. If `CLAUDE.md` has no explicit table, fall back to heuristics: any `uc-*` skill whose name matches the feature area touched, and any `domain-*` skill whose description covers the changed component. 3. **Enumerate every UC that flows through the changed code — don't stop at the one the ticket names.** For each `domain-*` skill you loaded, read its list of the `uc-*` skills that use that domain (each domain skill enumerates them), and load **every** UC on that list. Cross-check that list against the User Journey Chains in `product.md` and the Functional Domains table in `architecture.md` so no flow is missed. This is especially critical when the diff touches shared infrastructure (services, utilities, models, enums, factories used by several flows): each of those UCs is a regression candidate, and you cannot spot the regression in Step 4 without its skill loaded here. 4. **Read `.claude/skills/test-strategy/SKILL.md` when it exists** — Test Map, How to Scan and Test Locations drive Step 4's mapping checks. Read `/../../docs/test-map-contract.md`; inspect freshness, reuse unchanged relationships and refresh affected scope in memory. Never save map/strategy changes or modify source/tests, including under implement. Missing/malformed maps under the current strategy use temporary discovery across all placements; annotations are not required. The touched UCs/domains come from the plan's Domains/UCs Touched when implement supplies a plan, otherwise from the diff and loaded skills. With no strategy or an older strategy without `## Test Map`, skip Step 4's two mapping checks, report mapping unavailable in Coverage and recommend define-test-strategy. Do not interpret tags or convert old rules. **What each skill type provides:** - `uc-*` skills → what the business flow must do; the Main/Alternative flows, invariants, and field mappings the changed code must preserve. - `domain-*` skills → what architectural invariants must survive; the data contracts, status machine rules, and cross-cutting constraints the changed component owns. ## Step 3 — Business-correctness pass For each loaded `uc-*` skill, walk the changed logic against the documented flow and ask: - **Does the diff alter a documented behavior or validation rule** (product.md "Domain Validation Rules", the UC's Main/Alternative flows)? If the ticket doesn't explicitly mandate that change, it's a **Critical** finding — silent business changes are the most damaging class of bug here. - **Paired logic stays paired**: decision methods and their reporting/reason methods must check the same fields; a check-side change without the report-side change (or vice versa) produces impossible or misleading states. - **Field provenance**: is the *right* field used? Interfaces often carry near-duplicate fields (different IDs for the same concept, address fields, amount fields). Verify against the UC skill's documented mapping before assuming a rename or substitution is safe. - **Enumerate the state space**: for each function or code path the diff changes, list the inputs and states it can receive — null or missing, empty, single, multiple, merged or combined, boundary values, and any state where an upstream data source may be absent or unreliable — and check the changed behavior against each one. An enumerated state is only a candidate; it becomes a finding when Step 7 verifies it. - **Scope**: anything in the diff the ticket doesn't ask for (new utilities, drive-by refactors) → flag under "Exact scope". Small and harmless → Minor; behavior-affecting → higher. ## Step 4 — Regression pass **Hard architectural constraints** — open `architecture.md` and locate its "Architectural Constraints" section (or equivalent). Each constraint listed there is a **Critical** finding if violated. Read them now and check the diff against each one. Then hunt for the quieter regressions: - **Every UC through the changed code**: walk the change against *each* `uc-*` skill enumerated in Step 2 (the UCs the touched functional domains list as their users), not only the ticket's UC. For each one, trace its documented flow through the modified code and ask whether the new behavior still satisfies that UC's invariants and field mappings. A change that is correct for the ticket's flow routinely breaks a sibling flow that shares the same service, model, or factory — this cross-reference is where those regressions surface. - **Callers**: for every modified function signature or behavior, grep all call sites — do they all tolerate the change? - **Single-row DB reads**: any query feeding one record (`result[0]`) needs a soft-delete filter and a deterministic `ORDER BY ... LIMIT 1`. Compare with sibling queries in the same file — the codebase usually has a correctly-guarded sibling to point at. - **Source migrations**: when a diff changes where a value comes from (one field/table/config → another), the new source's empty/null/format space is different from the old one's. Trace **every downstream consumer** of the migrated value and check what happens when the new source yields empty, null, or absent — especially sentinel helpers whose behavior flips between `""` and `null`. A value the old source guaranteed non-empty may now arrive empty and silently flip a business decision. - **Cache keys and scoping**: cached lookups must be keyed by everything that varies the result. - **Tests**: new logic branches without unit tests, and tests deleted alongside behavior that still exists. With a strategy skill loaded in Step 2, also flag: - A relevant added/changed test absent from the shared test map, or with a stale/unresolved relationship — Minor index-maintenance finding, for any placement. Use in-memory reassessment to explain current relevance without hiding missing/stale persistent metadata. Never request source/test tags. - A touched UC/domain left with no current evidence-backed test relationship after complete assessment across Test Locations — Minor. Inspect actual assertions and behavior evidence; shared-module ownership alone is insufficient. Keep incomplete/stale/uncertain scope unresolved in the Coverage line rather than claiming a confirmed gap. A relationship alone does not establish sufficient behavioral coverage. - Tests in other repos are checked by scan only. - **Off-checklist hunt**: the checks above are a floor, not a ceiling. After completing them, re-read the diff once more asking only *"what else goes wrong for real inputs?"* — new utilities and helpers deserve the same scrutiny as the flows that call them. Findings from this pass meet the same evidence bar as everything else. ## Step 5 — Coding-standards pass `CLAUDE.md` (already in context) is the primary source of truth for this repo's conventions — apply what **it** states, not a remembered checklist from any other project. Also check the Claude rule files it references (e.g. under `.claude/rules/`), since standards often live there too, not only in `CLAUDE.md`. Read `CLAUDE.md` and those rule files, and turn each stated convention into a concrete check against the diff. To make sure nothing is skipped, walk `CLAUDE.md` section by section rather than from memory. A standards doc commonly defines rules in categories such as: general engineering principles (scope discipline, DRY/KISS, readability), naming conventions, logging/observability, type safety, error handling, architectural layering, configuration/secrets, security, and testing. For **every such category your `CLAUDE.md` actually defines**, check the diff against the specific rule it gives — and ignore any category it doesn't mention. If `CLAUDE.md` names a rule this list doesn't, it still applies; if this list names one `CLAUDE.md` omits, drop it. Severity follows the same calibration as everywhere else: a violation that changes behavior or hides a defect ranks higher than cosmetic hygiene (naming, missing explicit types, import order), which is Minor unless it demonstrably conceals a bug in this diff. **Unresolved `plan-the-work` stub check.** If this repo uses `make-it-work`'s TDD verification pipeline (`plan-the-work` writes per-step test files, including stub signatures for not-yet-implemented code), grep the diff for the fixed marker `plan-the-work stub — pending implementation`. Any match reaching this PR is, by definition, an unfinished step that never got real implementation — flag it as a **Critical** finding naming the exact file/line, regardless of what other category it might also fall under: a stub reaching review means the branch was merged (or is being reviewed for merge) before `execute` finished the plan it came from. ## Step 6 — Docs-sync check `CLAUDE.md`'s "After Any Feature Change" section (or equivalent) makes skill/doc updates part of the change itself — review them like code: - **Changed behavior in an existing flow or domain** → the matching `uc-*` / `domain-*` skill file must be updated **in this same diff**. Missing update → **Major** finding naming the exact skill file and what it should now say. - **New end-to-end flow** (new route + service + data path) → a new `uc-*` skill must exist and be registered in `CLAUDE.md`'s UC table and `product.md`'s UC summary. **New service/provider/route/domain** → `architecture.md`'s tables updated. - **Included updates are content, not checkbox**: when the diff does update a skill file, verify the new text actually matches the code change — a skill updated to describe behavior the code doesn't implement (or vice versa) is worse than no update. Flag mismatches at the severity of the confusion they will cause. - **Pure refactors/bugfixes that change no documented behavior** need no doc update — don't flag noise. Record the outcome in the Coverage line (`docs-sync: uc-02, uc-05 updated ✓` / `docs-sync gap: uc-05 not updated for sourcing change`). ## Step 7 — Verify, then report Adversarially re-check every candidate finding before writing it down: 1. Re-read the exact code at the reviewed commit — does the failure scenario actually occur? 2. Is it introduced by this diff, or pre-existing? (`git blame` / diff base when unsure.) Pre-existing issues go in a short separate note, never in the numbered findings. 3. Do the file/line references match the reviewed commit? Drop anything that fails these checks. **Report every verified finding in the changed region in the same review** — finish the Step 3 state-space enumeration for each changed function or path and don't stop at the first Major; a latent gap that surfaces one finding per review cycle forces repeated fix rounds. A short list of confirmed findings is worth more than a long list of maybes — every false positive costs the reviewer's trust. ## Output format Read [the review renderer contract](../../docs/deterministic-artifacts.md#review-reports). After semantic verification, use required `scripts/review-report.mjs` to validate source locations and derive counts, verdict, report tiers and version-1 handoff from one findings object. Do not manually produce contradictory Markdown/JSON. Existing report shapes below describe the rendered output; legacy implement callers still retain their outcome-line behavior without a new handoff. Deliver the review in **two tiers** — a durable, pasteable report in a file, and a scannable summary in chat. Both lead with a verdict so the reader knows the outcome before reading a single finding. ### Tier 1 — Full report (file) Save to `make-it-work/{TICKET}/{TICKET}-review.md` (create the `make-it-work/{TICKET}/` directory at the repo root if it doesn't exist), or to the path the user specifies. PR-comment style — verdict first, then findings, no checklist boilerplate: The file write is a required step of every completed review. Save it before printing Tier 2, then confirm it exists; do not substitute the chat summary or a `ReportFindings` tool call for the report file. ```markdown ### Code review — {TICKET} (, ) **Verdict:** — **Solid:** Found N issues: 1. **[Critical]** . `path/to/file.ts:123` — Introduced by fix of: <#N | none> (implement-mode re-reviews only; omit otherwise) 2. **[Major]** ... **Pre-existing (not blocking)**: **Coverage**: diffed `...`; skills consulted: ; requirements source: ; docs-sync: . ``` When the reviewed commit hash is known, link each finding to the source file at that commit using the repo's hosting URL (e.g. `https://github.com/org/repo/blob//path/file.ts#L123` or the equivalent for Bitbucket/GitLab). ### Tier 2 — Chat summary Print a compact, scannable version in chat — the verdict, one line per finding (severity + claim + `file:line`), and a pointer to the full report. No evidence paragraphs; the reader opens the file for those. ``` Verdict: Request changes (1 Critical, 2 Major) 1. [Critical] — : 2. [Major] — : 3. [Major] — : → full report: make-it-work/{TICKET}/{TICKET}-review.md ``` In an implement-mode re-review, append `(introduced by fix of #N)` to a finding's line when its `Introduced by fix of:` field names one. When there are **no findings**, say so plainly in both tiers — `Verdict: Approve — no issues found` plus the Coverage line — and don't invent filler. **Severity**: - **Critical** — must fix before merge: wrong data sent to an external system, undocumented business-rule change, state-machine/constraint violation, security/PCI exposure. Critical requires that the impact *matters to the business* per product.md, the UC skills, or the requirements. When the impact hinges on a business tolerance documented nowhere, report it as a **question to the author** with a provisional severity — not a Critical verdict. - **Major** — should fix: real defect with a narrower blast radius, misleading logs/reasons/rejection reasons, lint suppressions, missing tests for new logic branches, unrelated changes that alter behavior. - **Minor** — polish: type hygiene (`any`, missing explicit types) unless it demonstrably hides a defect in this diff, typos in messages, log-format inconsistencies, import order, DRY suggestions, missing/stale shared-index entry for a relevant changed test, touched UC/domain with no evidence-backed test relationship after complete assessment. **Causal-chain citation**: when a finding's impact runs through *unchanged* code — an exception caught by a generic handler, a downstream consumer, a shared utility — cite the whole chain, not just the diff line: the changed line that introduces the behavior AND the unchanged handler/consumer where the consequence materializes (`file:line` for each). The author must be able to verify the claim without re-tracing it. **Do not flag**: pre-existing issues (separate note only), formatting the linter will fix automatically, intentional pattern-breaking in test mocks/stubs, style preferences no `CLAUDE.md` rule names, optimizations without a measured need. ## When run by implement **Workspace runs owned by implement:** when supplied `workspace_contract` (`workspace_version: 1`), read [the workspace contract](../implement/references/workspace.md) before detection or artifact writes. Its artifact root and approved repository IDs/roots/branches/bases override standalone layout detection, any requirement for root `CLAUDE.md`, and repo-root artifact assumptions. Run source/Git/test operations in the explicit named repo; keep workflow artifacts under the supplied artifact root. Assess only approved scope, including selected test repos. Return newly affected repos or required branch changes before edits there so the parent obtains user confirmation. Forward the contract to nested skills and delegated agents. This override does not change standalone or legacy single-repo behavior. **Activity ownership** — follow [implement's activity contract](../implement/references/activity.md). The parent session saves the independent-review description before dispatch and retains it while awaiting this report. This review subagent must not write workflow activity or dashboards; return the review result for the parent to checkpoint. This section applies only when `/make-it-work:implement` dispatches the review; a standalone review ignores it entirely and runs every step above as written. **Inputs** — the dispatch prompt provides: the ticket key, the spec path, the current plan path (absent in Lightweight mode, which has no plan), the base branch, the literal `no PR`, the review cycle number, the list of known unrelated regressions (may be empty), and the list of decided findings (may be empty). When the prompt also provides a verification-ledger path, read it with the plan's AC traceability table. Check whether each claimed automated/manual verification has actual step evidence and call out unverified criteria in the report's coverage discussion; do not infer AC coverage from a passing full suite alone. This is an evidence review, not an instruction to rerun tests. The absence of the optional path on an older run changes nothing. When the plan has `## Verification Policy`, review its per-repo choices alongside that evidence. For manual repos, check the concrete actions/observations and nearby regression scenarios; automated coverage remains unavailable even after successful manual verification. An unperformed manual scenario or a missing AC observation is a verification gap, not evidence of success. Mixed policies require both automated results and manual observations. Proposals in a strategy's `## Recommended Setup` are not installed tooling or executed tests. **Skipped prompts** — do not run Step 0 (PR link), and skip Step 1's requirements question and its branch-pair confirmation. The requirements source is the provided spec plus plan — or the spec alone when no plan is provided — treated as authoritative exactly like user-provided requirements in Step 1. **Diff to review** — the work is not committed and there is no PR, so review the working tree against the base: - Tracked changes: `git diff $(git merge-base HEAD)` — this covers committed and uncommitted work together. - Untracked new files: `git ls-files --others --exclude-standard -- . ':!make-it-work'` — read each one in full. - Exclude `make-it-work/` from everything above. - In the report header, label the reviewed commit as `working tree on `, and omit hosted-source links. **Unchanged steps** — Steps 2–5 and Step 7 apply as written, including Step 5's unresolved-stub check. **Docs-sync override (Step 6)** — `implement` updates skills and rules files in its own final context sync, after review. A missing or stale `uc-*`, `domain-*`, or rules-file update is therefore **not** a finding here: list each one under a `**Context gaps (for final sync):**` block in the report instead. A skill or rules update that *is* in the diff but contradicts the code is still a finding. **Known unrelated regressions** — list the ones `implement` passed in under a `**Known unrelated regressions (documented, out of scope):**` block, not as findings — unless the diff demonstrably caused one, in which case report it as a normal finding with that evidence. **Decided findings** — a `Route: human` question the user has already decided in an earlier cycle (passed in by `implement`) is settled: do not raise it again unless the diff has since changed the code it concerns. List each one under a `**Decided (not re-raised):**` block in the report. **Fix-cycle regression check** — when the review cycle number is above 1 and the plan carries an `**Amended for fix cycle ...**` marker, the diff includes fix steps added in response to earlier findings (the steps after the plan's pre-amendment step count, each naming the finding it fixes). For each such step, explicitly check whether it regressed its own code path or introduced a new defect in the region it changed — re-run the Step 3 state-space enumeration over that region, including the complement of any guard, reject, or filter the fix added. Every finding in a re-review then carries `Introduced by fix of: #N`, naming the earlier finding or fix step it descends from, or `none`. A finding with a value other than `none` is a repeat offender: say so in the finding so `plan-the-work` can recommend a replan instead of stacking another patch. With no plan (Lightweight mode), the review cycle number above 1 alone marks a re-review: the fixes are the changes made since the previous review, and the prompt supplies that review's findings and regions. Apply the same check to each region those findings named, and set `Introduced by fix of:` against those findings. A repeat offender there makes `implement` escalate to its TDD workflow. **Finding routes** — tag every numbered finding with exactly one route: - `Route: fix` — fixable within the current approach (includes an unresolved `plan-the-work` stub). - `Route: replan` — the approach itself, or an assumption the plan relies on, is wrong. With no plan, it means the change needs the planned, test-first workflow; `implement` escalates to it. - `Route: human` — needs a product, business, or architectural decision no document settles (the "question to the author" cases above). **Outcome line** — end both the Tier 1 report and the Tier 2 chat summary with exactly one line: ``` Orchestrator outcome: CLEAN | FIX_REQUIRED | REPLAN_REQUIRED | HUMAN_DECISION ``` - Verdict **Approve** or **Approve with nits** → `CLEAN`. - Verdict **Request changes** → `REPLAN_REQUIRED` if any finding is `Route: replan`; otherwise `HUMAN_DECISION` if any is `Route: human`; otherwise `FIX_REQUIRED`. **No questions** — never ask the user anything in this mode. Anything that would need a question becomes a `Route: human` finding. **Version 1 handoff** — when the caller's workflow state contains `handoff_version: 1`, also read `../implement/references/handoffs.md` and write a new `review-.json` under `make-it-work//-handoffs/`. Use the actual saved Tier 1 review report as the `report` artifact. Record the current `review_cycle`, each actionable finding's id, route, stable code region, and `Introduced by fix of:` value (`null` for `none`); include the context gaps for final sync. The Minor test-map findings from Step 4 stay out of `findings`, so a Minor-only review remains `CLEAN`. Do not write this file for an older run. End the Tier 2 chat summary with `Handoff: ` after the `Orchestrator outcome:` line so `implement` can validate and route it.