Claude-skill-registry code-review-wasp
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.
git clone https://github.com/majiayu000/claude-skill-registry
T=$(mktemp -d) && git clone --depth=1 https://github.com/majiayu000/claude-skill-registry "$T" && mkdir -p ~/.claude/skills && cp -r "$T/skills/data/code-review-wasp" ~/.claude/skills/majiayu000-claude-skill-registry-code-review-wasp && rm -rf "$T"
skills/data/code-review-wasp/SKILL.mdCode 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
prefix instead of@wasp/wasp/ - ❌ Using
in .ts/.tsx files (should be relative paths)@src/ - ❌ Using relative paths in main.wasp (should be
)@src/ - ❌ Importing enum values from
(should bewasp/entities
)@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
type annotationGet{Name}<Args, Return> - All actions have
type annotationCreate/Update/Delete{Name}<Args, Return> - 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
(NOT custom logic)context.user - Operations filter by
or check ownershipuserId
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
arrayentities: [] - ❌ 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
by defaultawait action(args) -
ONLY used withuseAction
configoptimisticUpdates - Client imports from
wasp/client/operations - Error handling present (try/catch)
Red Flags:
- ❌ Using
without optimistic updatesuseAction - ❌ 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:
- Database schema changesschema.prisma
- Routes, operations, entities changesmain.wasp- 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
(not direct../scripts/safe-start.sh
)wasp start - Migration ran BEFORE restart (if schema changed)
- Types verified after restart
Red Flags:
- ❌ Schema/main.wasp changes without restart mention
- ❌ Using direct
(multi-worktree conflicts)wasp start - ❌ 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:
- ❌
- Genericexpect(result).toBeDefined() - ✅
- Specificexpect(result.name).toBe('Acme')
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
(NOT latest)shadcn@2.3.0 - Import path fixed to
../../lib/utils - No Tailwind v4 syntax
- Component works without import errors
Red Flags:
- ❌ Using
without version (uses latest)shadcn - ❌ 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:
is undefined at runtimecontext.entities - Impact: Runtime crash
- Fix: Add
type annotationGetQuery<Args, Return>
- Symptom:
-
Auth check not first line
- Symptom: Security vulnerability
- Impact: Unauthorized access to data
- Fix: Move
to first lineif (!context.user)
-
Using
prefix@wasp/- Symptom: Cannot find module error
- Impact: Build fails
- Fix: Use
(no @)wasp/
-
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
by defaultuseAction- 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
for relationsinclude
-
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
instead ofwasp startsafe-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
after schema/main.wasp changes../scripts/safe-start.sh - 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.