name	code-review-wasp
description	Comprehensive code review skill for Wasp applications. Use when reviewing code, PRs, or architectural changes. Covers Wasp-specific patterns, import rules, auth checks, TDD compliance, multi-tenant permissions, and performance. Integrates all CLAUDE.md critical rules.
triggers	review code, code review, review PR, check code, review this, pull request review, architectural review, review changes, check implementation
version	1
last_updated	Mon Oct 20 2025 00:00:00 GMT+0000 (Coordinated Universal Time)
allowed_tools	Read, Bash

Code Review Wasp Skill

Quick Reference

When to use this skill:

Reviewing pull requests
Checking code before commit
Architectural review of features
Pre-merge quality checks
Identifying Wasp-specific issues

Key Focus Areas:

Wasp Framework Patterns - Import rules, operations, entity access
Security - Server-side auth, multi-tenant isolation, input validation
TDD Compliance - 2-phase workflow, test quality (5 criteria)
Performance - N+1 queries, pagination, memoization
Code Quality - SOLID, DRY, testability

Review Philosophy:

This skill catches Wasp-specific issues that generic code review tools miss. It enforces the patterns documented in CLAUDE.md and your custom skills.

Review Checklist (10 Critical Categories)

Use this checklist systematically for every code review:

✅ Category 1: Import Rules

Verify correct import syntax based on context:

// ✅ CORRECT - In .ts/.tsx files
import { HttpError } from "wasp/server";
import type { Task } from "wasp/entities";
import { useQuery } from "wasp/client/operations";
import { TaskStatus } from "@prisma/client"; // For enum VALUES

// ❌ WRONG - Common mistakes
import { Task } from "@wasp/entities"; // Wrong prefix
import { helper } from "@src/utils/helper"; // Wrong in .ts/.tsx
import { TaskStatus } from "wasp/entities"; // Wrong for enum values

// ✅ CORRECT - In main.wasp files
query getTasks {
  fn: import { getTasks } from "@src/server/a3/operations",
  entities: [Task]
}

// ❌ WRONG - Common mistakes
fn: import { getTasks } from "src/server/a3/operations", // Missing @
fn: import { getTasks } from "../src/server/a3/operations", // Relative path

Red Flags:

❌ Using @wasp/ prefix instead of wasp/
❌ Using @src/ in .ts/.tsx files (should be relative paths)
❌ Using relative paths in main.wasp (should be @src/)
❌ Importing enum values from wasp/entities (should be @prisma/client)

Cross-reference: See wasp-operations skill for complete import patterns

✅ Category 2: Type Annotations (Operations)

CRITICAL: Type annotations are REQUIRED for context.entities access

// ✅ CORRECT - Type annotation present
export const getTasks: GetTasks<void, Task[]> = async (args, context) => {
  return context.entities.Task.findMany(); // Works!
};

// ❌ WRONG - No type annotation
export const getTasks = async (args, context) => {
  return context.entities.Task.findMany(); // context.entities is undefined!
};

Check:

All queries have Get{Name}<Args, Return> type annotation
All actions have Create/Update/Delete{Name}<Args, Return> type annotation
Imports include from "wasp/server/operations"
Args and Return types match actual function signature

Red Flags:

❌ Operations without type annotations
❌ Type annotation doesn't match actual function signature
❌ Missing imports for type definitions

Cross-reference: See wasp-operations skill, lines 88-155

✅ Category 3: Auth Checks (Security CRITICAL)

MANDATORY: Auth check must be FIRST line of every operation

// ✅ CORRECT - Auth check first
export const getTasks: GetTasks = async (args, context) => {
  if (!context.user) throw new HttpError(401); // FIRST line
  return context.entities.Task.findMany({ where: { userId: context.user.id } });
};

// ❌ WRONG - Auth check missing or late
export const getTasks: GetTasks = async (args, context) => {
  const tasks = await context.entities.Task.findMany(); // NO AUTH CHECK!
  return tasks;
};

Check:

