Agent Config Adapter

---
name: Code Review
description: Automatic code review before commits and pushes. Use when code changes are about to be committed or when user requests review. Auto-applies before git operations to ensure code quality and security.
allowed-tools: Bash(git:*), FileSystem, Bash(npm:*), Bash(yarn:*), Bash(lint:*), Bash(test:*), Bash(build:*), Read, Grep, Glob, Task
---

# Code Review Skill

Automatic code review that triggers BEFORE commits and pushes to ensure code quality, security, and adherence to standards.

## When This Skill Auto-Applies

This skill automatically reviews code when:
- User says "commit", "commit and push"
- Before git-best-practices skill commits changes
- During /completeWork validation phase
- User explicitly requests review
- Pre-commit hook triggered (if configured)

**Critical**: This skill runs BEFORE the commit is created, not after code is written.

## Review Process

### 1. Detect Review Trigger

**Triggers:**
- User intent to commit ("commit", "push")
- Checkpoint commit during /startWork
- Final commit during /completeWork
- Explicit review request

### 2. Quick Assessment

**Check:**
```bash
# Get changed files
git diff --name-only --cached  # Staged changes
git diff --name-only           # Unstaged changes

# Get diff stats
git diff --stat

# Count changes
git diff --numstat
```

**Classification:**
- Trivial changes: < 10 lines, non-critical files (README, comments, formatting)
- Small changes: < 100 lines, single file/module
- Medium changes: < 500 lines, multiple files
- Large changes: > 500 lines, requires deep review

### 3. Review Levels

**Level 1: Quick Review (Trivial/Small)**
- Code style and formatting
- Obvious errors
- Lint and type errors
- Import issues
- Duration: ~10 seconds

**Level 2: Standard Review (Medium)**
- All Level 1 checks
- Logic correctness
- Edge cases
- Error handling
- Test coverage
- Documentation
- Duration: ~30 seconds

**Level 3: Deep Review (Large/Security-Sensitive)**
- All Level 1 & 2 checks
- **Invoke security-code-reviewer subagent**
- Security vulnerabilities
- Performance implications
- Architecture impact
- Breaking changes
- Duration: ~2 minutes

## Review Criteria

### 1. Code Quality

**Check for:**
- Readability and clarity
- Naming conventions
- Code duplication
- Complexity (cyclomatic complexity)
- Error-prone patterns
- Magic numbers/strings
- TODO/FIXME comments

**Auto-fix when possible:**
- Formatting issues (via linters)
- Import organization
- Simple refactorings

### 2. Security

**Detect:**
- Hardcoded credentials
- SQL injection risks
- XSS vulnerabilities
- Path traversal
- Insecure dependencies
- Exposed secrets (.env, API keys)
- Unsafe eval/exec usage
- OWASP Top 10 issues

**Critical Security Files:**
If changes involve authentication, authorization, payment, or sensitive data:
→ **Auto-invoke security-code-reviewer subagent**

Security-sensitive patterns:
- Files: auth*.*, login*.*, payment*.*, api/key*.*, credential*.*
- Code: `eval(`, `exec(`, `dangerouslySetInnerHTML`, SQL queries, crypto operations

### 3. Standards Compliance

**Check:**
- Coding style (ESLint, Prettier, etc.)
- TypeScript/type safety
- Commit message format (handled by conventional-commit)
- File structure conventions
- API design patterns

**Run linters:**
```bash
npm run lint --silent 2>&1 || true
npm run typecheck --silent 2>&1 || true
```

### 4. Testing

**Verify:**
- Tests exist for new functionality
- Tests cover edge cases
- Test files follow naming conventions
- No skipped tests without reason
- Tests are passing (coordinate with checkpoint-validator)

**Don't run tests** - checkpoint-validator handles that
**Just verify** test coverage and quality

### 5. Documentation

**Check:**
- Code comments for complex logic
- JSDoc/TSDoc for public APIs
- README updates if needed
- Changelog updates if needed
- No outdated comments

### 6. Task Alignment

If taskFindings.md exists:
- Verify changes match requirements
- Check acceptance criteria
- Ensure no scope creep
- Validate implementation approach

## Decision Matrix

### Pass: Allow Commit
- All checks passed
- No critical issues
- Minor issues are acceptable
- Tests passing (per checkpoint-validator)
- Linter passing or auto-fixed

### Warn: Allow with Caution
- Minor code quality issues
- Missing documentation
- Test coverage could be better
- Non-critical security concerns

**Action:** Commit allowed, but report warnings

### Block: Prevent Commit
- Critical security vulnerabilities
- Linter errors that can't be auto-fixed
- Breaking changes without approval
- Tests failing
- Hardcoded secrets detected

**Action:** Block commit, provide fix suggestions

## Integration with Subagents

### With security-code-reviewer subagent:

**Auto-invoke when:**
- Changes to auth/payment/sensitive files
- Large changes (> 500 lines)
- User explicitly requests deep security review
- Security patterns detected in code

**Pass context:**
- Changed files
- Diff output
- Task context (if available)

**Receive:**
- Security assessment
- Vulnerability findings
- Fix recommendations

