kiro-review

Overview

This skill performs task-local adversarial review. It verifies that the implementation is real, complete, bounded, aligned with approved requirements and design, and supported by mechanical verification evidence.

Boundary terminology continuity:

• discovery identifies Boundary Candidates
• design fixes Boundary Commitments
• tasks constrain execution with _Boundary:_
• review rejects concrete Boundary Violations

When to Use

• After an implementer reports READY_FOR_REVIEW
• After remediation for a rejected review
• Before marking a task [x]
• Before accepting a task into feature-level validation

Do not use this skill to invent missing requirements or silently reinterpret the spec.

Inputs

Provide:

• Task ID and exact task text from tasks.md
• Relevant requirement section numbers
• Relevant design section numbers
• Spec file paths (requirements.md, design.md, optionally tasks.md)
• The implementer's status report
• The task _Boundary:_ scope constraints
• Validation commands discovered by the controller
• Relevant steering excerpts when applicable
• Relevant ## Implementation Notes entries when applicable

Outputs

Return one of:

• APPROVED
• REJECTED

Also return:

• Mechanical results
• Findings with severity
• Required remediation
• One-sentence summary

Use the language specified in spec.json.

First Action

Run git diff to inspect the actual code changes. If the diff is large or ambiguous, read the changed files directly. Do not trust the implementer report as source of truth.

Core Principle

Read the spec yourself. Read the diff yourself. Verify mechanically where possible. Reject on concrete failures rather than interpretive optimism. The main review question is not just "does it work?" but "does it stay inside the approved responsibility boundary without hiding new coupling?"

Mechanical Checks

Run these checks and use the result as primary signal.

1. Regression Safety

• Run the project's canonical test suite using the validation commands discovered by the controller.
• If tests fail, reject.

2. No Residual Placeholder Markers

• Check changed files for TBD, TODO, FIXME, HACK, XXX.
• Reject if new placeholder markers were introduced without explicit task justification.

3. No Hardcoded Secrets

• Check changed files for hardcoded secrets or credentials.
• Reject if concrete secret patterns are introduced.

4. Boundary Respect

• Compare changed files against the task _Boundary:_ scope.
• Reject if the change spills outside the approved boundary without explicit justification.
• Reject if the implementation introduces hidden cross-boundary coordination inside what should be a local task.

5. RED Phase Evidence

• For behavioral tasks, verify that the implementer status report includes RED_PHASE_OUTPUT.
• Reject if RED evidence is missing, empty, or unrelated to the task's acceptance criteria.

6. Runtime-Sensitive Static Checks

• If the project already has lint or equivalent static analysis for the touched stack, run the relevant command for the task boundary.
• Pay attention to patterns that can survive typecheck/build yet fail at runtime: type-only imports used as values, missing namespace value imports for qualified-name access, unresolved globals, and newly introduced runtime-sensitive dependencies without matching boot/runtime handling.
• If no project lint command exists, perform a targeted diff-based spot check in the changed files for those patterns.
• Reject on concrete findings that create a realistic boot-time or module-load failure.

Judgment Checks

7. Reality Check

• Confirm the implementation is real production code, not a placeholder, stub, fake path, or deferred-work shell.

8. Acceptance Criteria Coverage

• Read the task description and confirm all aspects are implemented, not only the primary happy path.

9. Requirements Alignment

• Read the referenced sections in requirements.md.
• Confirm each requirement is satisfied by concrete observable behavior.
• Use original section numbers only.

10. Design Alignment

• Read the referenced sections in design.md.
• Confirm the implementation uses the prescribed structures, interfaces, and dependency direction.
• Reject silent substitutions for design-mandated choices.

10.5 Boundary Audit

• Compare the implementation against the design's boundary commitments and out-of-boundary statements.
• Reject if downstream-specific behavior is pushed into an upstream boundary for convenience.
• Reject if the implementation creates new hidden dependencies, shared ownership, or undeclared coupling across adjacent boundaries.
• Reject if a task that is not an explicit integration task now behaves like one.

11. Test Quality

• Confirm tests prove the required behavior rather than only scaffolding.
• Confirm tests would fail if the implementation were removed or broken.

12. Error Handling

• Confirm relevant failure paths are handled and not silently swallowed.

13. Design Quality (筋の良い実装)

• Beyond "does it work?" and "does it match the spec?", assess whether this is a sound, well-shaped implementation per .claude/rules/coding-style.md (Reviewing for Design Quality). "筋が良い" is the shape that best fits this feature's context: judge which qualities matter most here (correctness, latency, memory, security, readability, maintainability), how they trade off, and whether the change resolves that tradeoff well. Reason from first principles and state the tradeoff — do not just pattern-match the repo's default markers. A well-argued deviation from a default, with the tradeoff named, is itself 筋が良い; the defaults are a floor, not a ceiling.
• Apply the severity discipline from that section: a spec-compliant change with a poor shape is normally a Suggestion or Important finding, not an automatic REJECTED. Reserve rejection for a shape that is genuinely hard to maintain, hides coupling, or will force a rewrite. Do not loop on taste.

Severity Model

Use:

• Critical for broken functionality, invalid verification, data loss, security risk, or major scope violation
• Important for required fixes before acceptance
• Suggestion for non-blocking improvements
• FYI for informational notes

Stop / Escalate

Escalate instead of papering over the issue when:

• The approved spec is ambiguous in a correctness-critical way
• The design conflicts with what is technically possible
• Required evidence cannot be gathered
• The implementation only works by silently deviating from approved scope
• Boundary ownership cannot be determined cleanly from requirements, design, and task scope

Common Rationalizations

Output Format

## Review Verdict
- VERDICT: APPROVED | REJECTED
- TASK: <task-id>
- MECHANICAL_RESULTS:
  - Tests: PASS | FAIL (command and exit code)
  - TBD/TODO grep: CLEAN | <count> matches
  - Secrets grep: CLEAN | <count> matches
  - Static checks: PASS | FAIL | SPOT_CHECKED
  - Boundary: WITHIN | <files outside boundary>
  - Boundary audit: CLEAN | <spillover / hidden dependency findings>
  - RED phase: VERIFIED | MISSING | N/A
- FINDINGS:
  1. <specific finding with exact files/spec refs>
- REMEDIATION: <mandatory if REJECTED>
- SUMMARY: <one sentence>