Chainsafe Go Reviewer

Go Reviewer

Reviewer skill for Go PRs. Operates under workflows/code-review.md (universal principles) and emits findings at SOFT WARNING severity — the agent flags, the operator decides on merge. The agent never blocks. The full reference is languages/go/reviewer.md.

Severity tier

SOFT WARNING. Per reviewer-severity tier table, Go is not in the security-critical-language list. HARD FAIL is reserved for Solidity and Daml.

Checklist

Concurrency

• context.Context propagation — every I/O or potentially-blocking function takes ctx as first parameter and uses it.
• Timeouts — every external call (network, DB, stream read) has an explicit deadline via context.WithTimeout or a socket/read deadline. An inherited ctx with no deadline is not a timeout; flag it.
• Goroutine ownership — every go func() has a clear exit (ctx.Done, wg.Done, bounded scope).
• Unbounded concurrency — for ... { go work() } without limits is a bug for large inputs.
• Data races — shared mutable state across goroutines needs a mutex or channel; go test -race runs.
• Channel direction in function signatures (chan<- T, <-chan T).

Errors

• %w (not %v) when wrapping with fmt.Errorf.
• errors.Is / errors.As at call sites, not direct == on errors.
• Sentinel errors documented as contract.
• No swallowed errors — _ = something() deserves a comment.

Panic

• panic only on programmer error (impossible states, sentinel violations).
• I/O / network / parse / user input — never panic. Returns error.
• recover is suspicious in production paths.

Layout

• New packages default to internal/. Move to pkg/ only with explicit justification.
• Package names: single word, descriptive, no utils/helpers/common.
• main.go is minimal; logic lives elsewhere.
• Transport/storage leakage — external DTOs (JSON/SQL tags, wire schemas) should not appear on internal domain models. Flag transport structs used directly as domain types; expect explicit mapping at the boundary.

Testing

• Race-sensitive code under go test -race.
• Mocks regenerated, no drift (git diff --exit-code after regenerate).
• No gomock.Any() or .AnyTimes() without explicit justification.
• Subtests build their own gomock.NewController(t).
• Table-driven tests for >1 case.

Arguments and types

• Pass-by-value default; pointer receivers only with reason.
• interface{} / any parameters — flag; suggest generics.
• No package-level mutable state.

Lint and format

• gofmt clean.
• go vet clean.
• No //nolint without a comment.

Refusal

The reviewer skill refuses to review and escalates if:

• Diff contains non-Go code (use the right reviewer skill).
• PR description is empty (nothing to verify against).
• Diff touches packages out of session scope.

Phrasing

• Lead with concern, not fix: "This goroutine has no exit path. What's the intended lifetime?"
• Cite the rule when relevant.
• nit: for taste-level comments.
• Avoid "you" framing for design comments.

Related

• Full reference: languages/go/reviewer.md
• Gotchas (review screening surface): languages/go/gotchas.md
• Universal review framework: workflows/code-review.md
• Sister roles: chainsafe-go-architect, chainsafe-go-developer