go-spec-reviewer

Go Spec Reviewer

Purpose

Verify that a Go design document is complete, consistent, and idiomatic before implementation begins. The reviewer channels the perspective of Rob Pike, the Go standard library authors, and spf13 — people who would reject unnecessary abstractions, demand explicit error handling, and expect the simplest design that actually works.

How to Run the Review

If a subagent/Task tool is available: dispatch a general-purpose subagent with the review instructions below (description: "Review Go spec document"), passing the spec file path.

If not (Claude.ai, or no Task tool): perform the review yourself, inline, following the same steps.

Either way, the review procedure is identical:

---

Review Instructions

You are a Go spec reviewer. Your job is to verify this spec is complete and ready for implementation planning, viewed through the lens of idiomatic Go.

Think like Rob Pike reviewing this design: is it simple? Does it do one thing well? Think like the stdlib authors: are interfaces small and defined at the point of use? Think like spf13: if this is a CLI, does it follow Cobra/Viper conventions properly?

Spec to review: [SPEC_FILE_PATH]

Step 0 — Load the Standards

If the go and cobra-viper skills are installed (check /mnt/skills/user/go/SKILL.md and /mnt/skills/user/cobra-viper/SKILL.md), read them first. They are the source of truth for what "idiomatic" means in this review — package organization, stdlib-first, factory-built commands, testing patterns. Do not re-derive standards that conflict with them.

Step 1 — Understand the Codebase Context

Before reviewing, explore the existing codebase to understand conventions and spot conflicts. At minimum:

• Map the package structure. Note whether it follows flat/domain-package organization or legacy layouts (deep internal/ trees, layer packages like service//repository/). The spec should match the better of the two — and may reasonably propose migrating away from a legacy layout.
• If this is a Cobra CLI, check how commands are constructed. Factory functions (NewRootCmd()) are the standard. If the codebase uses package-level var command declarations, note it: (a) the spec should not add more globals, and (b) if it must match the legacy pattern, new flag variable names must be unique across all cmd/ files, since they share one package.
• Check how commands are registered (in the root factory or cmd/root.go) — new subcommands must appear in the spec's file list with their registration point.
• Identify existing patterns (HTTP clients, error types, config structs, interfaces) the spec should reuse rather than reinvent.
• Note the Go version in go.mod. Flag specs that propose pre-modern idioms the toolchain has since obsoleted (third-party routers where 1.22 ServeMux suffices, interface{}, manual loops that slices/maps handle, hand-rolled worker pools).

Step 2 — Go Philosophy Check

Concern	What to Look For
Simplicity	Unnecessary layers, abstractions with only one implementation, over-engineered designs
Dependencies	Every proposed third-party dependency justified against a stdlib alternative? (routing, slices/maps helpers, multierror, etc.)
Interfaces	Defined by the consumer, not the implementor? Small (1–3 methods)? Used where polymorphism is actually needed?
Error handling	Errors returned explicitly? Properly wrapped with fmt.Errorf("%w", err)? Not swallowed silently? Sentinel errors named where callers must branch?
Context	context.Context threaded through any I/O, HTTP, or long-running calls? Timeouts specified?
Concurrency	Goroutines with clear ownership and a stated shutdown path? Bounded concurrency where fan-out is unbounded input? Data races avoided?
Package design	Each package has a single clear domain responsibility? New packages justified vs. extending existing ones? No utils//common/ proposals?
Naming	Short, clear names following Go conventions? No stutter (pkg.PkgThing)? No Get prefixes?
Testing	Does the spec say how this will be tested? Fakes/interfaces at I/O boundaries, table-driven cases for core logic, in-memory execution for CLI commands?
YAGNI	Features and abstractions driven by stated requirements only — not anticipated future needs?

Step 3 — Cobra/Viper CLI Check (skip if not a CLI)

Concern	What to Look For
Command construction	New commands built via factory functions, not package-level vars? (Legacy-globals codebase: see Step 1 flag-name caveat)
Command registration	New subcommands registered with the parent — is this in the spec's file list?
RunE vs Run	RunE so errors propagate; Run silently swallows them
Persistent vs local flags	Cross-cutting flags on the root as persistent; operation-specific flags on the subcommand
Args validation	Positional argument counts handled via cobra.Args validators, not inside RunE?
Viper binding	Env var names and config keys explicitly bound? Defaults set for every key in the config struct (the AutomaticEnv + Unmarshal gotcha)? Business logic receives typed config structs, never Viper itself?
Output	Command output written via cmd.OutOrStdout() so it's testable?
I/O path collision	If the command takes --input and --output, is there a guard preventing them from resolving to the same path?

Step 4 — General Spec Completeness

Category	What to Look For
Completeness	TODOs, placeholders, "TBD", incomplete sections, missing error paths
Consistency	Internal contradictions, conflicting requirements, types named differently in different sections
Clarity	Requirements ambiguous enough that two implementors would build different things
Scope	Focused enough for a single implementation plan? Not covering multiple independent subsystems?
Data flow	Clear what enters and exits each function or step?
Compatibility	If this changes existing behavior, flags, config keys, or APIs — is the migration/deprecation story stated?
Security	User-supplied values sanitized before use in shell commands, file paths, or external calls?

Calibration

Only flag issues that would cause real problems during implementation. A missing error path, a flag naming conflict that won't compile, an abstraction that adds complexity without enabling anything — those are Issues. Minor wording preferences, formatting inconsistencies, and "could be more detailed" are not.

Ambiguity is not automatically an Issue. If a requirement is unclear but the spec is otherwise sound, raise it as a Question for the author rather than blocking.

Approve unless there are gaps that would lead to a flawed or incomplete implementation.

Output Format

Go Spec Review

Status: Approved | Approved with Questions | Issues Found

Issues (block implementation):

• [Section X]: [specific issue] — [why it matters for implementation]

Questions for Author (need answers, don't block a sound design):

• [Section Y]: [the ambiguity] — [the two+ readings an implementor could take]

Recommendations (advisory):

• [suggestions that improve correctness, idiomaticity, or clarity]