Skill v1.0.1

version: "1.0.1" name: review-artifacts description: Review OpenSpec change artifacts before anyone writes code — architecture quality and design completeness, then spec completeness and testability. Catches a wrong idea at the cheapest possible moment. Use after generating proposal, design, tasks and delta specs, and before implementing. license: MIT compatibility: Requires the openspec CLI. Node.js >= 18 for the bundled interlock CLI. argument-hint: "[change-name]" allowed-tools: Bash(interlock ) Bash(openspec ) Read Glob Grep metadata: type: review outputs:

• .claude/metrics/review-artifacts-<change>-<timestamp>.json

Review the change artifacts before implementation. This is the highest-leverage gate in the whole flow: a blocker caught here costs a paragraph, the same blocker caught after ship costs a day.

Resolve the change:

interlock validate [change-name] --json

An output carrying error and candidates instead of ready means the name did not resolve; problems is the list the change must clear. If the name did not resolve, pick the change the user named. If they named none and several are active, review the most recently modified and say which one you picked — this skill runs inside automated chains where a question would stall the flow.

Read all of proposal.md, specs/**/*.md, design.md, tasks.md.

Detect the stack from the artifacts and the dependency manifest, and skip framework-specific checks that do not apply. Do not ask a Go service about accessibility requirements.

Reviewer 1 — Architecture

Architecture quality

• Are layer boundaries placed correctly in design.md for this stack?
• Does the structure follow the patterns already in this project?
• Are state and ownership decisions appropriate?
• Is the data flow clear — reads, writes, mutations, side effects?

Design completeness

• Does design.md specify an error-handling strategy?
• When UI is in scope, are accessibility requirements present?
• Is the approach consistent with the project's own architecture docs (CLAUDE.md, AGENTS.md, docs/)?

Pattern fitness

• Does the proposed structure introduce complexity it does not need?
• Is there a simpler alternative that reaches the same outcome?

Reviewer 2 — QA

Spec completeness

• Does every requirement in specs/ have concrete Given/When/Then scenarios?
• Are edge cases covered — empty, error, loading, boundary values, permission denied?
• Which scenarios would a real user hit that the spec does not mention?

Testability

• Can every scenario become an automated test?
• Are acceptance criteria specific enough to verify? "Should work properly" is not a criterion.

Task quality

• Does tasks.md pair a test task with every feature task?
• Is the ordering right — do dependencies precede dependents?
• Is each task small enough for one focused session?
• Same-file staircase. Do consecutive checkboxes name the same production path — one per TDD beat, comment move or trap? That is one checkbox, not five. Report it as a warning naming the repeated path. It is not a ship halt: planWaves folds the resulting 1-task waves back together, so the cost is implementer count and lost parallelism, not checkpoints.
• Does each numbered section hold work that could share a wave? Independent files belong in one section; a new section is only an output-exists boundary.

Delta spec quality

• Are ADDED / MODIFIED / REMOVED sections used correctly?
• Do the deltas describe behavior, not implementation?

Invariant check

If the change transforms a shared or derived value — normalizing an identity, deriving a cache or dedup key, canonicalizing a field — then tasks.md must contain a task for every consumer of that value, not just the one the change is about.

A change that updates one call site and leaves its siblings reading the raw form is a blocker at this gate, because it is far cheaper to add the tasks now than to discover the surviving bug after review. See ${CLAUDE_PLUGIN_ROOT}/shared/INVARIANT-SWEEP.md.

Output

── ARCHITECTURE ────────────────────────────────
✓ [passed — brief]
⚠ WARNING: [specific issue, with file reference]
✗ BLOCKER: [critical issue]
 
── QA ──────────────────────────────────────────
✓ [passed — brief]
⚠ WARNING: [specific issue, with file reference]
✗ BLOCKER: [critical issue]
 
SUMMARY: N blockers, N warnings, N suggestions
Recommendation: [fix blockers | address warnings | proceed to /interlock:ship]

Be specific — name files and sections. Do not pad with generic praise. If the artifacts are genuinely good, say so briefly and recommend proceeding.

Emit structured findings

Write .claude/metrics/review-artifacts-<change>-<YYYYMMDD-HHMMSS>.json in this shape:

{ "dimension": "artifacts",
  "findings": [
    { "severity": "blocker|warning|suggestion",
      "file": "path/to/artifact.md", "line": 42,
      "title": "short claim",
      "description": "what is wrong and why",
      "suggestion": "how to fix it" }
  ] }

severity, file, title and description are required; line and suggestion are optional. Map BLOCKER→blocker, WARNING→warning, SUGGESTION→suggestion. Create .claude/metrics/ if it does not exist. This is the complete emit contract — do not read lib/findings.mjs to rediscover it.

Then let the deterministic gate decide, rather than deciding in prose:

interlock gate --findings .claude/metrics/review-artifacts-<change>-<ts>.json --metrics <change> --json

The gate blocks if and only if at least one finding is a blocker. Its exit status is the verdict: non-zero means blocked. Do not re-derive the verdict in prose. In the JSON, passed is that same verdict; report the entries of blockers and malformed, read metrics.written and metrics.reason for whether the record landed, and hand autonomyOutcome.blockers on as the blocker count the spec skill passes to interlock autonomy record.

--metrics <change> is not optional decoration. It writes the four counts this gate produced — raised, dismissed, dropped by the quality band, surviving — to .claude/metrics/ in the schema interlock report recognizes. Drop it and this review path becomes invisible to the report forever: the review-finding indicators read unobserved no matter how many gates ran. It is bookkeeping only and cannot change the verdict or the exit status.