Go Code Reviewer

Go Code Reviewer

Purpose

Use this skill to review Go code for real defects and risk, not just style. The review must be evidence-based, policy-aligned, and actionable.

Quick Reference

| When you need to… | Jump to | |---|---| | Select review depth (Lite / Standard / Strict) | §Execution Modes | | Read repo policy before reviewing | §Review Policy Sources | | Execute full review checklist | §Review Checklist | | Determine finding severity | §Finding Severity | | Decide what NOT to report | §Review Discipline | | Format findings and report | §Output Format | | See a complete formatted output example | Load references/example-output.md |

When To Use

Trigger this skill when the user asks for:

• Go code review / PR review / diff review
• Code quality or best-practice checks
• Risk or regression analysis
• "Is this code compliant with project rules?"

Review Policy Sources (in order)

1. constitution.md (highest repository policy for this skill)
2. AGENTS.md (repo workflow/testing/style constraints)
3. Local package conventions (tests, interfaces, dependency patterns)
4. Go language/runtime best practices

If constitution.md is missing, explicitly state that and continue with AGENTS.md + Go best practices.

Execution Modes (Lite / Standard / Strict)

Choose a mode before starting review and state it in the report.

• Default mode: Standard
• Declare the selected mode in a dedicated Review Mode section.

Mode selection rules:

• Choose Lite only when scope is small (typically 15 files).
• Use Standard for everything else.

Lite (fast triage)

• Review focus: confirmed defects with high confidence, avoid speculative architecture commentary.
• Minimum execution:
• Run at least one static tool (golangci-lint preferred, else staticcheck/go vet).
• Run go test for impacted package(s).
• Run go test -race only if concurrency risk is present; if skipped, state reason.
• Baseline/Suppression/SLA gates still apply.
• Finding volume: soft target ≤ 5 findings (severity-tiered; see Workflow step 10).

Standard (default balanced review)

• Full workflow in this skill applies as written.
• Expected execution when feasible:
• golangci-lint run (config-aware fallback strategy),
• impacted-package tests,
• go test -race when concurrency/shared state risk exists.
• Finding volume: soft target ≤ 10 findings (severity-tiered; see Workflow step 10).

Strict (release/security gate)

• Perform deep impact-radius expansion and compatibility checks.
• Minimum execution when feasible:
• golangci-lint run + direct staticcheck/go vet when not explicitly covered by config.
• go test ./...
• go test -race ./...
• For unresolved findings, provide explicit SLA and, if deferred, complete risk acceptance entry.
• Finding volume: soft target ≤ 15 findings (severity-tiered; see Workflow step 10).

Mandatory Review Gates

1) Execution Integrity Gate

Never claim verification was executed unless it actually ran.

• If go test or go test -race is not run, you must output:
• Not run in this environment
• reason
• exact commands to run
• Do not imply pass/fail for commands you did not execute.

2) Baseline Comparison Gate

When prior review context exists (previous PR review comments, prior findings, or known issue list), classify each finding as:

• new
• regressed
• unchanged
• resolved

If no baseline is available, state: Baseline not found.

3) False-Positive Suppression Gate

Before reporting a finding, check whether the risk is already blocked by:

• upstream guard/middleware/policy
• non-user-controlled input path
• framework/runtime safe guarantees

If blocked, do not report as a finding. Put it in a short Suppressed items section with rationale.

4) Risk Acceptance and SLA Gate

For unresolved findings, include:

• recommended SLA by severity
• optional risk acceptance entry when immediate fix is not chosen

Default SLA guidance:

• High: fix or strong mitigation in 50 lines, nesting >4 levels
• Naked returns in long functions
• Mutable package-level variables
• Interface pollution (unused abstractions)
• Early return pattern (no else after error return)
• Nil interface trap (typed nil returned as interface)
• Inconsistent pointer vs value receivers
• Shadowed error variables
• Pointer slice []*T elements not nil-guarded before field access or method call

Performance (Medium) → references/go-performance-patterns.md

• String concatenation in loops (use strings.Builder)
• Slice/map without pre-allocation when size known
• sync.Pool misuse or missed opportunity for hot-path allocations
• Unnecessary allocations in hot paths
• Struct field alignment (use fieldalignment tool)
• Substring memory retention (use strings.Clone)
• Lock scope too wide; mutex where atomic suffices
• Missing buffered I/O (bufio) for frequent small writes/reads
• http.DefaultClient without timeout or transport tuning
• regexp.Compile in hot path instead of package-level compile-once

