Review Skill

Fast, opinionated code review for a solo dev shipping quickly. Catches real problems, skips bikeshedding.

Modes

Invocation	What it reviews
/review	All uncommitted changes (staged + unstaged)
/review last N	The last N commits on current branch
/review --scope <pkg>	Uncommitted changes filtered to a package
/review --file <path>	A single file

Rules

1. No style opinions — formatting, naming preferences, comment style → skip. That's linting.
2. No scope creep — only review what's in the diff. Never suggest "while you're here..." changes.
3. No praise — don't say "nice work" or "good use of". Just flag problems or say "clean".
4. Severity matters — categorize every finding. Don't mix critical bugs with nits.
5. Be specific — file, line, what's wrong, how to fix. No vague "consider improving".
6. Trust the dev — if something looks intentional, don't flag it. Flag things that look accidental.

Severity Levels

Level	Meaning	Action
🔴 Critical	Security hole, data loss risk, crash	Must fix before shipping
🟡 Warning	Bug likely, pattern violation, missing validation	Should fix soon
🔵 Note	Minor issue, potential improvement	Fix when convenient

Fast-Path for Non-Code Changes

When the diff contains only documentation and config files (.md, .yml, .yaml, .json, .toml, .txt) and no code files (.ts, .vue, .css, .scss, .js, .mjs), skip the full checklist and use this shortened review:

Check:

• Content is accurate and not misleading
• No typos or grammar issues
• Links and references are valid (file paths, URLs, anchors)
• Formatting is consistent with surrounding content
• No secrets, tokens, or credentials in the content

Skip: Security, correctness, project patterns, package boundaries, dead code, API endpoints, Vue components — these don't apply to non-code changes.

Detection:

# If this returns nothing, it's a doc-only diff → use fast-path
git diff --name-only <base>..<branch> | grep -E '\.(ts|vue|css|scss|js|mjs)$'

Produce the same structured report format, just with the shorter checklist.

Workflow

Step 1: Gather the diff

For uncommitted changes (/review or /review --scope):

git diff                  # unstaged
git diff --cached         # staged
git status               # new files

For recent commits (/review last N):

git log --oneline -N
git diff HEAD~N..HEAD

For single file (/review --file <path>):

git diff -- <path>        # if modified
# or just read the file if it's new

