Vibe Review

Vibe Review Skill

Mandatory quality gate after every phase. Acts as Senior Engineer, Architect, and Code Quality Auditor. Evidence-based. Every finding backed by file path and line number.

Always run in Plan Mode (Shift+Tab). Never writes feature code.

The O'Reilly principle this enforces

Issues caught at Phase 1: cheap — fix once, never propagates. Issues caught at Phase 3: expensive — retrofit across every feature. Issues caught at deploy: very expensive — production risk.

Without a review gate, bad patterns from Phase 1 get copied into every feature. Architecture drifts silently. Duplication builds. Quality degrades.

When this skill runs

Mandatory — phase gates:

Phase 1 complete → review: phase 1 → gate passed? → Phase 2 begins
Phase 2 complete → review: phase 2 → gate passed? → Phase 3 begins
Final phase      → review: final  → gate passed? → deploy unlocked

On demand:

review: phase 2 invoice management
review: audit the Supabase integration
review: full codebase before new team member

Gate rules:

• P0 → blocks progression, fix tasks in TASKS.md immediately
• P1 → logged to backlog.md, must resolve before deploy
• Final phase: P0 AND P1 both must be zero before deploy
• No P0s → gate passes, next phase begins

Documents this skill reads

Read in this order. Note if any are missing.

1. vibe/ARCHITECTURE.md — primary reference for drift detection
2. vibe/CODEBASE.md — exact file paths as-built
3. vibe/SPEC.md — acceptance criteria to verify against
4. vibe/DECISIONS.md — past decisions, context for why things are as they are
5. vibe/PLAN.md — phase scope, what was meant to be built
6. vibe/TASKS.md — what tasks completed this phase
7. vibe/reviews/ — all previous review files for carryover tracking
8. BRIEF.md (if exists) — original intent and core value
9. CLAUDE.md — boundaries and conventions

If vibe/ARCHITECTURE.md does not exist:

Flag this as a finding in the report: "ARCHITECTURE.md not found — no explicit architecture decisions documented. Pattern compliance reviewed against PLAN.md and CODEBASE.md instead." Use PLAN.md folder structure, tech stack, and conventions as the reference baseline. Recommend running architect: to formalise decisions. This is a P1 finding.

Step 0A — Concept boundary pre-screening (graph-assisted)

Check for dependency graph before reading any source files:

ls vibe/graph/CONCEPT_GRAPH.json 2>/dev/null && echo "GRAPH EXISTS" || echo "NO GRAPH"

If graph exists — run boundary pre-screening:

Read vibe/graph/DEPENDENCY_GRAPH.json and check for cross-concept imports that violate ARCHITECTURE.md patterns:

import json

dep_g = json.load(open('vibe/graph/DEPENDENCY_GRAPH.json'))
arch_violations = []

for file_path, node in dep_g.items():
    file_concept = node.get('concept', 'foundation')
    for imported in node.get('imports', []):
        imported_node = dep_g.get(imported, {})
        imported_concept = imported_node.get('concept', 'foundation')

        if file_concept != imported_concept and imported_concept != 'foundation':
            # Frontend component importing backend/agent directly → DIP violation
            if node.get('type') == 'component' and imported_node.get('type') in ['agent', 'service', 'model']:
                arch_violations.append({'file': file_path, 'imports': imported,
                    'issue': 'Frontend directly imports backend — DIP violation', 'severity': 'P1'})
            # Agent calling another agent directly (bypasses orchestrator)
            elif node.get('type') == 'agent' and imported_node.get('type') == 'agent':
                if 'base_agent' not in imported:
                    arch_violations.append({'file': file_path, 'imports': imported,
                        'issue': 'Agent directly imports agent — bypasses orchestrator', 'severity': 'P1'})

for v in arch_violations:
    print(f"{v['severity']}: {v['file']} → {v['imports']} ({v['issue']})")

Flagged files → deep review (read source, check thoroughly) Clean files → standard review (check naming, size, test coverage only)

