Go Code Review

Reviews Go code for idiomatic patterns, error handling, concurrency safety, and common mistakes. Use when reviewing .go files, checking error handling, gorou...

Security Scan

The skill's requirements and instructions are coherent with a Go code-review helper, but it references an external verification protocol file that isn't bundled and you should confirm what repository access the agent will have before using it.

Purpose & Capability

Name/description (Go code review) matches what the SKILL.md and reference docs ask the agent to do: read go.mod and .go files, check error handling, concurrency, interfaces, resources, and naming. No unrelated binaries, credentials, or installs are requested.

Instruction Scope

Instructions correctly limit actions to repository files (go.mod and changed .go files plus surrounding context). However the workflow requires following ../review-verification-protocol/SKILL.md (a relative path outside the skill bundle) which is not included in this package — that creates ambiguity about additional checks or external steps the agent will perform. Also the instructions require the agent to read full enclosing functions/contexts in the repo, which is expected for a review but means the agent will access repository source broadly.

Install Mechanism

Instruction-only skill with no install spec, no downloads, and no code files executed on install. This is the lowest-risk install model.

Credentials

No environment variables, credentials, or config paths are requested. The scope of secrets/credentials is minimal and proportionate for a code-review tool.

Persistence & Privilege

always is false and the skill does not request special persistent privileges. Autonomous invocation (default) is allowed but not excessive by itself. The skill does not modify other skills or system config.

Assessment

This skill appears to be what it says: an instruction-only Go code reviewer that reads go.mod and changed .go files and produces findings. Before installing or running it, confirm two things: (1) the referenced review-verification-protocol (../review-verification-protocol/SKILL.md) actually exists in the repository or platform and review its contents — the skill points to that external file but does not bundle it; (2) decide what repository access you will grant the agent. The skill will read source files (and surrounding context) in your repo — if those files contain secrets, credentials, or proprietary code you should limit access or inspect the verification protocol first. No credentials or network exfiltration are requested by the skill itself, but repository access is inherently sensitive.

Review Workflow

Follow this sequence in order. Do not emit findings until every Pass below is satisfied.

1. Baseline go.mod — Open go.mod and read the go directive.
   Pass: You can state the exact go X.YY value (in the review preamble or working notes). Apply version-gated advice only when it matches this baseline (loop capture pre-1.22, slog/structured logging from 1.21, errors.Join from 1.20).

2. Read surrounding code — For each changed .go file, read full functions or logical units that contain the edits, not only the diff hunk.
   Pass: At least one full enclosing function (or package-level init/var block) containing the change was read per changed file.

3. Scope the checklist — Decide which Review Checklist blocks apply (error handling, concurrency, interfaces/types, resources, naming). Load references for those blocks; skip blocks that are irrelevant to the diff.
   Pass: The review (or working notes) lists which checklist blocks you applied, or marks blocks N/A with a one-line reason tied to the diff (e.g. “no concurrency in change”).

4. Pre-report verification — Load and follow review-verification-protocol.
   Pass: The protocol’s Pre-Report Verification Checklist is satisfied for each finding you will report (actual code read, surrounding context checked, “wrong” vs “different style” distinguished, etc.).

Hard gates (same sequence, shorter)

Step	Objective pass condition
1	go X.YY from go.mod is recorded before version-specific advice.
2	Full enclosing context read per changed file, not diff-only.
3	In-scope checklist blocks listed or N/A with diff-tied reason; references opened as needed.
4	review-verification-protocol completed for every reported issue.

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

Issue Type	Reference
Missing error checks, wrapping, errors.Join	references/error-handling.md
Race conditions, channel misuse, goroutine lifecycle	references/concurrency.md
Interface pollution, naming, generics	references/interfaces.md
Resource leaks, defer misuse, slog, naming	references/common-mistakes.md

Review Checklist

Error Handling

• All errors checked (no _ = err without justifying comment)
• Errors wrapped with context (fmt.Errorf("...: %w", err))
• errors.Is/errors.As used instead of string matching
• errors.Join used for aggregating multiple errors (Go 1.20+)
• Zero values returned alongside errors

Concurrency

• No goroutine leaks (context cancellation or shutdown signal exists)
• Channels closed by sender only, exactly once
• Shared state protected by mutex or sync types
• WaitGroups used to wait for goroutine completion
• Context propagated through call chain
• Loop variable capture handled (pre-Go 1.22 codebases only)

Interfaces and Types

• Interfaces defined by consumers, not producers
• Interface names follow -er convention
• Interfaces minimal (1-3 methods)
• Concrete types returned from constructors
• any preferred over interface{} (Go 1.18+)
• Generics used where appropriate instead of any or code generation

Resources and Lifecycle

• Resources closed with defer immediately after creation
• HTTP response bodies always closed
• No defer in loops without closure wrapping
• init() functions avoided in favor of explicit initialization

Naming and Style

• Exported names have doc comments
• No stuttering names (user.UserService → user.Service)
• No naked returns in functions > 5 lines
• Context passed as first parameter
• slog used over log for structured logging (Go 1.21+)

Severity Calibration

Critical (Block Merge)

• Unchecked errors on I/O, network, or database operations
• Goroutine leaks (no shutdown path)
• Race conditions on shared state (concurrent map access without sync)
• Unbounded resource accumulation (defer in loop, unclosed connections)

Major (Should Fix)

• Errors returned without context (bare return err)
• Missing WaitGroup for spawned goroutines
• panic for recoverable errors
• Context not propagated to downstream calls

Minor (Consider Fixing)

• interface{} instead of any in Go 1.18+ codebases
• Missing doc comments on exports
• Stuttering names
• Slice not preallocated when size is known

Informational (Note Only)

• Suggestions to add generics where code generation exists
• Refactoring ideas for interface design
• Performance optimizations without measured impact

When to Load References

• Reviewing error return patterns → error-handling.md
• Reviewing goroutines, channels, or sync types → concurrency.md
• Reviewing type definitions, interfaces, or generics → interfaces.md
• General review (resources, naming, init, performance) → common-mistakes.md

Valid Patterns (Do NOT Flag)

These are acceptable Go patterns — reporting them wastes developer time:

• _ = err with reason comment — Intentionally ignored errors with explanation
• Empty interface / any — For truly generic code or interop with untyped APIs
• Naked returns in short functions — Acceptable in functions < 5 lines with named returns
• Channel without close — When consumer stops via context cancellation, not channel close
• Mutex protecting struct fields — Even if accessed only via methods, this is correct encapsulation
• //nolint directives with reason — Acceptable when accompanied by explanation
• Defer in loop — When function scope cleanup is intentional (e.g., processing files in batches)
• Functional options pattern — type Option func(*T) with With* constructors is idiomatic
• sync.Pool for hot paths — Acceptable for reducing allocation pressure in performance-critical code
• context.Background() in main/tests — Valid root context for top-level calls
• select with default — Non-blocking channel operation, intentional pattern
• Short variable names in small scope — i, err, ctx, ok are idiomatic Go

Context-Sensitive Rules

Only flag these issues when the specific conditions apply:

Issue	Flag ONLY IF
Missing error check	Error return is actionable (can retry, log, or propagate)
Goroutine leak	No context cancellation path exists for the goroutine
Missing defer	Resource isn't explicitly closed before next acquisition or return
Interface pollution	Interface has > 1 method AND only one consumer exists
Loop variable capture	go.mod specifies Go < 1.22
Missing slog	go.mod specifies Go >= 1.21 AND code uses log package for structured output

Before Submitting Findings

Satisfy step 4 in Review Workflow: load review-verification-protocol and complete its pre-report checks for each issue.