If --scope is set, filter to files matching packages/<pkg>/** or apps/<pkg>/**.

Step 2: Read the full context

For every changed file, read the COMPLETE file (not just the diff). You need surrounding context to catch:

• Unused imports after a refactor
• Broken references from renames
• Duplicated logic that already exists elsewhere

For files over 500 lines, read at minimum the changed sections ±50 lines.

Step 3: Run checks

Check every changed file against this checklist. Skip categories that don't apply (e.g., skip "API" checks for a component file).

Security

• No hardcoded secrets, API keys, tokens
• No v-html with user-controlled content (XSS)
• Server endpoints validate input (especially route params and body)
• Server endpoints check team authorization (useTeamContext())
• No raw SQL — should use drizzle queries
• No eval(), new Function(), or dynamic import() with user input

Correctness

• Async operations wrapped in try/catch or use { data, error } pattern
• Reactive references not destructured (loses reactivity)
• await not missing on async calls
• Correct HTTP method on API routes (filename matches: .get.ts, .post.ts, etc.)
• No infinite loops or unbounded recursion
• Proper cleanup in onUnmounted for listeners/intervals

Project Patterns (from CLAUDE.md)

• <script setup lang="ts"> — never Options API
• hub: { db: 'sqlite' } — never hub: { database: true }
• useTeamContext() — never raw route.params.id for team ID
• createError({ status, statusText }) — never statusCode/statusMessage
• Nuxt UI v4 names — USeparator, USwitch, UDropdownMenu (not v3 names)
• No resolveComponent() for optional cross-package components (use stub pattern)
• useState() for state — no Pinia
• Composition API only

Package Boundaries

• No direct DB queries for tables owned by another package (e.g., crouton-email querying teamSettings directly instead of going through crouton-auth exports)
• No type imports from deep internal paths of another package — use public exports or auto-imports
• API endpoints live in the package that owns the domain (e.g., team settings CRUD in crouton-core/crouton-admin, not scattered)
• No server utils in package A that duplicate logic from package B (e.g., re-implementing team resolution instead of using crouton-auth's resolveTeamAndCheckMembership)
• Components that belong to a domain live in that domain's package (e.g., email components in crouton-email, not crouton-core)
• Auto-imports from other packages are used via Nitro/Nuxt auto-import, not manual deep imports

Dead Code

• No unused imports
• No unreachable code after return/throw
• No commented-out code blocks (delete it, git has history)
• No unused variables (except intentional _ prefix)

API Endpoints (server/ files only)

• Input validation with zod or similar
• Consistent error responses using createError()
• Team-scoped endpoints use useTeamContext()
• Return types are consistent (not mixing shapes)

Gates, Checks & Flow Steps

Applies when the diff adds or changes a rule that judges other code (a validator, a CI check, a sign-off gate) or a step that mutates state (writes a file, commits, labels, dispatches). See AGENTS.md → Authoring a gate for the argument.

• Can this path exit 0 having done nothing? The recurring defect is a silent no-op, not a crash: git add on a gitignored path adds nothing (#1933) · --add-label X on an issue already carrying X fires no event, so nothing downstream wakes (#1750) · rows.filter(r => r.refTarget) dropped the relations missing a target, then reported "No relationships" (#1953)
• What in the output would prove it acted? A count, a path, a diff — if the run's own output can't distinguish "did the work" from "did nothing", say so and add the proof
• Where it can no-op, assert the effect instead of assuming it — check the artifact is committable rather than trusting the write; remove-then-add rather than hoping the event fires
• Was the rule run over the corpus, with the hit count reported? Green unit tests only prove the rule matches its author's imagination — #1957's first version passed 8 and would have refused 4 shipping schemas. Cheap to do: node scripts/corpus-check.mjs --glob 'apps/*/schemas/*.json' --rule ./my-rule.mjs (advisory — skip it when there's no meaningful corpus, e.g. a rule over one generated file)
• Does "no matches" mean clean, or mean the query was wrong? A scan that matched nothing has proved nothing

Vue Components (*.vue files only)

• Props have TypeScript types
• Emits are declared with defineEmits
• No direct DOM manipulation (use refs or VueUse)
• Modal/Slideover uses v4 pattern (v-model + #content="{ close }")

Step 4: Produce the report

If nothing found:

## Review: clean ✅

Reviewed N files, no issues found.

If issues found:

## Review Summary

| Severity | Count |
|----------|-------|
| 🔴 Critical | X |
| 🟡 Warning | Y |
| 🔵 Note | Z |

### 🔴 Critical

#### [Short title]
**File:** `path/to/file.ts:42`
**Issue:** [What's wrong]
**Fix:** [How to fix — be specific, show code if helpful]

---

### 🟡 Warning

#### [Short title]
**File:** `path/to/file.vue:15`
**Issue:** [What's wrong]
**Fix:** [How to fix]

---

### 🔵 Note

#### [Short title]
**File:** `path/to/file.ts:88`
**Issue:** [What's wrong]
**Fix:** [How to fix]

Step 5: Offer to fix

After presenting the report:

Want me to fix any of these? (all / critical only / pick by number)

If user says "all" or "critical only", fix them directly — no confirmation per fix. Use Edit tool. After fixing, run pnpm typecheck on affected apps.

Parallelization

When reviewing 5+ files, split into parallel agents:

• Group files by directory (server/, app/components/, app/composables/)
• Each agent reviews its group and returns findings
• Compile findings into single report

For <5 files, review sequentially in the main context — agent overhead isn't worth it.

Edge Cases

No changes

Nothing to review — working tree is clean and no commits specified.

Binary/generated files

Skip: *.sqlite, *.wasm, pnpm-lock.yaml, .nuxt/, dist/, .wrangler/, node_modules/

Large diffs (50+ files)

Warn the user and suggest scoping:

50+ files changed. Consider scoping:
  /review --scope crouton-core
  /review --file path/to/important-file.ts

Then proceed if they confirm, using parallel agents.