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¶
Pre-flight: For best results with
codedeliverables, run/simplifybefore this command. The parallel reviewers then focus on architectural and security concerns rather than code quality issues that/simplifyhandles in 90 seconds.Skip
/simplifyif: 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:
- Load constitution -
.specify/memory/constitution.mdfor principles - Derive active spec from branch — check
.active-specfile first, then parse branch name (feature/spec-NNN-*→ SPEC-NNN), then look upspecs_dirin.design-state.yaml - Load spec -
specs/[N]-feature/spec.mdfor compliance checking - Display status:
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¶
- Read
TOOLKIT_ROOT/intake/projects.yaml(resolve TOOLKIT_ROOT from this skill's base directory) - Check if the current working directory appears in the
projectslist - If NOT registered and the project has a CLAUDE.md or .design-state.yaml:
- Ask: "This project is not registered in the toolkit. Register now to enable cross-project scanning and journal aggregation?"
- Options: Register now (invoke /vt-c-project-register) | Skip for now
- 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.
- Walk up from cwd looking for
.design-state.yamlor.specify/orCLAUDE.md - The directory containing the first match is
PROJECT_ROOT - If none found, fall back to the git repository root:
git rev-parse --show-toplevel - Store this as
PROJECT_ROOTfor 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.
Step 0.7: Read Deliverable Types¶
- Read
deliverable_typesfromspecs/[N]-feature/spec.mdYAML frontmatter - If present and non-empty: use as-is
- If missing: warn "No
deliverable_typesin spec frontmatter — defaulting to[code]" and treat as[code] - 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:
docs/solutions/patterns/critical-patterns.md- Known patterns to checkspecs/[N]-feature/spec.md- Spec compliance (loaded in Step 0 if exists).specify/memory/constitution.md- Constitution compliance (loaded in Step 0 if exists)- Recent journal entries - Context for decisions made
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):
- Read
.test-gate.mdfromPROJECT_ROOT(resolved in Step 0.6) - If missing: Dispatch
test-runneragent in full-suite mode first (creates the gate file) -
If
status: FAIL: Display failing tests and STOP — tests must pass before review beginsSTOP. Do not proceed with review.━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 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] ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ -
If
status: PASSbutbranchdoesn't match current branch:Dispatch⚠ Test gate was for branch '[gate-branch]' but you are on '[current-branch]'. Test results may be stale. Re-running test suite...test-runnerin full-suite mode to refresh the gate file. -
If
status: PASSand branch matches: Display summary and proceed.
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:
- Dispatch the
spec-compliance-revieweragent with: specs/[N]-feature/spec.md(the specification)specs/[N]-feature/plan.md(if exists)- The branch diff (changes to review)
- The reviewer checks each functional requirement and acceptance criterion against the implementation
- If PASS → proceed to Step 2 (Quality Review)
- 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:
- Read
ui_scopefromspecs/[N]-feature/spec.mdYAML frontmatter - If
ui_scopeisbackendor absent → skip silently - If
ui_scopeisfrontendormixed: a. Read the## Visual Referencesection 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 Referencesection exists: - Add a Medium-severity finding: "Frontend-facing spec lacks visual references" d. Display findings: e. Major deviations are classified as High severity f. Minor deviations are classified as Medium severity
- 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):
- Apply file edits for all
[AUTO-FIXABLE]findings using Edit tool - Re-dispatch only the reviewers whose findings were auto-fixed (partial re-review via Task tool)
- Re-aggregate findings from the partial re-review
- Classify any new findings using the same heuristic
- If new
[AUTO-FIXABLE]findings exist AND iteration < 2 → loop to substep 1 - 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.
-
Availability check: If
/security-reviewis available in the current Claude Code session, invoke it scoped to the branch diff. -
Fallback: If the command is not recognized or returns an error:
Log the warning and continue to Step 5. Do not block the review. -
Finding severity mapping: Map
/security-reviewfindings to the same severity classification used by the parallel reviewers: - HIGH severity findings → quality gate failures (same as security-sentinel HIGH)
-
MEDIUM/LOW → advisory (included in report but not blocking)
-
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.
-
Merge results: Include
/security-reviewresults 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:
-
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.
-
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. -
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. -
Gate file fallback: If the
.review-gate.mdwrite 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. -
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:
- For each Critical or High finding, create a Beads issue:
- For Medium findings, create issues with priority 2:
- Low findings are not tracked as Beads issues (too noisy).
- Run
bd syncto 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¶
-
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]. -
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.
- Map dispositions:
- Findings tagged
[RESOLVED]by auto-fix →[RESOLVED]in triage, with the auto-fix description as rationale - Findings tagged
[HUMAN-REQUIRED]→[PENDING]in triage, with "awaiting review" as rationale
5.7b: Determine Output Path and Handle Multi-Pass¶
- Determine the output file path:
- If SpecKit is active and
specs_direxists in.design-state.yamlfor the active spec: write toPROJECT_ROOT/specs/[N]-feature/review-triage.md -
If no SpecKit (no
.specify/directory): write toPROJECT_ROOT/review-triage.md -
Check for existing triage file (multi-pass support):
- If the output file already exists: read
pass_numberfrom its YAML frontmatter, increment by 1, and prepare to append a new## Pass Nsection below the existing content. Do NOT overwrite prior passes. - If the output file does not exist: set
pass_number: 1and create a new file.
5.7c: Write Triage File¶
- 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, ornoneif no SpecKitbranch: Current git branch (git branch --show-current)date: Current UTC timestamptotal_findings,resolved,pending: Computed counts for this passaccepted,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: 0and: -
Multi-pass append: When appending Pass N to an existing file:
- Update the YAML frontmatter
pass_numberto N and update the counts cumulatively (sum of all passes) - Append a new
## Pass Nheading 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¶
- Only run when
codeis indeliverable_types. If not, skip this step silently. - If no active spec or no
specs_dirin.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:
package.jsonwithnycindevDependencies→npx nyc npm testpackage.jsonwithc8indevDependencies→npx c8 npm testpackage.jsonwithscripts.testcontaining--coverage→npm testpytest.iniorpyproject.tomlwith[tool.pytest]section →pytest --cov=src --cov-report=jsongo.modexists →go test -coverprofile=coverage.out ./...Gemfilewithsimplecov→ 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¶
- Run the detected command using the Bash tool with
timeout: 300000(5 minutes). - If the command fails (non-zero exit) or times out: display a brief note and skip metrics. Do not block.
- If successful: parse output for line coverage % and branch coverage %.
- For nyc/c8/Jest: parse the text summary table (look for
StmtsandBranchcolumns) - For pytest-cov: read
coverage.jsonif--cov-report=jsonwas used - For go cover: parse
coverage.outwithgo tool cover -func=coverage.out - 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¶
- If a prior
metrics.jsonalready exists in the same spec directory (from a previous review pass): - Read the previous
line_coveragevalue - Compute delta:
current - previous - If delta < -2.0: display regression warning
- If delta >= 0: display improvement note
- If -2.0 <= delta < 0: display minor note
- 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:
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.
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:
5. Capture results and screenshots:
- Parse test output for pass/fail counts
- If tests produce screenshots (check
test-results/or Playwright output directory), copy tospecs/[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:
- Derive the active spec from the branch name (
feature/spec-NNN-*→ SPEC-NNN) - Look up
specs_dirin.design-state.yaml - Read
specs/[N]-feature/state.yaml - Add or update the
review_gatesection: - Write the updated state.yaml
- 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¶
- Scan
todos/files created during this review - Categorize findings by type (security, performance, complexity, patterns, testing)
- Read
docs/review-history.md— check the Recurring Findings table - If any finding type appears 2+ times across reviews:
- Update the Recurring Findings table (increment frequency or add new entry)
- Check
docs/solutions/patterns/critical-patterns.md— is this pattern already documented? - If NOT documented: append the pattern to
critical-patterns.mdwith the format:
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:
- Read the Manual/Acceptance Tests table
- Check if all manual tests are marked as completed (status column)
- If unchecked tests remain:
- List them in the review report
- Flag as:
"ℹ Manual tests incomplete — [N] of [M] tests not verified" - This is advisory (not blocking) — noted in review output but does not affect the quality gate
- If all manual tests checked:
"✓ All manual tests verified ([M] of [M])" - If no
docs/test-plan.mdexists: skip silently (manual tests are optional)
7e: Documentation Completeness (if docs/user-manual/ exists)¶
If docs/user-manual/ exists in PROJECT_ROOT:
- Read the current specs (or
specs/[N]-feature/spec.md) for "Documentation Requirements" checklist items - Check if the referenced documentation files exist and are non-empty
- Report in the review findings:
- docs_complete: true/false
- docs_missing: list of missing documentation files
- Severity: Medium (does not block review gate, but flagged in review report)
Add to .review-gate.md frontmatter:
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¶
- LLM stochasticity — Non-deterministic output means each run explores slightly different paths through the code
- Context window noise — Code changes between runs shift which sections get the most scrutiny
- Fix-induced findings — Fixing one issue can introduce or reveal adjacent issues
Recommended Workflow¶
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.