/dev-review – Code Review

Skill Awareness: See skills/_registry.md for all available skills.

• Before: After /dev-coding implementation and /dev-test passes
• Reads: _quality-attributes.md (ALL Levels – verification)
• If issues: Fix with /dev-coding, then review again
• If major changes: Create CR via /debrief

Review code changes for quality, security, and adherence to standards.

When to Use

• After implementing a feature
• Before merging a PR
• To get feedback on approach
• To catch issues before production

Usage

/dev-review                      # Review uncommitted changes
/dev-review --staged             # Review staged changes only
/dev-review src/auth/            # Review specific directory
/dev-review UC-AUTH-001          # Review changes for specific UC

Input

Reviews can be based on:

1. Git diff – Uncommitted or staged changes
2. File list – Specific files to review
3. UC reference – Files changed for a use case (from spec)

Output

## Review Summary

**Verdict**: ✅ Approve | ⚠️ Request Changes | ❓ Needs Discussion

**Stats**: X files, Y additions, Z deletions

### Issues Found

#### 🔴 Critical (must fix)
- [security] SQL injection risk in `src/api/users.ts:45`
- [bug] Null pointer in `src/utils/parse.ts:23`

#### 🟡 Important (should fix)
- [performance] N+1 query in `src/api/posts.ts:67`
- [error-handling] Unhandled promise rejection

#### 🔵 Suggestions (nice to have)
- [style] Consider extracting to helper function
- [naming] `data` is too generic, suggest `userProfile`

### By File
- `src/api/auth.ts` - 2 issues
- `src/components/Form.tsx` - 1 suggestion

### Positives
- Good error handling in login flow
- Clean separation of concerns

Expected Outcome

Review report with verdict and categorized issues.

Report includes:

• Verdict (Approve/Request Changes/Needs Discussion)
• Issues found (by severity: critical/important/suggestion)
• Issues by file
• Positives (acknowledge good work)
• Spec compliance (if UC provided)

Review Workflow

1. Load Context (once):

• Read plans/brd/tech-context.md → Identify stack(s)
• Load knowledge/stacks/{stack}/_index.md → Focus on “For /dev-review” section
• Read _quality-attributes.md → Get all level checklists
• Read spec (if UC provided) → Get requirements
• Get git diff or file list → Understand what changed

2. Analyze Changes:

• Review each changed file
• Apply: Security + Quality + Conventions + Stack-Specific checks
• Note issues by severity (critical/important/suggestion)
• Acknowledge good practices

3. Verify Spec Compliance (if UC provided):

• Check all requirements met
• Identify missing items
• Flag unauthorized additions

4. Generate Report:

• Verdict (Approve/Request Changes/Needs Discussion)
• Issues by severity
• Issues by file
• Spec compliance status
• Positives

5. Offer Fix (if issues found):

• Critical/Important → Suggest auto-fix via /dev-coding
• Suggestions → User decides

Success Criteria

• Stack knowledge loaded and applied
• Security risks identified
• Quality issues found
• Spec compliance verified
• Conventions checked
• Framework patterns verified (from stack knowledge)
• Constructive feedback with suggested fixes
• Clear verdict given

Review Focus Areas

Security

• Input validation (sanitized, injection prevented, XSS prevented)
• Authentication (required where needed, tokens validated, sessions secure)
• Authorization (permissions checked, data access restricted, admin protected)
• Data protection (passwords hashed, sensitive data not logged, no secrets in code)
• API security (rate limiting, CORS, no sensitive data in URLs)

Quality

• Error handling (caught, user-friendly messages, logged, not swallowed)
• Performance (no N+1 queries, pagination on lists, async heavy ops, no memory leaks)
• Maintainability (readable, functions not too long, no magic values, DRY)
• Testing (new code tested, edge cases covered, meaningful tests)

Conventions

• Naming (variables, files, components per scout)
• Structure (correct location, follows patterns, organized imports)
• Style (matches configs, consistent, no linting errors)
• Git (proper commit format, no unrelated changes, no debug code)

Spec Compliance

• Requirements met (all implemented)
• Requirements not met (missing items)
• Not in spec (unauthorized additions)

Stack-Specific

CRITICAL: Load framework-specific review checklists

After loading stack knowledge (from Context Sources → Step 1), apply framework-specific checks from the “For /dev-review” section:

React projects:

• Hooks follow Rules of Hooks (no conditional calls, correct order)
• All dependencies included in useEffect/useCallback/useMemo
• No infinite loops, no stale closures
• State updates are immutable
• Performance optimizations appropriate (memo, lazy)
• Lists have stable keys

Vue 3 projects:

• Uses <script setup> (not Options API)
• Composables follow naming convention (use*)
• No destructuring reactive without toRefs
• ref() accessed with .value in script (not in template)
• v-for has unique :key
• No v-for + v-if on same element
• Props not mutated directly

Next.js projects:

• Server vs Client Components used correctly
• Server Actions have “use server” directive
• Data fetching uses proper Next.js patterns
• No client-side code in Server Components

Nuxt projects:

• Composables follow naming convention (useX)
• Server routes in correct location
• Auto-imports used correctly
• SSR-safe code (no window access in setup)

Directus projects:

• Collections accessed properly
• Permissions configured
• Field types match requirements
• Flows and hooks follow patterns

If stack knowledge file doesn’t exist, apply general best practices only.

Context Sources

Read to understand what was changed:

• git diff (uncommitted or staged)
• Specific files (if provided)

Read to understand standards:

Step 1: Load stack knowledge (CRITICAL)

