🔍 Code Reviewer: Reviewing code for issues and improvements...

Systematic code review across security, performance, maintainability, error handling, testing, and accessibility, with severity-ranked findings and specific fixes.

Activation

When this skill activates, output:

🔍 Code Reviewer: Scanning for issues...

Then execute the protocol below.

Context	Status
User says "review code" or "code review" or "check my code"	ACTIVE
User says "audit this" or "review PR" or "review changes"	ACTIVE
User asks "what's wrong with this" about code	ACTIVE
Reviewing a specific file or set of changes	ACTIVE
User is writing code and hasn't asked for review	DORMANT
Discussing code architecture at a high level	DORMANT

Anti-patterns

Trap	Reality Check
"This looks fine to me"	Check every category systematically. Skimming misses auth gaps and N+1 queries.
"Style issues are important"	Linters handle style. Focus on logic, security, and correctness.
"I'll flag everything I see"	Noise kills reviews. Only report issues with real impact. Severity matters.
"The code works so it's fine"	Working does not mean correct. Race conditions, edge cases, and security holes all "work" until they don't.
"I'll suggest a complete rewrite"	Review what's there. Propose targeted fixes, not architectural overhauls.

Severity Levels

Level	Label	Meaning	Action
🔴	Critical	Security vulnerability, data loss risk, crash in production	Fix before merge
🟠	High	Bug, incorrect behavior, significant performance issue	Fix this sprint
🟡	Medium	Code smell, minor performance issue, missing edge case	Fix when touching the file
🔵	Low	Style preference, minor improvement, documentation gap	Consider for future

Protocol

Step 1: Security Review

Scan for security vulnerabilities: this category takes priority.

Authentication gaps:

Search for route handlers and verify each has auth checks:

grep -rn "export async function\|export function" --include="*.ts" app/api/ | head -20

Flag these patterns:

• Route handler without auth check (e.g., getAuthContext) at the top
• Org-scoped route without verifyOrgAccess
• Admin action without role verification
• Webhook endpoint without signature verification

Exposed secrets:

Search for hardcoded credentials:

grep -rn "sk_live\|sk_test\|password\s*=\s*['\"]" --include="*.ts" --include="*.tsx" --include="*.js" --include="*.env" . | grep -v node_modules | grep -v .env.example

Flag these patterns:

• API keys or tokens hardcoded in source
• .env files committed to git
• Secrets in client-side code (NEXT_PUBLIC_ prefix on secret values)
• Database connection strings in source files

Injection vulnerabilities:

Search for unsafe input handling:

grep -rn "\.raw(\|\.unsafeRaw\|innerHTML\s*=" --include="*.ts" --include="*.tsx" --include="*.js" . | grep -v node_modules

Flag these patterns:

• String concatenation in SQL queries (use parameterized queries)
• Unsafe HTML rendering with user-supplied content
• Unsanitized URL parameters used in redirects (open redirect)
• User input reflected in HTML without escaping (XSS)

Step 2: Performance Review

Identify patterns that degrade under load.

N+1 queries:

# Find loops that might contain database calls
grep -rn "\.forEach\|\.map\|for.*of\|for.*in" --include="*.ts" --include="*.tsx" . | grep -v node_modules | head -20

Flag these patterns:

• Database query inside a loop (fetch all in one query, then map)
• await inside .map() without Promise.all() (sequential when it could be parallel)
• Sequential await calls that could be Promise.all([...]) (parallelizable)

Missing indexes:

• Foreign key columns without an index (causes slow JOINs)
• Columns used in WHERE or ORDER BY without indexes
• Composite queries that need composite indexes

Frontend performance:

# Check for large imports that should be tree-shaken
grep -rn "import .* from ['\"]lodash['\"]" --include="*.ts" --include="*.tsx" . | grep -v node_modules

Flag these patterns:

• Full library imports instead of tree-shakeable imports (import _ from 'lodash')
• Large dependencies in client bundles (check with next build --analyze)
• Missing React.memo, useMemo, or useCallback on expensive renders
• Images without width/height or next/image optimization
• Unthrottled event handlers (scroll, resize, input without debounce)

Data fetching:

• Fetching more data than needed (SELECT * instead of specific columns)
• Missing pagination on list endpoints
• No caching on expensive, infrequently-changing queries
• Client-side fetching for data that could be server-rendered

Step 3: Maintainability Review

Evaluate code clarity and organization.

Dead code:

grep -rn "export " --include="*.ts" --include="*.tsx" . | grep -v node_modules | head -30

Flag these patterns:

• Functions/components that are never imported
• Commented-out code blocks (delete it: git has history)
• Unused variables, imports, or parameters
• Feature flags for features that shipped months ago

Duplicated logic:

• Same validation logic in multiple route handlers (extract to shared schema)
• Repeated auth patterns (extract to middleware or helper)
• Copy-pasted components with minor differences (extract shared component)
• Same error handling try/catch in every handler (extract to wrapper)

Naming clarity:

• Single-letter variables outside of loops (d, x, t: what are these?)
• Boolean variables without is/has/should prefix (active vs isActive)
• Functions that don't describe their action (process(), handle(), doStuff())
• Inconsistent naming between related files (camelCase in one, snake_case in another)

