Code Review Skill

Performs thorough, structured code reviews for GitHub pull requests with focus on problem-solving effectiveness, security, and code quality.

When to Use This Skill

• •User provides a GitHub PR URL for review
• •User asks to review a pull request or code change
• •User wants to verify if a PR fixes a specific issue
• •User asks about security implications of a change
• •User wants alternative approaches to a fix

Input Format

The user will typically provide:

1. •PR URL (required): https://github.com/{owner}/{repo}/pull/{number}
2. •Issue URL (optional): https://github.com/{owner}/{repo}/issues/{number}
3. •Additional context (optional): What to focus on, concerns, etc.

Review Process

Step 1: Gather Information

Use GitHub MCP tools to fetch all relevant data:

1. Parse the PR URL to extract: owner, repo, pull_number
2. Parse the Issue URL (if provided) to extract: owner, repo, issue_number

Fetch PR Data:

• •Get pull request details (title, body, author, state, base/head)
• •Get pull request diff/files changed
• •Get pull request comments and review comments
• •Get commits in the pull request

Fetch Issue Data (if provided):

• •Get issue details (title, body, labels, state)
• •Get issue comments (the full discussion)
• •Check for linked issues or references

Fetch Repository Context:

• •Search for similar implementations in the codebase
• •Check if feature exists in related areas (e.g., MVC vs Blazor)
• •Look for existing patterns that should be followed

Step 2: Analyze Against Review Criteria

Evaluate the PR against each of these categories:

2.1 Problem Alignment

• •Does the PR actually solve the problem stated in the issue?
• •Are ALL cases mentioned in the issue discussion addressed?
• •Does the PR scope match the issue scope (not under/over-engineering)?
• •Are there requirements in comments that were missed?

2.2 Code Quality & Design

• •Does the change make logical sense?
• •Is the code readable and maintainable?
• •Does it follow existing patterns in the codebase?
• •What code should be refactored?
• •Can the fix be done simpler?
• •Is there unnecessary complexity?

2.3 Security Analysis

• •What security threats could this PR introduce?
• •Input validation: Is user input properly sanitized?
• •Authentication/Authorization: Any bypasses possible?
• •Data exposure: Could sensitive data leak?
• •Injection risks: SQL, XSS, command injection?
• •Dependencies: Any vulnerable packages added?

2.4 Alternative Approaches

• •What other ways could this issue be fixed?
• •What are the trade-offs of each approach?
• •Is the PR's approach the best one? Why or why not?
• •Are there simpler solutions that were overlooked?

2.5 Edge Cases & Completeness

• •Are all use cases covered?
• •What edge cases might fail?
• •Error handling: What happens when things go wrong?
• •Boundary conditions: Empty inputs, null values, max limits?
• •Concurrency: Thread safety issues?

2.6 Duplication & Consistency

• •Does this re-implement something that already exists?
• •Is there similar functionality elsewhere that should be reused?
• •For ASP.NET Core: Does this feature exist in MVC? How does the Blazor implementation compare?
• •Are there patterns in the codebase that should be followed?
• •Is the implementation consistent with similar features?

2.7 Testing

• •Are there adequate unit tests?
• •Are edge cases tested?
• •Integration tests where needed?
• •Are tests actually testing the right things?

Step 3: Generate Review Report

Structure the output using the review report template.

Review Report Format

Present findings in this structure:

# Code Review: PR #{number}

## Summary
[One paragraph executive summary of the PR and overall assessment]

## Problem Alignment ✅/⚠️/❌
[Does the PR solve the issue? Missing requirements?]

## Code Quality
### What Works Well 👍
- [Positive observations]

### Concerns 🔍
- [Issues that should be addressed]

### Suggestions for Improvement 💡
- [Refactoring opportunities, simplifications]

## Security Analysis 🔒
[Security threats and mitigations]

## Alternative Approaches
| Approach | Pros | Cons |
|----------|------|------|
| Current PR | ... | ... |
| Alternative 1 | ... | ... |

## Edge Cases
- [ ] Case 1: [covered/not covered]
- [ ] Case 2: [covered/not covered]

## Duplication Check
[Existing implementations, MVC vs Blazor comparison if applicable]

## Final Verdict
[APPROVE / REQUEST CHANGES / NEEDS DISCUSSION]

### Must Fix 🔴
- [Blocking issues]

### Should Fix 🟡  
- [Important but not blocking]

### Consider 🟢
- [Nice to have]

Examples

Example 1: Basic PR Review

User Input:

Review this PR: https://github.com/dotnet/aspnetcore/pull/65306 It's supposed to fix: https://github.com/dotnet/aspnetcore/issues/49683

Agent Actions:

1. •Extract: owner=dotnet, repo=aspnetcore, pull_number=65306, issue_number=49683
2. •Fetch PR #65306 details, diff, and comments
3. •Fetch Issue #49683 details and all comments
4. •Search codebase for related implementations
5. •Analyze against all criteria
6. •Generate structured review report

Example 2: Security-Focused Review

User Input:

Review https://github.com/myorg/api/pull/123 with focus on security

Agent Actions:

1. •Fetch PR data
2. •Prioritize security analysis section
3. •Check for auth, input validation, data exposure
4. •Generate report with expanded security section

Checklist Reference

See review-checklist.md for detailed checklist. See security-checklist.md for security-specific checks.

Important Notes

• •Always read the FULL issue discussion, not just the issue body
• •Check linked issues and references for additional context
• •For ASP.NET Core repos, always compare MVC and Blazor implementations
• •Consider the PR in context of the broader architecture
• •Be constructive - suggest improvements, don't just criticize