Prospec Review

---
name: prospec-review
description: "Adversarial Code Review → Fix Loop - Between implement and verify, an independent fresh-context reviewer audits the whole change diff; verifier-confirmed criticals are auto-fixed, majors are proposed, and a spec-aware lens checks delta-spec/dependency-direction. Triggers: review, code review, adversarial review, find bugs, 審查, 程式碼審查, 對抗式審查, 找 bug, 找問題"
---
<!-- Generated by `prospec agent sync`. Do not edit — this file is regenerated on every sync. -->

# Prospec Review Skill

## Activation

When triggered, briefly describe:
- That an independent reviewer (fresh context) will audit the entire change diff between implement and verify
- That only verifier-confirmed criticals are auto-fixed; majors are proposed and passed to verify as WARN
- That the loop converges to zero unresolved critical or escalates to you at a hard cap — review never silently passes

## CLI Prerequisite (required)

> The prospec CLI is a required file for this skill — its deterministic steps call `prospec`
> commands. Probe BEFORE any other step; there is no manual fallback.

1. Run `prospec --version` (Bash).
2. **Command not found / not executable** → STOP. Ask the user to install the prospec standalone
   executable — the one-click installer script from the project README (macOS/Linux `install.sh`,
   Windows `install.ps1`) or a release binary from GitHub Releases; prospec is NOT published to
   npm. Then re-run this skill.
3. **Version older than 2.0.0** → STOP. Report the installed vs required version
   and ask the user to upgrade, then re-run this skill.

Hand-executing a CLI-owned mutation is NEVER the fallback — that re-introduces the
nondeterministic serialization this contract exists to remove.

## Startup Loading

1. [STABLE] Read `prospec/CONSTITUTION.md` — principles and dependency/layering rule
2. [STABLE] **MANDATORY** — Read [`references/review-format.md`](references/review-format.md) for the severity contract, review.md format, and reviewer lenses
3. [DYNAMIC] Read `.prospec/changes/[name]/tasks.md`, `plan.md`, `delta-spec.md`, `proposal.md` — the contract this change must honour
4. [DYNAMIC] Read `prospec/ai-knowledge/_conventions.md` + each affected module `README.md` — patterns and ripple effects
5. [DYNAMIC] Compute the change diff relative to the branch base (`git diff`), reviewing source + tests; exclude generated artifacts (`dist/`, lockfiles, deployed skills)

## Entry Gate

> Blocking precondition check before this skill runs. If any item FAILs, stop and tell the user what is missing — do not proceed.

- Implementation is done: metadata status is `implemented` **or later** and code tasks are complete (`prospec change status implemented` already enforces that; unchecked `[M]`/`[V]` do not block; kind schema: tasks-format reference); if still before `implemented` (`story`/`plan`/`tasks`), FAIL and point to `prospec-implement`. This item is a **floor, not a ceiling** — a `verified` change whose code moved after verify re-enters here (its stale `review-provenance` demands it), stamping a fresh baseline with no status regression.
- Planning artifacts exist: proposal.md, plan.md, delta-spec.md, tasks.md. **Exception — `metadata.scale: quick`**: only proposal.md + tasks.md are required (a quick change legitimately has no plan/delta-spec); do not FAIL on their absence. **Exception — `metadata.scale: backfill`**: only proposal.md + delta-spec.md are required (a backfill change records existing code — no forward plan/tasks); do not FAIL on their absence.
- Prior unresolved WARN: surface the `warn:` lines `prospec status` prints for this change.

## Core Workflow

### Reviewer Modes

- **B — single reviewer, multi-lens (default)**: one fresh-context reviewer covers every must-run lens in a single pass. Token-friendly; independence from the implementer is already satisfied.
- **A — parallel lenses (opt-in)**: N independent lens agents run concurrently — available only when the capability line below resolves `can_spawn_subagent` to yes; it has no single-context equivalent. Use for large or high-risk diffs, or Scale=Full. Higher first-round cost buys maximum inter-lens independence.

**Route the reviewer to the strongest model / agent tier the harness makes available** — a review gate's detection power is bounded by its reviewer, so the strongest available tier on a fresh context is the goal (named abstractly — never a specific model or vendor; "strongest available" is resolved by the harness). When you record the review baseline (`prospec check --record-review`), declare the grading context honestly with `--graded-by`: `fresh-subagent` for an independent context, `in-session` when the same session that implemented the change also reviewed it — recorded into `review_provenance.graded_by` as the honest grading-context record that informs the downstream verify station (the verify grade cap itself keys on each verify dimension's own `graded_by`).

### Review Lenses

