Review - Pre-Merge Code Review Tool

Pre-merge PR review tool that analyzes code diffs and provides actionable feedback.

When to Use This Skill

Use this skill when the user says:

• "review this PR"
• "code review"
• "pre-landing review"
• "check my diff"
• "review the current branch"
• "review my changes"
• Code is about to be merged

How This Skill Works

This skill performs a systematic code review with automatic execution.

Execution Workflow

Step 0: Detect Base Branch

First, determine the base branch to compare against:

1. Check for existing PRs and get the target branch
2. If no PR exists, use the repository's default branch (usually

   main

   or

   master

   )
3. Verify the base branch exists locally or fetch it if needed

Step 1: Branch Validation

Ensure conditions are met for a proper review:

1. Verify not on base branch: Current branch should be a feature branch, not main/master
2. Check for changes: Confirm there are actual differences to review
3. Check git status: Ensure working directory is clean or document uncommitted changes

If conditions aren't met:

# Check current branch
git branch --show-current

# Check for uncommitted changes
git status

# Show diff if exists
git diff HEAD

Step 2: Scope Drift Detection

Compare the original task/intent with actual code changes:

1. Review the task description or commit message
2. Analyze the actual files changed
3. Identify:
   • Scope creep: Changes beyond the original task
   • Missing requirements: Intended changes not implemented
   • Unexpected additions: Changes that weren't planned

Report any scope drift issues before proceeding.

Step 3: Load Review Checklist

Load the review checklist if it exists:

# Check for review checklist
REVIEW_CHECKLIST.md
.code-review-checklist.md
docs/review-checklist.md

Use the checklist to ensure all review points are covered.

Step 4: Get the Diff

Generate a comprehensive git diff:

# Get full diff against base branch
git diff origin/main...HEAD

# Or with commit range
git diff <base-branch>...HEAD

Parse the diff to understand:

• Files changed (added, modified, deleted)
• Lines added/removed
• Types of changes (logic, tests, docs, config)

Step 5: Two-Phase Review

Perform review in two phases:

Phase 1: Critical Issues (Must Fix)

Review for critical issues that must be addressed:

1. SQL Security

   • Check for SQL injection vulnerabilities
   • Verify parameterized queries
   • Ensure proper escaping of user input
   • Review database access patterns

2. Race Conditions

   • Identify concurrent access issues
   • Check for missing locks/transactions
   • Verify atomic operations
   • Review state mutations

3. LLM Trust Boundaries

   • Check for prompt injection risks
   • Verify input validation
   • Ensure output sanitization
   • Review data flow to/from LLMs

4. Enum Completeness

   • Check switch/case statements cover all cases
   • Verify enum values are handled
   • Look for default case handling

5. Authentication/Authorization

   • Check permission checks
   • Verify session management
   • Review API security
   • Ensure proper authentication

Phase 2: Informational Issues (Should Fix)

Review for quality improvements:

1. Conditional Side Effects

   • Check for side effects in conditions
   • Identify hidden mutations
   • Review function purity

2. Magic Numbers

   • Identify hardcoded values
   • Suggest named constants
   • Document special values

3. Dead Code

   • Find unused variables/functions
   • Identify commented-out code
   • Remove unreachable code

4. Test Coverage

   • Check for missing tests
   • Verify test quality
   • Suggest edge cases

5. Performance Issues

   • Identify N+1 queries
   • Check for unnecessary loops
   • Review algorithm complexity
   • Suggest optimizations

Step 6: Design Review (Conditional)

If frontend/UI files were changed, perform design review:

1. Compare against design documents if available
2. Check for:
   • Design system adherence
   • Accessibility compliance
   • Responsive design
   • User experience issues
   • Visual consistency

Step 7: Provide Fixes

For issues found, categorize and provide fixes:

AUTO-FIX Level

Automatically fix mechanical issues:

• Code formatting
• Simple logic errors
• Unused imports/variables
• Basic security fixes

ASK Level

Require user confirmation for complex changes:

• Design decisions
• Refactoring suggestions
• Breaking changes
• Architectural concerns

Example feedback format:

## Critical Issues

### 1. SQL Injection Risk
**File**: `src/db/queries.py:42`
**Severity**: 🔴 Critical
**Status**: ❌ Must Fix

