Gsd-2 review

<objective> Review code changes and provide structured feedback covering security, performance, bug risks, code quality, and test coverage gaps. This skill analyzes diffs and surrounding context to catch issues before they reach production. </objective> <context> This skill reviews code changes at various stages of the development workflow. It can review staged changes before a commit, unstaged work-in-progress, a specific commit, or the full set of changes on a branch that are ready for a pull request.

The reviewer reads both the diff and the surrounding source files to understand intent and catch issues that only appear in context. </context>

<core_principle> FIND REAL ISSUES, NOT STYLE NITS. Focus on problems that cause bugs, security vulnerabilities, performance degradation, or maintainability pain. Avoid nitpicking formatting or subjective style preferences unless they harm readability. </core_principle>

<analysis_only_rule> THIS SKILL IS READ-ONLY. DO NOT MODIFY CODE.

The purpose is to review and report findings. Making changes during review conflates the reviewer and author roles. Present findings and let the user decide what to act on. </analysis_only_rule>

<quick_start>

<determine_review_scope>

Parse the user's input to determine what to review:

1. No arguments - Review staged changes first. If nothing is staged, review unstaged changes.

   • Staged:

     git diff --cached

   • Unstaged:

     git diff

   • If both are empty, review the most recent commit:

     git show HEAD

2. Commit hash argument (e.g.,

   /review abc1234

   ) - Review that specific commit.

   • git show <hash>

3. File path argument (e.g.,

   /review src/foo.ts

   ) - Review unstaged changes in that file.

   • git diff -- <path>

     then fall back to

     git diff --cached -- <path>

4. "pr" argument (e.g.,

   /review pr

   ) - Review all changes since branching from main.

   • git diff main...HEAD

   • If on main, review

     git diff HEAD~1

After obtaining the diff, if it is empty, inform the user that there are no changes to review and stop.

</determine_review_scope>

<gather_context>

Before analyzing the diff:

1. Read changed files in full - Do not review a diff in isolation. Read each modified file to understand the surrounding code, imports, types, and control flow.
2. Identify the tech stack - Note languages, frameworks, and libraries in use. This affects what patterns are risky.
3. Check for related test files - For each changed source file, look for corresponding test files. Note whether tests were updated alongside the changes.
4. Check for configuration changes - If config files changed (env, CI, package.json, tsconfig, etc.), pay extra attention to side effects.

</gather_context>

<review_categories>

Analyze the changes against each category below. Only report findings that are actually present. Skip categories with no issues.

A. Security Issues (Severity: CRITICAL or HIGH)

• Injection vulnerabilities (SQL injection, command injection, template injection)
• Cross-site scripting (XSS) - unsanitized user input rendered in HTML
• Authentication and authorization flaws (missing auth checks, privilege escalation)
• Secrets or credentials hardcoded or logged
• Insecure deserialization or unsafe eval usage
• Path traversal or file access vulnerabilities
• Missing input validation on external data

B. Performance Concerns (Severity: HIGH or MEDIUM)

• N+1 query patterns in database access
• Unnecessary memory allocations in hot paths or loops
• Blocking operations on the main thread or in async contexts
• Missing pagination on unbounded queries
• Redundant computation that could be cached or memoized
• Large payloads without streaming or chunking

C. Bug Risks (Severity: HIGH or MEDIUM)

• Off-by-one errors in loops or array access
• Null/undefined dereferences without guards
• Race conditions in concurrent or async code
• Incorrect error handling (swallowed errors, wrong error types)
• Type mismatches or unsafe type assertions
• Logic errors in conditionals (inverted checks, missing cases)
• Resource leaks (unclosed connections, file handles, listeners)

D. Code Quality (Severity: MEDIUM or LOW)

• Unclear or misleading naming
• Significant code duplication that should be extracted
• Excessive complexity (deeply nested logic, functions doing too many things)
• Dead code or unreachable branches
• Missing or misleading comments on non-obvious logic
• Inconsistency with patterns used elsewhere in the codebase

E. Test Coverage Gaps (Severity: MEDIUM or LOW)

• New logic paths without corresponding test cases
• Changed behavior without updated tests
• Edge cases not covered (empty inputs, boundary values, error paths)
• Missing integration tests for new API endpoints or database changes

</review_categories>

<format_findings>

For each finding, use this structure:

### [SEVERITY] Category: Brief Title

**File**: `path/to/file.ext` (lines X-Y)

**Issue**: Clear description of the problem.

**Why it matters**: What could go wrong if this is not addressed.

**Suggestion**: How to fix it, with a code snippet if helpful.

Severity levels:

• CRITICAL - Must fix before merge. Security vulnerability or data loss risk.
• HIGH - Should fix before merge. Likely bug or significant performance issue.
• MEDIUM - Should fix soon. Code quality or moderate risk issue.
• LOW - Consider fixing. Minor improvement opportunity.

</format_findings>

</quick_start>

<critical_rules>

1. READ THE FULL FILE: Never review a diff without reading the complete source file for context
2. NO FALSE ALARMS: Only report issues you can explain concretely. Do not report vague concerns
3. PRIORITIZE: Lead with the most severe findings. Do not bury critical issues under style nits
4. BE SPECIFIC: Include file paths, line numbers, and code references for every finding
5. EXPLAIN THE RISK: For each finding, explain what could actually go wrong
6. CHECK TESTS: Always check whether changes have corresponding test updates
7. CONSIDER THE STACK: Apply language-specific and framework-specific knowledge to your review
8. DO NOT MODIFY CODE: Present findings only. The user decides what to act on

</critical_rules>

<output_format>

## Code Review: [brief description of what was reviewed]

**Scope**: [staged changes | unstaged changes | commit abc1234 | PR changes from main]
**Files reviewed**: [count] files changed, [additions] additions, [deletions] deletions

---

### Findings

[Findings grouped by severity, highest first. Use the format from <format_findings>.]

---

### Summary

| Severity | Count |
|----------|-------|
| CRITICAL | X     |
| HIGH     | X     |
| MEDIUM   | X     |
| LOW      | X     |

### Recommended Actions

1. [Most important action to take]
2. [Next most important action]
3. [...]

If no issues are found:

## Code Review: [brief description]

**Scope**: [what was reviewed]
**Files reviewed**: [count]

No significant issues found. The changes look good to merge.

</output_format>

<decision_gate>

After presenting findings, ALWAYS offer these options:

─────────────────────────────────────────
REVIEW COMPLETE

What would you like to do?

1. **Fix issues** - I'll address the findings starting with the most critical
2. **Save review** - Export findings to a markdown file
3. **Review again** - Re-review with different scope or focus
4. **Discuss a finding** - Ask questions about a specific issue
5. **Other** - Tell me what you need
─────────────────────────────────────────

Wait for user response before taking any action.

This gate is MANDATORY. Never skip it. Never auto-implement fixes.

</decision_gate>