Code Reviewer

# Code Review Examples

Complete end-to-end examples showing how to use the code-reviewer skill effectively.

---

## Table of Contents

1. [Example 1: Good Comprehensive Code Review](#example-1-good-comprehensive-code-review)
2. [Example 2: Security-Focused Review](#example-2-security-focused-review)
3. [Example 3: Bad Review (What NOT to Do)](#example-3-bad-review-what-not-to-do)

---

## Example 1: Good Comprehensive Code Review

### Context
**Feature:** User authentication API endpoint
**Files Changed:** `src/api/auth.ts`, `src/middleware/validate.ts`
**Lines Changed:** +150/-30
**Review Type:** Pre-merge PR review

### User Request
```
"Review this authentication PR for security, performance, and code quality issues"
```

### Review Process

**Step 1: Run Automated Tools**
```bash
bash scripts/quick-audit.sh
```

**Results:**
- ✅ npm audit: No vulnerabilities
- ✅ ESLint: 0 errors, 2 warnings
- ✅ TypeScript: All type checks pass
- ⚠️ Prettier: 3 files need formatting

**Step 2: Manual Security Review (OWASP Top 10)**

Checked `src/api/auth.ts`:

**A01 - Broken Access Control:**
- ✅ Authentication middleware present on all protected routes
- ✅ Token validation checks user permissions
- ✅ Direct object references prevented with user ID validation

**A02 - Cryptographic Failures:**
- ✅ Passwords hashed with bcrypt (cost factor 12)
- ✅ JWT secrets in environment variables
- ✅ HTTPS enforced in production

**A03 - Injection:**
- ✅ Parameterized queries used (TypeORM)
- ✅ Input validation with Zod schemas
- ✅ No string concatenation in SQL

**A07 - Authentication Failures:**
- ✅ Rate limiting implemented (5 attempts/15 minutes)
- ⚠️ **ISSUE FOUND:** Session timeout set to 7 days (too long)
- ✅ Password requirements enforced (12 chars min)

**Step 3: Performance Analysis**

```typescript
// File: src/api/auth.ts:45
async function getUserWithPosts(userId: string) {
  const user = await User.findOne({ where: { id: userId } });
  const posts = await Post.find({ where: { userId } });  // 🔴 N+1 potential
  return { ...user, posts };
}
```

**🔴 ISSUE FOUND:** Potential N+1 query if called in loop. Should use JOIN.

**Step 4: Code Quality**

- ✅ Functions < 50 lines
- ✅ No `any` types
- ✅ Proper error handling
- ⚠️ **ISSUE FOUND:** `console.log` on line 78 should be removed
- ✅ Test coverage: 85%

### Review Report Generated

```markdown
# Code Review Report: User Authentication API

**Reviewed by:** Claude (code-reviewer skill)
**Date:** 2025-11-03
**PR:** #123
**Review Type:** Standard Pre-Merge Review

---

## Executive Summary

**Verdict:** ⚠️ APPROVED WITH RESERVATIONS

**Overview:**
Solid authentication implementation with proper security practices. Two medium-priority issues identified that should be fixed before merge.

**Key Metrics:**
- **Files Modified:** 2
- **Lines Changed:** +150/-30
- **Estimated Risk:** 🟡 Medium
- **Test Coverage:** 85%
- **Review Duration:** 8 minutes

---

## Critical Issues (Blocking - Must Fix Before Merge)

**None identified** ✅

---

## High Priority Issues (Fix Within 48 Hours)

**None identified** ✅

---

## Medium Priority Issues (Fix This Sprint)

### 1. Session Timeout Too Long 🟡

**Location:** `src/api/auth.ts:23`

**Issue:**
```typescript
const token = jwt.sign({ userId }, JWT_SECRET, { expiresIn: '7d' });
```

Session timeout of 7 days is excessive and increases risk if token is compromised.

**Recommendation:**
```typescript
const token = jwt.sign({ userId }, JWT_SECRET, { expiresIn: '24h' });
```

Change to 24 hours (or 1 hour for high-security applications).

**Severity:** 🟡 Medium
**Category:** Security - Authentication
**OWASP:** A07:2021 - Identification and Authentication Failures

---

### 2. Potential N+1 Query Pattern 🟡

**Location:** `src/api/auth.ts:45-48`

**Issue:**
```typescript
async function getUserWithPosts(userId: string) {
  const user = await User.findOne({ where: { id: userId } });
  const posts = await Post.find({ where: { userId } });
  return { ...user, posts