```python
# Current (Vulnerable)
query = f"SELECT * FROM users WHERE name = '{user_input}'"

# Suggested Fix
query = "SELECT * FROM users WHERE name = %s"
cursor.execute(query, (user_input,))

Reason: Direct string interpolation allows SQL injection attacks.

---

2. Missing Authentication Check

File:

src/api/routes.py:87

Severity: 🔴 Critical Status: ❌ Must Fix

The

/admin/delete

endpoint lacks authentication.

Suggested Fix:

@app.delete("/admin/delete")
@require_auth  # Add this decorator
def delete_item():

---

Informational Issues

1. Magic Number

File:

src/config.py:15

Severity: 🟡 Medium Status: 💡 Should Fix

# Current
MAX_RETRIES = 3

# Suggested
MAX_RETRIES = 3  # Retry 3 times before giving up

---

Auto-Fixes Applied

1. ✅ Fixed 2 unused imports in

   src/utils/helpers.py

2. ✅ Formatted

   src/models/user.py

   with black
3. ✅ Added missing semicolons in

   src/styles/main.css

### Step 8: Generate Review Status

Assign a final status to the review:

- **DONE**: Review complete, code can merge
- **DONE_WITH_CONCERNS**: Review complete, but has concerns to monitor
- **BLOCKED**: Cannot proceed, blocking issues exist
- **NEEDS_CONTEXT**: Need more information to complete review

### Step 9: Output Review Report

Generate a comprehensive review report:

```markdown
# Code Review Report

## Review Summary
- **Branch**: `feature/user-authentication`
- **Base**: `main`
- **Files Changed**: 12 files
- **Lines Added**: +342
- **Lines Removed**: -89
- **Status**: ✅ DONE

## Critical Issues: 2
1. SQL Injection Risk - ❌ Must Fix
2. Missing Authentication - ❌ Must Fix

## Informational Issues: 5
1. Magic Number - 💡 Should Fix
2. Dead Code - 💡 Should Fix
3. Missing Tests - 💡 Should Fix
4. Performance Issue - 💡 Should Fix
5. Documentation - 💡 Should Fix

## Auto-Fixes Applied: 3
1. ✅ Fixed unused imports
2. ✅ Formatted code
3. ✅ Fixed typos

## Design Review
✅ No design issues found

## Recommendations
1. Address critical SQL injection vulnerability
2. Add authentication to admin endpoint
3. Increase test coverage to 80%+
4. Remove unused code in `src/utils/legacy.py`

## Next Steps
1. Fix critical issues
2. Re-run review
3. Proceed to /qa for testing

Safety Checks

Before making any automatic fixes:

1. Create backup: Document current state
2. Review changes: Show what will be changed
3. Get confirmation: Ask user before applying fixes
4. Preserve logic: Ensure fixes don't break functionality

Integration with Other Skills

This skill integrates with:

• office-hours: Earlier in workflow, for planning
• qa: Next in workflow, for testing fixes
• ship: Final step, for merging and deploying

Boil the Lake Principle

"Don't be half-invested, boil the whole lake"

• Don't just report bugs, fix them: When issues are found, actually fix them
• Complete the task: A review isn't done until issues are addressed
• No "good enough": Pursue 100% quality with AI assistance

Review Checklist Template

Use this checklist to ensure thorough reviews:

## Code Review Checklist

### Security
- [ ] SQL injection checks
- [ ] XSS prevention
- [ ] Authentication/authorization
- [ ] Input validation
- [ ] Output sanitization

### Functionality
- [ ] Logic correctness
- [ ] Edge cases handled
- [ ] Error handling
- [ ] Data validation

### Code Quality
- [ ] Naming conventions
- [ ] Code organization
- [ ] Comments where needed
- [ ] DRY principle followed

### Performance
- [ ] No N+1 queries
- [ ] Efficient algorithms
- [ ] Proper caching
- [ ] Resource cleanup

### Testing
- [ ] Unit tests added
- [ ] Integration tests added
- [ ] Edge cases covered
- [ ] Test quality

### Documentation
- [ ] README updated
- [ ] API docs updated
- [ ] Changes documented
- [ ] Deprecation noted

Error Handling

If review fails:

1. Check git status: Ensure repo is in valid state
2. Verify branch: Confirm branch tracking is correct
3. Check permissions: Ensure file access is available
4. Provide clear error messages: Explain what went wrong and how to fix

Example Usage

User:

/review my current branch

AI: I'll review your current branch changes.

[Executes review workflow]

Output: Comprehensive review report with identified issues and fixes

User: Fix the critical issues

AI: Applying fixes...

[Applies auto-fixes]

Output: Updated review report with fixes applied, suggests next step:

/qa

---

Original: gstack/review by Garry Tan
Adaptation: OpenClaw/WorkBuddy version with automated execution
Version: 2.0.0