SuperCheck Code Review

Review Workflow

Step 1: Gather Changes

Uncommitted changes (staged + unstaged): Use the get_changed_files tool to retrieve current diffs. If the user specifies, filter by staged or unstaged.

Specific files: Read the files the user points to and review them in context.

Branch comparison: Run git diff main...HEAD or git log --oneline main..HEAD in the terminal to see all commits on the current branch vs main.

Step 2: Classify Each Change

Categorize every modified file:

Step 3: Apply Review Checklist

Run through every applicable section below for each changed file. Report findings as:

• CRITICAL: Must fix before merge (security, data leak, broken functionality)
• WARNING: Should fix (conventions violated, potential bugs, performance)
• SUGGESTION: Nice to have (readability, patterns, minor improvements)
• OK: Passes review

Review Checklists

1. Security (Apply to ALL changes)

Multi-Tenant Data Isolation (CRITICAL)

Every database query in API routes and server actions MUST scope by both projectId AND organizationId:

// CORRECT
const result = await db.select().from(table).where(
  and(
    eq(table.projectId, projectId),
    eq(table.organizationId, organizationId),
    ...otherFilters
  )
)

// WRONG — missing organizationId scoping
const result = await db.select().from(table).where(
  eq(table.projectId, projectId)
)

// WRONG — no scoping at all
const result = await db.select().from(table).where(
  eq(table.id, id)
)

Check for:

• Every db.select(), db.update(), db.delete() includes tenant scoping
• No queries that fetch by id alone without verifying ownership
• JOIN queries still enforce scoping on the primary table
• Subqueries maintain scoping context

Authentication & Authorization

• API routes use requireUserAuthContext() or requireProjectContext()
• Server actions use requireProjectContext() + requirePermissions() or checkPermissionWithContext()
• Permission checks happen BEFORE any data mutation
• No auth bypass paths (early returns, catch blocks that skip auth)
• API key routes validate scopes match the operation
• isAuthError() properly caught and returns 401 (not 500)

Input Validation

• All user inputs validated with Zod schemas before use
• .parse() called (throws on invalid) — not .safeParse() when errors should halt execution
• No raw request.body or request.query used without validation
• File uploads validate size AND MIME type
• URL/path parameters validated (especially UUIDs)
• No user input interpolated into SQL (use Drizzle parameterized queries)
• No user input in shell commands (check execa calls in worker)
• No user input rendered as raw HTML (XSS)

Secrets & Sensitive Data

• No hardcoded secrets, API keys, or passwords
• Secrets use encryptValue() / decryptValue() with project-scoped context
• List APIs return [ENCRYPTED] for secret fields, not the actual value
• No secrets logged (check console.log, console.error, this.logger.*)
• Environment variables accessed via process.env, never committed
• Error messages don't expose internal details (stack traces, DB errors, file paths)

SSRF / Network

• No user-controlled URLs passed to fetch() or HTTP clients without allowlist validation
• Worker execution pods block RFC1918, metadata endpoints, K8s API (verify NetworkPolicy if changed)