Modern Go & Best Practices (Medium) → references/go-modern-practices.md

• Generics vs interface choice appropriateness
• any overuse where type constraint is possible
• slog for structured logging (Go 1.21+)
• Typed atomic operations (atomic.Int64 etc., Go 1.19+)
• context.WithCancelCause / context.WithoutCancel (Go 1.20+/1.21+)
• Goroutine lifecycle: recover panics, leak prevention, concurrency control
• Channel buffer selection semantics
• Error message format (lowercase, no punctuation)
• context.Background() vs context.TODO() usage
• Package naming (short, lowercase, no underscore)
• Godoc on exported symbols

Dependency & Module (Low)

• go.sum synchronized
• Stale replace directives
• Unnecessary dependencies
• Deprecated standard library functions

1. Apply False-Positive Suppression Gate.

1. Apply Baseline Comparison Gate.

1. Apply Change Origin Classification Gate.

• For each finding, determine whether the defective code was introduced in this PR/diff or existed before.
• Use diff hunks as the primary signal: finding location inside a changed hunk → introduced; outside → pre-existing.
• When in doubt, use git blame or file history to confirm.
• Attach Origin and Action to each finding.

1. Consolidate, prioritize, and report findings.

• Merge rule: When the same conceptual issue (e.g., "missing error wrapping") appears at ≥ 3 locations, report ONE finding with a location list, not N separate findings. Merged findings share the same Origin only if all locations have the same origin; otherwise list origins per location.
• Volume cap — severity-tiered strategy:
• Soft targets by mode: Lite ≤ 5, Standard ≤ 10, Strict ≤ 15.
• Phase 1 — High: Report ALL High-severity findings regardless of the soft target. High findings are never dropped by volume cap.
• Phase 2 — Medium: Fill remaining slots (soft target minus High count) with Medium-severity findings, prioritizing introduced over pre-existing.
• Phase 3 — Low: If slots remain, include Low-severity findings.
• Overflow: If total candidates exceed the soft target after Phase 1, move the lowest-severity candidates that did not make the cut to Residual Risk / Testing Gaps with a one-line summary each, and note N additional lower-priority issues moved to Residual Risk in Summary.
• Example: Standard mode, 4 High + 8 Medium found → report 4 High + 6 Medium as findings, move 2 Medium to Residual Risk.
• Sort order: introduced before pre-existing within the same severity level. High → Medium → Low.
• Every finding must include policy mapping, file references, Origin, and Action.
• Apply Go Version Gate: remove or downgrade findings that recommend features above the project's Go version.

1. Add risk acceptance/SLA recommendations.

Severity Rubric

• High: functional breakage, data loss/corruption, security risk, race/deadlock, production instability
• Medium: maintainability or reliability risk without immediate failure
• Low: clarity, readability, or non-critical consistency gap

Evidence Rules

• Do not report speculative findings as confirmed defects.
• Every finding must include:
• exact location (path:line)
• concrete impact
• why current behavior is risky/incorrect
• actionable fix direction
• Clearly label inference vs directly observed behavior.

Anti-examples (DO NOT report these)

See references/go-review-anti-examples.md (always loaded — see Appendix). Before suppressing a finding using an anti-example, you MUST quote specific code evidence satisfying the anti-example's stated precondition. Category match alone is not sufficient — if you cannot cite evidence, the finding must be reported.

Output Format (Required)

Review Mode

• Lite|Standard|Strict
• mode selection rationale (1-2 lines)

Findings

List findings first, ordered by severity.

[High|Medium|Low] Short Title

• ID: REV-001
• Origin: introduced|pre-existing|uncertain
• Baseline: new|regressed|unchanged (or N/A if baseline missing)
• Principle: constitution.md clause (or N/A with reason)
• Location: path:line (or location list for merged findings)
• Impact: user/business/runtime impact
• Evidence: concrete observed behavior
• Recommendation: specific and minimal fix direction
• Action: must-fix | follow-up issue (aligned with Origin actionability table)

Suppressed Items

Only include items filtered by the suppression gate.

