| name | review-changes |
| description | Use when docs/specs/changes/{name}/ contains delta specs but no tasks.md yet, or when a previously-reviewed change has been revised in writing-specs — validates each delta operation's preconditions against current stable specs, checks scenario coverage and RFC 2119 language, then hands inline findings to plan-review which drafts the approval-gated plan. |
| metadata | {"tags":["tech:markdown","spec-driven","workflow","review"]} |
Review Changes
Overview
Review a change folder (delta specs) against current stable specs. Validate preconditions, scenario coverage, and RFC 2119 language. Surface ambiguity before a plan is built on a shaky spec.
Violating the letter of the rules is violating the spirit of the rules.
Do NOT hand off to `plan-review` until every delta operation's preconditions hold against the CURRENT stable specs on disk. Any CRITICAL finding returns the change to `writing-specs`. Approval is a single gate on the draft plan (`tasks.md`) produced by `plan-review` — NOT here.
The Iron Law
NO PLAN WITHOUT VALIDATED DELTAS FIRST
When to Use
Always:
- A delta spec exists under
docs/specs/changes/{name}/ and has not been planned yet.
- A previously-planned change has been revised in
writing-specs and needs re-validation.
Exceptions: none. Planning without validation produces plans that cite requirements that don't cleanly merge.
Process
- List inputs. Identify every file in
docs/specs/changes/{name}/. Identify every stable spec domain the delta references.
- Read the delta spec(s) and confirm each has a
## Change Overview section (Why / Impact / Non-goals / Rollback).
- For each delta operation, verify preconditions against the current stable spec (see
references/delta-preconditions.md):
- ADDED: name MUST NOT exist in stable spec.
- MODIFIED: name MUST exist in stable spec; delta block MUST contain full updated body.
- REMOVED: name MUST exist; Reason and Migration present.
- RENAMED: FROM exists in stable; TO does not exist in stable and is not also created by ADDED.
- Scenario coverage: every ADDED and MODIFIED requirement has ≥1
#### Scenario: with Given/When/Then.
- RFC 2119: statements use uppercase MUST/SHALL/SHOULD/MAY; no lowercase ambiguity.
- Ambiguity check: each scenario's Then clause is testable. Requirements describe behavior, not implementation.
- Three-tier severity (see
references/review-checklist.md):
- CRITICAL: blocks approval. Precondition failures, missing scenarios, malformed headings, implementation leaks.
- WARNING: surfaces but does not auto-block. Weak scenario wording, SHOULD used where MUST is clearer.
- SUGGESTION: informational.
- Summarize findings inline with severity tiers (CRITICAL / WARNING / SUGGESTION), including requirement names and line references. These findings are carried into
plan-review's draft for the Review summary appendix in tasks.md.
- Decision gate:
- Any CRITICAL → return to
writing-specs with the inline findings.
- No CRITICAL → hand off to
plan-review with the inline findings for the Review summary.
Checklist
You MUST work through each of these items and complete them in order:
- Inputs listed; all files in change folder enumerated.
- Change Overview section present in delta spec.
- Preconditions verified for every delta operation.
- Scenario coverage verified for every ADDED/MODIFIED requirement.
- RFC 2119 language verified.
- Ambiguity/implementation-leak check complete.
- Decision made: return-to-
writing-specs (CRITICAL) or handoff-to-plan-review (clean).
Common Rationalizations
| Excuse | Reality |
|---|
| "The delta is self-evident, skip preconditions" | Preconditions catch name collisions and silent drift from the current stable spec. Always verify. |
| "Scenario wording is close enough" | If Then clauses aren't testable, plan-review can't map them to tasks. |
"I'll let plan-review catch the issues" | plan-review assumes the delta is valid. It doesn't re-run precondition checks. |
| "I'll gate approval here to be safe" | The workflow has ONE approval gate: on the draft plan produced by plan-review. Double-gating defeats the purpose. |
Red Flags — STOP and Re-review
- Found an ADDED requirement whose name already exists in stable
- MODIFIED block shorter than the corresponding stable block
- REMOVED with no Migration
- RENAMED where FROM name cannot be found in stable
- Requesting user approval in this stage (approval is on the draft plan in
plan-review)
All of these mean: stop the handoff, fix or escalate to writing-specs.
Anti-Pattern: "The delta is self-evident, skip preconditions"
Preconditions are where silent bugs live. A MODIFIED pointing at a renamed requirement becomes an orphaned block on merge. An ADDED that duplicates an existing name produces two requirements with the same identity. Every operation gets checked against the current stable spec on disk, without exception.
Example
Inline review findings for add-admin-2fa:
## CRITICAL
- **ADDED Requirement: Admin Two-Factor Authentication** — name collides with existing `### Requirement: Admin Two-Factor Authentication` in `docs/specs/auth/spec.md`. Expected: operation should be MODIFIED, not ADDED.
## WARNING
- **MODIFIED Requirement: Session Expiration** — scenario "Admin idle" uses "will return" instead of "MUST return". Tighten to RFC 2119.
## SUGGESTION
- Change Overview's Impact section omits rollback strategy. Consider adding feature flag note.
Verification Checklist
Integration
Terminal states:
- Clean (no CRITICAL): post a one-line summary of findings (e.g. "Review clean — N WARNINGs, M SUGGESTIONs — invoking plan-review") and invoke
plan-review with the inline findings for the Review summary appendix. plan-review owns the sole approval gate. Do NOT ask the user "should I proceed to plan-review?" — the summary IS the handoff signal.
- Rejected (any CRITICAL): invoke
writing-specs to fix.
Do NOT request user approval here. Do NOT invoke execution or code-review.
REQUIRED SUB-SKILL (approved path): Use plan-review. Announce findings in one line, then invoke.
REQUIRED SUB-SKILL (rejected path): Use writing-specs.
