🔍 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
.envfiles 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)
awaitinside.map()withoutPromise.all()(sequential when it could be parallel)- Sequential
awaitcalls that could bePromise.all([...])(parallelizable)
Missing indexes:
- Foreign key columns without an index (causes slow JOINs)
- Columns used in
WHEREorORDER BYwithout 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, oruseCallbackon expensive renders - Images without width/height or
next/imageoptimization - 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/shouldprefix (activevsisActive) - Functions that don't describe their action (
process(),handle(),doStuff()) - Inconsistent naming between related files (camelCase in one, snake_case in another)
Type safety:
anytype used where a specific type is known- Type assertions (
as Type) hiding real type errors - Missing return types on exported functions
@ts-ignoreor@ts-expect-errorwithout 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:
awaitcalls withouttry/catchin route handlers (returns 500 with no context).then()chains without.catch()(unhandled rejection)- Event handlers with
asyncbut no error boundary - Fire-and-forget promises (no
await, no.catch(), novoid)
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>withoutaltattribute (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-labelon icon-only buttons - Using generic elements for navigation instead of
<nav> - Form inputs without associated
<label>elements - Missing
roleattributes 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:
- [Most critical issue with file:line]
- [Second most critical]
- [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)