code-review

Code Review

Comprehensive AI-powered code review for PRs and local changes — enterprise-grade alternative to CodeRabbit

When to use

• "code review"
• "review my PR"
• "review PR #123"
• "check my changes"
• "what's wrong with my code"
• "security review"
• "full review"
• "/review"
• "/review-pr"

Dependencies

• External: gh CLI (GitHub), git

Modes

1. Local review (uncommitted changes)

Reviews git diff — changes not yet committed.

2. Branch review (vs main/master)

Reviews all changes in current branch compared to main.

3. PR review (GitHub)

Fetches diff from GitHub PR and can post comments.

4. Focused review

User can request specific focus: security, performance, bugs, style, etc.

How to execute

Step 0: Check if review needed

Skip review if:

• PR is draft (gh pr view --json isDraft)
• PR is already closed/merged
• Only documentation changes (.md, .txt, LICENSE)
• Only config changes (.json, .yaml, .toml) without code impact
• Trivial changes (<5 lines, whitespace only, version bumps)

Inform user and ask to confirm if they still want review.

Step 1: Determine mode

Ask user or detect automatically:

• If PR number provided → PR review
• If uncommitted changes exist → local review
• If on feature branch → branch review
• If specific focus requested → apply focus filter

Step 2: Get diff

Local:

git diff HEAD

Branch (vs main):

DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' || echo "main")
git diff $DEFAULT_BRANCH...HEAD

PR:

gh pr diff <PR_NUMBER>

Step 3: Get context

For thorough review, read related files:

# List changed files
git diff --name-only HEAD

# Read each file fully for context
# Check package.json for dependencies
# Check tsconfig/eslint config for project standards

Step 3b: Filter pre-existing issues

Before reporting an issue, check if it was introduced in this PR:

# Check when the problematic line was last modified
git blame -L <start>,<end> <file> --porcelain | head -1

Skip issues that:

• Existed before this PR (old blame hash)
• Are in unchanged lines
• Were introduced by a different author long ago

Only report issues introduced or modified in current changes.

This prevents noise from legacy code and focuses review on new changes.

Step 4: Comprehensive Analysis

Apply ALL relevant checks from the checklist below.

Step 5: Confidence Scoring

Rate each issue 0-100:

Only report issues with confidence ≥70.

Mark as false positive and skip:

• Pre-existing issues (caught by Step 3b)
• Issues that linters will catch (eslint, prettier)
• Pedantic nitpicks without real impact
• Code that looks wrong but has valid reason (check comments)
• Issues with explicit ignore comments (// eslint-disable, # noqa)

Step 6: Output result

Format:

## Code Review Summary

**Reviewed:** X files, Y lines changed
**Risk Level:** Critical / High / Medium / Low

### Critical Issues (must fix)
- [file:line] Description — Why it matters

### High Priority
- [file:line] Description

### Medium Priority
- [file:line] Description

### Low Priority / Suggestions
- [file:line] Description

### Good Practices
- What was done well

For GitHub PR — post comments:

# General comment on PR
gh pr comment <PR_NUMBER> --body "## AI Code Review
[Review content]"

# Line-by-line comments via API (for specific file/line feedback)
# Replace {owner}, {repo}, {pr} with actual values
gh api repos/{owner}/{repo}/pulls/{pr}/comments \
  --method POST \
  -f body="Issue description and fix suggestion" \
  -f path="src/file.ts" \
  -f line=42 \
  -f side="RIGHT"

COMPREHENSIVE REVIEW CHECKLIST

1. SECURITY (OWASP Top 10 + Extended)

1.1 Injection

• SQL Injection — User input in SQL queries without parameterization
• NoSQL Injection — Unsanitized input in MongoDB/similar queries
• Command Injection — User input passed to shell commands (exec, spawn, system)
• LDAP Injection — User input in LDAP queries
• XPath Injection — User input in XML queries
• Template Injection — User input in template engines (SSTI)
• Header Injection — User input in HTTP headers (CRLF)
• Log Injection — Unsanitized data written to logs

1.2 Broken Authentication

• Weak password requirements — No complexity enforcement
• Missing brute-force protection — No rate limiting on login
• Session fixation — Session ID not regenerated after login
• Insecure session storage — Sessions in localStorage (XSS vulnerable)
• Missing logout — No session invalidation
• Predictable tokens — Using weak random generators for tokens
• Password in URL — Credentials in query parameters
• Missing MFA on critical operations — No 2FA for sensitive actions

1.3 Sensitive Data Exposure

• Hardcoded secrets — API keys, passwords, tokens in code
• Secrets in logs — Sensitive data written to console/logs
• Secrets in error messages — Stack traces exposing internals
• Unencrypted sensitive data — PII/credentials not encrypted at rest
• Weak cryptography — MD5, SHA1 for passwords, short keys
• Missing HTTPS — HTTP links for sensitive operations
• Sensitive data in URLs — Tokens/IDs in GET parameters
• Excessive data exposure — Returning more fields than needed in API

1.4 XML External Entities (XXE)

• XML parsing without disabling DTD — External entity processing enabled
• Unsafe XML deserialization — User-controlled XML parsed

1.5 Broken Access Control

• Missing authorization checks — Actions without permission verification
• IDOR (Insecure Direct Object Reference) — Accessing resources by ID without ownership check
• Privilege escalation — User can access admin functions
• CORS misconfiguration — Wildcard or overly permissive origins
• Missing function-level access control — API endpoints without role checks
• Path traversal — User input in file paths (../)
• Forced browsing — Unprotected admin/debug endpoints

1.6 Security Misconfiguration

• Debug mode in production — Verbose errors, stack traces exposed
• Default credentials — Unchanged default passwords
• Unnecessary features enabled — Unused endpoints, methods
• Missing security headers — No CSP, X-Frame-Options, etc.
• Directory listing enabled — Exposed file structure
• Outdated dependencies — Known vulnerabilities in packages
• Permissive file permissions — World-readable sensitive files

1.7 Cross-Site Scripting (XSS)

• Reflected XSS — User input echoed without encoding
• Stored XSS — Database content rendered without sanitization
• DOM XSS — Client-side JS using unsafe sinks (innerHTML, eval)
• Missing Content-Security-Policy — No CSP headers
• Unsafe React patterns — dangerouslySetInnerHTML with user content
• Template literal injection — User input in template strings

1.8 Insecure Deserialization

• Unsafe JSON parsing — eval() for JSON
• Object deserialization — pickle, serialize without validation
• Prototype pollution — Object.assign/merge with user input

1.9 Using Components with Known Vulnerabilities

• Outdated dependencies — Check package.json against vulnerability databases
• Abandoned packages — Dependencies with no recent updates
• Typosquatting — Suspicious package names

1.10 Insufficient Logging & Monitoring

• Missing audit logs — No logging for security events
• Missing error handling — Silent failures
• No alerting — Security events not monitored

1.11 Additional Security Checks

• SSRF (Server-Side Request Forgery) — User-controlled URLs in server requests
• Open Redirect — User input in redirect URLs
• CSRF protection missing — No tokens for state-changing operations
• JWT issues — Weak secret, algorithm confusion, no expiry
• Regex DoS (ReDoS) — Catastrophic backtracking in regex
• Mass assignment — Binding request body directly to models
• File upload vulnerabilities — No type validation, path traversal
• Race conditions — TOCTOU (time-of-check-time-of-use) issues

2. BUGS & LOGIC ERRORS

2.1 Null/Undefined Handling

• Null pointer dereference — Accessing properties of null/undefined
• Missing null checks — Optional chaining not used where needed
• Null coalescing issues — Wrong default values
• Falsy value confusion — 0, '', false treated as null

2.2 Type Issues

• Type coercion bugs — == instead of ===
• Implicit type conversion — String + Number concatenation
• Missing type checks — typeof/instanceof not used
• Type assertion abuse — Unsafe 'as' casts in TypeScript
• Any type overuse — Losing type safety

2.3 Async/Concurrency

• Missing await — Unhandled promises
• Unhandled promise rejection — No .catch() or try/catch
• Race conditions — Shared state without synchronization
• Deadlocks — Circular waiting for resources
• Callback hell — Nested callbacks instead of async/await
• Memory leaks from listeners — Event listeners not removed
• Stale closures — Capturing old values in closures

2.4 Loop/Iteration Errors

• Off-by-one errors — Wrong loop bounds
• Infinite loops — Missing break condition
• Mutating while iterating — Modifying collection during loop
• Wrong loop variable — Using outer variable in nested loop
• forEach with async — Not waiting for async operations

2.5 Boundary/Edge Cases

• Empty array/object — Not handling empty collections
• Single element — Edge case for arrays with one item
• Negative numbers — Not handling negative input
• Zero division — Dividing without checking denominator
• Integer overflow — Large number calculations
• Floating point precision — 0.1 + 0.2 !== 0.3
• Unicode/encoding — Special characters not handled
• Timezone issues — Date handling without timezone awareness
• Daylight saving time — Date calculations across DST

2.6 State Management

• Stale state — React setState with stale closure
• Missing state updates — UI not reflecting state
• State mutation — Direct state modification
• Prop drilling issues — Props passed too deep
• Unnecessary re-renders — Missing memoization

2.7 Error Handling

• Empty catch blocks — Swallowing errors silently
• Generic error handling — Catching all without specificity
• Missing finally — Resources not cleaned up
• Rethrowing without context — Losing stack trace
• Error in error handler — Handler itself can throw

2.8 Resource Management

• Resource leaks — Files, connections not closed
• Memory leaks — Objects not garbage collected
• Connection pool exhaustion — Not returning connections
• File handle leaks — Streams not closed
• Timer leaks — setInterval not cleared

2.9 Business Logic

• Wrong calculations — Incorrect formulas
• Missing validation — Business rules not enforced
• State machine violations — Invalid state transitions
• Duplicate operations — Same action performed twice
• Missing rollback — Partial updates on failure

3. PERFORMANCE

3.1 Database

• N+1 queries — Loop with individual queries
• Missing indexes — Queries on unindexed columns
• **SELECT *** — Fetching unnecessary columns
• Missing pagination — Loading all records
• Missing connection pooling — New connection per request
• Expensive JOINs — Large table joins without optimization
• Missing query caching — Repeated identical queries
• Transaction scope too large — Holding locks too long
• Inefficient aggregations — COUNT/SUM without indexes

3.2 API/Network

• Missing caching — No Cache-Control headers
• Over-fetching — Returning more data than needed
• Under-fetching — Multiple requests for related data
• No compression — Missing gzip/brotli
• Chatty APIs — Too many small requests
• Missing pagination — Large response payloads
• Synchronous external calls — Blocking on I/O
• Missing timeouts — No timeout on HTTP calls
• Missing retry logic — No retry for transient failures

3.3 Frontend

• Large bundle size — Missing code splitting
• Render blocking resources — CSS/JS blocking paint
• Unnecessary re-renders — Missing React.memo, useMemo
• Large images — Unoptimized images
• Missing lazy loading — Loading offscreen content
• Layout thrashing — Forced synchronous layouts
• Heavy animations — Expensive CSS/JS animations
• Memory leaks — Detached DOM nodes
• Long tasks — Blocking main thread >50ms

3.4 Algorithms

• O(n²) or worse — Nested loops on large data
• Repeated calculations — No memoization
• String concatenation in loops — Using += for strings
• Inefficient data structures — Array instead of Set/Map
• Unnecessary copying — Deep clone when not needed
• Redundant sorting — Sorting already sorted data

3.5 Caching

• Missing caching layer — No Redis/Memcached
• Cache invalidation issues — Stale data served
• Cache stampede — Many requests on cache miss
• No cache warming — Cold start performance
• Unbounded cache — No eviction policy

4. CODE QUALITY

4.1 Readability

• Poor naming — Unclear variable/function names
• Magic numbers — Hardcoded values without constants
• Long functions — Functions >50 lines
• Deep nesting — >3 levels of indentation
• Complex conditionals — Hard to understand if/else chains
• Missing comments for complex logic — Unclear algorithms
• Inconsistent naming convention — camelCase mixed with snake_case
• Abbreviations — Unclear shortened names

4.2 Maintainability

• DRY violations — Copy-pasted code
• God objects/functions — Doing too much
• Tight coupling — Hard dependencies between modules
• Missing abstraction — Repeated patterns not extracted
• Premature abstraction — Over-engineering simple code
• Circular dependencies — Modules importing each other
• Dead code — Unused functions, variables, imports
• Commented-out code — Old code left in comments
• TODO/FIXME/HACK — Technical debt markers

4.3 SOLID Principles

• Single Responsibility — Class/function doing multiple things
• Open/Closed — Modifying instead of extending
• Liskov Substitution — Subclasses breaking parent contract
• Interface Segregation — Fat interfaces
• Dependency Inversion — Depending on concretions

4.4 Error Messages

• Generic error messages — "Something went wrong"
• Technical jargon for users — Stack traces shown to users
• Missing error codes — No way to identify errors
• Non-actionable errors — User doesn't know what to do

4.5 API Design

• Inconsistent response format — Different structures per endpoint
• Wrong HTTP methods — POST for GET operations
• Wrong status codes — 200 for errors
• Missing versioning — No API version in path/header
• Breaking changes — Removing/renaming fields
• Missing documentation — No OpenAPI/Swagger

4.6 Configuration

• Hardcoded configuration — No environment variables
• Missing defaults — Required config without fallback
• Scattered config — Configuration in multiple places
• Secrets in config files — Credentials in committed files

5. TESTING

5.1 Test Coverage

• Missing unit tests — Critical functions untested
• Missing integration tests — API endpoints untested
• Missing edge case tests — Only happy path tested
• Missing error case tests — Error handling untested
• Low coverage on changed files — New code not tested

5.2 Test Quality

• Flaky tests — Tests that sometimes fail
• Slow tests — Tests taking too long
• Test interdependence — Tests depending on order
• Missing assertions — Tests without expect/assert
• Testing implementation — Testing how, not what
• Mocking too much — Testing mocks, not code
• Missing test data cleanup — Side effects between tests

5.3 Test Patterns

• Missing describe blocks — No test organization
• Unclear test names — "test1", "should work"
• Arrange-Act-Assert — Unclear test structure
• Missing fixtures — Repeated test data setup

6. ACCESSIBILITY (a11y)

• Missing alt text — Images without alt attributes
• Missing form labels — Inputs without labels
• Color contrast — Insufficient contrast ratio
• Keyboard navigation — Not usable without mouse
• Missing ARIA labels — Interactive elements without labels
• Focus management — Focus not handled in modals/SPAs
• Missing skip links — No way to skip navigation
• Missing heading hierarchy — Improper h1-h6 usage
• Auto-playing media — No way to stop audio/video
• Missing screen reader text — Icons without labels

7. INTERNATIONALIZATION (i18n)

• Hardcoded strings — Text not in translation files
• Date/time formatting — Not using locale-aware formatting
• Number formatting — Not handling decimal separators
• Currency formatting — Not handling currency symbols
• RTL support — Not handling right-to-left languages
• Pluralization — Not handling singular/plural
• String concatenation — Building sentences from parts
• Hardcoded date formats — MM/DD/YYYY vs DD/MM/YYYY

8. DOCUMENTATION

• Missing README — No project documentation
• Outdated documentation — Docs don't match code
• Missing API docs — No endpoint documentation
• Missing JSDoc/TSDoc — Public APIs undocumented
• Missing changelog — No record of changes
• Missing setup instructions — Hard to get started
• Broken links — Dead URLs in README, docs, or code comments
• Relative link errors — Wrong paths to internal docs/files

9. DEVOPS / INFRASTRUCTURE

• Missing health checks — No /health endpoint
• Missing graceful shutdown — Not handling SIGTERM
• Missing retry logic — No resilience patterns
• Missing circuit breaker — No failure isolation
• Logging not structured — Plain text instead of JSON
• Missing correlation IDs — Can't trace requests
• Missing metrics — No observability
• Missing alerts — No monitoring
• Container issues — Running as root, large images
• Missing resource limits — No CPU/memory limits

10. GIT/VERSION CONTROL

• Large files committed — Binary files, node_modules
• Secrets in history — API keys in past commits
• Merge conflicts markers — Unresolved conflicts
• Mixed line endings — CRLF/LF issues
• Missing .gitignore entries — Temporary files tracked
• Large commits — Too many changes in one commit
• Unclear commit messages — "fix", "update", "wip"

LANGUAGE-SPECIFIC CHECKS (apply if relevant files detected)

11. REACT / NEXT.JS (if .tsx, .jsx files)

• useEffect missing deps — Dependencies array incomplete or missing
• useEffect cleanup — Missing cleanup for subscriptions, timers
• useState stale closure — Using state in callbacks without functional update
• Key prop issues — Missing key, using index as key in dynamic lists
• useMemo/useCallback overuse — Premature optimization
• Prop drilling — Props passed through many levels (use context)
• Component too large — >300 lines, should be split
• Inline function in JSX — Creating new function on every render
• Direct DOM manipulation — Using document.querySelector in React
• Missing error boundary — No error handling for component tree

12. TYPESCRIPT (if .ts, .tsx files)

• Any type abuse — Using any to bypass type checking
• Type assertion abuse — Unsafe as casts without validation
• Missing return types — Public functions without explicit return type
• Implicit any — Variables without type annotation
• Non-null assertion — Overuse of ! operator
• Enum vs union — Using enum where union type is better
• Unused exports — Exported types/functions never imported

13. PYTHON (if .py files)

• Mutable default args — def foo(items=[]) anti-pattern
• Missing type hints — Public functions without annotations
• Bare except — except: without specific exception
• Context manager — File/connection not using with
• Global state — Modifying global variables
• Import side effects — Code execution at import time
• f-string security — User input in f-strings (potential injection)

14. NODE.JS / EXPRESS (if package.json with express)

• Async error handling — Missing try/catch in async routes
• Missing helmet — No security headers middleware
• Missing rate limiting — No protection against abuse
• Callback hell — Nested callbacks instead of async/await
• Blocking event loop — Sync operations in request handlers
• Missing input validation — No validation middleware (joi, zod)
• Hardcoded CORS — Wildcard or hardcoded origins

15. SQL / DATABASE (if .sql files or ORM usage)

• Raw queries — String concatenation instead of parameterized
• Missing indexes — Queries on frequently filtered columns
• N+1 in ORM — Lazy loading in loops
• Wide transactions — Transaction scope too large
• Missing migrations — Schema changes without migration files
• Cascading deletes — Unintended data loss on delete

Parameters

Examples

Example 1: Full review

User: full code review

Claude: [git diff HEAD]
Claude: [comprehensive analysis using all checklists]
Claude:
## Code Review Summary

**Reviewed:** 5 files, 234 lines changed
**Risk Level:** High

### Critical Issues (2)
1. [src/api/auth.ts:45] SQL Injection - User input passed directly to query
2. [src/utils/crypto.ts:12] Weak cryptography - Using MD5 for password hashing

### High Priority (3)
1. [src/services/user.ts:78] Missing authorization check - IDOR vulnerability
2. [src/api/data.ts:23] N+1 query - 50 queries in loop
3. [src/components/Form.tsx:156] XSS - dangerouslySetInnerHTML with user content

### Medium Priority (5)
...

Example 2: Security-focused review

User: security review PR 123

Claude: [applies only security checklist]
Claude: Found 3 security issues...

Example 3: Performance review

User: check performance issues in my changes

Claude: [applies only performance checklist]
Claude: Found 2 performance issues...

Severity Levels

Troubleshooting

Limitations

• Static analysis only — cannot run code
• Cannot see runtime behavior
• May miss complex cross-file issues
• Needs context for architectural decisions
• GitHub API rate limits when posting many comments
• Language-specific checks may vary in depth

If you're getting false positives

Step 1: Use severity filter

Most false positives are low/medium severity. Start with high-only:

"review PR 123 --severity=high"
"review my changes, only critical and high issues"

Step 2: Use focus filter

Narrow to specific categories you care about:

"security review PR 123"
"review PR 123 --focus=bugs,security"
"check only performance issues"

Step 3: Tell Claude to skip specific issues

In the same conversation, provide context:

"ignore the N+1 warning in admin routes - it's intentional, low traffic"
"skip any type warnings in src/legacy/ - that's legacy code"
"the raw SQL in migrations/ is fine, we use raw migrations"

Step 4: Add inline comment in code

For persistent false positives that keep appearing:

// @review-ok: parameterized query handled by ORM
const query = `SELECT * FROM users WHERE id = ${sanitizedId}`;

# @review-ok: global cache intentional for performance
CACHE = {}

Step 5: Report to improve the skill

If the same false positive keeps appearing across reviews:

1. Open issue at github.com/your-org/claude-code-review-skill/issues
2. Include:
   • File and line number
   • What was flagged
   • Why it's a false positive
   • Code snippet if possible

This helps improve the skill for everyone.

Default behavior

The skill is designed to minimize false positives out of the box:

• Baseline filtering — Only reports issues introduced in current PR/changes (via git blame)
• Confidence threshold — Only reports issues with ≥70% confidence
• Skips linter territory — Doesn't flag formatting, style issues that ESLint/Prettier catch
• Skips pre-existing issues — Won't complain about old code you didn't touch
• Skips trivial changes — Version bumps, whitespace, documentation-only changes