Code Review

Code Review Checklist

Thorough, structured approach to reviewing code. Work through each dimension systematically rather than scanning randomly.

Installation

OpenClaw / Moltbot / Clawbot

npx clawhub@latest install code-review

Review Dimensions

Security Checklist

Review every change for these vulnerabilities:

• SQL Injection — All queries use parameterized statements or an ORM; no string concatenation with user input
• XSS — User-provided content is escaped/sanitized before rendering; dangerouslySetInnerHTML or equivalent is justified and safe
• CSRF Protection — State-changing requests require valid CSRF tokens; SameSite cookie attributes are set
• Authentication — Every protected endpoint verifies the user is authenticated before processing
• Authorization — Resource access is scoped to the requesting user's permissions; no IDOR vulnerabilities
• Input Validation — All external input (params, headers, body, files) is validated for type, length, format, and range on the server side
• Secrets Management — No API keys, passwords, tokens, or credentials in source code; secrets come from environment variables or a vault
• Dependency Safety — New dependencies are from trusted sources, actively maintained, and free of known CVEs
• Sensitive Data — PII, tokens, and secrets are never logged, included in error messages, or returned in API responses
• Rate Limiting — Public and auth endpoints have rate limits to prevent brute-force and abuse
• File Upload Safety — Uploaded files are validated for type and size, stored outside the webroot, and served with safe Content-Type headers
• HTTP Security Headers — Content-Security-Policy, X-Content-Type-Options, Strict-Transport-Security are set

Performance Checklist

• N+1 Queries — Database access patterns are batched or joined; no loops issuing individual queries
• Unnecessary Re-renders — Components only re-render when their relevant state/props change; memoization is applied where measurable
• Memory Leaks — Event listeners, subscriptions, timers, and intervals are cleaned up on unmount/disposal
• Bundle Size — New dependencies are tree-shakeable; large libraries are loaded dynamically; no full-library imports for a single function
• Lazy Loading — Heavy components, routes, and below-the-fold content use lazy loading / code splitting
• Caching Strategy — Expensive computations and API responses use appropriate caching (memoization, HTTP cache headers, Redis)
• Database Indexing — Queries filter/sort on indexed columns; new queries have been checked with EXPLAIN
• Pagination — List endpoints and queries use pagination or cursor-based fetching; no unbounded SELECT *
• Async Operations — Long-running tasks are offloaded to background jobs or queues rather than blocking request threads
• Image & Asset Optimization — Images are properly sized, use modern formats (WebP/AVIF), and leverage CDN delivery

Correctness Checklist

• Edge Cases — Empty arrays, empty strings, zero values, negative numbers, and maximum values are handled
• Null/Undefined Handling — Nullable values are checked before access; optional chaining or guards prevent runtime errors
• Off-by-One Errors — Loop bounds, array slicing, pagination offsets, and range calculations are verified
• Race Conditions — Concurrent access to shared state uses locks, transactions, or atomic operations
• Timezone Handling — Dates are stored in UTC; display conversion happens at the presentation layer
• Unicode & Encoding — String operations handle multi-byte characters; text encoding is explicit (UTF-8)
• Integer Overflow / Precision — Arithmetic on large numbers or currency uses appropriate types (BigInt, Decimal)
• Error Propagation — Errors from async calls and external services are caught and handled; promises are never silently swallowed
• State Consistency — Multi-step mutations are transactional; partial failures leave the system in a valid state
• Boundary Validation — Values at the boundaries of valid ranges (min, max, exactly-at-limit) are tested

Maintainability Checklist

• Naming Clarity — Variables, functions, and classes have descriptive names that reveal intent
• Single Responsibility — Each function/class/module does one thing; changes to one concern don't ripple through unrelated code
• DRY — Duplicated logic is extracted into shared utilities; copy-pasted blocks are consolidated
• Cyclomatic Complexity — Functions have low branching complexity; deeply nested chains are refactored
• Error Handling — Errors are caught at appropriate boundaries, logged with context, and surfaced meaningfully
• Dead Code Removal — Commented-out code, unused imports, unreachable branches, and obsolete feature flags are removed
• Magic Numbers & Strings — Literal values are extracted into named constants with clear semantics
• Consistent Patterns — New code follows the conventions already established in the codebase
• Function Length — Functions are short enough to understand at a glance; long functions are decomposed
• Dependency Direction — Dependencies point inward (infrastructure to domain); core logic does not import from UI or framework layers

Testing Checklist

• Test Coverage — New logic paths have corresponding tests; critical paths have both happy-path and failure-case tests
• Edge Case Tests — Tests cover boundary values, empty inputs, nulls, and error conditions
• No Flaky Tests — Tests are deterministic; no reliance on timing, external services, or shared mutable state
• Test Independence — Each test sets up its own state and tears it down; test order does not affect results
• Meaningful Assertions — Tests assert on behavior and outcomes, not implementation details
• Test Readability — Tests follow Arrange-Act-Assert; test names describe the scenario and expected outcome
• Mocking Discipline — Only external boundaries (network, DB, filesystem) are mocked
• Regression Tests — Bug fixes include a test that reproduces the original bug and proves it is resolved

Review Process

Work through the code in three passes. Do not try to catch everything in one read.