This pre-screening surfaces most P1 architectural findings before reading a single source file. On a clean codebase: 0 violations, shallower reads. On a drifting codebase: violations surface immediately, guide where to look.

If no graph exists — proceed with standard full read: No pre-screening available. Read all files in the phase as normal.

Step 0 — Run automated checks first

Prefer first-party review tools when available. If the session has the /code-review and/or /security-review commands, run them here — they are deeper and produce structured findings. Fold their output into this review's P0-P3 findings (map their severities onto ours) rather than re-deriving everything by hand. The stack commands below still run (tests/lint/typecheck/ audit are ground truth); the manual Steps 3-8 then focus on what the tools don't cover (architecture drift vs ARCHITECTURE.md, spec alignment). Treat the hand-rolled grep checks as the fallback when those tools aren't available.

Run these before reading any code. Adapt to the project stack from CODEBASE.md section 2.

Node / npm projects:

npm test              # Failures are P0
npx eslint . --quiet  # Errors are P0, warnings are P2
npx tsc --noEmit      # TypeScript errors are P0
npm audit             # High/critical = P0, moderate = P1

Python projects:

pytest                # Failures are P0
ruff check .          # Errors are P0
mypy .                # Type errors are P0
pip-audit             # Critical vulnerabilities are P0

Flutter / Dart projects:

flutter test          # Failures are P0
flutter analyze       # Errors are P0
dart pub audit        # Critical vulnerabilities are P0

No test runner configured: Flag as P1: "No automated test runner configured. Tests exist but cannot be run automatically."

Include full tool output in the review report. If a check fails with P0-level findings: document output, do not stop — continue full review. Machines verify first, agent verifies second.

Step 1 — Establish review scope

Scoping rule: If this phase modified more than 30 files, review feature by feature. Generate a sub-report per feature, then synthesise into one phase report. Do not attempt to review 30+ files in a single pass — quality degrades.

Review focus by phase:

Phase 1 — Foundation integrity: Is this a solid base? Patterns, abstractions, folder structure, TypeScript setup, shared utilities.

Phase 2+ — Incremental delta + integration: What this phase added + does it integrate cleanly? Cross-phase consistency. Emerging duplication.

Final phase — Full production readiness: Everything. Performance, security, accessibility, error handling, bundle size. Strictest gate.

Step 2 — Carryover check

Read all vibe/reviews/phase-N-review.md files. Read vibe/reviews/backlog.md.

If no previous reviews exist (this is Phase 1): State: "No previous reviews — this is the first review. No carryover to check." Skip carryover section in the report.

If previous reviews exist: For each previously logged issue, check current state:

✅ P1-001 — [Issue] — RESOLVED
⚠️ P1-002 — [Issue] — STILL PRESENT — escalating to P0 (appeared in 2+ reviews)
❌ P1-003 — [Issue] — NOT ADDRESSED — remains P1

Escalation rule: unresolved P0 from previous phase → highest priority. P1 appearing unresolved across 2+ phase reviews → escalate to P0. P2 appearing unresolved across 3+ phase reviews → escalate to P1.

Step 3 — Architecture drift detection

🧠 Effort: drift detection and the SOLID/security judgment in Steps 3-7 are the highest-value reasoning in a review — run the review at high/xhigh effort with adaptive thinking. For the final-gate review, consider claude-opus-5.

Most important section. Check ARCHITECTURE.md (or PLAN.md if no ARCHITECTURE.md) first.

For each documented decision, check the actual code.

Drift format:

🔴 ARCHITECTURE DRIFT — [Section violated]
   Decision: "[exact quote from ARCHITECTURE.md or PLAN.md]"
   Found: [file path] line [N]
          [exact violation]
   Decision origin: [architect: session / D-ID / PLAN.md]
   Impact: [what breaks or degrades]
   Fix: [specific action]

Check:

