• Domain-Scoped References: Each agent loads ONLY its domain-specific reference file (see Reference Loading Strategy). The essential rules are inline in each agent's dispatch prompt.
• Domain Specialist Model: Agents are assigned by rule domain (testing, errors, types, etc.), NOT by package. Each agent scans ALL packages for their domain's violations.
• Code-Level Findings Only: Every finding must include actual code from the repo and a concrete CORRECT version. Abstract suggestions are forbidden.
• Cite Rule Source: Every finding must cite the section number from sapcc-code-patterns.md (e.g., "§30.1: httptest.Handler migration").
• Directive Review Voice: Frame findings as the lead reviewer would state them in a PR comment. Use the project's established review phrases where applicable.
• Audit Only By Default: READ and REPORT. Do NOT modify code unless

  --fix

  is specified.
• Skip Generic Go: Do NOT report generic Go best practices. Only report patterns that are specifically sapcc/project-convention divergences.

• gopls MCP Integration: All review agents MUST use gopls MCP tools when available —

  go_workspace

  at start,

  go_file_context

  for dependency analysis,

  go_symbol_references

  for cross-package impact tracing,

  go_diagnostics

  for build verification. This gives type-aware analysis instead of text-only grep.
• Save Report: Write findings to

  sapcc-review-report.md

  in repo root
• Quick Wins Section: Identify the 5 easiest fixes with highest impact
• Cross-Repository Reinforcement: Weight findings higher when the violated rule appears in 4+ repos per §35

• --fix: Create worktree, apply fixes, run tests, create branch
• --focus [package]: Audit only one package (runs 3 agents instead of 10)
• --severity [critical|high|medium|all]: Only report findings at or above severity

skills/go-sapcc-conventions/references/

~/.claude/skills/go-sapcc-conventions/references/

review-standards-lead.md

architecture-patterns.md

api-design-detailed.md

error-handling-detailed.md

testing-patterns-detailed.md

architecture-patterns.md

anti-patterns.md

• sapcc-code-patterns.md

  — Comprehensive 36-section reference. Only load specific sections, not the entire file.

• pr-mining-insights.md

  — Review severity calibration

• library-reference.md

  — Approved/forbidden dependency table

• Phase 1: Discover repo structure
• Phase 2: Dispatch 10 specialist agents
• Phase 3: Aggregate findings
• Phase 4: Write report

• Module: [module path]
• Packages: [N]
• Go files: [M] (excluding vendor)
• Test files: [T]
• Key imports: [list]

1. The path to sapcc-code-patterns.md to read
2. Their assigned sections to focus on
3. Their domain-specific reference file(s) to read
4. Instructions to scan ALL .go files in the repo
5. The exact output format for findings

REFERENCE FILES TO READ FIRST (mandatory):
1. Read ~/.claude/skills/go-sapcc-conventions/references/sapcc-code-patterns.md
   (Focus on sections listed below, but skim all for context)
2. Read [domain-specific reference file]

REPO TO REVIEW: [current working directory]

