Skip to content

vt-c-4-review

Phase 4 - Comprehensive code review using the implementation-orchestrator. Runs 6 parallel reviewers for security, quality, performance, and patterns.

Plugin: core-standards
Category: Development Workflow
Command: /vt-c-4-review


Phase 4: Review - Code Quality

Run comprehensive code review with 6 parallel specialized reviewers.

Workflow Position

/activate → /vt-c-2-plan → /vt-c-3-build → [ /vt-c-4-review ] → /vt-c-5-finalize → /vt-c-complete
                                              ▲ YOU ARE HERE
                                              ↑ deliverable_types determines reviewer dispatch
                                                code → 6 code agents + accessibility + design (when frontend)
                                                document/presentation/research/project-plan → type-specific reviewer

Invocation

/vt-c-4-review                    # Review all changes
/vt-c-4-review --scope src/api/   # Review specific path

Pre-flight: For best results with code deliverables, run /simplify before this command. The parallel reviewers then focus on architectural and security concerns rather than code quality issues that /simplify handles in 90 seconds.

Skip /simplify if: tests are failing, diff >50 files, or changes are config-only.

Intent Constraints

This skill MUST NOT: - Auto-fix findings classified as human-required — present them to the user instead - Exceed 2 iterations of the fix-review cycle — reclassify unresolved findings as human-required - Modify files outside the reviewed branch diff scope during auto-fix - Dismiss security-sentinel HIGH findings without explicit user acknowledgment - Dispatch quality reviewers before the test gate and spec-compliance gate pass

This skill MUST STOP and surface to the user when: - A reviewer agent times out, fails to return, or returns an unexpected format - The auto-fix loop does not converge (findings persist or multiply after fix attempts) - All findings are human-required and no auto-fix is possible - The review discovers a potential data loss or security vulnerability requiring immediate attention

Execution Instructions

This command invokes the implementation-orchestrator agent.

Step 0a: Read Phase Checkpoint

If .claude-checkpoint.md exists in the project root: 1. Read the file 2. Display: Resuming from checkpoint: {completed_phase} completed at {timestamp} 3. Display the "Context for Next Phase" section content 4. If next_phase doesn't match 4-review: warn "Checkpoint suggests /{next_phase}, but you're running /vt-c-4-review" (advisory, not blocking)

If no checkpoint exists: proceed silently.

Step 0: Check for Spec-Driven Mode

If .specify/ directory exists:

  1. Load constitution - .specify/memory/constitution.md for principles
  2. Derive active spec from branch — check .active-spec file first, then parse branch name (feature/spec-NNN-* → SPEC-NNN), then look up specs_dir in .design-state.yaml
  3. Load spec - specs/[N]-feature/spec.md for compliance checking
  4. Display status:
    ✓ SpecKit detected - Spec compliance review enabled
    • Implementation will be validated against spec requirements
    • Constitution principles will be verified
    

If NO .specify/:

Standard code review mode (no spec validation).
TIP: For spec-driven development, run /vt-c-specify-and-validate

