dasha-ws/ai-dev-framework/review-code

review-code

Purpose

/review-code is the independent code-quality gate that follows successful verification. It reviews one already working work item of a target project and answers one question:

"Is the implementation technically sound, maintainable, appropriately simple, and consistent with the approved technical design and project conventions?"

The review itself is done by the independent code-reviewer. This skill owns the process around it: prerequisites, scope, sources, baseline, gate semantics, integrity and routing. It is not a second /test-feature, a builder, a requirements, architecture, product, UX, security or deployment review, or a documentation stage. It fixes nothing. A defect goes back to where it was made. For substantial work:

/review-code → FAIL → /build-feature → /test-feature → /review-code again.

The detour through /test-feature is mandatory there. After a substantive fix, /review-code can rely on a /test-feature PASS only if that PASS is still current under /test-feature's own version-specific rules, so "fix the code, then /review-code" is not allowed. For a microchange the proportional re-verification rule of step 12 applies instead: a clearly non-behaviour-affecting fix does not need a full /test-feature solely because code changed, and a behaviour-affecting or uncertain one needs the appropriate functional re-verification.

Relation to `/test-feature`. /test-feature asks whether the behaviour works. /review-code asks whether the code that implements it is good enough to accept. Functional QA is not repeated. For substantial work, and for a microchange that went through /test-feature, a current /test-feature PASS is a prerequisite, but it is not evidence of code quality: the code is read and judged independently, and a concrete bug visible in it is a valid FAIL even if the tests are green.

Roles. The skill is the orchestrator and the gate. code-reviewer owns the detailed code-review criteria and the independent, read-only inspection. It is mandatory whenever /review-code is actually run, for substantial work and for a microchange alike. The skill never imitates it. There is no fallback where the skill "does the review" itself.

Source of truth.

• Product and Feature requirements: the expected behaviour, as far as it sets technical intent.
• The reviewed Tech Spec: the approved technical decisions, contracts, dependencies, persistence and integration design, constraints.
• architecture.md: the durable architectural boundaries.
• development.md: the conventions, commands and development rules. infrastructure.md only where it materially affects the implementation.
• Code is the implementation under review. Tests are implementation artifacts, not a source of requirements.
• For a microchange: the user's request, the affected implementation and existing behaviour, the project conventions.
• A material conflict between sources is never resolved by picking the convenient one: BLOCKED — NO VERDICT, routed upstream (step 5).

Tech Spec conformance is semantic, not literal. The implementation is reviewed against the material approved decisions of the Tech Spec: contracts, constraints, boundaries, dependencies, data and persistence design, the integration approach, lifecycle behaviour. It need not match the Tech Spec structurally. Helpers, private methods, file splits, internal names and small refactors are not prescribed unless a detail is an explicit required decision. A reasonable choice inside the approved design is not a FAIL because the reviewer would have done it differently.

The three results.

• `PASS`. No unresolved substantive code-quality defect must be fixed before the work goes on. It may carry non-blocking observations. It does not mean perfect code, security approval, deployment readiness or an architecture re-approval.
• `FAIL`. Independent review established a concrete, evidence-based, actionable, substantive defect in the current work item or in code it materially depends on. Never for a stylistic preference.
• `BLOCKED — NO VERDICT`. Valid independent review cannot be completed and no valid blocking defect is established. A broken review process is not a FAIL.

Findings are of two kinds only: blocking findings and non-blocking observations. No severity scores. FAIL needs at least one blocking finding.

A PASS is version-specific. It applies only to the implementation as reviewed, against the sources and conventions used for that review. An old PASS is no longer sufficient after a reliably known substantive change to the production implementation (migrations included), the required tests and their required fixtures and snapshots, implementation or review-relevant configuration, dependency manifests and lockfiles, any other implementation artifact in the review scope, or an applicable source (requirements, Tech Spec, architecture contracts and boundaries, development conventions) that materially changes the review expectations or scope. Unrelated changes, documentation outside code and normal transient outputs (build output, coverage, reports, caches, screenshots) do not invalidate it. A PASS is never carried over to changed expectations by assumption (steps 4 and 5). Nothing is recorded: no hashes, approval registry, review status file or PASS metadata.

One work item per run. Three scopes are kept apart: the work item; the review scope needed to assess it (the code it changed, shared code it changed or materially depends on, and the tests, configuration, dependencies and migrations needed to judge it); and unrelated repository code, which is not part of this gate.

When to use

• After a /test-feature PASS for substantial work: reviewed requirements → architecture as applicable → reviewed Tech Spec → /build-feature → /test-feature PASS → /review-code.
• For a real microchange that the user explicitly sends here. The scope is proportional. code-reviewer is still mandatory.
• After a FAIL was fixed and re-verified, after a BLOCKED cause was resolved, or after a reliably known change that voids an earlier PASS.