First line is: if (!context.user) throw new HttpError(401)
No operations bypass auth (unless explicitly public)
Auth check uses context.user (NOT custom logic)
Operations filter by userId or check ownership

Red Flags:

❌ Missing auth check entirely
❌ Auth check after business logic
❌ Custom auth logic instead of context.user
❌ No filtering by userId (potential data leak)

Cross-reference: See wasp-operations skill, Section 3 (lines 120-220)

✅ Category 4: Error Sequence (HTTP Status Codes)

CRITICAL: Enforce correct error sequence

Order (MUST follow):

401 Unauthorized - No user (unauthenticated)
404 Not Found - Resource doesn't exist
403 Forbidden - User lacks permission
400 Bad Request - Invalid input

// ✅ CORRECT - Proper error sequence
export const updateTask: UpdateTask = async (args, context) => {
  // 1. Auth check (401)
  if (!context.user) throw new HttpError(401);

  // 2. Fetch resource
  const task = await context.entities.Task.findUnique({
    where: { id: args.id },
  });

  // 3. Check existence (404)
  if (!task) throw new HttpError(404, "Task not found");

  // 4. Check permission (403)
  if (task.userId !== context.user.id) {
    throw new HttpError(403, "Not authorized");
  }

  // 5. Validate input (400)
  if (!args.data.description?.trim()) {
    throw new HttpError(400, "Description required");
  }

  // 6. Perform update
  return context.entities.Task.update({
    where: { id: args.id },
    data: args.data,
  });
};

// ❌ WRONG - Wrong error order
export const updateTask: UpdateTask = async (args, context) => {
  // Checking permission before auth and existence - WRONG ORDER!
  const task = await context.entities.Task.findUnique({
    where: { id: args.id },
  });
  if (task.userId !== context.user.id) throw new HttpError(403);
  // ... rest of checks
};

Check:

401 comes first (auth)
404 before 403 (existence before permission)
400 after auth/existence/permission checks
Error messages are descriptive

Red Flags:

❌ Wrong error order (e.g., 403 before 404)
❌ Using wrong HTTP status code
❌ Generic error messages ("Error", "Failed")
❌ Not using HttpError class

Cross-reference: See error-handling skill for complete patterns

✅ Category 5: Entity Listing (main.wasp)

CRITICAL: Entities must be listed for context.entities access AND auto-invalidation

// ✅ CORRECT - Entities listed
query getTasks {
  fn: import { getTasks } from "@src/server/a3/operations",
  entities: [Task]  // Enables context.entities.Task + auto-invalidation
}

action createTask {
  fn: import { createTask } from "@src/server/a3/operations",
  entities: [Task]  // Same as getTasks → auto-invalidation works!
}

// ❌ WRONG - No entities
query getTasks {
  fn: import { getTasks } from "@src/server/a3/operations",
  // Missing entities: [] → context.entities undefined!
}

// ❌ WRONG - Different entities prevent auto-invalidation
action createTask {
  entities: [Task, User]  // Extra entity prevents auto-invalidation of getTasks
}

Check:

All queries list entities in main.wasp
All actions list entities in main.wasp
Entities match actual database access
Query + Action share same entities (for auto-invalidation)

Red Flags:

❌ Missing entities: [] array
❌ Empty entities array when accessing database
❌ Query and action have different entities (breaks auto-invalidation)
❌ Entities listed but not actually accessed in code

Cross-reference: See wasp-operations skill, lines 40-87

✅ Category 6: Client Patterns (Actions)

DEFAULT: Use direct async/await for actions, NOT useAction

// ✅ CORRECT - Direct call (default pattern)
const handleCreate = async (description: string) => {
  try {
    await createTask({ description }); // Simple + enables auto-invalidation
    toast.success("Task created");
  } catch (err) {
    toast.error(err.message);
  }
};

// ❌ WRONG - useAction by default
const createTaskFn = useAction(createTask);
const handleCreate = async (description: string) => {
  try {
    await createTaskFn({ description }); // Blocks auto-invalidation!
    toast.success("Task created");
  } catch (err) {
    toast.error(err.message);
  }
};