Type safety:

• any type used where a specific type is known
• Type assertions (as Type) hiding real type errors
• Missing return types on exported functions
• @ts-ignore or @ts-expect-error without explanation

Step 4: Error Handling Review

Check that errors are caught and handled appropriately.

Uncaught promises:

grep -rn "await " --include="*.ts" --include="*.tsx" . | grep -v "try\|catch\|\.catch" | grep -v node_modules | head -20

Flag these patterns:

• await calls without try/catch in route handlers (returns 500 with no context)
• .then() chains without .catch() (unhandled rejection)
• Event handlers with async but no error boundary
• Fire-and-forget promises (no await, no .catch(), no void)

Error quality:

• Generic catch (e) { throw e } (adds nothing, let it propagate or add context)
• Swallowed errors: catch (e) {} (at minimum, log them)
• Stack traces or internal details exposed to users in API responses
• Missing error boundaries in React component trees

Edge cases:

• No handling for empty arrays/null results from database queries
• No handling for network timeouts or external API failures
• No handling for concurrent modification (optimistic locking)
• No handling for file not found, permission denied, or disk full

Step 5: Testing Review

Assess test coverage for critical paths.

Untested critical paths:

• Auth flows (login, logout, token refresh) without tests
• Payment/billing logic without tests
• Data mutation endpoints (POST, PATCH, DELETE) without tests
• RLS policies without tests (test that users CAN'T access other orgs' data)

Missing edge cases:

• Only happy path tested (what about invalid input? empty data? max limits?)
• No tests for error responses (401, 403, 404, 422)
• No tests for boundary conditions (0 items, 1 item, max items)
• No tests for concurrent operations (race conditions)

Test quality:

• Tests that test implementation details instead of behavior
• Tests with no assertions (they "pass" but verify nothing)
• Flaky tests that depend on timing, external services, or test order
• Test data that doesn't represent realistic scenarios

Step 6: Accessibility Review

Check that UI code is usable by everyone.

Image and media:

• <img> without alt attribute (screen readers announce nothing)
• Decorative images without alt="" (screen readers read the filename)
• Video/audio without captions or transcripts
• Icon buttons without accessible labels

Keyboard navigation:

• Click handlers on non-interactive elements instead of <button> (not keyboard accessible)
• Custom dropdowns/modals without focus trapping
• No visible focus indicator on interactive elements
• Tab order that doesn't match visual layout

ARIA and semantics:

• Missing aria-label on icon-only buttons
• Using generic elements for navigation instead of <nav>
• Form inputs without associated <label> elements
• Missing role attributes on custom interactive components
• Live regions (toast notifications) without aria-live

Color and contrast:

• Information conveyed by color alone (add icons or text)
• Low contrast text (below 4.5:1 ratio for normal text, below 3:1 for large text)
• Focus indicators that rely on color change alone

Step 7: Output Per-File Report

For each file reviewed, output findings grouped by file:

file: app/api/organizations/[orgId]/route.ts

🔴 Critical: No auth check on DELETE handler
   Line 45: export async function DELETE(req) { ... }
   Fix: Add getAuthContext + verifyOrgAccess + admin role check
   ```typescript
   const auth = await getAuthContext(req);
   if (!auth) return apiError('Authentication required', 401);
   const access = await verifyOrgAccess(auth.userId, params.orgId);
   if (!access || access.role !== 'owner') return apiError('Access denied', 403);

🟠 High: N+1 query in project listing Line 62: projects.map(async (p) => await getProjectMembers(p.id)) Fix: Batch fetch members for all projects in one query

const membersByProject = await db.members.findByProjectIds(
  projects.map(p => p.id)
);

🟡 Medium: Generic error message Line 78: catch (e) { return apiError('Something went wrong', 500); } Fix: Log the error with context, return safe message

catch (error) {
  console.error('DELETE /organizations failed:', { orgId: params.orgId, error });
  return apiError('Failed to delete organization', 500);
}

🔵 Low: Missing return type on handler Line 45: export async function DELETE(req) Fix: Add explicit return type

export async function DELETE(req: NextRequest): Promise<NextResponse>

### Step 8: Summary Report

After reviewing all files, output a summary:

🔍 Code Review: Complete

Files reviewed: [count] Issues found: [total]

By severity: 🔴 Critical: [count], fix before merge 🟠 High: [count], fix this sprint 🟡 Medium: [count], fix when touching the file 🔵 Low: [count]: consider for future

By category: Security: [count] issues Performance: [count] issues Maintainability: [count] issues Error handling: [count] issues Testing: [count] issues Accessibility: [count] issues

Top 3 priorities:

1. [Most critical issue with file:line]
2. [Second most critical]
3. [Third most critical]

Estimated fix effort: Critical + High: ~[X] hours All issues: ~[X] hours

## Level History

- **Lv.1**: Base: Six-category systematic review (security, performance, maintainability, error handling, testing, accessibility), severity-ranked findings, per-file reports with specific code fixes, summary with prioritized action items. (Origin: MemStack Pro v3.2, Mar 2026)