Memstack memstack-development-code-reviewer
Use this skill when the user says 'review code', 'code review', 'check my code', 'audit this', 'review PR', 'review changes', 'what\'s wrong with this', or is requesting a structured review of code quality, security, performance, or maintainability. Do NOT use for refactoring plans or test generation.
git clone https://github.com/cwinvestments/memstack
T=$(mktemp -d) && git clone --depth=1 https://github.com/cwinvestments/memstack "$T" && mkdir -p ~/.claude/skills && cp -r "$T/skills/development/code-reviewer" ~/.claude/skills/cwinvestments-memstack-memstack-development-code-reviewer && rm -rf "$T"
skills/development/code-reviewer/SKILL.md🔍 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.,
) at the topgetAuthContext - 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
files committed to git.env- Secrets in client-side code (
prefix on secret values)NEXT_PUBLIC_ - 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)
insideawait
without.map()
(sequential when it could be parallel)Promise.all()- Sequential
calls that could beawait
(parallelizable)Promise.all([...])
Missing indexes:
- Foreign key columns without an index (causes slow JOINs)
- Columns used in
orWHERE
without indexesORDER BY - 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
, oruseMemo
on expensive rendersuseCallback - Images without width/height or
optimizationnext/image - 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
— what are these?)t - Boolean variables without
/is
/has
prefix (should
vsactive
)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:
type used where a specific type is knownany- Type assertions (
) hiding real type errorsas Type - Missing return types on exported functions
or@ts-ignore
without explanation@ts-expect-error
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:
calls withoutawait
in route handlers (returns 500 with no context)try/catch
chains without.then()
(unhandled rejection).catch()- Event handlers with
but no error boundaryasync - Fire-and-forget promises (no
, noawait
, no.catch()
)void
Error quality:
- Generic
(adds nothing — let it propagate or add context)catch (e) { throw e } - Swallowed errors:
(at minimum, log them)catch (e) {} - 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:
without<img>
attribute (screen readers announce nothing)alt- Decorative images without
(screen readers read the filename)alt="" - Video/audio without captions or transcripts
- Icon buttons without accessible labels
Keyboard navigation:
- Click handlers on non-interactive elements instead of
(not keyboard accessible)<button> - 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
on icon-only buttonsaria-label - Using generic elements for navigation instead of
<nav> - Form inputs without associated
elements<label> - Missing
attributes on custom interactive componentsrole - 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)