Go Code Review Skill

Operator Context

Hardcoded Behaviors (Always Apply)

• CLAUDE.md Compliance: Read and follow repository CLAUDE.md before reviewing
• Over-Engineering Prevention: Report issues only. Do not suggest speculative improvements or "while reviewing" refactors
• Read-Only Mode: NEVER modify code during review. Analyze, identify, report -- do not fix
• All Phases Required: Complete all 6 review phases. No skipping, no shortcuts
• Evidence-Based Findings: Every issue must reference file, line, and concrete impact
• Compilation First: Run

  go build ./...

  before any linting or analysis commands

Default Behaviors (ON unless disabled)

• Automated Checks: Run go build, go test -race, go vet, and coverage analysis
• Gopls MCP Analysis: Use gopls MCP tools when available:

  go_workspace

  for structure,

  go_file_context

  for dependencies,

  go_symbol_references

  for impact analysis,

  go_diagnostics

  for build errors (fallback to grep/LSP)
• Priority Classification: Tag every finding as CRITICAL, HIGH, MEDIUM, or LOW
• Structured Output: Use the PR Review Output Template for all reports
• Cross-Platform Build: Verify compilation on linux, darwin, and windows
• Module Verification: Run go mod verify and check for tidy state

Optional Behaviors (OFF unless enabled)

• Security Audit: Run gosec and perform deep security analysis
• Benchmark Review: Evaluate benchmark coverage for performance-critical code
• API Compatibility Check: Verify exported API changes against semver expectations
• Dependency Audit: Deep review of new or changed dependencies

Available Scripts

• scripts/check-interface-compliance.sh

  — Find exported interfaces missing compile-time

  var _ I = (*T)(nil)

  checks. Run

  bash scripts/check-interface-compliance.sh --help

  for options.

What This Skill CAN Do

• Systematically review Go code across 6 structured phases
• Run automated checks (build, test, vet, staticcheck, coverage)
• Use gopls MCP for semantic analysis (implementations, references, symbols, diagnostics, vulnerability checks)
• Classify findings by severity with actionable recommendations
• Produce structured review reports with executive summary and detailed analysis

What This Skill CANNOT Do

• Modify, edit, or fix any code (reviewers report, they do not fix)
• Replace domain-specific skills (use go-concurrency for concurrency design, go-error-handling for error patterns)
• Skip phases or approve without completing all analysis areas
• Guarantee security (use dedicated security audit tools for compliance)

Instructions

Phase 1: Context Understanding

## Review: [PR/Change Title]
- Problem solved: [what]
- Expected behavior change: [what]
- Linked issues: [references]
- Testing strategy: [approach]

• List all modified files and packages
• Identify core changes vs supporting changes
• Note deleted code and understand why
• Check for generated or vendored code

Phase 2: Automated Checks

# Verify compilation across platforms
GOOS=linux go build ./...
GOOS=darwin go build ./...
GOOS=windows go build ./...

# Run tests with race detector
go test -race -count=1 -v ./...

# Run tests with coverage
go test -coverprofile=coverage.out ./...
go tool cover -func=coverage.out | tail -10

# Static analysis
go vet ./...
staticcheck ./...    # if available
golangci-lint run    # if available

# Module integrity
go mod verify
go mod tidy && git diff go.mod go.sum

.mcp.json

# Detect workspace structure (MUST run first)
go_workspace

# Understand file dependencies after reading .go files (MUST use)
go_file_context({"file": "/path/to/changed_file.go"})

# Find all references before modifying any symbol (MUST use)
go_symbol_references({"file": "/path/to/interface.go", "symbol": "Handler.ServeHTTP"})

# Fuzzy search for symbols
go_search({"query": "Server"})

# Check for build/analysis errors after edits (MUST use)
go_diagnostics({"files": ["/path/to/changed_file.go"]})

# Inspect package public API
go_package_api({"packagePaths": ["example.com/internal/storage"]})

# LSP tool: goToDefinition, findReferences, hover, documentSymbol
# CLI fallback:
command -v gopls >/dev/null && echo "gopls available" || echo "gopls not found"
gopls implementation path/to/interface.go:42:10
gopls references path/to/changed_function.go:100:5
gopls symbols path/to/changed_file.go

• Interface changes: find all implementations that need updating
• Function signature changes: find all callers that may break
• Renaming proposals: verify safe rename with

  go_symbol_references

  first
• Post-edit verification:

  go_diagnostics

  catches errors before running full test suite

gosec ./...  # if available

Phase 3: Code Quality Analysis

• SOLID principles followed?
• Appropriate abstraction levels?
• Clear separation of concerns?
• Dependency injection used properly?
• Interfaces focused and minimal?

• Using

  any

  instead of

  interface{}

  ?