SCAN METHOD:
- Use Glob to find all .go files: **/*.go (excluding vendor/)
- Use Read to examine each file
- Use Grep to search for specific patterns across all files

OUTPUT FORMAT for each finding:

path/to/file.go:LINE

[actual code, 3-10 lines]

[fixed code]

Why: [One sentence]

---

review-standards-lead.md

• Constructor taking option struct or functional options instead of positional params
• Functions with >8 params (should they be split?)
• context.Context in wrong position (should be first only for external calls)
• Config loaded from files/viper instead of env vars via osext.MustGetenv
• Constructors that return errors (should be infallible)
• Missing Override methods for test doubles
• Missing time.Now / ID-generator injection in constructors

architecture-patterns.md

• Interfaces with only one implementation (should be concrete type)
• Interfaces defined in implementation package instead of consumer package

• *T

  used for optional fields instead of

  Option[T]

  from majewsky/gg/option
• Missing dot-import for option package (

  import . "github.com/majewsky/gg/option"

  )

• iota

  used for enums instead of typed string constants
• Named types for domain concepts missing (raw

  string

  where

  AccountName

  type should exist)
• Pointer receivers where value receiver is appropriate (or vice versa)
• Type exported when only constructor needs to be public
• Contract cohesion (§36): Constants, error sentinels, or validation functions in a different file from the interface/type they belong to. If

  ErrFoo

  is returned by

  FooDriver

  methods, both must live in

  foo_driver.go

  . MEDIUM for new violations, LOW for pre-existing.
• Interface consumer audit: When a sentinel value or special parameter is introduced on an interface method, grep for ALL implementations AND all callers of that interface method across the entire repo. Use gopls

  go_symbol_references

  when available. Verify every caller validates the sentinel before passing it. Do not rely on the PR description's claim about authorization — verify the call chain independently.

api-design-detailed.md

• Auth done as middleware instead of inline at top of handler
• JSON responses not using respondwith.JSON
• Error responses using JSON instead of text/plain (http.Error for 4xx, respondwith.ErrorText for 500)
• Missing DisallowUnknownFields on json.NewDecoder
• Route registration not using httpapi.Compose pattern
• Handler not a method on *API struct
• Handler signature not

  handleVerbResource(w, r)

• Request structs not parsed into purpose-specific types
• API docs in Markdown instead of Go types on pkg.go.dev

error-handling-detailed.md

• Error messages not following "cannot <verb>: %w" format
• "failed to" instead of "cannot" in error messages
• Logging AND returning the same error (must choose one)
• Missing error wrapping (bare

  return err

  without context)
• Generic error messages ("internal error", "something went wrong")
• logg.Fatal used outside cmd/ packages
• Swallowed errors (err checked but not returned or logged)
• fmt.Errorf with %s when %w is needed (or vice versa)
• Validation functions returning (bool, string) instead of error

• SQL queries not declared as package-level

  var

  with

  sqlext.SimplifyWhitespace()

• Using

  ?

  placeholders instead of

  $1, $2

  (PostgreSQL style)
• Missing

  defer sqlext.RollbackUnlessCommitted(tx)

  after

  db.Begin()

• TIMESTAMP used instead of TIMESTAMPTZ
• NULL columns that should be NOT NULL
• Migrations that modify existing migrations (immutable rule)
• Missing down migrations
• App-level validation that should be DB constraints
• Using explicit transaction for single statements
• Not using

  sqlext.ForeachRow

  for row iteration

testing-patterns-detailed.md

• Using deprecated

  assert.HTTPRequest

  instead of

  httptest.Handler.RespondTo()

• Using

  assert.DeepEqual

  where generic

  assert.Equal

  works
• Table-driven tests (project convention prefers sequential scenario-driven narrative)
• Missing

  t.Helper()

  in test helper functions
• Using reflect.DeepEqual instead of assert.DeepEqual
• Test fixtures as large JSON files instead of programmatic builders
• Duplicated test setup across test functions (should extract)
• Using

  require

  package instead of

  must

  from go-bits
• Not using

  must.SucceedT

  /

  must.ReturnT

  for error-checked returns
• Not using

  assert.ErrEqual

  for flexible error matching
• Assertion depth check: For security-sensitive code (auth, filtering, tenant isolation), presence-only assertions (

  NotEmpty

  ,

  NotNil

  ,

  assert.True(t, ok)

  ) are INSUFFICIENT. Tests must verify the actual VALUE matches the expected input (e.g.,

  assert.Equal(t, expectedID, filters[0]["term"]["tenant_ids"])

  )

architecture-patterns.md

• Import groups not in stdlib / external / internal order (3 groups)
• Dot-import used for anything other than

  majewsky/gg/option

• Missing SPDX license header
• Comments using

  /* */

  instead of

  //

  for doc comments
• Missing 80-slash separator comments (

  ////////////////...

  ) between type groups

• //NOTE:

  markers missing for non-obvious logic
• Exported symbols without godoc comments
• cmd/ packages using wrong CLI patterns (if CLI repo)
• Package names not reading as English ("package utils" instead of meaningful name)
• Contract cohesion (§36): Files named generically (

  interface.go

  ,

  types.go

  ,

  constants.go

  ) when they should be named for the domain concept (

  storage_driver.go

  ,

  rbac_policy.go

  ). Constants/sentinels in

  util.go

  that belong to a specific interface's file. The test: if you can name the owning interface, the artifact must live in that interface's file.