2. API Routes (app/src/app/api/**)

Structure

// Expected pattern:
export async function METHOD(request: NextRequest, { params }: { params: Promise<{ id: string }> }) {
  try {
    const { userId, organizationId } = await requireUserAuthContext()
    // OR for project-scoped:
    const { userId, project, organizationId } = await requireProjectContext()

    // Permission check
    const allowed = await hasPermissionForUser(userId, "resource", "action", { organizationId })
    if (!allowed) return NextResponse.json({ error: "Forbidden" }, { status: 403 })

    // Validate input
    const body = schema.parse(await request.json())

    // Business logic (scoped queries)
    // ...

    return NextResponse.json({ success: true, data: result })
  } catch (error) {
    if (isAuthError(error)) return NextResponse.json({ error: "..." }, { status: 401 })
    console.error("[ROUTE_NAME] Error:", error)
    return NextResponse.json({ error: "..." }, { status: 500 })
  }
}

Check for:

• Response format consistency: { success, data, error }
• Proper HTTP status codes (200, 201, 400, 401, 403, 404, 500)
• isAuthError() and isProjectConfigError() handled distinctly
• No uncaught promise rejections
• params properly awaited (Next.js App Router pattern: const { id } = await params)
• Rate limiting on public endpoints

3. Server Actions (app/src/actions/*.ts)

Structure

"use server"

export async function actionName(data: ValidatedType) {
  try {
    const { userId, project, organizationId } = await requireProjectContext()
    await requirePermissions({ resource: ["action"] })

    const validated = schema.parse(data)

    // Scoped DB operations
    // ...

    await logAuditEvent({ action: "...", resourceId: "..." })
    revalidatePath("/relevant-path")

    return { success: true, message: "Done" }
  } catch (error) {
    console.error("[ACTION_NAME] Failed:", error)
    return { success: false, error: "Operation failed" }
  }
}

Check for:

• "use server" directive at top of file
• Return type: { success: boolean, message?: string, error?: string }
• Never throws to client — always returns error object
• revalidatePath() or revalidateTag() called after mutations
• Audit logging via logAuditEvent() for all mutations
• Console logging with [OPERATION_NAME] prefix
• Permission check inside try/catch (return { success: false }, don't throw)

4. Database Schema (app/src/db/schema/*.ts)

Check for:

• UUIDs use $defaultFn(() => sql\uuidv7()`)` for time-ordered IDs
• Foreign keys have proper references() with onDelete cascading
• Indexes on foreign keys and frequently-filtered columns
• Unique constraints where business logic requires uniqueness
• notNull() on required fields
• timestamp fields for createdAt/updatedAt
• Zod schemas created: createInsertSchema(), createSelectSchema()
• Schema exported from app/src/db/schema/index.ts
• Migration generated (npm run db:generate) — migration file should be present if schema changed

5. Database Migrations

Check for:

• Migration matches schema changes exactly
• No data-destructive operations without explicit user confirmation
• DROP TABLE, DROP COLUMN reviewed carefully
• Index naming is consistent
• Default values for new NOT NULL columns on existing tables
• Migration is idempotent (safe to re-run)

6. React Components (app/src/components/**)

Check for:

• Client vs Server component boundary correct ("use client" only when needed)
• No server-only imports in client components (e.g., db, requireProjectContext)
• RBAC checks: use permissions-client.ts on client, permissions.ts on server
• Form submissions via server actions, not direct API calls
• Loading/error states handled
• No inline styles where Tailwind classes exist
• Accessible: proper ARIA attributes, keyboard navigation
• No dangerouslySetInnerHTML with user content
• React Hook Form + Zod resolver for forms

7. Worker Services (worker/src/**)

NestJS Patterns

• Services use @Injectable() decorator
• Dependencies injected via constructor
• Logger: private readonly logger = new Logger(ServiceName.name)
• Structured logging: this.logger.log(), .warn(), .error() — not console.log

Processor Patterns

• @Processor(QUEUE_NAME, { concurrency: 1 }) — concurrency stays at 1
• Job data properly typed
• Error handling: catch, log, and let BullMQ handle retries
• No blocking operations in hot paths
• Timeouts set on external operations

Container Execution

• Use resolveWorkerDir() and resolveBrowsersPath() — not hardcoded Docker paths
• SELF_HOSTED === "true" gates checked correctly (string comparison)
• K8s client uses projected token auth (not static tokens)

8. Queue Constants

CRITICAL: Queue names must stay synchronized across:

• app/src/lib/queue.ts (source of truth)
• worker/src/execution/constants.ts
• worker/src/k6/k6.constants.ts
• worker/src/monitor/monitor.constants.ts

Check for:

• Use queue name builder functions (PLAYWRIGHT_QUEUE, K6_GLOBAL_QUEUE, k6QueueName(), monitorQueueName())
• No inline template strings for queue names
• After location CRUD: invalidateLocationCache(), invalidateQueueMaps(), invalidateQueueEventHub(), invalidateBullBoard() called
• Queue name not renamed in one place only

9. Testing

Unit Tests (*.spec.ts)

Check for:

• Tests exist for new logic (especially utils, services, validators)
• Mocks set up BEFORE imports (jest.mock() hoisting)
• jest.clearAllMocks() in beforeEach()
• Descriptive test names: should [expected behavior] when [condition]
• Both happy path and error/edge cases covered
• No test-only state leaking between tests
• Assertions are specific (not just toBeTruthy())
• Mock return values match actual types

E2E Tests (e2e/**/*.spec.ts)