• Folder structure — all new files in correct folder?
• Naming conventions — files, components, hooks, utilities match exactly?
• State management — correct layer for each type of state?
• Data fetching — all calls going through the abstraction layer?
• Component patterns — functional, one per file, props typed?
• TypeScript — strict mode, no any, all params typed?
• Error handling — following the documented strategy?
• Testing — co-located files, names describe behaviour?

Any violation = P0. Architecture drift that propagates is expensive.

Step 4 — SOLID principles review

Read each source file created or modified this phase. Every violation cites file path and line.

SRP: Each component/hook/service has one reason to change.

• 500 lines → P1. >1000 lines → P0 CRITICAL.

• Component handling both UI AND business logic → extract hook.

OCP: New features added without modifying existing working code?

• Conditional chains replacing polymorphism? Flag.

LSP: Components/functions behave consistently across implementations?

• Hook return shapes consistent — all returning { data, loading, error }?

ISP: Props interfaces not forcing unused props?

• Props interface >10 props (especially optional) → P1.

DIP: High-level components depend on abstractions, not concrete implementations?

• Direct DB client import in component → P0 CRITICAL
• Direct localStorage in component → P1

Step 5 — Platform-specific review

Read references/PLATFORM_CHECKS.md. Apply the section matching this project's stack (from CODEBASE.md section 2).

Platforms covered: React Web · React Native · Node/Express · Supabase · Security (universal)

Step 6 — Code quality analysis

Component size audit: Log every component exceeding thresholds. Duplication analysis: Patterns repeated across 3+ files → extract. TypeScript quality: Count any usages — each is P0. Test quality: Names describe behaviour, not implementation. AAA structure present.

Performance-tagged findings (handoff to vibe-perf): when a finding is a performance concern (N+1 queries, unbounded lists, missing memoisation, oversized bundles, sequential calls that could parallelise), tag it category: performance in the report's structured findings and note it. This is what vibe-perf Entry B triggers on — after review, if any performance P1/P2 findings exist, suggest running perf: to audit and fix them properly. Do not attempt deep perf fixes inside review.

Step 7 — Security review

Universal — all projects, all phases:

# Include npm audit / pip-audit output from Step 0

• Hardcoded secrets, tokens, API keys → P0 CRITICAL
• .env in .gitignore → P0 if missing
• User input rendered without sanitisation (XSS) → P0
• Auth checks before protected content → P0 if missing
• Sensitive data not logged → P1
• Input validation at API boundary → P1 if missing
• Dependency vulnerabilities: high/critical = P0, moderate = P1

Final phase review only (additional checks):

• CORS not configured as * in production → P1
• Security headers (Helmet or equivalent) → P1
• Rate limiting on public endpoints → P1
• HTTPS enforced in production config
• Content Security Policy (CSP) headers set
• Session management correct
• Zero high/critical dependency vulnerabilities (must be zero before deploy)

Step 8 — Testing review

• Tests present for all business logic? (missing → P1)
• Test names describe behaviour? (implementation names → P2)
• AAA structure? (missing → P2)
• Edge cases and error states covered for critical paths? (missing → P1)
• Coverage estimate >70% on business logic? (below → P1)

Step 9 — Generate the review report

Read references/REVIEW_REPORT.md for the full template.

Save to vibe/reviews/phase-[N]-review.md.

Quality score formula:

• Start at 10.0
• Subtract 1.0 per P0 finding
• Subtract 0.5 per P1 finding
• Subtract 0.2 per P2 finding
• Subtract 0.1 per P3 finding
• Subtract 0.5 additional per architecture drift violation

Evidence standard — non-negotiable: Every P0 and P1 must have file path + line number + specific actionable recommendation. "Some components are too complex" — rejected, no file path. If a finding cannot be backed by evidence — it does not go in the report.

Step 10 — Update vibe/TASKS.md

P0 issues found — insert blocking tasks:

🔴 Review fixes required — Phase [N] gate (0/N)
   Must complete before Phase [N+1] begins.
   [ ] RFX-001 · [P0 fix — plain English]
                 File: [path] · Issue: [one line]
   → Full report: vibe/reviews/phase-[N]-review.md