ONLY use useAction for optimistic UI updates (advanced):

// ✅ CORRECT - useAction ONLY for optimistic updates
const deleteTaskFn = useAction(deleteTask, {
  optimisticUpdates: [
    {
      getQuerySpecifier: () => [getTasks],
      updateQuery: (oldTasks, { id }) => oldTasks.filter((t) => t.id !== id),
    },
  ],
});

Check:

Actions use direct await action(args) by default
useAction ONLY used with optimisticUpdates config
Client imports from wasp/client/operations
Error handling present (try/catch)

Red Flags:

❌ Using useAction without optimistic updates
❌ Importing operations from wrong location
❌ No error handling around action calls
❌ Manual cache invalidation (Wasp does this automatically)

Cross-reference: See CLAUDE.md, lines 275-304 (Client-Side Action Usage)

✅ Category 7: Restart Requirements

MANDATORY: Restart after schema.prisma or main.wasp changes

Files that require restart:

schema.prisma - Database schema changes
main.wasp - Routes, operations, entities changes
Database migrations - After wasp db migrate-dev

Verify:

# After changes, MUST restart with safe-start.sh (multi-worktree safe)
../scripts/safe-start.sh

# ❌ WRONG - Direct wasp start (not multi-worktree safe)
wasp start

Check:

PR mentions restart if schema/main.wasp changed
Uses ../scripts/safe-start.sh (not direct wasp start)
Migration ran BEFORE restart (if schema changed)
Types verified after restart

Red Flags:

❌ Schema/main.wasp changes without restart mention
❌ Using direct wasp start (multi-worktree conflicts)
❌ Restart before migration (wrong order)
❌ Assuming types update without restart

Cross-reference: See wasp-database skill for complete workflow

✅ Category 8: Test Coverage (TDD Compliance)

MANDATORY: ≥80% statements, ≥75% branches

Check test quality using 5 MUST PASS criteria:

1. Tests Business Logic (NOT Existence):

// ❌ WRONG - Test theater
it("should exist", () => {
  expect(createTask).toBeDefined(); // Meaningless!
});

// ✅ CORRECT - Tests behavior
it("should create task with description", async () => {
  const result = await createTask({ description: "Test" }, mockContext);
  expect(result.description).toBe("Test");
  expect(mockContext.entities.Task.create).toHaveBeenCalledWith({
    data: { description: "Test", userId: "user-1" },
  });
});

2. Meaningful Assertions:

❌ expect(result).toBeDefined() - Generic
✅ expect(result.name).toBe('Acme') - Specific

3. Tests Error Paths:

✅ 401 (unauthenticated)
✅ 400 (validation error)
✅ 404 (not found)
✅ 403 (no permission)

4. Tests Edge Cases:

✅ Empty strings, null, undefined
✅ Boundary conditions (min/max length)
✅ Special characters

5. Behavior NOT Implementation:

❌ Testing internal variables (component.state.loading)
✅ Testing observable UI (screen.getByText('Loaded'))

Check:

All 5 quality criteria pass
Coverage ≥80% statements, ≥75% branches
Tests follow 2-phase TDD (RED → GREEN + REFACTOR)
No test theater (existence checks)

Red Flags:

❌ Low coverage (<80%/<75%)
❌ Only happy path tests (no error paths)
❌ Existence checks instead of behavior tests
❌ Testing implementation details

Cross-reference: See tdd-workflow skill, lines 226-555

✅ Category 9: ShadCN Version Lock

CRITICAL: ONLY use ShadCN v2.3.0

# ✅ CORRECT - Version locked
npx shadcn@2.3.0 add button

# ❌ WRONG - Uses latest (Tailwind v4, breaks Wasp)
npx shadcn add button

After EVERY component installation - Fix import path:

// In src/components/ui/{component}.tsx
-import { cn } from "s/lib/utils"
+import { cn } from "../../lib/utils"

Check:

Uses shadcn@2.3.0 (NOT latest)
Import path fixed to ../../lib/utils
No Tailwind v4 syntax
Component works without import errors

Red Flags:

❌ Using shadcn without version (uses latest)
❌ Import path still s/lib/utils
❌ Tailwind v4 syntax (@theme, etc)
❌ Component import errors

Cross-reference: See shadcn-ui skill and CLAUDE.md lines 82-107

✅ Category 10: Multi-Tenant Permissions

CRITICAL: Verify multi-tenant isolation

Architecture:

Organization → Many Departments (hierarchical via parentId)
Users ↔ Departments = Many-to-Many via UserDepartment
DepartmentRole: MANAGER | MEMBER | VIEWER

Permission checks REQUIRED:

// ✅ CORRECT - Multi-tenant filtering
export const getTasks: GetTasks = async (args, context) => {
  if (!context.user) throw new HttpError(401);

  // Get user's departments
  const userDepartments = await context.entities.UserDepartment.findMany({
    where: { userId: context.user.id },
  });

  const departmentIds = userDepartments.map((ud) => ud.departmentId);

  // Filter by departments user has access to
  return context.entities.Task.findMany({
    where: {
      departmentId: { in: departmentIds },
    },
  });
};

// ❌ WRONG - No multi-tenant filtering
export const getTasks: GetTasks = async (args, context) => {
  if (!context.user) throw new HttpError(401);
  return context.entities.Task.findMany(); // Returns ALL tasks (data leak!)
};

Check:

Operations filter by organization/department
Permission checks use DepartmentRole
No raw queries without multi-tenant WHERE clause
VIEWER role has read-only access

Red Flags:

❌ Queries without organization/department filtering
❌ Assuming single organization
❌ No role-based permission checks
❌ Data leaks across organizations

Cross-reference: See permissions skill for complete patterns

Wasp-Specific Red Flags

These issues are UNIQUE to Wasp - generic tools won't catch them:

🚩 Critical Red Flags (BLOCK PR)

Missing type annotations on operations
- Symptom: context.entities is undefined at runtime
- Impact: Runtime crash
- Fix: Add GetQuery<Args, Return> type annotation
Auth check not first line
- Symptom: Security vulnerability
- Impact: Unauthorized access to data
- Fix: Move if (!context.user) to first line
Using @wasp/ prefix
- Symptom: Cannot find module error
- Impact: Build fails
- Fix: Use wasp/ (no @)
ShadCN version > 2.3.0
- Symptom: Tailwind v4 incompatibility
- Impact: Styles break, build fails
- Fix: Uninstall and reinstall with @2.3.0
No restart after schema/main.wasp changes
- Symptom: Types are stale, imports fail
- Impact: Development blocked
- Fix: Stop wasp, run ../scripts/safe-start.sh
Test theater (5 criteria fail)
- Symptom: Tests pass but don't verify behavior
- Impact: False confidence, bugs in production
- Fix: Rewrite tests following 5 MUST PASS criteria

⚠️ Warning Red Flags (Request Changes)

Using useAction by default
- Impact: Blocks auto-invalidation
- Fix: Use direct await action(args)
Different entities in query + action
- Impact: Manual cache invalidation needed
- Fix: Use same entities for auto-invalidation
N+1 query problem
- Impact: Performance degradation
- Fix: Use Prisma include for relations
No multi-tenant filtering
- Impact: Data leaks across organizations
- Fix: Filter by organization/department in WHERE clause
Wrong error sequence
- Impact: Misleading error messages
- Fix: Follow 401 → 404 → 403 → 400 order
Direct wasp start instead of safe-start.sh
- Impact: Multi-worktree port conflicts
- Fix: Use ../scripts/safe-start.sh

Code Quality Criteria

SOLID Principles

Single Responsibility:

✅ Operations do ONE thing (get, create, update, delete)
❌ Operations mixing CRUD operations
❌ Operations handling multiple entities

Open/Closed:

✅ Helper functions for reusable logic
❌ Copy-pasted auth checks
❌ Copy-pasted validation logic

Liskov Substitution:

✅ Consistent return types
❌ Sometimes returns entity, sometimes null
❌ Inconsistent error handling

Interface Segregation:

✅ Minimal operation arguments
❌ Operations accepting huge config objects
❌ Optional parameters that are actually required

Dependency Inversion:

✅ Using Wasp operations abstraction
❌ Direct Prisma imports in client code
❌ Tight coupling to specific implementations

DRY (Don't Repeat Yourself)

Extract common patterns:

// ✅ CORRECT - Extracted helper
function requireAuth(context: any) {
  if (!context.user) throw new HttpError(401);
  return context.user;
}

export const getTasks: GetTasks = async (args, context) => {
  requireAuth(context); // Reused
  return context.entities.Task.findMany();
};

// ❌ WRONG - Repeated auth check
export const getTasks: GetTasks = async (args, context) => {
  if (!context.user) throw new HttpError(401); // Repeated
  // ...
};

export const getTask: GetTask = async (args, context) => {
  if (!context.user) throw new HttpError(401); // Repeated
  // ...
};

Check:

Auth checks extracted to helper
Validation logic reused
Permission checks centralized
No copy-pasted code

Testability

Code should be easy to test:

✅ Pure functions (input → output, no side effects)
✅ Dependencies injected (via context)
✅ Minimal mocking required
❌ Global state access
❌ Hard-coded values
❌ Untestable private logic

Security Review

Server-Side Authorization

CRITICAL: Never trust client

// ❌ WRONG - Client-side check only
if (user.role !== "ADMIN") return <div>Access denied</div>;
// Easy to bypass! Client-side is cosmetic only.

// ✅ CORRECT - Server-side enforcement
export const sensitiveOperation: Operation = async (args, context) => {
  if (!context.user) throw new HttpError(401);
  if (context.user.role !== "ADMIN") throw new HttpError(403);
  // Safe - enforced server-side
};

Check:

All permissions checked server-side
Operations throw 403 if unauthorized
Client checks are cosmetic only (UX)
No sensitive data leaked to client

Input Validation

Validate ALL user input:

// ✅ CORRECT - Validation
if (!args.description?.trim()) {
  throw new HttpError(400, "Description required");
}
if (args.description.length > 500) {
  throw new HttpError(400, "Description too long");
}

// ❌ WRONG - No validation
const task = await context.entities.Task.create({
  data: { description: args.description }, // Accepts empty string, null, etc.
});

Check:

Required fields validated
Length constraints enforced
Format validation (email, URL, etc.)
Special characters handled

SQL Injection Prevention

Use Prisma (auto-escapes):

// ✅ CORRECT - Prisma auto-escapes
const tasks = await context.entities.Task.findMany({
  where: { status: args.status }, // Safe
});

// ❌ WRONG - Raw SQL (dangerous!)
const tasks = await context.entities.$queryRaw`
  SELECT * FROM Task WHERE status = ${args.status}
`; // SQL injection risk!

Check:

All queries use Prisma (not raw SQL)
No string concatenation in queries
User input never directly in SQL

Performance Review

N+1 Query Problem

CRITICAL: Avoid multiple queries in loop

// ❌ WRONG - N+1 queries
const tasks = await context.entities.Task.findMany();
for (const task of tasks) {
  const user = await context.entities.User.findUnique({
    where: { id: task.userId },
  }); // N queries!
}

// ✅ CORRECT - Single query with include
const tasks = await context.entities.Task.findMany({
  include: { user: true }, // 1 query with JOIN
});

Check:

No queries inside loops
Related data fetched with include
Uses Prisma select for specific fields

Pagination

Required for large lists:

// ❌ WRONG - Loading all records
const tasks = await context.entities.Task.findMany(); // Could be millions!

// ✅ CORRECT - Paginated
const tasks = await context.entities.Task.findMany({
  skip: page * pageSize,
  take: pageSize,
  orderBy: { createdAt: "desc" },
});

Check:

Lists use pagination (skip/take)
Default page size reasonable (20-50)
Returns total count for UI

React Re-renders

Memoization for expensive components:

// ✅ CORRECT - Memoized
const ExpensiveComponent = memo(({ data }) => {
  const processed = useMemo(() => expensiveCalc(data), [data]);
  return <div>{processed}</div>;
});

// ❌ WRONG - Re-renders every time
function ExpensiveComponent({ data }) {
  const processed = expensiveCalc(data); // Runs on every render!
  return <div>{processed}</div>;
}

Check:

Expensive components use memo
Expensive calculations use useMemo
Stable callbacks use useCallback

PR Review Template

Use this template for structured feedback:

## Code Review Summary

**Overall:** [APPROVE / REQUEST CHANGES / BLOCK]

**Wasp-Specific Issues:** [count] found
**Security Issues:** [count] found
**Performance Issues:** [count] found
**Test Quality:** [PASS / FAIL - specify criteria]

---

## 🚨 Critical Issues (MUST FIX)

### Issue 1: [Title]

**Location:** `app/src/server/a3/operations.ts:45`

**Problem:**
Missing type annotation on operation. `context.entities` will be undefined at runtime.

**Current Code:**

```typescript
export const getTasks = async (args, context) => {
  return context.entities.Task.findMany(); // FAILS
};
```

Fix:

export const getTasks: GetTasks<void, Task[]> = async (args, context) => {
  return context.entities.Task.findMany(); // Works
};

Why: Type annotations are CRITICAL for Wasp operations to enable context.entities access.

Reference: wasp-operations skill, lines 88-155

⚠️ Warnings (Request Changes)

Issue 2: [Title]

[Same structure as Critical Issues]

✅ What Went Well

Auth checks present and correct
Test coverage excellent (87%/82%)
Code follows DRY principle
Multi-tenant filtering correct

📚 Additional Resources

CLAUDE.md - Project guidelines
wasp-operations skill - Operation patterns
tdd-workflow skill - Test quality criteria
Troubleshooting Guide - Common issues

🎯 Action Items

Fix all CRITICAL issues (block PR)
Address all WARNING issues
Verify test coverage ≥80%/≥75%
Run ../scripts/safe-start.sh after schema/main.wasp changes
Update PR description with testing notes


---

## Summary

**This skill provides comprehensive Wasp-specific code review.**

**Key Features:**

- ✅ Catches Wasp-specific issues (generic tools miss these)
- ✅ Enforces all CLAUDE.md critical rules
- ✅ Integrates with existing skills (cross-references)
- ✅ Provides actionable feedback (not just "good/bad")
- ✅ Context-efficient (activates only when explicitly asked)

**When to activate:**

- Reviewing PRs before merge
- Pre-commit quality checks
- Architectural reviews
- Identifying framework-specific issues

**Expected outcome:**

Structured review with:

1. Critical issues (block PR)
2. Warnings (request changes)
3. What went well (positive feedback)
4. Actionable fixes (code examples)
5. Cross-references (related skills)

**For complete patterns:** See cross-referenced skills listed throughout this document.

code-review-wasp

Install Skill

SKILL.md

Code Review Wasp Skill

Quick Reference

Review Checklist (10 Critical Categories)

✅ Category 1: Import Rules

✅ Category 2: Type Annotations (Operations)

✅ Category 3: Auth Checks (Security CRITICAL)

✅ Category 4: Error Sequence (HTTP Status Codes)

✅ Category 5: Entity Listing (main.wasp)

✅ Category 6: Client Patterns (Actions)

✅ Category 7: Restart Requirements

✅ Category 8: Test Coverage (TDD Compliance)

✅ Category 9: ShadCN Version Lock

✅ Category 10: Multi-Tenant Permissions

Wasp-Specific Red Flags

🚩 Critical Red Flags (BLOCK PR)

⚠️ Warning Red Flags (Request Changes)

Code Quality Criteria

SOLID Principles

DRY (Don't Repeat Yourself)

Testability

Security Review

Server-Side Authorization

Input Validation

SQL Injection Prevention

Performance Review

N+1 Query Problem

Pagination

React Re-renders

PR Review Template

⚠️ Warnings (Request Changes)

Issue 2: [Title]

✅ What Went Well

📚 Additional Resources

🎯 Action Items