• Proper error handling with context?
• Appropriate use of pointers vs values?
• Channels used correctly?
• Context propagation proper?

• Unnecessary allocations in loops?
• String concatenation in loops (use strings.Builder)?
• Improper use of defer in loops?
• Missing buffer pooling for high-frequency allocations?
• Unbuffered channels in producer-consumer patterns?

Phase 4: Specific Analysis Areas

• Data races possible?
• Proper synchronization (mutex, channels)?
• Goroutine leaks prevented?
• Context cancellation handled?
• Deadlock potential?

• All errors checked or explicitly ignored?
• Error messages provide context?
• Wrapped appropriately with %w?
• Custom error types when needed?
• Sentinel errors properly defined?

• Test coverage adequate (aim for >80%)?
• Table-driven tests used?
• Edge cases covered?
• Error conditions tested?
• Benchmarks for performance-critical code?
• Test helpers marked with t.Helper()?
• No test interdependencies?

• Input validation present?
• SQL injection prevented?
• Path traversal prevented?
• Sensitive data not logged?
• Crypto/rand used for security-critical randomness?
• Time-constant comparisons for secrets?

Phase 5: Line-by-Line Review

1. Is the change necessary?
2. Is the implementation correct?
3. Are there edge cases missed?
4. Could this be simpler?
5. Will this be maintainable?
6. Performance implications?
7. Security implications?

Phase 6: Documentation Review

• Package comments present and helpful?
• Public APIs documented?
• Complex algorithms explained?
• Examples provided for public APIs?
• README updated if needed?

Review Output Template

## PR Review: [Title]

### Executive Summary
- **Risk Level**: [LOW/MEDIUM/HIGH]
- **Recommendation**: [Approve/Request Changes/Comment]
- **Key Finding**: [Most important discovery]

**Critical Positives:**
- [What was done well]

**Areas for Improvement:**
- [Issues that need attention]

### Detailed Analysis

#### 1. Architecture & Design
**Score: [A/B/C/D/F]**
- [pass/fail] Modern Go patterns
- [pass/fail] Proper package structure
- [pass/fail] Interface design
- [pass/fail] SOLID principles

#### 2. Code Quality
**Score: [A/B/C/D/F]**
[Code snippets showing excellent patterns and improvements needed]

#### 3. Testing Strategy
**Score: [A/B/C/D/F]**
Coverage: X%, Target: 80%+, Gaps: [areas needing tests]

#### 4. Security & Performance
**Score: [A/B/C/D/F]**

### Specific Issues

**[CRITICAL]** Issue Title
- **File**: `path/to/file.go:123`
- **Problem**: [Explanation]
- **Impact**: [What could go wrong]
- **Fix**: [How to fix]

**[HIGH/MEDIUM/LOW]** Issue Title
- **File**: `path/to/file.go:456`
- **Details**: [Explanation]
- **Suggestion**: [Improvement]

### Recommendations
1. **Must Fix Before Merge**: [Critical issues]
2. **Should Address**: [Important improvements]
3. **Consider for Future**: [Nice to have]

### Final Assessment
[Summary with actionable next steps]

Review Priority Guidelines

Death Loop Prevention

1. Channel Direction Changes: NEVER change

   chan Type

   to

   <-chan Type

   without verifying the function does not send or close
2. Function Signature Changes: NEVER change return types without updating ALL call sites
3. Compilation Before Linting: Run

   go build ./...

   FIRST. If code does not compile, report compilation errors before linting issues

Error Handling

Error: "Automated Tool Not Available"

1. Check tool availability with

   command -v

2. Proceed with available tools (go build, go test, go vet are always present)
3. Note in report which tools were unavailable
4. Use grep-based fallback for gopls queries (warn about false positives)

Error: "Tests Fail on Current Code"

1. Run tests on the base branch to confirm pre-existing failures
2. Distinguish between pre-existing failures and newly introduced ones
3. Report new failures as findings; note pre-existing as context

Error: "Cannot Determine Change Scope"

1. Read all changed files to infer intent
2. Check commit messages for context
3. Ask the user to clarify the change scope before proceeding

Anti-Patterns

Anti-Pattern 1: Fixing Code During Review

Anti-Pattern 2: Skipping Automated Checks

Anti-Pattern 3: Severity Downgrading to Avoid Conflict

Anti-Pattern 4: Approving Small PRs Without Full Review

Anti-Pattern 5: Reviewing Only Changed Lines

References

• Anti-Rationalization: Review - Review-specific rationalization prevention
• Anti-Rationalization: Core - Universal shortcut prevention
• Verification Checklist - Pre-completion checks
• Severity Classification - Issue severity standards

Domain-Specific Anti-Rationalization

Reference Files

• ${CLAUDE_SKILL_DIR}/references/common-review-comments.md

  : Go code patterns with good/bad examples for error handling, concurrency, and testing