Must-run every round:
- **correctness & edge cases**
- **security & data integrity**
- **spec-architecture** — the prospec differentiator, always layered on regardless of reviewer engine: implementation **contradicting** a `delta-spec` REQ, the project's declared dependency direction (per its Constitution / `_conventions.md`), module conventions, and unhandled ripple effects. Whether every REQ is *fully satisfied* is `prospec-verify` 2/5's contract check, not this lens — that station's `## Key Difference from Other Skills` states the review/verify division once; do not restate or re-audit it here.
  - **Quick degradation** (`metadata.scale: quick`): the delta-spec REQ comparison is `not-applicable` (there is no delta-spec — never report it as PASS); dependency direction, module conventions, and ripple checks still run in full. Additionally, when the diff appears to touch behavior covered by existing `prospec/specs/features/` REQs, raise an early warning — the `prospec-archive` Entry Gate re-checks this, but catching it at review is cheaper.

Conditional: **security & data integrity** (untrusted input, auth, external integrations), efficiency/performance (hot-path or data-layer changes), maintainability/DRY (new abstractions, or an existing helper / guard / writer re-implemented), **docs-claims** (the change adds/edits README or doc claims about behavior — check claim ⊆ implementation), **parallel-site completeness** (the change touches a shared resolver / invariant / data source — grep EVERY consumer), and **test-quality** (the change adds/edits tests — section-scoped + structural + negative + mutation-verified). When any conditional lens applies, load [`references/review-lenses-content.md`](references/review-lenses-content.md) **on demand** for its concrete, severity-pre-mapped criteria (OWASP/IDOR/SSRF/injection/secrets; N+1/CWV/blocking I/O; DRY/complexity/Rule-of-N; docs-claims/parallel-site/test-quality) — severity vocabulary stays defined in `review-format.md`, the lens-content reference only maps onto it. This reference is on-demand only — it is NOT a Startup Loading item. A pluggable language-specific engine may add further language lenses; the spec-architecture lens is always added by prospec and is never replaced by the vendored lens criteria.

### Severity Routing

