Code Review

Comprehensive code analysis for quality, security, maintainability, and performance optimization.

When to Use This Skill

Use this skill when:

• •Reviewing code before merging (pre-merge review)
• •After implementation is complete in openspec-apply-change
• •Refactoring existing code for improvement
• •Auditing code for security vulnerabilities
• •Optimizing performance-critical sections
• •Ensuring adherence to project standards

Instructions

Step 1: Initial Assessment

1. •Read the code being reviewed
2. •Understand the context:
   • •What problem does this solve?
   • •What are the requirements?
   • •Are there related files/components?
3. •Identify the scope:
   • •Lines changed
   • •Files modified
   • •Dependencies introduced

Step 2: Security Review (OWASP)

Check for common security vulnerabilities:

Input Validation:

• • All user inputs are validated/sanitized
• • No SQL injection vulnerabilities
• • No command injection risks
• • Path traversal prevented
• • File uploads restricted

Authentication & Authorization:

• • Proper authentication checks
• • Role-based access control (RBAC)
• • Session management secure
• • No hardcoded secrets or credentials
• • API keys not exposed in client code

Data Protection:

• • Sensitive data encrypted
• • Proper handling of PII (Personally Identifiable Information)
• • Secure communication (HTTPS/TLS)
• • No sensitive data in logs

Output Encoding:

• • XSS prevention (output encoding)
• • CSRF protection
• • Content Security Policy headers

Step 3: Architecture Review

Evaluate against design principles:

SOLID Principles:

• •Single Responsibility: Each class/function has one reason to change
• •Open/Closed: Open for extension, closed for modification
• •Liskov Substitution: Subtypes are substitutable for base types
• •Interface Segregation: Small, focused interfaces
• •Dependency Inversion: Depend on abstractions, not concretions

Design Patterns:

• •Appropriate use of patterns
• •No over-engineering
• •Consistent with project conventions

Coupling & Cohesion:

• •Low coupling between modules
• •High cohesion within modules
• •Clear module boundaries

Step 4: Code Quality Review

Readability:

• • Clear naming (variables, functions, classes)
• • Consistent formatting
• • Appropriate comments (why, not what)
• • No magic numbers/strings
• • Functions are focused and small

Maintainability:

• • DRY (Don't Repeat Yourself)
• • No code duplication
• • Easy to test
• • Documentation exists
• • Error handling comprehensive

Error Handling:

• • Exceptions are caught appropriately
• • Error messages are user-friendly
• • Failures are logged
• • Graceful degradation

Step 5: Performance Review

Efficiency:

• • No unnecessary computations
• • Efficient data structures
• • Database queries optimized
• • No N+1 query problems
• • Caching used appropriately

Resource Usage:

• • Memory leaks prevented
• • Large objects disposed properly
• • No blocking operations on main thread
• • Async/await used correctly

Step 6: Testing Review

• • Unit tests exist for new logic
• • Tests cover edge cases
• • Integration tests for API contracts
• • Test names are descriptive
• • Mocks are appropriate
• • Test coverage is adequate (>80%)

Step 7: Documentation Review

• • Code comments explain complex logic
• • Public APIs are documented (JSDoc, docstrings)
• • README updated if needed
• • Architecture Decision Records (ADRs) for significant changes
• • Changelog entries added

Step 8: Compile Findings

Organize issues by severity:

## Code Review Report

### Critical (Block Merge)
1. [Security vulnerability description]
   - File: path/to/file.ext:line
   - Issue: [detailed explanation]
   - Fix: [recommended solution]

### High (Should Fix)
1. [Performance or architecture issue]
   ...

### Medium (Nice to Have)
1. [Style or readability improvement]
   ...

### Low (Suggestions)
1. [Minor improvements]
   ...

### Positive Feedback
- [What was done well]

Step 9: Provide Actionable Feedback

For each issue:

1. •Explain the problem (why it's an issue)
2. •Show the problematic code
3. •Suggest a fix (with code example if possible)
4. •Reference best practices (OWASP, SOLID, etc.)

Step 10: Follow-up

1. •Track fixes - Ensure all critical/high issues are addressed
2. •Re-review if significant changes are made
3. •Update documentation based on review findings
4. •Share learnings - Common issues can inform team guidelines

Integration with OpenSpec

• •Run after openspec-apply-change completes
• •Block merge in openspec-verify-change until critical issues resolved
• •Link review findings to tasks in tasks.md
• •Document architectural decisions in design.md

Guardrails

• •Be constructive - Focus on the code, not the person
• •Explain why - Don't just say "change this", explain the reason
• •Balance thoroughness with pragmatism - Not everything needs to be perfect
• •Respect existing patterns - Don't force changes unless there's a good reason
• •Consider the context - Startup MVP vs. enterprise system have different standards

Review Checklist Template

## Security
- [ ] Input validation
- [ ] Authentication/authorization
- [ ] No secrets in code
- [ ] XSS/CSRF protection

## Architecture
- [ ] SOLID principles
- [ ] Appropriate patterns
- [ ] Low coupling

## Quality
- [ ] Naming clarity
- [ ] No duplication
- [ ] Error handling
- [ ] Comments where needed

## Performance
- [ ] Efficient algorithms
- [ ] No N+1 queries
- [ ] Proper async usage

## Testing
- [ ] Unit tests
- [ ] Integration tests
- [ ] Edge cases covered

## Documentation
- [ ] Code comments
- [ ] API docs
- [ ] README updated

Requirements

• •Access to the codebase
• •Understanding of project conventions
• •Knowledge of security best practices (OWASP)
• •Familiarity with design patterns
• •Understanding of performance optimization

See Also

• •secure-coding-cybersecurity - Deep security audit
• •performance-optimizer - Performance analysis
• •code-maintainability - Maintainability assessment
• •openspec-verify-change - Verification with review gates