## Phase gates
Phase [N] → Phase [N+1]:  🔴 BLOCKED — [N] P0 issues, fix tasks above

No P0 issues — update gate status:

## Phase gates
Phase [N] → Phase [N+1]:  ✅ reviewed [date] — 0 P0, [N] P1 logged to backlog

Step 11 — Update vibe/reviews/backlog.md

Add all P1/P2/P3 findings. Use the format in references/REVIEW_REPORT.md.

Step 11.5 — Record the gate state (enforcement)

Read references/GATES.md. Write the outcome to vibe/.gates.json so phase advancement and the deploy gate can check this review rather than trust memory — this is what makes the gate hold in manual mode. Merge into the existing file; never drop other phases' entries.

# Set these from the review you just completed:
#   PHASE = the phase reviewed ("1", "2", … or "final")
#   P0 / P1 = the P0 / P1 finding counts from this review
PHASE="[phase]"; P0=[p0 count]; P1=[p1 count]
python3 - "$PHASE" "$P0" "$P1" << 'PY'
import json, sys, pathlib, datetime
phase, p0, p1 = sys.argv[1], int(sys.argv[2]), int(sys.argv[3])
p = pathlib.Path("vibe/.gates.json")
g = json.loads(p.read_text()) if p.exists() and p.read_text().strip() else {"phases": {}}
g.setdefault("phases", {})
status = "passed" if p0 == 0 else "open"
entry = {"review": status, "p0": p0, "p1": p1,
         "date": datetime.date.today().isoformat(),
         "report": f"vibe/reviews/phase-{phase}-review.md"}
if phase == "final":
    g["final"] = {"review": "passed" if (p0 == 0 and p1 == 0) else "open", **entry}
else:
    g["phases"][phase] = entry
p.write_text(json.dumps(g, indent=2) + "\n")
print(f"gate recorded: phase {phase} → {status} (P0={p0}, P1={p1})")
PY

If P0 > 0, the phase gate is open — the generated CLAUDE.md's advancement rule will block the next phase from starting until a re-review passes.

Step 12 — Update vibe/ARCHITECTURE.md (if gaps found)

If review reveals a pattern being used that isn't documented:

• Add to the relevant section with > 📝 [date] · Added during phase-[N] review
• Log in DECISIONS.md if it changes something previously decided

Step 13 — Signal done and generate hooks

Read execution mode:

grep "VIBE_MODE" CLAUDE.md 2>/dev/null | cut -d= -f2 | tr -d ' '

Standard output (both modes):

## Phase [N] Review Complete

📊 Score: [X/10] — Grade [A-F]
🏗️  Architecture drift: [N issues / none]
🔴 P0 issues: [N — fix tasks in TASKS.md / none]
🔶 P1 issues: [N — logged to backlog]
📋 Report: vibe/reviews/phase-[N]-review.md

If manual or not set — standard gate message:

Gate decision:
[✅ PASS — Phase [N+1] may begin. Say "next" to continue.]
[🔴 BLOCKED — complete RFX tasks in TASKS.md first, then say "next".]

If autonomous — structured signal for calling skill:

On PASS (0 P0s):

REVIEW_RESULT: PASS
P0: 0 | P1: [N] | P2: [N]
AUTONOMOUS: Phase [N] complete — proceeding to next phase automatically.

Return control to the calling skill. It continues the autonomous loop.

On FAIL (any P0s):

REVIEW_RESULT: FAIL
P0: [N] | P1: [N] | P2: [N]
AUTONOMOUS: PAUSED — [N] P0 issue(s) require human resolution.

[List each P0 with file path, line, and specific fix]

Fix the above, then say "resume" to continue autonomous execution.

Wait for human. Do not proceed. When human says "resume" — re-run this review step automatically, then signal result again.

Step 14 — Generate .claude/settings.json hooks (Phase 1 review only)

After the very first phase review (Phase 1), check if .claude/settings.json has a PostToolUse lint hook. If not — offer to add it:

"Phase 1 review complete. One quick setup: adding a lint+typecheck hook will catch TypeScript errors and ESLint violations as soon as they're introduced rather than at the next phase gate. Want me to add it? (y/n)"

If yes — merge the hook into .claude/settings.json (never overwrite it). .claude/settings.json may already hold permissions, env, or other hooks (e.g. a PreToolUse hook written by vibe-doctor). A cat > … <<EOF overwrite would wipe all of that. Read-merge-write instead, and no-op if an equivalent lint hook is already present:

# Pick the lint command for the stack (see per-stack variants below):
LINT_CMD='npm run lint --silent 2>&1 | tail -10 || true'

mkdir -p .claude
python3 - "$LINT_CMD" << 'PY'
import json, sys, pathlib
lint_cmd = sys.argv[1]
p = pathlib.Path(".claude/settings.json")
cfg = {}
if p.exists() and p.read_text().strip():
    try:
        cfg = json.loads(p.read_text())          # preserve existing config
    except json.JSONDecodeError:
        print("settings.json is not valid JSON — leaving it untouched"); sys.exit(0)
hooks = cfg.setdefault("hooks", {})
post = hooks.setdefault("PostToolUse", [])
# Skip if any PostToolUse hook already runs a lint/typecheck command.
already = any(
    "lint" in h.get("command", "") or "tsc" in h.get("command", "") or "ruff" in h.get("command", "")
    for entry in post for h in entry.get("hooks", [])
)
if not already:
    post.append({"matcher": "Edit|Write",
                 "hooks": [{"type": "command", "command": lint_cmd}]})
    p.write_text(json.dumps(cfg, indent=2) + "\n")
    print("Lint hook merged into .claude/settings.json")
else:
    print("A lint/typecheck PostToolUse hook already exists — no change")
PY

Set LINT_CMD per stack — Python: ruff check . 2>&1 | tail -10 || true; TypeScript with typecheck: npm run lint --silent 2>&1 | tail -5; npx tsc --noEmit 2>&1 | tail -5 || true.

Prefer the update-config skill if it is available in the session — it owns settings.json edits and merges hooks safely. Use the inline merge above only as a fallback.

Tell the user:

"Hook added to .claude/settings.json. From now on, lint runs automatically after every file edit. ESLint errors surface immediately — no more discovering them at phase gate review."

If the merge reported that a lint/typecheck hook already exists — skip silently. If the user says no — skip and note they can add it manually later.

Always-flag anti-patterns

P0 — Critical:

1. Direct DB client import in component (DIP violation)
2. Component >1000 lines (SRP violation CRITICAL)
3. any type anywhere (TypeScript violation)
4. Hardcoded secrets/tokens/API keys (security CRITICAL)
5. Auth check missing on protected route/endpoint (security CRITICAL)
6. Architecture drift from ARCHITECTURE.md (or PLAN.md if no ARCHITECTURE.md)
7. npm/pip audit high/critical vulnerability
8. TypeScript compilation errors
9. All tests failing (if test runner exists)

P1 — Fix before deploy: 9. Component >500 lines (SRP violation) 10. Direct storage/localStorage without abstraction (DIP) 11. Missing error boundaries on data-fetching components 12. Hooks managing multiple unrelated concerns 13. Props interface >10 props (ISP violation) 14. console.log in production code 15. Test names describing implementation not behaviour 16. Missing tests for business logic 17. ARCHITECTURE.md not present (recommend running architect:)

Pre-submission checklist

• Automated checks run — output included in report
• Every P0 and P1 has file path and line number
• Drift checked against ARCHITECTURE.md (or PLAN.md if missing) — not generic rules
• Carryover from previous reviews addressed (or stated as first review)
• Quality score calculated using the formula — not estimated
• Strengths acknowledged with file references
• RFX tasks in TASKS.md for all P0 issues
• P1/P2/P3 logged to vibe/reviews/backlog.md
• Gate decision stated clearly
• No section TBD or skipped
• Report saved to vibe/reviews/phase-[N]-review.md