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
- Select the change — run
node {{SKILL_DIR}}/scripts/spec-driven.js modifyto list active changes. Ask which change to review. If already specified, use it. - Confirm readiness — run:
node {{SKILL_DIR}}/scripts/spec-driven.js apply <name>Ifremaining > 0, stop — the change is not ready for review. Suggest/spec-driven-applyfirst. - 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
- 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.
- 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.
- 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.
- 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?
- 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
- Output a review report:
MUST FIX (blocks archive): - [list or "none"] SHOULD FIX (recommended): - [list or "none"] NITS (optional): - [list or "none"] - 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