b4san/code-review

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