Apply `references/review-format.md`. In short: **critical** blocks the loop and is auto-fixed; **major** does not block (proposed, passed to verify as WARN, never counted in verify's grade); **nit** is dropped.

### The Loop

1. Run the reviewer (mode B or A) over the change diff, by the mechanism **Harness Degradation** resolves below. The reviewer reads whole functions/classes and greps ripple, not just diff hunks, and hands its round back by the Persistence contract below:
   - **Physical Receipt Verification**: The orchestrator MUST verify that the returned payload path resolves to a readable regular file that exists, has `size > 0` bytes, and parses as valid JSON against `ReviewFindingsInputSchema` before calling `prospec review merge`.
   - **Lifecycle Probe & Await**: If the subagent returns a completion message or path but the file is not yet written, inspect abstract subagent lifecycle state or transcript logs and await completion.
   - **Explicit Degradation**: If the subagent crashes, times out, or fails to execute, trigger explicit Harness Degradation (e.g. running in-session with honest `--graded-by in-session` disclosure).
2. For each reported **critical**, confirm the issue's **existence** by **running its `repro`** and reading the cited code, then have an **independent verifier** write a single-entry `ReviewFindingsInputSchema` JSON array using the same finding `id` and `repro`, with a `status` of `confirmed` or `not-found`; it returns only the payload path plus that status. Verify that receipt is a readable regular file with `size > 0` and schema-valid JSON before acting. If the receipt is pending, inspect lifecycle/transcript evidence and await completion. On terminal verifier failure, disclose the degraded path, leave the critical unresolved, and escalate rather than self-confirming it; NEVER fabricate a mock/dummy verdict or synthetic confirmation. Before applying any fix, construct a **fail-then-pass regression test (pin)** with mutation verification to serve as the mechanical receipt guarding against regression in subsequent rounds. Only confirmed criticals with a concrete, local, drop-in fix and a regression pin are auto-fixed; architectural, large-refactor, or ambiguous fixes are **escalated to the human**, not auto-applied.
3. Apply each fix to the **working tree** (no commit), then **re-run the test suite** (via the project test command; see [`references/project-test-runner.md`](references/project-test-runner.md)); the suite must stay green — if a fix turns a test red, roll that fix back and re-decide, never proceed on red.
4. Re-review defaults to **full-lens re-review**: treat the fix as a new diff and re-run all review lenses over the cumulative diff in a fresh minimal context (input: cumulative diff + lenses + list of pinned findings). Previously pinned findings are mechanically guarded by their regression tests and are not re-adjudicated with judgment tokens. Continue until **0 unresolved critical** (review-clean).
5. **Circuit Breakers & Hard Cap**: Read [`references/circuit-breaker.md`](references/circuit-breaker.md) on demand. Hard cap: **3 rounds** (maximum 5). **Dual-Axis Circuit Breaker**: evaluate fix-induced defect ratio (trips if > 0.5 in round > 1) and cumulative token spend (trips if > declared budget), alongside the **Oscillation Breaker** (state flips `FAIL → PASS → FAIL` ≥ 2) and early-stop conditions.
6. **Escalation**: at the cap, circuit breaker trip, or early-stop with unresolved criticals, emit an `EscalationReport` recommending revert-and-redesign to the human for decision — never silently pass.

### Harness Degradation

**Harness capabilities** (resolved by `prospec agent sync` from this agent's registry entry — act on them, do not re-derive them at runtime): `can_spawn_subagent`: yes · `can_worktree`: no · `can_background`: yes

Sub-agents are available here, so take the sub-agent path. Should a spawn fail at runtime anyway, degrade — offer a choice between the harness's own reviewer command and a single-pass fresh-context review, and let the developer decide — and name the path you took. A degraded path is never a silent skip: the developer is told which path ran, every time.

### Persistence

Emit this round's findings as a `ReviewFindingsInputSchema` JSON array — whose executable field projection and cross-field rules live in [`references/delegated-evidence-format.md`](references/delegated-evidence-format.md) — to a temp file, then run `prospec review merge --findings <file> --lenses <lens,lens,…> [--round <n>] [--spend <tokens>] [--budget <tokens>]` (Bash). Pass `--lenses` with every lens this round actually ran (the must-run set plus any conditional lens you loaded), even on a clean round: a lens that ran and found nothing is essential data for `prospec learn yield`. The CLI stamps `origin_round` on each finding based on the current round — `origin_round` is stamped by the CLI, never declared by the reviewer. When passing `--round <n>`, `<n>` is this review loop's round counter starting at 1 (a change re-entering review after verify starts again at 1; the CLI offsets it internally by earlier loops so the `Origin` column stays monotonic). Omit `--round` and the CLI re-runs the round `review.md` records until `prospec change log` has closed it, then opens the next — so re-running one round's merge is byte-idempotent. A delegated reviewer or verifier **writes that file itself and returns only its path plus the counts** — evidence never travels back. The CLI owns the cumulative `review.md` bookkeeping: merge by finding identity, origin round tracking (`origin_round`), severity taken as the maximum, rows carried forward across rounds so resolved items are not re-raised, `repro` into its own table column, and `evidence` into the evidence section (both cumulative — a round re-reporting a finding without them keeps what the artifact holds). Every `critical` needs a `repro`; the shared reference owns its exact cross-field contract. **Finding IDENTITY is your judgment**: code edits shift line numbers, so reuse the prior round's `id` for a finding you consider the same one — the CLI never infers identity from the location string. An id no row carries opens a NEW row (only a row with no id at all — the pre-ids shape — is adopted by one), and a finding you leave id-less keys by location+lens against the rows that predate this round, so two id-less findings you file at one Location in one round stay two rows.

> **Clean review (0 findings)**: When your JSON array is empty (`[]`), the CLI generates an empty table with English headers. To satisfy the Constitution's Language Policy, you **MUST** manually append a summary sentence in the artifact language (e.g., a short sentence stating that no issues were found) to `.prospec/changes/[name]/review.md` after running the CLI merge command.

### Review Provenance (machine gate)

Review must leave a machine-queryable record so `prospec-verify`'s Entry Gate can prove it ran and is still current:

1. **Every round** — including a **review-clean** round (0 critical / 0 major) and **circuit-breaker tripped rounds** — record the round via `prospec change log --skill prospec-review --result PASS|WARN --criticals-found <n> --criticals-fixed <n> --majors <n> [--warning "circuit breaker tripped: <type>"|"<unresolved item>"]` (Bash; result `PASS` when clean, `WARN` when unresolved majors/FAIL or circuit-breaker trips carry forward). The counts come straight from `prospec review merge`'s round report. A clean review that records nothing is indistinguishable from a review that never ran — a round that finds nothing records the counts as `0`.
2. **At loop convergence** (review-clean or escalation), run `prospec check --record-review --graded-by <fresh-subagent|in-session>` — it code-computes the reviewed change's digest and writes `review_provenance` (with the declared grading context) to `metadata.yaml`. This is the baseline the `review-provenance` drift check compares against.

Because the digest is code-computed, editing the change's code after this point flips `review-provenance` to stale — `prospec-verify` will then require a fresh review round before it runs.

## Output Contract

> After running, self-assess and emit a concise Output Summary. Every Success Criterion must be objectively checkable (file existence / grep / test result / count) — no subjective adjectives.

### Success Criteria
- [ ] no unresolved critical (loop converged, or escalated to the human with the list)
- [ ] every fix round left the project test suite (per [`references/project-test-runner.md`](references/project-test-runner.md)) green
- [ ] `review.md` written with the findings table
- [ ] a `prospec-review` `quality_log` entry recorded (every round, incl. review-clean) and the review baseline stamped via `prospec check --record-review --graded-by <context>`
- [ ] every auto-fixed critical was verifier-confirmed before the fix and guarded by a fail-then-pass regression pin (manual)

### Failure Conditions
- a critical auto-fixed without an existence-verification step or without a regression pin
- tests left red, or the loop exceeded the hard cap without escalating

### Output Summary
Emit one line: `Met N/M | Unmet: <items> | Overall: PASS|WARN|FAIL | Next: <one-line>`

### Exit Gate (Constitution)

Verify the output against this skill's **site-specific** Constitution rule (**dependency-direction/layering** — the spec-architecture lens's concern), not the full Constitution; the every-principle audit is `prospec-verify` V3/5 only. When the rule carries RFC-2119 severity, grade by weight — MUST→FAIL, SHOULD→WARN, MAY→informational (the grade vocabulary stays PASS/WARN/FAIL). A free-text Constitution falls back to judgment-based grading. **Always** record a `prospec-review` entry to `metadata.yaml` `quality_log` (`skill: prospec-review` / `date` / `result` / `warnings`) — **every round, including review-clean** (result `PASS` when clean, `WARN` when unresolved majors/FAIL carry forward) — so `prospec-verify` can machine-verify review ran and surface any majors; majors are advisory and do not block. Then record the review baseline (`prospec check --record-review --graded-by <context>`, see Review Provenance) and suggest `prospec-verify`.

## NEVER

- **NEVER** return evidence prose to this context — a delegated reviewer or verifier writes it to the payload file and returns the path; an unbounded relay costs the very context delegation was meant to save
- **NEVER** proceed on verbal completion claims or promises without physical receipt verification of the payload file — trusting prose without file inspection masks execution errors
- **NEVER** fabricate mock findings, dummy JSON `[]`, or synthetic passes when a payload file is missing or unreadable — fail closed with concrete diagnostic errors
- **NEVER** auto-fix a critical that was not independently confirmed to exist — acting on a hallucinated finding edits correct code and erodes trust
- **NEVER** apply a fix to a confirmed critical without a fail-then-pass regression test (pin) with mutation verification — unpinned fixes risk silent regression during subsequent full-lens re-reviews
- **NEVER** proceed to the next round with the test suite red — a fix that breaks a test must be rolled back; silent green→red regression defeats the loop
- **NEVER** loop without a hard cap or silently pass unresolved criticals — unbounded retries waste tokens; unresolved criticals must escalate to the human
- **NEVER** auto-apply an architectural or large-refactor fix — only concrete, local, drop-in fixes are safe to apply unattended; the rest are proposed
- **NEVER** count major findings in verify's grade — review and verify are separate axes; majors pass as advisory WARN, not as a grade penalty
- **NEVER** silently skip review when the capability line says no sub-agents, or when a spawn fails — offer the degraded path so the developer decides knowingly
- **NEVER** commit during review — the commit boundary is after `prospec-verify` reaches S/A; review only edits the working tree

## Error Handling

| Scenario | Action |
|----------|--------|
| metadata status is BEFORE `implemented` (`story`/`plan`/`tasks` — implementation unfinished) | Stop; point to `prospec-implement` to finish tasks first. A status at or past `implemented` — `verified` included — is NOT this scenario: the Entry Gate item is a floor |
| No change diff vs branch base | Report nothing to review; suggest proceeding to `prospec-verify` |
| Sub-agent spawn fails at runtime | Offer the harness reviewer or single-pass fallback; do not skip |
| Fix repeatedly turns tests red | Roll back, mark the critical unresolved, escalate to the human |
| Reviewer and verify disagree on layering | Keep both — review catches it first, verify re-checks independently; no mutual exemption |

## Next-Step Handoff

After the Output Summary, recommend the next step in the SDD workflow order
(`story → plan → tasks → implement → review → verify → knowledge-update → archive`, then periodic `learn`) — read
`metadata.yaml` status and `prospec/ai-knowledge/_status-lifecycle.md` (review and learn own no
status transition, so follow this order, not status alone). Provide the direct, actionable Skill
identity or CLI command for the next step (e.g. `prospec-plan`), allowing smooth continuation
without blocking on a separate confirmation turn. If the stage is terminal (`archived`), the linear
flow is complete — point to periodic `prospec-learn` rather than a workflow successor. If the result
does not advance (e.g. verify grade B/C/D), say so and point to the corrective step instead of
offering the next skill.