When not to use

• The current directory is the ai-dev-framework repository itself. That repo is the source of the framework, not a target. If the root contains the framework's own skills/ folder, stop and ask which project the user actually means.
• The framework structure created by /init-project is missing (step 1).
• Nothing is implemented, or substantial work, or a microchange that went through /test-feature, has no current /test-feature PASS (steps 3 and 4).
• Substantial work has no requirements or no Tech Spec (step 5), or a "microchange" turns out to be substantial (step 2).
• The request covers several work items.
• The user wants findings fixed, code written, functional verification, documentation updated, deployment readiness checked, a security audit, or a review of requirements or architecture.
• A true microchange that the framework or project rules let go without code review may skip this skill. Once it is sent here, the skill applies in full, code-reviewer included.

Inputs

• Target project root. Default: the current working directory. Confirm it if there is any doubt.
• The work item. Substantial work: .claude/work/<work-item-name>/tech-spec.md. Microchange: the user's request.
• The applicable approved requirements. The Feature Spec of the work item, or the Product Spec for an initial product implementation.
• The framework structure created by `/init-project`. Substantial work: root CLAUDE.md, .claude/skills/project-context/SKILL.md, in .claude/skills/project-context/context/ the files product.md, architecture.md, development.md and infrastructure.md, plus .claude/product/product-spec.md and .claude/work/. Microchange: root CLAUDE.md and the router.
• The implementation with its tests, fixtures, configuration, manifests and migrations.
• The current `/test-feature` PASS (substantial work, and a microchange that went through /test-feature), established by the current conversation or workflow (step 4).
• `code-reviewer`, as provided by the current installation. Agents are built after the skills, so it may not be installed yet (step 6).
• The `/build-feature` and `/test-feature` reports, if any. Context only.
• The user, for the few questions only they can settle.

Files to read

All reads are read-only. Never read or expose real .env files, credentials, tokens or secrets. Read selectively, not the whole repository:

• Substantial work: the root CLAUDE.md; the router; the current Tech Spec; the relevant architecture context and development.md (infrastructure.md only where it bears on the implementation); the requirements only where needed for technical intent; the relevant code, tests, fixtures and snapshots, configuration, manifests and lockfiles, migrations and generated-code inputs; the /build-feature report and the /test-feature result, as context only.
• Microchange: the root CLAUDE.md, the router, and the minimum relevant subset of code, tests, configuration and context.

Execution steps

1. Confirm the target and the framework structure

Verify that the working directory is a target project, not the framework repo, and that the structure required for the mode exists (see "Inputs"). If it is missing: stop with `BLOCKED — NO VERDICT`, say what is missing and suggest /init-project. Do not create or rebuild the structure.

Follow the target's root CLAUDE.md and conventions. They may refine how the review is done and cannot silently override a mandatory framework gate. If a project instruction directly conflicts with a mandatory framework rule, stop, report the conflict and ask for a decision.

2. Decide the mode

Spec-driven substantial work. The work item has a Tech Spec and approved requirements, and the implementation has been through /test-feature. Do not rerun /test-feature or /review-spec here.

Real microchange. No artificial specs. The basis is the user's request, the affected implementation, existing behaviour, project conventions and minimum technical context. The scope is proportional. A microchange that never went through /test-feature has no /test-feature PASS prerequisite, and none is invented for it. If the current workflow actually invoked /test-feature for it, a current reliably established /test-feature PASS is required, exactly as for substantial work (step 4).

Do not shrink substantial work into a microchange. If it turns out to be substantial, stop and route to the normal workflow: /new-feature-spec or /product-spec, /tech-spec, /review-spec, /build-feature, /test-feature, then back here.

3. Determine the work item and the review scope

Exactly one work item. If the request or Tech Spec covers several, take the one the sources and the user point to, and ask if that is unclear. For substantial work, establish the applicable requirements (the feature-spec.md in the same work folder, or the Product Spec for an initial product implementation) and ask if the workflow cannot be determined.

If nothing of the work item is in the code: stop with `BLOCKED — NO VERDICT` and route to /build-feature.

Do not rely on `git diff` alone to find the scope. The implementation may already be committed and the diff empty. Identify the scope from the work item, the Tech Spec, the conversation or workflow, the build report, code references and the project structure. Git status and diff serve state inspection and safety. They are not the source of the work item's identity, of approvals or of workflow completion. History may be read to understand how the code evolved, never as approval metadata. If the scope cannot be identified: BLOCKED — NO VERDICT.

4. Check the /test-feature PASS