Step 0.5: Project Registration Check

  1. Read TOOLKIT_ROOT/intake/projects.yaml (resolve TOOLKIT_ROOT from this skill's base directory)
  2. Check if the current working directory appears in the projects list
  3. If NOT registered and the project has a CLAUDE.md or .design-state.yaml:
  4. Ask: "This project is not registered in the toolkit. Register now to enable cross-project scanning and journal aggregation?"
  5. Options: Register now (invoke /vt-c-project-register) | Skip for now
  6. If registered: continue silently

Step 0.6: Resolve Project Root

Before any file reads or writes, determine the project root directory. Test and build commands executed during the review can shift cwd into subdirectories (e.g., running ng test from consignee-landingpage/), so all file operations must use an explicit root path — never rely on cwd alone.

  1. Walk up from cwd looking for .design-state.yaml or .specify/ or CLAUDE.md
  2. The directory containing the first match is PROJECT_ROOT
  3. If none found, fall back to the git repository root: git rev-parse --show-toplevel
  4. Store this as PROJECT_ROOT for all subsequent file operations

All gate files (.review-gate.md, .test-gate.md) and review artifacts (review-triage.md) MUST be written relative to PROJECT_ROOT, not the current working directory.

✓ Project root resolved: [PROJECT_ROOT]

Step 0.7: Read Deliverable Types

  1. Read deliverable_types from specs/[N]-feature/spec.md YAML frontmatter
  2. If present and non-empty: use as-is
  3. If missing: warn "No deliverable_types in spec frontmatter — defaulting to [code]" and treat as [code]
  4. Display: "Deliverable types: {list}"

The detected types determine which reviewers are dispatched in Step 2 and whether the test gate is enforced in Step 1.5.

Step 1: Load Institutional Knowledge

Before reviewing, load:

  1. docs/solutions/patterns/critical-patterns.md - Known patterns to check
  2. specs/[N]-feature/spec.md - Spec compliance (loaded in Step 0 if exists)
  3. .specify/memory/constitution.md - Constitution compliance (loaded in Step 0 if exists)
  4. Recent journal entries - Context for decisions made
  5. specs/[N]-feature/build-log.md - Implementation narrative (if exists)

Step 1.5: Test Gate Verification

Before dispatching reviewers, verify that tests have passed.

When code is NOT in deliverable_types: Write .test-gate.md to PROJECT_ROOT (resolved in Step 0.6) with status: NOT_RUN, display "Test gate: NOT_RUN (no code deliverables)", and skip to Step 1.8. Do not dispatch test-runner.

When code in deliverable_types (existing behavior):

  1. Read .test-gate.md from PROJECT_ROOT (resolved in Step 0.6)
  2. If missing: Dispatch test-runner agent in full-suite mode first (creates the gate file)
  3. If status: FAIL: Display failing tests and STOP — tests must pass before review begins

    ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    BLOCKED: Tests Failed
    ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    
    [failed_count] tests failed. Code review cannot proceed with failing tests.
    Fix the failing tests and re-run /vt-c-4-review.
    
    Failing tests:
    [list from .test-gate.md]
    ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    
    STOP. Do not proceed with review.

  4. If status: PASS but branch doesn't match current branch:

    ⚠ Test gate was for branch '[gate-branch]' but you are on '[current-branch]'.
      Test results may be stale. Re-running test suite...
    
    Dispatch test-runner in full-suite mode to refresh the gate file.

  5. If status: PASS and branch matches: Display summary and proceed.

    ✓ Test gate: PASS ([total] tests, [passed] passed, [duration])
      Browser tests: [browser_tests status]
    

This ensures reviewers never waste time on code that doesn't pass its own tests.

Step 1.8: Spec-Compliance Gate (if spec exists; corresponds to implementation-orchestrator Step 3.8)

If .specify/ exists and a spec was loaded in Step 0:

  1. Dispatch the spec-compliance-reviewer agent with:
  2. specs/[N]-feature/spec.md (the specification)
  3. specs/[N]-feature/plan.md (if exists)
  4. The branch diff (changes to review)
  5. The reviewer checks each functional requirement and acceptance criterion against the implementation
  6. If PASS → proceed to Step 2 (Quality Review)
  7. If FAIL → report missing requirements and deviations, STOP. Do not dispatch the 6 parallel quality reviewers.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
BLOCKED: Spec Compliance Failed
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Implementation does not match specification.
[Missing/deviated requirements from spec-compliance-reviewer]

Fix spec compliance first, then re-run /vt-c-4-review.
Quality review is skipped — no point reviewing code quality
on an implementation that doesn't match its spec.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

If no spec exists (no .specify/ or no matching spec file), skip this step and proceed directly to Step 2.

Step 1.9: Visual Compliance Check (Frontend Specs)

If SpecKit is detected and spec loaded:

  1. Read ui_scope from specs/[N]-feature/spec.md YAML frontmatter
  2. If ui_scope is backend or absent → skip silently
  3. If ui_scope is frontend or mixed: a. Read the ## Visual Reference section from the spec b. For each referenced screenshot path:
    • Use Read tool to load the image
    • Compare visual design intent against the implementation files
    • Classify findings: match | minor deviation | major deviation c. If no ## Visual Reference section exists:
    • Add a Medium-severity finding: "Frontend-facing spec lacks visual references" d. Display findings:
      Visual Compliance Check
      ─────────────────────────────────────────────────
      Reference: [path]
        [match | minor deviation | major deviation]: [description]
      ─────────────────────────────────────────────────
      
      e. Major deviations are classified as High severity f. Minor deviations are classified as Medium severity
  4. Findings from this step are merged into the Step 3 finding aggregation

Note: Visual compliance runs as a sequential pre-check before the 6 parallel quality reviewers in Step 2.

Step 2: Invoke Implementation Orchestrator

Dispatch reviewers in parallel based on deliverable_types (only after spec-compliance and visual compliance pass or are skipped).

Framework-conditional dispatch: Before dispatching reviewers, check for framework config files in PROJECT_ROOT: - If angular.json exists → dispatch angular-reviewer instead of kieran-typescript-reviewer - If nest-cli.json exists → dispatch nestjs-reviewer instead of kieran-typescript-reviewer - If both exist → dispatch both framework reviewers, skip kieran-typescript-reviewer - If neither exists → dispatch kieran-typescript-reviewer (default)

code type reviewers (dispatched when code in types):

Reviewer Focus
security-sentinel OWASP vulnerabilities, XSS, injection, auth issues
kieran-typescript-reviewer TypeScript quality, strict typing (default — skipped when angular.json or nest-cli.json detected)
angular-reviewer Angular 21 patterns, Signals, PrimeNG, zoneless (conditional — when angular.json detected, replaces kieran-typescript-reviewer)
nestjs-reviewer NestJS modules, controllers, DTOs, guards, pipes (conditional — when nest-cli.json detected, replaces kieran-typescript-reviewer)
julik-frontend-races-reviewer Race conditions, async issues, DOM state
code-simplicity-reviewer YAGNI, complexity, unnecessary abstractions
performance-oracle N+1 queries, memory leaks, bundle size
pattern-recognition-specialist Design patterns, anti-patterns, consistency
accessibility-reviewer WCAG 2.1 AA compliance (always for code)
design-implementation-reviewer Figma/design match (only when ui_scope: frontend)

Non-code type reviewers (dispatched when the corresponding type is in deliverable_types):

Type Reviewer Focus
document document-quality-reviewer Completeness, clarity, accuracy, audience fit, goal alignment, formatting
presentation presentation-quality-reviewer Narrative arc, slide density, speaker notes, visual consistency, timing, CD compliance
research research-quality-reviewer Methodology rigor, source diversity/recency, conclusion support, bias detection, actionability
project-plan project-plan-reviewer Phase completeness, dependency integrity, resource allocation, risk coverage, timeline realism, measurability, stakeholder alignment

Multi-type specs: dispatch the union of all applicable reviewers, all in parallel.

Display: "Dispatching N reviewers for types: {list}"

Step 3: Aggregate Findings

Collect all findings and classify by severity:

  • Critical - Must fix before deployment (security vulnerabilities, data loss risks)
  • High - Should fix (significant quality issues)
  • Medium - Recommended (improvements)
  • Low - Optional (style, minor suggestions)

Step 4: Produce Review Report

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Phase 4: Code Review Results
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Quality Gate: [PASS/FAIL]

Findings Summary:
─────────────────────────────────────────────────────────────────
Critical: X issues (must fix)
High:     X issues (should fix)
Medium:   X issues (recommended)
Low:      X issues (optional)

[Detailed findings by category...]

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Step 4.5: Auto-Fix Loop

After producing the review report (Step 4), classify findings and auto-fix mechanical issues. This step runs entirely within the fork context — all decisions are deterministic, rule-based heuristics. No interactive questions.

4.5a: Classify Findings

For each finding from Step 3, apply the classification heuristic defined in implementation-orchestrator.md Step 5.5. Tag each finding:

  • [AUTO-FIXABLE] — Mechanical fix, safe to apply autonomously
  • [HUMAN-REQUIRED] — Requires human judgment, architectural decision, or policy choice

Classification rules:

  • All Critical severity findings → always [HUMAN-REQUIRED]
  • Ambiguous findings → default to [HUMAN-REQUIRED] (conservative)
  • No auto-fix may change public API signatures or interfaces
  • No auto-fix may modify files outside the reviewed scope
  • No auto-fix may create new files (edits to existing files only)

4.5b: Skip Condition

If zero findings are classified as [AUTO-FIXABLE], skip directly to Step 5. No Auto-Fix Summary is added to the report.

4.5c: Fix-and-Re-Review Loop

Execute the auto-fix loop (maximum 2 iterations):

  1. Apply file edits for all [AUTO-FIXABLE] findings using Edit tool
  2. Re-dispatch only the reviewers whose findings were auto-fixed (partial re-review via Task tool)
  3. Re-aggregate findings from the partial re-review
  4. Classify any new findings using the same heuristic
  5. If new [AUTO-FIXABLE] findings exist AND iteration < 2 → loop to substep 1
  6. If iteration reaches 2 OR no new auto-fixable findings → exit loop

If a finding persists after 2 fix attempts, reclassify it as [HUMAN-REQUIRED].

Timeout constraints:

  • 60 seconds max per individual auto-fix attempt. If exceeded → reclassify as [HUMAN-REQUIRED]
  • 120 seconds total wall clock for the entire loop. If exceeded → exit loop immediately

4.5d: Merge Findings

Produce the final finding set for Step 5:

  • Auto-fixed findings marked [RESOLVED] with fix description
  • Human-required findings unchanged from original classification
  • Reclassified findings (failed to auto-fix after retries) shown as human-required

4.5e: Auto-Fix Summary

Append to the review report output (after detailed findings, before quality gate):

Auto-Fix Summary (Step 4.5)
─────────────────────────────────────────────────────────────────
Attempt 1: Fixed [N] findings, re-reviewed [M] reviewers
  [RESOLVED] [ID]: [description] in [file:line]
  [RESOLVED] [ID]: [description] in [file:line]
Attempt 2: [outcome description]

Remaining human-required findings: [N]
─────────────────────────────────────────────────────────────────

If no auto-fixable findings existed (Step 4.5b skip), omit this section entirely.

Proceed to Step 5 with the merged finding set.


Step 4.6: Built-In Security Review

After the auto-fix loop completes, invoke the built-in /security-review command for additional AI-driven security analysis. This supplements the security-sentinel agent from Step 2.

  1. Availability check: If /security-review is available in the current Claude Code session, invoke it scoped to the branch diff.

  2. Fallback: If the command is not recognized or returns an error:

    Warning: Built-in /security-review not available in this Claude Code version.
    Relying on security-sentinel results only.
    
    Log the warning and continue to Step 5. Do not block the review.

  3. Finding severity mapping: Map /security-review findings to the same severity classification used by the parallel reviewers:

  4. HIGH severity findings → quality gate failures (same as security-sentinel HIGH)
  5. MEDIUM/LOW → advisory (included in report but not blocking)

  6. EC4 compliance: All HIGH findings must be reported individually in the review summary. Do not aggregate or summarize — enumerate each HIGH finding with its location and description.

  7. Merge results: Include /security-review results in the review summary alongside the 6 parallel reviewer results.


Step 5: Quality Gate Decision

Note: The quality gate operates on the post-auto-fix finding set from Step 4.5. Findings marked [RESOLVED] by auto-fix do not count toward the gate decision. If Step 4.5 was skipped (zero auto-fixable findings), the original finding set is used unchanged.

If PASS with zero High findings:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Review Passed
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Code quality meets standards.

Optional improvements identified - address if time permits.

TIP: Consider running /simplify before creating your PR for additional
     code quality improvements. See references/simplify-guide.md for
     focused mode and best practices.

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
NEXT STEP: /vt-c-5-finalize
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

The deploy phase will:
• Run dependency security audit
• Verify migration safety
• Check compliance requirements
• Produce Go/No-Go decision

If PASS but High findings exist (PASS WITH RECOMMENDATIONS):

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Review Passed — Fix Findings and Re-Run Recommended
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Quality gate: PASS (no critical issues)
However, [N] HIGH findings were identified that should be fixed.

Findings to address:
[List of HIGH findings with IDs and brief descriptions]

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
RECOMMENDED: Fix findings, commit, then re-run /vt-c-4-review
ALTERNATIVE: Proceed to /vt-c-5-finalize (findings are documented)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

A second review pass typically catches issues masked by the
first round's findings. Plan for 2-3 review cycles for
production readiness.

TIP: Consider running /simplify before creating your PR for additional
     code quality improvements. See references/simplify-guide.md for
     focused mode and best practices.

If FAIL (Critical or many High issues):

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Review Failed - Issues Found
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Critical/High issues must be addressed before deployment.

[List of issues with fix guidance...]

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
ACTION: Fix issues, then run /vt-c-4-review again
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Recommended: Run /vt-c-3-build to address findings, then re-run /vt-c-4-review.

Reducing Approval Prompts

The allowed-tools frontmatter pre-approves read-only operations (Read, Glob, Grep, git commands, ls, wc) so reviewers can inspect code without triggering manual approval prompts for each command. Write operations (Edit, Write) remain gated — only the auto-fix loop (Step 4.5) writes files, and those are intentional.

If you still see excessive prompts for safe read-only compound commands (e.g., cd /path && git diff), consider running the review in acceptEdits permission mode which allows all read operations plus edits.

Error Handling (context: fork)

This skill runs with context: fork — it executes as an isolated sub-agent that returns only its output to the parent conversation. Fork sub-agents CANNOT interact with the user.

Critical rules for fork context:

  1. NEVER ask interactive questions. No "Would you like...?", no "Should I...?", no AskUserQuestion calls. The question will be swallowed, the sub-agent will die, and the parent conversation receives no output at all.

  2. If a file write fails, report it inline and continue. Use this format:

    Warning — file not persisted: Could not write {filename}. The full content is included in the output above. To persist, copy the content manually or re-run with write permissions.

  3. Always produce the full formatted output as the primary deliverable. The formatted review summary returned to the parent conversation is the primary output. File persistence (.review-gate.md, docs/review-history.md) is secondary. Complete all analysis and formatting BEFORE attempting any file writes.

  4. Gate file fallback: If the .review-gate.md write is denied in Step 6, output the full gate file content (YAML frontmatter + markdown body) inline in the review summary. The parent conversation can then persist it manually.

  5. Step 7 always runs: The Review Retrospective (Step 7) MUST execute regardless of whether Step 6 succeeds. Do not make Step 7 conditional on Step 6 completion.

What Each Reviewer Checks

security-sentinel

  • SQL injection vulnerabilities
  • XSS attack vectors
  • Authentication/authorization flaws
  • Hardcoded secrets
  • Insecure dependencies

kieran-typescript-reviewer

  • Strict type usage (no any)
  • Proper error handling
  • Interface definitions
  • Async/await patterns
  • Import organization

julik-frontend-races-reviewer

  • Race conditions in event handlers
  • Async state management
  • DOM manipulation timing
  • Animation frame issues
  • Event listener cleanup

code-simplicity-reviewer

  • Unnecessary abstractions
  • Over-engineering
  • Dead code
  • Complex conditionals
  • Function length

performance-oracle

  • N+1 database queries
  • Memory leaks
  • Bundle size impact
  • Render performance
  • Caching opportunities

pattern-recognition-specialist

  • Design pattern usage
  • Anti-pattern detection
  • Consistency with codebase
  • Naming conventions
  • File organization

visual-compliance (Step 1.9 — sequential pre-check)

  • Layout/structure match against referenced screenshots
  • Color palette consistency with visual references
  • Spacing and typography alignment
  • Component hierarchy match (nesting, grouping, order)
  • Missing visual references on frontend-facing specs

Spec Compliance Review (When SpecKit Active)

If .specify/ exists, spec compliance is checked via the spec-compliance-reviewer agent in Step 1.8. This agent runs as a sequential gate BEFORE the 6 parallel quality reviewers. See agents/review/spec-compliance-reviewer.md for the detailed output format (structured compliance report with requirement statuses).

What the Spec-Compliance Reviewer Checks

  • Each functional requirement (FR-N) against the implementation
  • Each user story acceptance criterion (Given/When/Then)
  • Each edge case defined in the spec
  • Scope compliance (no unspecified changes)

What It Does NOT Check

  • Code quality, style, naming — that's the quality reviewers' job
  • Security, performance — that's security-sentinel and performance-oracle
  • Constitution adherence — checked separately

Constitution Adherence

Constitution compliance is checked in addition to spec compliance:

Constitution Compliance:
─────────────────────────────────────────────────────────────────
Checking against .specify/memory/constitution.md principles...

[✓/✗] Principle 1: [principle name]
[✓/✗] Principle 2: [principle name]
...

Violations: [none / list of violations]


Step 5.5: Create Beads Issues for Findings (if .beads/ exists)

If .beads/issues.jsonl exists in PROJECT_ROOT:

  1. For each Critical or High finding, create a Beads issue:
    bd create "[Finding title]" --type bug --priority [0 for Critical, 1 for High] --label review,[reviewer-name]
    
  2. For Medium findings, create issues with priority 2:
    bd create "[Finding title]" --type task --priority 2 --label review,[reviewer-name]
    
  3. Low findings are not tracked as Beads issues (too noisy).
  4. Run bd sync to ensure all findings are committed.

This enables the next session to pick up unresolved review findings via bd ready even after conversation clearing.

If .beads/ does not exist, skip silently.


Step 5.7: Review Triage Generation

After Beads issue creation and before writing the gate file, generate a structured review-triage log that records each finding's disposition. This creates an auditable record of review decisions and enables the continuous-learning skill to analyze accept/reject patterns across reviews.

5.7a: Collect Findings and Assign IDs

  1. Collect the merged finding set from Step 4.5d (the post-auto-fix set with [RESOLVED] and [HUMAN-REQUIRED] tags). If Step 4.5 was skipped (zero auto-fixable findings), use the original finding set from Step 3 — all findings are [HUMAN-REQUIRED].

  2. Assign finding IDs using a reviewer prefix + sequential number. The prefix is unique per reviewer, so IDs are globally unique across the triage document. Reset the counter for each reviewer.

Reviewer Prefix
security-sentinel SEC
kieran-typescript-reviewer TS
julik-frontend-races-reviewer RACE
code-simplicity-reviewer SIMP
performance-oracle PERF
pattern-recognition-specialist PAT
spec-compliance-reviewer SPEC
visual-compliance VIS
built-in /security-review BSEC
accessibility-reviewer A11Y
design-implementation-reviewer DES
document-quality-reviewer DOC
presentation-quality-reviewer PRES
research-quality-reviewer RES
project-plan-reviewer PLAN

Example: The first finding from security-sentinel gets ID SEC-1, the second gets SEC-2. The first from code-simplicity-reviewer gets SIMP-1.

  1. Map dispositions:
  2. Findings tagged [RESOLVED] by auto-fix → [RESOLVED] in triage, with the auto-fix description as rationale
  3. Findings tagged [HUMAN-REQUIRED][PENDING] in triage, with "awaiting review" as rationale

5.7b: Determine Output Path and Handle Multi-Pass

  1. Determine the output file path:
  2. If SpecKit is active and specs_dir exists in .design-state.yaml for the active spec: write to PROJECT_ROOT/specs/[N]-feature/review-triage.md
  3. If no SpecKit (no .specify/ directory): write to PROJECT_ROOT/review-triage.md

  4. Check for existing triage file (multi-pass support):

  5. If the output file already exists: read pass_number from its YAML frontmatter, increment by 1, and prepare to append a new ## Pass N section below the existing content. Do NOT overwrite prior passes.
  6. If the output file does not exist: set pass_number: 1 and create a new file.

5.7c: Write Triage File

  1. Write (or append to) the triage file with YAML frontmatter and structured findings:
---
spec_id: SPEC-NNN
branch: feature/spec-NNN-slug
pass_number: 1
date: 2026-03-09T00:00:00Z
total_findings: 5
resolved: 2
pending: 3
accepted: 0
rejected: 0
deferred: 0
---

# Review Triage — SPEC-NNN (Pass 1)

## security-sentinel (3 findings)
- [RESOLVED] (SEC-1) SQL injection risk in user_search → auto-fixed in Step 4.5
- [PENDING] (SEC-2) CSRF token missing in form handler → awaiting review
- [PENDING] (SEC-3) Rate limiting not implemented → awaiting review

## code-simplicity-reviewer (2 findings)
- [RESOLVED] (SIMP-1) Extract method for validation → auto-fixed in Step 4.5
- [PENDING] (SIMP-2) Rename variable for clarity → awaiting review
  • spec_id: From the active spec, or none if no SpecKit
  • branch: Current git branch (git branch --show-current)
  • date: Current UTC timestamp
  • total_findings, resolved, pending: Computed counts for this pass
  • accepted, rejected, deferred: Always 0 on initial generation (updated manually by the user)
  • Group findings by reviewer. Omit reviewers with zero findings.
  • Each finding line: - [DISPOSITION] (ID) description → rationale

  • Zero findings: If no findings exist (clean review), write the file with total_findings: 0 and:

    # Review Triage — SPEC-NNN (Pass 1)
    
    No findings — clean review.
    

  • Multi-pass append: When appending Pass N to an existing file:

  • Update the YAML frontmatter pass_number to N and update the counts cumulatively (sum of all passes)
  • Append a new ## Pass N heading followed by the findings for this pass
  • Do not modify previous pass sections

  • Fork write fallback: If the file write is denied (context: fork), output the full triage file content inline using a fenced code block, preceded by:

    Warning — file not persisted: Could not write review-triage.md. The full triage content is included below. To persist, copy the content manually or re-run with write permissions.

5.7d: Report Integration

After the triage file is written (or output inline), include a "Review Triage Log" summary in the review output. Insert this block after the quality gate decision and auto-fix summary, alongside the other review artifacts:

Review Triage Log
─────────────────────────────────────────────────────────────────
  File: [output path from 5.7b]
  Findings: [N] total ([M] resolved, [K] pending)
  Update [PENDING] dispositions after reviewing findings.
─────────────────────────────────────────────────────────────────

Step 5.8: Quality Metrics Snapshot (Optional)

After review triage and before writing the gate file, collect a quality metrics snapshot when coverage tooling is available. This step is advisory only — it never blocks the review gate.

5.8a: Guard Check

  1. Only run when code is in deliverable_types. If not, skip this step silently.
  2. If no active spec or no specs_dir in .design-state.yaml: skip silently.

5.8b: Auto-Detect Coverage Tooling

Check the project root (resolved in Step 0.6) for coverage tooling, using this priority order:

  1. package.json with nyc in devDependenciesnpx nyc npm test
  2. package.json with c8 in devDependenciesnpx c8 npm test
  3. package.json with scripts.test containing --coveragenpm test
  4. pytest.ini or pyproject.toml with [tool.pytest] section → pytest --cov=src --cov-report=json
  5. go.mod exists → go test -coverprofile=coverage.out ./...
  6. Gemfile with simplecov → coverage auto-generated by test run

If no tooling detected: skip silently — no warning, no metrics in output. Proceed to Step 6.

5.8c: Run Coverage Command

  1. Run the detected command using the Bash tool with timeout: 300000 (5 minutes).
  2. If the command fails (non-zero exit) or times out: display a brief note and skip metrics. Do not block.
    Note: Coverage command failed — skipping metrics snapshot.
    
  3. If successful: parse output for line coverage % and branch coverage %.
  4. For nyc/c8/Jest: parse the text summary table (look for Stmts and Branch columns)
  5. For pytest-cov: read coverage.json if --cov-report=json was used
  6. For go cover: parse coverage.out with go tool cover -func=coverage.out
  7. For SimpleCov: read coverage/.last_run.json

5.8d: Save Metrics Snapshot

Write the snapshot to specs/[N]-feature/metrics.json:

{
  "timestamp": "ISO-8601",
  "command": "npm test -- --coverage",
  "line_coverage": 85.2,
  "branch_coverage": 72.1
}

If the file write is denied (context: fork): output the JSON inline and continue.

5.8e: Regression Detection

  1. If a prior metrics.json already exists in the same spec directory (from a previous review pass):
  2. Read the previous line_coverage value
  3. Compute delta: current - previous
  4. If delta < -2.0: display regression warning
    ⚠ Coverage Regression Detected
    ─────────────────────────────────────────────────
      Line coverage dropped: 85.2% → 81.0% (delta: -4.2%)
      Threshold: -2%
      This is advisory — review gate is not affected.
    ─────────────────────────────────────────────────
    
  5. If delta >= 0: display improvement note
    ✓ Coverage stable/improved: 83.0% → 85.2% (delta: +2.2%)
    
  6. If -2.0 <= delta < 0: display minor note
    Coverage slightly decreased: 85.2% → 84.0% (delta: -1.2%, within threshold)
    
  7. If no prior metrics.json: this is the first snapshot, no comparison.

5.8f: Display Metrics Summary

Quality Metrics:
─────────────────────────────────────────────────
  Line coverage:   85.2%
  Branch coverage: 72.1%
  Regression:      None
─────────────────────────────────────────────────

If regression was detected, show the delta in the Regression line:

  Regression:      ⚠ Line coverage -4.2% (was 85.2%)

Store the collected metrics values (line_coverage, branch_coverage, regression boolean) for use in Step 6 and Step 8.

Step 5.9: Playwright E2E Tests (Frontend Specs Only)

This step only runs when the active spec has ui_scope: frontend or ui_scope: mixed in its YAML frontmatter. If ui_scope is backend or absent, skip this step silently.

1. Check ui_scope guard:

Read: specs/[N]-feature/spec.md
Extract: ui_scope from YAML frontmatter

IF ui_scope is "backend" or absent → skip silently, proceed to Step 6
IF ui_scope is "frontend" or "mixed" → continue

2. Detect Playwright configuration:

Check for playwright.config.ts or playwright.config.js in the project root.

If no Playwright config found:

Note: No Playwright tests found for frontend spec.
  ui_scope: [frontend|mixed] but no playwright.config.ts/.js detected.
  Consider adding e2e tests in e2e/ or tests/ directory.
Skip to Step 6.

3. Detect test files:

Glob for test files: e2e/**/*.spec.ts, tests/**/*.spec.ts, e2e/**/*.spec.js, tests/**/*.spec.js.

If no test files found: display same advisory as step 2, skip to Step 6.

4. Run Playwright tests:

Run: npx playwright test --reporter=list
(use Bash tool timeout parameter set to 300000ms)

5. Capture results and screenshots:

  • Parse test output for pass/fail counts
  • If tests produce screenshots (check test-results/ or Playwright output directory), copy to specs/[N]-feature/screenshots/
  • Create the screenshots directory if it doesn't exist

6. Display summary:

On pass:

Playwright E2E Tests:
─────────────────────────────────────────────────
  Tests:       [N] passed, 0 failed
  Screenshots: specs/[N]-feature/screenshots/ ([M] files)
─────────────────────────────────────────────────

On failure:

Playwright E2E Tests:
─────────────────────────────────────────────────
  Tests:       [N] passed, [M] failed
  Screenshots: specs/[N]-feature/screenshots/ ([K] files)

  ⚠ Failed tests (advisory — does not block review gate):
  • [test name]: [failure reason]
─────────────────────────────────────────────────

E2E test failures are advisory only — they do not affect the PASS/FAIL decision of the review gate. They are reported for developer awareness.

Store the e2e test results (passed count, failed count, screenshot path) for use in Step 6 and Step 8.


Step 6: Write Review Gate File (Mandatory)

After the Quality Gate decision (Step 5), always write a .review-gate.md file to PROJECT_ROOT (resolved in Step 0.6). This file is read by /vt-c-5-finalize and /vt-c-promote to enforce mandatory review before deployment.

Gate File Format

---
status: [PASS or FAIL]
date: [ISO 8601 timestamp]
branch: [current git branch]
reviewers:
  - spec-compliance-reviewer
  - visual-compliance
  - security-sentinel
  - kieran-typescript-reviewer
  - julik-frontend-races-reviewer
  - code-simplicity-reviewer
  - performance-oracle
  - pattern-recognition-specialist
spec_compliance: [PASS or FAIL or N/A]
critical_count: [number]
high_count: [number]
medium_count: [number]
low_count: [number]
findings_dir: todos/
tests_status: [PASS or FAIL or NOT_RUN]
tests_total: [number]
tests_failed: [number]
autofix_applied: [true or false]
autofix_attempts: [0, 1, or 2]
autofix_resolved: [number]
autofix_findings:
  - id: [finding-id]
    file: [file-path]
    fix: [description of fix applied]
    reviewer: [reviewer-name]
metrics:
  line_coverage: [number or omit field]
  branch_coverage: [number or omit field]
  regression: [true or false or omit field]
e2e_tests:
  passed: [number or omit field]
  failed: [number or omit field]
  screenshots: [path or omit field]
---

# Review Gate

Quality Gate: [PASS or FAIL]
Reviewed: [date]
Branch: [branch]

## Summary
[1-2 sentence summary of review outcome]

## Blocking Issues (if FAIL)
- [List of critical/high issues that caused failure]

Writing the Gate File

BRANCH=$(git branch --show-current 2>/dev/null || echo "unknown")
TIMESTAMP=$(date -u +"%Y-%m-%dT%H:%M:%SZ")

Write .review-gate.md to PROJECT_ROOT using the format above, filling in: - status: PASS if quality gate passed, FAIL if it failed - date: current UTC timestamp - branch: current git branch - Counts from the severity classification in Step 3 - Summary from the review report in Step 4 - metrics: from Step 5.8 (line_coverage, branch_coverage, regression). Omit the entire metrics: field if Step 5.8 was skipped or no tooling was detected - e2e_tests: from Step 5.9 (passed, failed, screenshots path). Omit the entire e2e_tests: field if Step 5.9 was skipped (backend spec or no Playwright config)

IMPORTANT: Write the gate file on BOTH pass and fail. On fail, the file records why deployment is blocked.

If the write is denied (context: fork): Output the full gate file content (YAML frontmatter + markdown body) inline in the review summary using a fenced code block, preceded by:

Warning — file not persisted: Could not write .review-gate.md. The full gate file content is included below. To persist, copy the content manually or re-run with write permissions.

Step 6b: Write Test Gate File (CRITICAL — DO NOT SKIP)

BLOCKING REQUIREMENT: Write a .test-gate.md file to PROJECT_ROOT (resolved in Step 0.6). Without this file, /vt-c-5-finalize will refuse to proceed (Step 0b). This is the #1 cause of failed deployments.

This file is separate from .review-gate.md — both must exist for the workflow to complete.

---
status: [PASS or FAIL or NOT_RUN]
date: [ISO 8601 timestamp]
branch: [current git branch]
tests_total: [number]
tests_passed: [number]
tests_failed: [number]
test_framework: [detected framework or "unknown"]
duration: [test duration if available]
---

# Test Gate

Status: [PASS or FAIL or NOT_RUN]
Date: [date]
Branch: [branch]

## Summary
[1-2 sentence summary: e.g., "All 47 tests passed in 12.3s" or "Tests not available for this project"]

Populate from test data: - If tests were run in Step 1.5: use the test-runner results - If .test-gate.md already exists from Step 1.5 (created by test-runner): keep it, do not overwrite - If no test infrastructure exists: write with status: NOT_RUN and note "No test infrastructure detected"

If the write is denied (context: fork): Output the full .test-gate.md content inline, similar to the .review-gate.md fallback above.

Step 6c: Persist Review Gate in state.yaml

After writing the ephemeral gate files, also persist the review result in specs/[N]-feature/state.yaml for committed audit trail:

  1. Derive the active spec from the branch name (feature/spec-NNN-* → SPEC-NNN)
  2. Look up specs_dir in .design-state.yaml
  3. Read specs/[N]-feature/state.yaml
  4. Add or update the review_gate section:
    review_gate:
      status: PASS|FAIL
      date: "YYYY-MM-DD"
      branch: [current git branch]
      critical: [count]
      high: [count]
    
  5. Write the updated state.yaml
  6. Stage: git add specs/[N]-feature/state.yaml

If no active spec or no state.yaml: skip silently. If write is denied (context: fork): skip silently — the ephemeral .review-gate.md is sufficient for the current session.

Step 6d: Write Phase Checkpoint

Write .claude-checkpoint.md to the project root: - completed_phase: 4-review - next_phase: 5-finalize - next_command: /vt-c-5-finalize - branch: current git branch - active_spec: derived from branch or .active-spec - clear_recommended: true - clear_reason: "Deploy checks are independent of review conversation; review findings are captured in the report" - Key Outcomes: review verdict (PASS/FAIL), finding counts by severity - Context for Next Phase: critical/high findings that need attention, review report location

Display the clear recommendation per the phase-checkpoint protocol.


Step 7: Review Retrospective (Learning Capture)

This step MUST run regardless of Step 6 outcome. Even if the gate file write was denied, proceed with retrospective analysis.

After the gate file step, analyze findings for patterns and update project knowledge.

7a: Update Review History

Check if docs/review-history.md exists in the project. If not, initialize it from the template:

# Review History

## Reviews
| Date | Branch | Gate | Critical | High | Medium | Low | Agents |
|---|---|---|---|---|---|---|---|

## Recurring Findings
| Finding Type | Frequency | First Seen | Agent | Action Taken |
|---|---|---|---|---|

## Agent Effectiveness
| Agent | Reviews Run | Findings | Critical Catches |
|---|---|---|---|

Append this review's data to the Reviews table.

7b: Pattern Analysis

  1. Scan todos/ files created during this review
  2. Categorize findings by type (security, performance, complexity, patterns, testing)
  3. Read docs/review-history.md — check the Recurring Findings table
  4. If any finding type appears 2+ times across reviews:
  5. Update the Recurring Findings table (increment frequency or add new entry)
  6. Check docs/solutions/patterns/critical-patterns.md — is this pattern already documented?
  7. If NOT documented: append the pattern to critical-patterns.md with the format:
    ### [Pattern Name] (Added from review retrospective [date])
    ❌ WRONG: [Anti-pattern description]
    ✅ CORRECT: [Correct approach]
    Source: Review findings on [branch] — [agent] detected [count] times
    

7b+: CLAUDE.md Evolution Suggestion

If recurring patterns were detected in Step 7b (same finding type appeared 2+ times across reviews):

TIP: Recurring patterns found. Consider running /claudemd-evolve collect
to propose CLAUDE.md rules for these patterns.

This is a suggestion only — do not auto-invoke /claudemd-evolve.

7c: Update Agent Effectiveness

Update the Agent Effectiveness table in docs/review-history.md: - Increment "Reviews Run" for each agent used - Add finding counts per agent - Note any critical catches

This data informs agent prompt improvements via /vt-c-research-ingest.

7d: Manual Test Verification (Advisory)

If docs/test-plan.md exists in the project:

  1. Read the Manual/Acceptance Tests table
  2. Check if all manual tests are marked as completed (status column)
  3. If unchecked tests remain:
  4. List them in the review report
  5. Flag as: "ℹ Manual tests incomplete — [N] of [M] tests not verified"
  6. This is advisory (not blocking) — noted in review output but does not affect the quality gate
  7. If all manual tests checked: "✓ All manual tests verified ([M] of [M])"
  8. If no docs/test-plan.md exists: skip silently (manual tests are optional)

7e: Documentation Completeness (if docs/user-manual/ exists)

If docs/user-manual/ exists in PROJECT_ROOT:

  1. Read the current specs (or specs/[N]-feature/spec.md) for "Documentation Requirements" checklist items
  2. Check if the referenced documentation files exist and are non-empty
  3. Report in the review findings:
  4. docs_complete: true/false
  5. docs_missing: list of missing documentation files
  6. Severity: Medium (does not block review gate, but flagged in review report)

Add to .review-gate.md frontmatter:

docs_status: COMPLETE    # or INCOMPLETE or N/A
docs_missing: []         # list of missing files, or empty

If docs/user-manual/ does not exist, set docs_status: N/A and skip silently.


Step 8: TL;DR Summary (Final Output)

This MUST be the absolute last output of the review. Nothing may follow it. Max 10 lines between delimiters.

After ALL previous steps (including retrospective and file writes), output a concise summary using the format below. Fill in values from Steps 3–5.

If PASS:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
TL;DR
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Quality Gate: PASS
Findings: [N] critical | [N] high | [N] medium | [N] low
Spec Compliance: [X]/[Y] ([Z]%)
Branch: [branch-name]

Next: /vt-c-5-finalize
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

If PASS with HIGH findings (recommended re-run):

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
TL;DR — PASS WITH RECOMMENDATIONS
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Quality Gate: PASS (but [N] HIGH findings need attention)
Findings: [N] critical | [N] high | [N] medium | [N] low
Spec Compliance: [X]/[Y] ([Z]%)
Branch: [branch-name]

Fix these: [HIGH finding IDs and brief descriptions]
Next: Fix findings, commit, re-run /vt-c-4-review
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

If FAIL:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
TL;DR
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Quality Gate: FAIL
Findings: [N] critical | [N] high | [N] medium | [N] low
Spec Compliance: [X]/[Y] ([Z]%)
Branch: [branch-name]

Blockers: [critical/high finding IDs]
Next: Fix blockers, then re-run /vt-c-4-review
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

If zero findings:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
TL;DR
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Quality Gate: PASS
Findings: 0 critical | 0 high | 0 medium | 0 low
Spec Compliance: [X]/[Y] ([Z]%)
Branch: [branch-name]

Next: /vt-c-5-finalize
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

If error mid-execution: Still produce the TL;DR with whatever information is available, plus a note:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
TL;DR
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Quality Gate: INCOMPLETE
Findings: [partial counts if available]
Branch: [branch-name]
Error: [brief description of what failed]

Next: Investigate error, then re-run /vt-c-4-review
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Notes: - Omit the "Spec Compliance" line if SpecKit is not active (no .specify/ directory) - Omit the "What to fix" / "Blockers" line if there are no findings to report - Include Auto-fixed: [N] findings resolved in [M] attempts after the "Findings:" line when Step 4.5 applied auto-fixes. Omit this line if Step 4.5 was skipped - Include Coverage: [N]% line, [N]% branch (no regression) after the "Findings:" line (or after the "Auto-fixed:" line if present) when Step 5.8 collected metrics. If regression was detected, show Coverage: [N]% line, [N]% branch (⚠ regression: -[N]%) instead. Omit this line if Step 5.8 was skipped or no tooling was detected - Include E2E Tests: [N] passed, [M] failed ([K] screenshots) when Step 5.9 ran Playwright tests. Omit this line if Step 5.9 was skipped - The TL;DR summarizes Steps 3–5; do not repeat the full report


Why Multiple Review Passes Are Expected

Plan for 2-3 review cycles before considering code production-ready. A single pass rarely catches everything.

The Attention-Budget Effect

LLM-based reviewers exhibit an "attention budget": HIGH-severity findings in pass 1 consume reviewer focus, crowding out MEDIUM and LOW observations. Once the HIGHs are fixed and the reviewer re-runs, a fresh layer of findings emerges — not because they were invisible before, but because the reviewer's attention is no longer dominated by the loudest issues.

Contributing Factors

  1. LLM stochasticity — Non-deterministic output means each run explores slightly different paths through the code
  2. Context window noise — Code changes between runs shift which sections get the most scrutiny
  3. Fix-induced findings — Fixing one issue can introduce or reveal adjacent issues
Pass 1: Fix all CRITICAL and HIGH findings
Pass 2: Fix remaining HIGH + MEDIUM findings
Pass 3: Clean sweep — LOW findings and polish

Each pass should be a separate commit. The review gate status reflects the latest pass only.