Code Review Expert

Expert assistant for comprehensive code review including security vulnerability detection, code quality assessment, performance analysis, and best practice recommendations.

How It Works

1. Analyze Repository Context - Scan existing codebase to understand established patterns
2. Understands the purpose and scope of changes
3. Checks for security vulnerabilities (highest priority)
4. Evaluates code quality and maintainability
5. Validates consistency with repo standards - Coding style, design patterns, architecture
6. Identifies performance issues
7. Provides actionable improvement suggestions

Usage

Fetch PR Diff

bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh <pr-number> [repo] [format]

Arguments:

• pr-number - Pull request number (required)
• repo - Repository in owner/repo format (default: from git remote)
• format - Output format: markdown, json, plain (default: markdown)

Examples:

bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh 123
bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh 123 owner/repo markdown
bash /mnt/skills/user/code-review-master/scripts/pr-diff.sh 456 owner/repo json

Output:

• PR metadata (title, author, base branch)
• Changed files list
• Diff content formatted for review

Requirements: gh CLI installed and authenticated

Review Dimensions

1. Security (OWASP Top 10)

Critical Checks:

• SQL Injection - Parameterized queries used?
• XSS - User input sanitized for output?
• CSRF - Tokens validated for state-changing requests?
• Sensitive Data - Secrets in code? Logging sensitive info?
• Auth/Authz - Proper access control checks?

Security Code Patterns:

// BAD: SQL Injection
db.query(`SELECT * FROM users WHERE id = ${userId}`);

// GOOD: Parameterized query
db.query('SELECT * FROM users WHERE id = $1', [userId]);

// BAD: XSS vulnerability
element.innerHTML = userInput;

// GOOD: Safe text content
element.textContent = userInput;

2. Code Quality

Checks:

• Naming clarity and consistency
• Function length and complexity
• DRY (Don't Repeat Yourself)
• Error handling completeness
• Single Responsibility Principle

Metrics:

• Cyclomatic complexity < 10
• Function length < 50 lines
• Nesting depth < 4 levels

3. Performance

Checks:

• N+1 query problems
• Memory leak risks
• Unnecessary computations
• Missing async/parallel opportunities
• Inefficient data structures

Performance Patterns:

// BAD: N+1 queries
for (const user of users) {
  const orders = await getOrdersByUserId(user.id);
}

// GOOD: Batch query
const userIds = users.map(u => u.id);
const orders = await getOrdersByUserIds(userIds);

4. Maintainability

Checks:

• Test coverage for changes
• Documentation for public APIs
• Proper module structure
• Dependency management
• Backward compatibility

5. Repository Consistency (Repo-Specific Standards)

Before reviewing, analyze the existing codebase to understand:

Coding Style Analysis

• Naming conventions (camelCase, snake_case, PascalCase)
• File/folder naming patterns
• Import ordering and grouping
• Comment styles and documentation format
• Indentation and formatting preferences
• Error handling patterns used in the repo

How to Analyze:

# Scan existing code for naming patterns
# Look at 5-10 representative files in src/ or lib/
# Identify consistent patterns across the codebase

Design Pattern Detection

• Creational: Factory, Singleton, Builder patterns in use
• Structural: Adapter, Decorator, Facade patterns
• Behavioral: Observer, Strategy, Command patterns
• Repository-specific patterns: Custom patterns unique to this codebase

Common Patterns to Check:

// If repo uses Factory pattern
// BAD: Direct instantiation breaking existing pattern
const service = new UserService(db, config);

// GOOD: Follow existing factory pattern
const service = ServiceFactory.create('user', { db, config });

// If repo uses Repository pattern
// BAD: Direct database access in controller
const users = await db.query('SELECT * FROM users');

// GOOD: Follow existing repository pattern
const users = await userRepository.findAll();

Layered Architecture Compliance

Identify the repo's architecture style:

• Clean Architecture: Entities → Use Cases → Controllers → Frameworks
• Hexagonal (Ports & Adapters): Core → Ports → Adapters
• MVC/MVVM: Model → View → Controller/ViewModel
• Domain-Driven Design: Domain → Application → Infrastructure
• Microservices patterns: Service boundaries, API contracts

Architecture Checks:

// Analyze directory structure to understand layers
src/
├── domain/          # Business logic (should not import from infrastructure)
├── application/     # Use cases (orchestrates domain)
├── infrastructure/  # External concerns (DB, API, etc.)
└── presentation/    # UI/API

...