review-pass
Thin orchestrator over the review stages of the canonical chain. It does NOT re-implement any check — each sub-skill owns its own logic and its own skip rules. This skill only scopes the diff, orders the stages, drives them, and merges their output into one verdict. Point it at a change that's already written; it decides nothing about how the change was built.
Executor note: this is a prompt, not a script. Drive one stage via the Skill tool, wait for it to finish, checkpoint, then start the next. Never fire all four blind.
Protocol
Scope — determine what's under review. Default: the local working-tree diff (staged + unstaged) vs the merge-base. Accept an explicit target (--staged, a commit range, a path). From the diff note two things that drive the plan: (a) does it have runtime surface (product source, not docs/test-only) — gates the live-run check (/run); (b) does it touch a trust boundary (auth/authz, input parsing, secrets, file upload, external calls, SQL) — gates security-audit.
Harvest existing review — if a PR exists for this branch, check what already reviewed it before running anything. gh pr view <n> --json reviews plus the inline comments; see references/automated-reviewers.md for the discovery command and, importantly, for why you must discriminate bots by user.type rather than by an [bot] login suffix. Two outcomes matter: findings a bot already posted go into the merged verdict with attribution instead of being rediscovered by a stage, and a configured reviewer that is paused, absent, or stale is itself worth reporting. Skip when there's no PR — a local-only diff has nothing to harvest.
Print the plan — the four stages, each marked RUN or SKIP with a one-line reason (table below). Never skip silently. Note anything the harvest already covered.
Confirm — user says go / edit / abort. Accept partial edits ("skip the run check", "add security-audit"). Skip this step when an orchestrator drove you here — dev-flow runs this as one chain stage and already had the user approve the whole chain; a second confirm inside an approved run is friction, not safety. Print the plan as a notice and proceed.
Drive — invoke each RUN stage via the Skill tool, in order. After each returns, checkpoint: report its findings (blockers vs nits) in one line, then start the next. A blocker from an early stage doesn't halt the pass — collect everything, decide at the end.
Verdict — merge all findings into ONE list, deduped, ranked blocker → nit. Bot findings from step 2 merge in here too, attributed to their source; where you disagree with one, say so and why — a reasoned disagreement is worth more to the reader than either verdict alone. Close with a go/no-go: ship (no blockers), fix-first (blockers listed), or needs-a-human (findings you can't adjudicate). This consolidated verdict is the whole point of the skill — don't just concatenate the sub-skill outputs.
Act — if there are actionable findings, ask how to apply them (skip this if the verdict is clean; when orchestrator-driven, a clean verdict returns straight to the conductor with no prompt):
- Post as PR comments — re-run
/code-review --comment so findings land as inline comments on the PR. Only offer this when a PR actually exists for the branch; if none does, say so and drop the option.
- Fix directly — re-run
/code-review --fix to apply the findings to the working tree. Then re-verify the touched paths.
- Just report — leave the verdict as-is; the user handles it.
Don't pick for the user — ask, then do exactly what they choose.
Stages
Run in this order — cheapest signal and hardest gate first.
Depth. Default runs all four gates. quick runs stages 1–2 only, and exists for one case: a single-file fix with no plan, where implementation-review's plan and coverage checks have nothing to read against and code-review already covers a diff that small. dev-flow passes quick on its bug tier and the default everywhere else. Anything beyond a one-file fix uses the default — when unsure, use the default.
| Stage |
Order |
Gate (RUN only when true) |
/run (live check) |
1 |
Diff has runtime surface. SKIP for docs/test-only diffs — nothing to drive. |
| code-review |
2 |
Anything beyond a trivial one-liner. Hunts correctness bugs in the diff. |
| implementation-review |
3 |
Always at default depth, for a real change. SKIP at quick. Seven parallel checks; its check 7 already runs security-audit, so this covers the baseline security pass. |
| security-audit |
4 |
SKIP by default — implementation-review already ran it. RUN standalone ONLY when the change sits squarely on a trust boundary and you want the full WSTG pass, not the summary check. On a plain UI/CRUD diff this is pure double-pay. |
Rules
- Orchestrate, don't decide inside a stage. Once a sub-skill is invoked, follow its instructions — don't second-guess its internals.
- This skill owns the review gates. dev-flow delegates its entire review tail here and deliberately does not restate these gates. They were duplicated in both files once and drifted — dev-flow said "SKIP implementation-review on the bug tier" while this file said "always". A gate policy changes here, and only here.
- Never skip silently. Print SKIP + reason so the user can override.
- Don't double-pay security. implementation-review → security-audit is redundant unless the diff is genuinely security-shaped. Say so in the SKIP reason.
- One verdict, not four. The value is the merged, deduped, ranked finding list + a single go/no-go — not a wall of concatenated reports.
- Review first, act second. The four stages are read-only; only the step-6 Act phase touches anything, and only with the user's pick. Fixing is
/code-review --fix, PR comments are /code-review --comment — this skill routes to them, it doesn't re-implement either. Committing is still pr-craft's job.
- Obey CLAUDE.md conventions (shell wrappers, etc.) — sub-skills that touch the shell already honor these.
When to skip review-pass entirely
- Reviewing a remote GitHub PR — use
/code-review <pr-number> (it takes a PR number, branch, or path, and supports --comment / --fix).
- Mid-build task with no diff yet — use /dev-flow (it builds first, then drives this skill as its review stage).
- One-line edit, typo, rename — just eyeball it.
1---2name: review-pass3description: Single review-only entry point for an existing diff — runs a live-run check (`/run`) → code-review → implementation-review (and security-audit only when the change touches a trust boundary), then consolidates every finding into one go/no-go verdict. Use when you have a change ready and want it reviewed without running the whole dev-flow build chain. Triggers on /review-pass, "review this diff", "review my changes", "is this ready to commit", "give this a once-over". NOT for reviewing a remote GitHub PR (use /code-review) or for a task still being built (use /dev-flow).4---56# review-pass78Thin orchestrator over the review stages of the canonical chain. It does NOT re-implement any check — each sub-skill owns its own logic and its own skip rules. This skill only scopes the diff, orders the stages, drives them, and merges their output into one verdict. Point it at a change that's already written; it decides nothing about how the change was built.910Executor note: this is a prompt, not a script. Drive one stage via the Skill tool, wait for it to finish, checkpoint, then start the next. Never fire all four blind.1112## Protocol13141. **Scope** — determine what's under review. Default: the local working-tree diff (staged + unstaged) vs the merge-base. Accept an explicit target (`--staged`, a commit range, a path). From the diff note two things that drive the plan: (a) does it have **runtime surface** (product source, not docs/test-only) — gates the live-run check (`/run`); (b) does it touch a **trust boundary** (auth/authz, input parsing, secrets, file upload, external calls, SQL) — gates `security-audit`.152. **Harvest existing review** — if a PR exists for this branch, check what already reviewed it before running anything. `gh pr view <n> --json reviews` plus the inline comments; see [references/automated-reviewers.md](references/automated-reviewers.md) for the discovery command and, importantly, for why you must discriminate bots by `user.type` rather than by an `[bot]` login suffix. Two outcomes matter: findings a bot already posted go into the merged verdict with attribution instead of being rediscovered by a stage, and a configured reviewer that is paused, absent, or stale is itself worth reporting. Skip when there's no PR — a local-only diff has nothing to harvest.163. **Print the plan** — the four stages, each marked RUN or SKIP with a one-line reason (table below). Never skip silently. Note anything the harvest already covered.174. **Confirm** — user says go / edit / abort. Accept partial edits ("skip the run check", "add security-audit"). **Skip this step when an orchestrator drove you here** — dev-flow runs this as one chain stage and already had the user approve the whole chain; a second confirm inside an approved run is friction, not safety. Print the plan as a notice and proceed.185. **Drive** — invoke each RUN stage via the Skill tool, in order. After each returns, checkpoint: report its findings (blockers vs nits) in one line, then start the next. A blocker from an early stage doesn't halt the pass — collect everything, decide at the end.196. **Verdict** — merge all findings into ONE list, deduped, ranked blocker → nit. Bot findings from step 2 merge in here too, attributed to their source; where you disagree with one, say so and why — a reasoned disagreement is worth more to the reader than either verdict alone. Close with a go/no-go: **ship** (no blockers), **fix-first** (blockers listed), or **needs-a-human** (findings you can't adjudicate). This consolidated verdict is the whole point of the skill — don't just concatenate the sub-skill outputs.207. **Act** — if there are actionable findings, ask how to apply them (skip this if the verdict is clean; when orchestrator-driven, a clean verdict returns straight to the conductor with no prompt):21 - **Post as PR comments** — re-run `/code-review --comment` so findings land as inline comments on the PR. Only offer this when a PR actually exists for the branch; if none does, say so and drop the option.22 - **Fix directly** — re-run `/code-review --fix` to apply the findings to the working tree. Then re-verify the touched paths.23 - **Just report** — leave the verdict as-is; the user handles it.2425 Don't pick for the user — ask, then do exactly what they choose.2627## Stages2829Run in this order — cheapest signal and hardest gate first.3031**Depth.** Default runs all four gates. `quick` runs stages 1–2 only, and exists for one case: a single-file fix with no plan, where implementation-review's plan and coverage checks have nothing to read against and code-review already covers a diff that small. dev-flow passes `quick` on its bug tier and the default everywhere else. Anything beyond a one-file fix uses the default — when unsure, use the default.3233| Stage | Order | Gate (RUN only when true) |34|-------|-------|---------------------------|35| **`/run`** (live check) | 1 | Diff has runtime surface. SKIP for docs/test-only diffs — nothing to drive. |36| **code-review** | 2 | Anything beyond a trivial one-liner. Hunts correctness bugs in the diff. |37| **implementation-review** | 3 | Always at default depth, for a real change. SKIP at `quick`. Seven parallel checks; **its check 7 already runs security-audit**, so this covers the baseline security pass. |38| **security-audit** | 4 | SKIP by default — implementation-review already ran it. RUN standalone ONLY when the change sits squarely on a trust boundary and you want the full WSTG pass, not the summary check. On a plain UI/CRUD diff this is pure double-pay. |3940## Rules4142- **Orchestrate, don't decide inside a stage.** Once a sub-skill is invoked, follow its instructions — don't second-guess its internals.43- **This skill owns the review gates.** dev-flow delegates its entire review tail here and deliberately does not restate these gates. They were duplicated in both files once and drifted — dev-flow said "SKIP implementation-review on the bug tier" while this file said "always". A gate policy changes here, and only here.44- **Never skip silently.** Print SKIP + reason so the user can override.45- **Don't double-pay security.** implementation-review → security-audit is redundant unless the diff is genuinely security-shaped. Say so in the SKIP reason.46- **One verdict, not four.** The value is the merged, deduped, ranked finding list + a single go/no-go — not a wall of concatenated reports.47- **Review first, act second.** The four stages are read-only; only the step-6 Act phase touches anything, and only with the user's pick. Fixing is `/code-review --fix`, PR comments are `/code-review --comment` — this skill routes to them, it doesn't re-implement either. Committing is still pr-craft's job.48- **Obey CLAUDE.md conventions** (shell wrappers, etc.) — sub-skills that touch the shell already honor these.4950## When to skip review-pass entirely5152- Reviewing a remote GitHub PR — use `/code-review <pr-number>` (it takes a PR number, branch, or path, and supports `--comment` / `--fix`).53- Mid-build task with no diff yet — use /dev-flow (it builds first, then drives this skill as its review stage).54- One-line edit, typo, rename — just eyeball it.
mqmalagris