**Example invocation:**
```
Detected security-sensitive changes in:
  - src/auth/login.ts
  - src/api/payment.ts

Invoking security-code-reviewer for deep analysis...

[Security subagent runs isolated analysis]

Result: 2 vulnerabilities found
1. SQL injection risk in payment.ts:45
2. Weak password validation in login.ts:120

BLOCKING commit until fixed.
```

## Integration with Other Skills

### With git-best-practices:
- Code-review runs BEFORE git-best-practices commits
- If blocked → git-best-practices aborts
- If passed → git-best-practices proceeds

### With conventional-commit:
- Code-review focuses on code quality
- conventional-commit focuses on message format
- Both happen before commit creation

### With checkpoint-validator:
- checkpoint-validator runs tests
- code-review verifies test quality and coverage
- Both inform commit decision

## Output Format

### Pass (No Issues)
```
✓ Code review passed
  - Code quality: Good
  - Security: No issues detected
  - Standards: Compliant
  - Testing: Adequate coverage
  - Duration: 8s

Ready to commit.
```

### Pass with Warnings
```
✓ Code review passed with warnings
  - Code quality: Good
  - Security: No issues detected
  - Standards: Compliant
  - Testing: Could be improved

⚠️  Warnings:
  1. Missing JSDoc for new function in utils.ts:45
  2. Test coverage could be better for edge cases

Proceeding with commit. Consider addressing warnings.
```

### Blocked (Critical Issues)
```
❌ Code review BLOCKED

Critical issues found:
1. SECURITY: Hardcoded API key in config.ts:12
   Fix: Move to environment variable

2. LINT ERROR: TypeScript error in handler.ts:89
   Fix: Add type annotation for 'data' parameter

3. SECURITY: SQL injection risk in query.ts:34
   Fix: Use parameterized queries

COMMIT BLOCKED. Please fix these issues and retry.
```

### Deep Review (with subagent)
```
🔍 Deep security review in progress...

Detected security-sensitive changes:
  - src/auth/middleware.ts
  - src/api/endpoints/payment.ts

Invoking security-code-reviewer subagent...

[Security analysis complete]

❌ Code review BLOCKED

Security vulnerabilities found:
1. HIGH: JWT token not validated properly (middleware.ts:67)
2. MEDIUM: Payment amount not validated for negative values (payment.ts:123)

Recommendations from security-code-reviewer:
- Use proper JWT verification library
- Add input validation for payment amounts
- Consider rate limiting for payment endpoints

COMMIT BLOCKED. Security issues must be fixed.
```

## Auto-Fix Capabilities

When safe, auto-fix these issues:
- Formatting (via Prettier, ESLint --fix)
- Import organization
- Missing semicolons
- Trailing whitespace
- Simple type annotations

**Report auto-fixes:**
```
✓ Auto-fixed 5 issues:
  - Formatted 3 files with Prettier
  - Organized imports in 2 files
  - Added missing semicolons (8 locations)
```

## Special Cases

### 1. Emergency/Hotfix
If user explicitly says "emergency" or "hotfix":
- Run expedited review (Level 1 only)
- Allow commit with warnings
- Still block on critical security issues

### 2. WIP Commits
If commit message starts with "WIP:" or "🚧 wip:":
- Relaxed review
- Allow commit with more warnings
- Note: "WIP commit - full review at final commit"

### 3. Doc-Only Changes
If only markdown/documentation changed:
- Skip code quality checks
- Check markdown formatting
- Verify links are valid
- Quick pass

### 4. Config-Only Changes
If only config files changed (.json, .yaml, etc.):
- Validate JSON/YAML syntax
- Check for exposed secrets
- Quick pass

## Error Handling

**If linter fails to run:**
- Continue review without lint results
- Note in output: "Linter unavailable"

**If subagent fails:**
- Fall back to basic security checks
- Note in output: "Deep security review unavailable"

**If git commands fail:**
- Report error clearly
- Don't block commit unnecessarily
- Provide troubleshooting steps

## Workflow Integration

### During /startWork (Checkpoint Commits):
```
Code change →
checkpoint-validator runs tests →
code-review checks quality →
If passed: conventional-commit + git-best-practices commit →
Continue development
```

### User Says "commit and push":
```
User intent →
code-review runs (this skill) →
If blocked: Stop, show issues →
If passed: conventional-commit + git-best-practices commit + push
```

### During /completeWork:
```
Final changes →
code-review (comprehensive) →
If large/sensitive: Invoke security-code-reviewer →
If passed: checkpoint-validator full test suite →
conventional-commit + git-best-practices final commit →
pr-automation (if available)
```

## Configuration

Can be configured in task context or project settings:

```yaml
code-review:
  level: standard  # quick | standard | deep
  block-on-warnings: false
  auto-fix: true
  security-check: auto  # always | auto | never
  invoke-subagent-threshold: 500  # lines changed
```

## Notes

- This skill is **proactive** - runs before commits
- **Never** runs after code is written without commit intent
- Integrates with security-code-reviewer for deep analysis
- Can auto-fix simple issues
- Blocks commits when critical issues found
- Provides actionable feedback
- Works with existing git workflow
- Respects emergency/hotfix scenarios

## Escape Hatch

If user needs to override block (not recommended):
```
User can say: "commit anyway" or "override review"
→ Warn about risks
→ Allow commit with disclaimer
→ Note in commit that review was overridden
```

But this should be rare and discouraged.

No companion files

Add companion files to enhance this skill