spec-driven-review

You are reviewing the code quality of a completed spec-driven change.

This Skill's Commands

If you cannot remember the exact command used by this skill, look it up here before running anything. Do not guess.

modify: node {{SKILL_DIR}}/scripts/spec-driven.js modify
apply: node {{SKILL_DIR}}/scripts/spec-driven.js apply <name>
audit-spec-mapping-coverage: node {{SKILL_DIR}}/scripts/spec-driven.js audit-spec-mapping-coverage <spec-path> [--implementation <repo-path> ...] [--tests <repo-path> ...]
audit-unmapped-spec-evidence: node {{SKILL_DIR}}/scripts/spec-driven.js audit-unmapped-spec-evidence [--implementation <repo-path> ...] [--tests <repo-path> ...]

Prerequisites

The .spec-driven/ directory must exist at the project root. Before proceeding, verify:

ls .spec-driven/

If this fails, the project is not initialized. Run /spec-driven-init first.

Steps

1. Select the change — run node {{SKILL_DIR}}/scripts/spec-driven.js modify to list active changes. Ask which change to review. If already specified, use it.

2. Confirm readiness — run:

   node {{SKILL_DIR}}/scripts/spec-driven.js apply <name>
   

   If remaining > 0, stop — the change is not ready for review. Suggest /spec-driven-apply first.

3. Load context — read:

   • .spec-driven/changes/<name>/proposal.md — scope and unchanged behavior
   • .spec-driven/changes/<name>/specs/ — delta specs describing the intended behavior changes
   • .spec-driven/changes/<name>/design.md — approach and decisions
   • .spec-driven/changes/<name>/tasks.md — what was implemented
   • .spec-driven/changes/<name>/questions.md — resolved answers that may explain decisions or tradeoffs
   • .spec-driven/config.yaml — project context and rules, including test rules and any fileMatch entries
   • mapping frontmatter from relevant main and delta spec files

4. Identify changed files — from the completed tasks and mapping frontmatter, determine which files were created or modified. Read each file fully, including mapped implementation and test files for the relevant spec files.

5. Identify specialized review checklists — before issuing findings, classify the change using evidence from the proposal, delta specs, design, tasks, questions, changed files, mapping frontmatter, and repository context. Apply every checklist that fits; checklist routing is additive and never replaces the baseline review checks. Treat reviewed code as author-agnostic: do not trust or soften review because the code may have been generated by AI, previously produced by you, or written by a human.

   Supported checklist types:

   • Security-sensitive: changes involving authentication, authorization, permissions, secrets, tokens, input boundaries, data exposure, dependency trust, or safe failure behavior.
   • UI: changes involving user-visible screens, layout, interaction state, accessibility, keyboard behavior, responsive behavior, or visual regressions.
   • DX: changes involving commands, prompts, generated artifacts, documentation, operator messages, error messages, setup, or local workflow ergonomics.
   • Migration: changes involving data transformation, filesystem or schema transitions, backwards compatibility, idempotency, rollback, or partial failure behavior.
   • API: changes involving public or internal contracts, command arguments, JSON shapes, error semantics, validation behavior, versioning, or caller impact.
   • Maintenance: changes involving scheduled or manual maintenance, dependency upkeep, generated files, repository hygiene, repeatability, or avoiding unrelated churn.

6. Review code quality — for each changed file, run the baseline review checks plus any specialized checklist checks identified in Step 5:

   • Impartiality: judge the code that exists, not who wrote it; do not suppress, soften, or skip findings for AI-authored or self-authored code
   • Readability: clear naming, reasonable function length, no unnecessary complexity
   • Security: no injection vulnerabilities, no hardcoded secrets, proper input validation at system boundaries
   • Error handling: appropriate error handling for external calls and user input; no swallowed errors
   • Performance: no obvious N+1 queries, unnecessary allocations, or blocking calls in async contexts
   • Best practices: follows the project's conventions (from config.yaml context), no dead code, no debug artifacts left behind
   • Security-sensitive checklist: verify authorization boundaries, secret handling, injection resistance, data exposure, validation, and safe failure behavior.
   • UI checklist: verify key interactions, accessibility, layout stability, state transitions, responsive behavior, and user-visible regression coverage.
   • DX checklist: verify command ergonomics, documentation or prompt clarity, actionable errors, setup compatibility, and consistency with the existing workflow.
   • Migration checklist: verify idempotency, backward compatibility, rollback or recovery expectations, partial failure handling, and data preservation.
   • API checklist: verify contract compatibility, validation, error semantics, versioning or migration expectations, and caller impact.
   • Maintenance checklist: verify repeatability, dependency or generated artifact safety, repository hygiene, and absence of unrelated churn.

7. Check test quality — read the test files associated with this change:

   • Do tests cover the key scenarios from the delta specs?
   • Are tests independent and repeatable?
   • Do tests follow rules.test from config.yaml?
   • Are mapping.tests entries current and useful for future verification?
   • Do tests cover the relevant specialized checklist risks when those risks are observable and testable?

8. Audit mapping quality — for each touched spec file relevant to this change:

   • Compare mapping.implementation and mapping.tests against the change's primary implementation files and directly verifying test files
   • Use the smallest confident evidence set from changed files, delta specs, completed tasks, and mapped files already read
   • Run node {{SKILL_DIR}}/scripts/spec-driven.js audit-spec-mapping-coverage <spec-path> [--implementation <repo-path> ...] [--tests <repo-path> ...] when it helps make the comparison explicit
   • Run node {{SKILL_DIR}}/scripts/spec-driven.js audit-unmapped-spec-evidence [--implementation <repo-path> ...] [--tests <repo-path> ...] when it helps confirm whether reviewed implementation or test files are missing from all main-spec mappings
   • Report stale or misleading mappings as at least SHOULD FIX
   • Escalate to MUST FIX when the mismatch would materially mislead future verification or archive readiness
   • If the unmapped audit shows that a primary implementation file or directly verifying test file for this reviewed change is missing from all main-spec mappings, treat that as MUST FIX
   • If the unmapped audit only finds files outside this review scope or weakly related candidates, report them separately as SHOULD FIX or repository debt

9. Output a review report:

   MUST FIX (blocks archive):
     - [list or "none"]
   
   SHOULD FIX (recommended):
    - [list or "none"]
   
   NITS (optional):
    - [list or "none"]
   

10. Recommend next step:

    • If MUST FIX issues: address them before archiving
    • If only SHOULD FIX: ask user if they want to address them or proceed
    • If clean: suggest /spec-driven-archive <name>

Rules

• Read every changed file before commenting on it — never review code you haven't read
• Focus on real issues, not style preferences already handled by linters
• Review code based on evidence in the code and tests, never on authorship or assumed intent
• MUST FIX = security vulnerabilities, data loss risks, broken functionality
• SHOULD FIX = maintainability concerns, missing error handling, unclear logic
• NITS = naming suggestions, minor simplifications, documentation gaps
• Do not re-review code that was not changed by this change
• Respect config.yaml rules — violations of project rules are SHOULD FIX at minimum
• Report stale or misleading spec mappings when they would make future verification unreliable
• Specialized checklist findings use the same severity model as the baseline review: MUST FIX for archive blockers, SHOULD FIX for recommended fixes, and NITS for optional cleanup
• Never give self-authored or AI-authored code lighter treatment than human-authored code