Code Review Wasp

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:

1. Wasp Framework Patterns - Import rules, operations, entity access
2. Security - Server-side auth, multi-tenant isolation, input validation
3. TDD Compliance - 2-phase workflow, test quality (5 criteria)
4. Performance - N+1 queries, pagination, memoization
5. 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):

1. 401 Unauthorized - No user (unauthenticated)
2. 404 Not Found - Resource doesn't exist
3. 403 Forbidden - User lacks permission
4. 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)

1. Missing type annotations on operations

   • Symptom: context.entities is undefined at runtime
   • Impact: Runtime crash
   • Fix: Add GetQuery<Args, Return> type annotation

2. Auth check not first line

   • Symptom: Security vulnerability
   • Impact: Unauthorized access to data
   • Fix: Move if (!context.user) to first line

3. Using @wasp/ prefix

   • Symptom: Cannot find module error
   • Impact: Build fails
   • Fix: Use wasp/ (no @)

4. ShadCN version > 2.3.0

   • Symptom: Tailwind v4 incompatibility
   • Impact: Styles break, build fails
   • Fix: Uninstall and reinstall with @2.3.0

5. No restart after schema/main.wasp changes

   • Symptom: Types are stale, imports fail
   • Impact: Development blocked
   • Fix: Stop wasp, run ../scripts/safe-start.sh

6. 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)

7. Using useAction by default

   • Impact: Blocks auto-invalidation
   • Fix: Use direct await action(args)

8. Different entities in query + action

   • Impact: Manual cache invalidation needed
   • Fix: Use same entities for auto-invalidation

9. N+1 query problem

   • Impact: Performance degradation
   • Fix: Use Prisma include for relations

10. No multi-tenant filtering

    • Impact: Data leaks across organizations
    • Fix: Filter by organization/department in WHERE clause

11. Wrong error sequence

    • Impact: Misleading error messages
    • Fix: Follow 401 → 404 → 403 → 400 order

12. 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.