• Using

  sort.Slice

  instead of

  slices.SortFunc

  (Go 1.21+)
• Using manual

  keys := make([]K, 0, len(m)); for k := range m { ... }

  instead of

  slices.Sorted(maps.Keys(m))

• Using

  strings.HasPrefix + strings.TrimPrefix

  instead of

  strings.CutPrefix

  (Go 1.20+)
• Using manual

  if a < b { return a }

  instead of

  min(a, b)

  (Go 1.21+)
• Loop variable capture workaround (

  v := v

  ) in Go 1.22+ code
• Goroutines without proper context cancellation
• Missing SIGINT context handling in main()

• os.Exit

  used instead of proper shutdown sequence

• sync.Mutex

  on struct value instead of per-resource
• Missing

  for range N

  syntax where applicable (Go 1.22+)

• Prometheus metrics missing application prefix (e.g.,

  keppel_

  or

  logrouter_

  )
• Counter metrics not initialized to zero
• Counter metric names not plural
• Gauge used where Counter is appropriate (or vice versa)
• Background jobs not using

  jobloop.ProducerConsumerJob

  pattern
• HTTP client creating new

  http.Client

  per request instead of using

  http.DefaultClient

• Custom HTTP transport instead of

  http.DefaultTransport

• Missing jitter in polling/retry loops

• fmt.Sprintf

  for simple string concatenation (use

  +

  )

• +

  for complex multi-part string building (use

  fmt.Sprintf

  )

anti-patterns.md

• Functional options pattern (project convention: positional params)
• Table-driven tests (project convention: sequential scenario narrative)
• Interface segregation / many small interfaces (project convention: 1-2 interfaces max per domain)
• Middleware-based auth (project convention: inline at handler top)
• Config validation layer (project convention: no separate validation)

• *T

  for optional fields (project convention:

  Option[T]

  )
• Config files / viper (project convention: pure env vars)
• Error messages starting with capital letter
• Error messages using "failed to" (project convention: "cannot")
• Helper functions extracted for cyclomatic complexity (project convention: "contrived edit to satisfy silly metrics")
• Exported types when only constructor is public
• Plugin creating its own DB connection (project convention: receive dependencies)

• errors.New

  +

  fmt.Sprintf

  instead of

  fmt.Errorf

• Manual row scanning instead of

  sqlext.ForeachRow

• Test setup in

  TestMain

  instead of per-test
• Verbose error checking instead of

  assert.ErrEqual

  /

  must.SucceedT

• Extraction without guard transfer: When inline code is extracted into a named helper, ALL defensive checks that relied on "the caller handles it" must be re-evaluated. A missing guard rated LOW as inline code becomes MEDIUM as a reusable function. Flag extracted helpers that lack self-contained validation.

git status --short

git diff --stat

• Single-line changes (regex replace, import reorder)
• No behavioral change (pure style/naming)
• Low risk of breaking tests

sapcc-review-report.md

undefined

EnterWorktree

sapcc-review-fixes

go build ./...    # Must still compile
go vet ./...      # Must pass vet
make check 2>/dev/null || go test ./...  # Must pass tests

git add -A
git commit -m "fix: apply sapcc-review findings (N fixes across M files)"

sapcc-review-report.md

• Which findings were fixed
• Which findings were skipped (and why)
• Test results after fixes

1. Complete coverage: Every section in sapcc-code-patterns.md has a specialist agent checking for it
2. Low false positives: Agents skip generic Go advice and only report sapcc-specific divergences
3. Actionable findings: Every finding has REJECTED code and CORRECT code
4. Prioritized output: Cross-repo reinforcement weights findings by importance
5. Reproducible: Same repo + same reference = same findings
6. Quick Wins first: Operator can fix 10 easy things immediately for rapid improvement

• Router:

  /do

  routes via "sapcc review", "sapcc lead review", "comprehensive sapcc audit"
• Complements:

  /sapcc-audit

  (package-level generalist) — use both for maximum coverage
• Prerequisite: go-sapcc-conventions skill must be installed at

  ~/.claude/skills/go-sapcc-conventions/

• Sync: After creating, run

  cp -r skills/sapcc-review ~/.claude/skills/sapcc-review

  for global access