Check for:

• loginIfNeeded() in beforeEach()
• No shared Playwright storage-state auth
• Tests are independent (no ordering dependency)
• Proper selectors (data-testid preferred over CSS classes)

10. TypeScript Quality

Check for:

• No any types (use unknown + type narrowing)
• No @ts-ignore or @ts-expect-error without documented reason
• Proper type inference (avoid redundant type annotations)
• Discriminated unions for state management
• Zod .infer<> for schema-derived types
• Strict null checks respected (no ! non-null assertions without justification)
• Enums from permissions-client.ts for roles (DRY)

11. Error Handling

Check for:

• No silent catch blocks (catch (e) { /* empty */ })
• Error context included in log messages
• Errors don't expose internals to users
• Async errors properly propagated or handled
• Worker: structured logging with Pino, not console
• Actions: return { success: false, error } — never throw to client

12. Performance

Check for:

• No N+1 query patterns (use JOINs or batch queries)
• Large result sets paginated
• Database indexes for new WHERE/ORDER BY columns
• No unnecessary await in loops (use Promise.all for independent operations)
• Redis caching used where appropriate
• No blocking operations in request handlers
• select() specific columns, not select(*) for large tables

13. Self-Hosted vs Cloud

Check for:

• SELF_HOSTED checks use string comparison: === "true" or === "1"
• Feature gates don't break in either mode
• Billing/Polar code guarded by !isSelfHosted()
• Email verification skipped in self-hosted
• CAPTCHA disabled in self-hosted
• New features work in both modes unless explicitly scoped

14. Docker & Deployment (deploy/**)

Check for:

• Image version bumped in ALL compose files (see version bump checklist)
• Environment variables documented
• No secrets in Dockerfiles or compose files
• Health check endpoints exist for new services
• Resource limits set appropriately
• docker-compose-local.yml NOT versioned (builds from source)

Review Output Format

Structure your review as:

## Code Review Summary

**Files Reviewed:** [count]
**Scope:** [uncommitted/staged/branch/specific files]

### Critical Issues (must fix)
1. **[FILE:LINE]** — [Description of issue]
   - Why: [Security/correctness impact]
   - Fix: [Specific recommendation]

### Warnings (should fix)
1. **[FILE:LINE]** — [Description]
   - Fix: [Recommendation]

### Suggestions (nice to have)
1. **[FILE:LINE]** — [Description]

### Passed Checks
- [x] Multi-tenant scoping verified
- [x] Auth/RBAC checks present
- [x] Input validation with Zod
- [x] No secrets exposed
- [x] Tests updated
- [ ] Missing: [specific check that wasn't applicable]

### Summary
[1-3 sentence overall assessment]

Common Anti-Patterns to Flag

Quick Commands Reference

# App
npm run lint                    # ESLint check
npm run build                   # Type check + build
npm test                        # Jest unit tests
npm run e2e                     # Playwright E2E

# Worker
npm run lint                    # ESLint check
npm run build                   # Type check + build
npm test                        # Jest unit tests

# Database
npm run db:generate             # Generate migration from schema changes
npm run db:migrate              # Apply pending migrations