[Suppressed] Short Title

• Reason: upstream guard / non-user-controlled input / framework safety
• Location: path:line
• Residual risk: short note

Execution Status

• Go version: X.Y (from go.mod) or unknown
• Excluded (generated): list of generated files excluded, or None
• References loaded: list of reference files loaded for this review
• go vet: PASS|FAIL|Not available|Covered by golangci-lint config
• staticcheck: PASS|FAIL|Not available|Covered by golangci-lint config
• golangci-lint: PASS|FAIL|Not available
• go test: PASS|FAIL|Not run in this environment
• go test -race: PASS|FAIL|Not run in this environment
• If not run, include reason and exact commands.

Risk Acceptance / SLA

• SLA recommendation by severity in this review
• Optional risk acceptance entries for deferred fixes:
• finding ID
• owner
• compensating control
• expiry/review date

Open Questions

Only include blockers that materially affect confidence.

Residual Risk / Testing Gaps

This section captures items that are valuable context but do not belong in Findings:

1. Verification gaps: tools or tests that were not run, and the reason.
2. Volume-cap overflow: findings that were evaluated and confirmed but displaced by the severity-tiered volume cap (see Workflow step 10). List each with a one-line summary (severity | origin | location | short description) so no validated issue is silently dropped.
3. Pre-existing issues (non-High): Low/Medium-severity pre-existing defects found in impact-radius files (not in the diff) go here with a one-line summary each. High-severity pre-existing issues may be promoted to Findings with Origin: pre-existing.
4. Areas not covered: parts of the change whose risk could not be assessed (e.g., missing test fixtures, untraceable dynamic dispatch).

Summary

1-3 lines only, after findings. Include origin breakdown: X introduced / Y pre-existing / Z uncertain. If findings were capped by volume limit, note: N additional lower-priority issues moved to Residual Risk.

Example Output Reference

When you need to verify report formatting or check what a complete review looks like: → Load references/example-output.md for a full example covering Review Mode, Findings with all required fields (ID, Origin, Baseline, Principle, Location, Impact, Evidence, Recommendation, Action), and Summary.

No-Finding Case

If no issues are found:

• Explicitly say: No actionable findings found.
• Still provide:
• Review Mode
• Execution Status
• baseline status (resolved list if available, otherwise Baseline not found)
• residual risks/testing gaps

Review Discipline

• Prefer small, precise, defensible findings over generic advice.
• Findings are the primary deliverable.
• Recommend tests whenever behavior changes are proposed.
• Execute ALL checklist categories regardless of how many High findings have already been identified; the presence of multiple High findings does not reduce the obligation to complete remaining categories; finish the full checklist scan before consolidating findings.

Skill Maintenance

Run regression checks for this skill with:

bash "/scripts/run_regression.sh"

Appendix: Reference Loading Triggers

This is a mandatory gate (see §7 Reference Loading Gate). Use the table below as a compact trigger map; detailed patterns live in the reference files themselves.

| Reference | Representative triggers | |---|---| | references/go-review-anti-examples.md | Any review (always loaded) | | references/pr-review-quick-checklist.md | Any PR or diff review | | references/go-security-patterns.md | auth/token flows, hardcoded string literals, SQL/command/path use, outbound fetches, TLS, HTML/template output, upload/body limits | | references/go-concurrency-patterns.md | go func, channels, mutexes, wait groups, errgroup, lifecycle or cancellation code | | references/go-error-and-quality.md | ignored errors, panic(, errors.Is/As, naked returns, receiver or shadowing issues | | references/go-test-quality.md | _test.go, httptest, testing.B/F, testdata/ | | references/go-api-http-checklist.md | net/http, handlers, servers, gin/echo/chi, gRPC, pb. or proto. | | references/go-database-patterns.md | database/sql, pgx, sqlx, gorm, ent, query/tx/rows code | | references/go-performance-patterns.md | hot loops, builders, preallocation, sync.Pool, atomics, regex, client or JSON hot paths | | references/go-modern-practices.md | generics, any, slog, typed atomics, modern context helpers, slices/maps |

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: johnqtcg
• Source: johnqtcg/awesome-skills
• License: MIT
• Homepage: https://johnqtcg.github.io/awesome-skills/

Install and usage instructions live in the source repository linked above.