• Read plans/brd/tech-context.md → Identify stack(s)
• Extract stack names (look for “Primary Stack” or “Tech Stack” section)
• Load corresponding knowledge files:
  • If “React” → Read knowledge/stacks/react/_index.md → Focus on “For /dev-review” section
  • If “Vue” → Read knowledge/stacks/vue/_index.md → Focus on “For /dev-review” section
  • If “Next.js” → Read knowledge/stacks/nextjs/_index.md → Focus on “For /dev-review” section
  • If “Nuxt” → Read knowledge/stacks/nuxt/_index.md → Focus on “For /dev-review” section
  • If “Directus” → Read knowledge/stacks/directus/_index.md → Focus on “For /dev-review” section

Step 2: Load project and spec context

• tech-context.md – Project conventions
• architecture.md – Architecture decisions (if exists)
• spec – Requirements (if UC provided)
• _quality-attributes.md – ALL levels (Architecture, Specification, Implementation, Review)
• Config files (.eslintrc, tsconfig.json, etc.)

Severity Levels

Verdicts

Review Best Practices

Be Constructive

// BAD
"This code is bad"

// GOOD
"This could cause a SQL injection. Consider using parameterized queries:
```sql
SELECT * FROM users WHERE id = $1
```"

Explain Why

// BAD
"Don't use var"

// GOOD
"Use const/let instead of var - var has function scope which can lead to
unexpected behavior. const also signals intent that the value won't change."

Suggest Alternatives

// Issue + Solution
"The N+1 query here will cause performance issues with many posts.

Consider using an include/join:
```typescript
const posts = await db.posts.findMany({
  include: { author: true }
});
```"

Acknowledge Good Work

### Positives
- Clean separation of API and business logic
- Good error messages for users
- Comprehensive input validation

Tools Used

Integration

Example Review

User: /dev-review UC-AUTH-001

Step 1: Load Context
- Read tech-context.md → Stack: Next.js 14 + TypeScript
- Read knowledge/stacks/nextjs/_index.md → "For /dev-review" section
  → Check Server Actions, App Router, Server Components
- Read knowledge/stacks/react/_index.md → "For /dev-review" section
  → Check Hooks, state management, performance
- Read _quality-attributes.md → All level checklists
- Read UC-AUTH-001 spec → Requirements
- Get git diff → Changed files

Step 2: Analyze
- src/api/auth/login.ts: Clean ✓
- src/components/LoginForm.tsx: 1 issue (React hooks deps missing)
- src/lib/api.ts: 1 suggestion (naming)

Step 3: Generate Report

## Review Summary

**Verdict**: ⚠️ Request Changes

**Stats**: 3 files, +245 additions, -12 deletions

### Issues Found

#### 🔴 Critical
None

#### 🟡 Important
- [error-handling] `src/components/LoginForm.tsx:34`
  Promise rejection not handled. If API fails, user sees nothing.
  ```tsx
  // Add error state
  .catch(err => setError(err.message))

🔵 Suggestions

• [naming] src/lib/api.ts:12 data is generic. Consider credentials for clarity.

Spec Compliance

• POST /api/auth/login works
• Returns token
• Validates input
• Missing: Rate limiting (spec requirement)

Positives

• Good validation on both client and server
• Clean component structure
• Proper TypeScript types

## Fix Loop

When issues are found, `/dev-review` can trigger `/dev-coding` to fix them:

/dev-review ↓ Issues found? ├── No → Pass ✅ → Suggest /dev-changelog └── Yes → Offer to fix ↓ “1 important issue found. Fix now?” ├── Yes → Load /dev-coding with fix context │ ↓ │ Fix applied │ ↓ │ Re-run /dev-review (auto) └── No → Output review, user fixes manually

### Fix Flow

```markdown
**Review found issues:**

| # | Severity | File | Issue |
|---|----------|------|-------|
| 1 | 🟡 Important | LoginForm.tsx:34 | Promise rejection not handled |
| 2 | 🔵 Suggestion | api.ts:12 | Generic variable name |

**Options:**
- A: Fix important issues automatically (1 issue)
- B: Fix all issues automatically (2 issues)
- C: I'll fix manually

If user chooses A or B:

1. Load /dev-coding skill
   → Pass: files to fix, issues to address

2. /dev-coding applies fixes
   → Uses existing patterns from scout
   → Follows spec requirements

3. Auto re-run /dev-review
   → Verify fixes applied correctly
   → Check for new issues introduced

4. If pass → Suggest /dev-changelog

After Pass: Changelog

When review passes (no critical/important issues):

## Review Complete ✅

**Verdict**: Approved

No critical or important issues found.

**Next Steps:**
1. Run `/dev-changelog` to document what was built
2. Commit and push changes
3. Create PR

💡 Run `/dev-changelog {feature}` to create implementation summary.

Integration

Complete Flow

/dev-coding → /dev-test → /dev-review
                              │
                    ┌─────────┴─────────┐
                    │   Issues found?   │
                    └─────────┬─────────┘
                              │
              ┌───────────────┼───────────────┐
              ↓               ↓               ↓
         Critical        Important        None
              │               │               │
              ↓               ↓               ↓
         Must fix        Offer fix       Pass ✅
              │               │               │
              └───────┬───────┘               │
                      ↓                       │
              /dev-coding (fix)               │
                      ↓                       │
              /dev-review (re-run)            │
                      ↓                       │
                    Pass ────────────────────→│
                                              ↓
                                    /dev-changelog
                                              ↓
                                    summary.md created