From 615639076a154bee692e1d2d98e6b00f71769e47 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 18:15:51 +0300 Subject: [PATCH 01/15] feat: enhance Go skills and agents with new error handling and testing guidelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added new review dimensions to `go-reviewer`: silent dispatch defaults, sensitive-value echo in errors/logs, and comment–code drift. - Introduced fail-loudly-on-impossible-dispatch and boundary-errors-carry-classification-not-payload rules in `go-errors`. - Updated `go-testing` to clarify that golden files pin shape, not behavior, and emphasized the need for live-execution tests alongside them. These updates improve the robustness of Go code reviews and error handling practices. --- CHANGELOG.md | 7 +++++++ agents/go-reviewer.md | 17 ++++++++++++++++- skills/go-errors/SKILL.md | 10 +++++++++- skills/go-testing/SKILL.md | 3 +++ 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e55b4e2..f186270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Keep a Changelog: https://keepachangelog.com/en/1.1.0/ - Semantic Versioning: https://semver.org/spec/v2.0.0.html +## [Unreleased] + +### Added +- Agents: `go-reviewer` — three review dimensions: **silent dispatch defaults** (pass-through `default` over an internal enum; paired dispatch sites maintained as independent switches), **sensitive-value echo in errors/logs** at a boundary (driver messages quoting stored values, request bodies in wrapped errors), and **comment–code drift** in the diff. +- Skills: `go-errors` — fail-loudly-on-impossible-dispatch rule (loud `default` + `exhaustive` linter or enum-completeness test) and boundary-errors-carry-classification-not-payload rule (pass the class/code, keep the raw message internal). +- Skills: `go-testing` — golden files pin *shape*, not behaviour: pair a golden of an externally executed artefact (SQL, wire requests, rendered configs) with at least one live-execution test. + ## [0.3.0] - 2026-07-01 Retargets the standards baseline to **Go 1.26** (1.26.4+) while keeping version-gated guidance valid for 1.25 modules. diff --git a/agents/go-reviewer.md b/agents/go-reviewer.md index 587735c..0299d9d 100644 --- a/agents/go-reviewer.md +++ b/agents/go-reviewer.md @@ -3,7 +3,8 @@ name: go-reviewer description: > Use this agent to review Go (`.go`) diffs or files for the bugs and smells that linters miss — silent error swallowing, goroutine leaks, context misuse, resource leaks, sentinel-error breakage, - unsafe atomics, stale modernization debt, and slog hot-path waste. Invoke it after writing or + silent dispatch defaults, sensitive-value echo in errors/logs, comment–code drift, unsafe atomics, + stale modernization debt, and slog hot-path waste. Invoke it after writing or changing Go code, before opening a PR, or whenever the user asks for a Go code review. It is read-only, works alone, and returns severity-ranked findings; it does not edit code or dispatch other agents. Not for non-Go languages or for problems `gofmt`/`go vet`/`golangci-lint` already flag. @@ -90,6 +91,20 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v inside a loop accumulating handles. - **Sentinel / typed-error breakage** — `err == ErrX` or a type assertion where wrapping is in play (use `errors.Is`/`errors.As`); a documented sentinel removed, or its wrapping changed (an API break). +- **Silent dispatch defaults** — a `switch` over an internal enum/kind tag whose `default` arm + silently passes through, returns a zero value, or picks the weakest behaviour: a member added + later rides the wrong arm with no error. Expect a loud `default` (error, or panic only for the + genuinely unreachable) plus something pinning exhaustiveness (the `exhaustive` linter or a + completeness test iterating the enum). Same smell when two dispatch sites over one enum must agree + (encode/decode, compare/order) but are maintained as independent switches — look for a shared + discipline function or a test pinning the pairing. +- **Sensitive-value echo in errors/logs** — an error crossing a logging or API boundary carrying + payload data: database-driver messages quote the offending stored value, validation errors embed + the request body, wrapped errors accumulate user input. At the boundary the stable classification + (error code, SQLSTATE-class) should cross; the raw message stays internal. +- **Comment–code drift** — a comment, doc string, or doc file in the diff asserting what the final + code no longer does (stale counts, renamed symbols, behaviour claims the change invalidated). + Cheap to fix at review, expensive once trusted. - **Concurrency hazards** — bare-int `atomic.Add*` instead of typed `atomic.Int64`/`Bool` (and non-atomic reads of those fields); `sync.Mutex`/`WaitGroup` copied by value; a map written concurrently without a lock; check-then-act races. diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index 0b37b4a..28de4ae 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -1,6 +1,6 @@ --- name: go-errors -description: Idiomatic Go error handling. This skill should be used when the user writes, reviews, or debugs Go error code — wrapping with `%w`, inspecting via `errors.Is`/`errors.As`, sentinel vs typed errors, `errors.Join`, or chasing a silently-swallowed or context-losing error. Pair with the `errorlint` linter (set it up via `go-linting`). Not for panics-as-control-flow or non-Go languages. +description: Idiomatic Go error handling. This skill should be used when the user writes, reviews, or debugs Go error code — wrapping with `%w`, inspecting via `errors.Is`/`errors.As`, sentinel vs typed errors, `errors.Join`, enum-switch dispatch defaults, keeping payload values out of boundary errors/logs, or chasing a silently-swallowed or context-losing error. Pair with the `errorlint` linter (set it up via `go-linting`). Not for panics-as-control-flow or non-Go languages. --- # go-errors — Go error handling @@ -25,6 +25,14 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch — replaces manual concatenation and most third-party multierror use. - **Never swallow:** no `_ = f()` on an error you care about; no empty `if err != nil {}`. Handle, wrap-and-return, or (deliberately, with a comment) ignore. +- **Fail loudly on impossible dispatch:** a `switch` over an internal enum/kind gets a `default` + that returns an error (panic only for the genuinely unreachable) — never a silent pass-through + that lets a later-added member ride the weakest arm. Pin exhaustiveness with the `exhaustive` + linter or a completeness test that iterates the enum. +- **Boundary errors carry classification, not payload:** upstream messages can embed data values — + a database driver quoting the offending stored value, a validator echoing the request body. At a + logging or API boundary, pass the stable class/code (e.g. SQLSTATE) and keep the raw message + internal — the message-content analogue of severing an internal error *type* with `%v`. - **Add context at each layer, log once at the boundary.** Wrapping at every level *and* logging at every level produces duplicate noise — return wrapped, log at the top. - **Error strings:** lowercase, no trailing punctuation (they get wrapped): `"cannot parse %q"`. diff --git a/skills/go-testing/SKILL.md b/skills/go-testing/SKILL.md index a83458b..deb4327 100644 --- a/skills/go-testing/SKILL.md +++ b/skills/go-testing/SKILL.md @@ -26,6 +26,9 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, `GOEXPERIMENT=synctest` API — `synctest.Run` — was removed in Go 1.26; use the stable `synctest.Test`.) - **Fuzzing** (`func FuzzX(f *testing.F)`) for parsers, codecs, and anything consuming untrusted bytes. **Golden files** (an `-update` flag writing `testdata/*.golden`) for large structured output. + A golden pins *shape*, not behaviour — when it records something another system executes (SQL, + wire requests, rendered configs), pair it with at least one test that executes the artefact for + real; a snapshot can be stable and wrong. - **Deterministic crypto tests (Go 1.26):** `testing/cryptotest.SetGlobalRandom(t, seed)` pins a deterministic randomness source for the test's duration — reach for it instead of hand-injecting a custom `io.Reader` when testing code that draws from `crypto/rand`. It's process-global, so it From c890254793b6f43ef7036aba0edda00fd4e31f5f Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:22:11 +0300 Subject: [PATCH 02/15] feat(skills): ground go-errors, go-testing and go-concurrency in current stdlib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each of these skills predated stdlib APIs the Go 1.26 baseline already implies, so they taught the older form by omission. go-errors: prefer `errors.AsType[E]` over `errors.As` (generic target, no pointer to prepare, cannot panic on a mistyped target; `errorsastype` converts call sites); `%w` goes last unless the sentinel is the sentence; check `Close` on written files via `errors.Join` into a named result — a bare `defer f.Close()` hides a failed flush behind an apparently successful write; keep the happy path unindented; never let a panic cross a package boundary. go-testing: `t.Context`, `t.ArtifactDir` vs `t.TempDir`, `t.Output`/`t.Attr`, and the footgun that ties them together — `t.Setenv`, `t.Chdir` and `cryptotest.SetGlobalRandom` are process-global, so they fail under `t.Parallel` or a parallel ancestor. Plus failure messages that carry call/input/got/want, and helpers that set up while the test body asserts. go-concurrency: cancellation causes (`WithCancelCause` + `context.Cause`, `WithTimeoutCause`) so an expiring layer names itself instead of reporting a bare `context.DeadlineExceeded`; `context.WithoutCancel` for work outliving a request; `context.AfterFunc`; prefer-synchronous-APIs; and explicit cleanup over `runtime.AddCleanup`/`SetFinalizer`. Every rule cites go.dev, pkg.go.dev, Code Review Comments or the Google style guide; version annotations verified against the "added in go1.NN" markers. Co-Authored-By: Claude Opus 5 (1M context) --- skills/go-concurrency/SKILL.md | 31 ++++++++++++++++++++++++++---- skills/go-errors/SKILL.md | 25 +++++++++++++++++++++--- skills/go-testing/SKILL.md | 35 ++++++++++++++++++++++++++++++---- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/skills/go-concurrency/SKILL.md b/skills/go-concurrency/SKILL.md index dbc2c8b..11ff072 100644 --- a/skills/go-concurrency/SKILL.md +++ b/skills/go-concurrency/SKILL.md @@ -1,15 +1,18 @@ --- name: go-concurrency -description: Idiomatic, leak-free Go concurrency. This skill should be used when the user writes or reviews Go goroutines, channels, `sync`/`atomic`, `context`, `errgroup`, or worker pools — goroutine lifetimes/leaks, context propagation, typed atomics, mutex misuse, or data races. Pair with `go test -race`, `go vet`, and `goleak`. Defers time/concurrency *testing* mechanics to `go-testing` (synctest). Not for non-Go languages. +description: Idiomatic, leak-free Go concurrency. This skill should be used when the user writes or reviews Go goroutines, channels, `sync`/`atomic`, `context`, `errgroup`, or worker pools — goroutine lifetimes/leaks, context propagation, cancellation causes (`context.Cause`, `WithTimeoutCause`), work that must outlive a request (`context.WithoutCancel`, `AfterFunc`), typed atomics, mutex misuse, cleanup/finalizers, or data races. Pair with `go test -race`, `go vet`, and `goleak`. Defers time/concurrency *testing* mechanics to `go-testing` (synctest). Not for non-Go languages. --- # go-concurrency — Go concurrency Deterministic backstop: `go test -race ./...`, `go vet ./...` (catches copylocks, lost cancel), and `go.uber.org/goleak`. The race detector is the source of truth — run it before reasoning. -On **Go 1.26+** the runtime also ships an experimental `goroutineleak` profile in `runtime/pprof` +The runtime also ships an experimental `goroutineleak` profile in `runtime/pprof` (Go 1.26) that reports leaked goroutines — a toolchain-native complement to `goleak` for leak hunts (enable it -with `GOEXPERIMENT=goroutineleakprofile` at build time). +with `GOEXPERIMENT=goroutineleakprofile` at build time). The implementation is production-ready; the +experiment flag is only about API feedback, and it costs nothing unless in use. +*Go 1.27 (draft, expected Aug 2026) enables it by default — no `GOEXPERIMENT`, and +`/debug/pprof/goroutineleak` via `net/http/pprof`.* ## Rules @@ -27,17 +30,37 @@ with `GOEXPERIMENT=goroutineleakprofile` at build time). - **Context discipline:** pass `ctx context.Context` as the first parameter; **never store it in a struct** (`containedctx`); don't reach for `context.Background()` deep in a call stack — thread the caller's ctx. HTTP/SQL/RPC calls must carry it (`noctx`), and set a client timeout. +- **Make cancellation say *why*.** `ctx.Err()` only ever reports `context.Canceled` or + `DeadlineExceeded`, which is useless for diagnosis when several budgets nest. Use + `context.WithCancelCause` + `cancel(err)` (Go 1.20) and read `context.Cause(ctx)`, or + `WithTimeoutCause`/`WithDeadlineCause` (1.21) so the expiring layer names itself. `errors.Is(err, + context.Canceled)` keeps working — the cause rides alongside, it doesn't replace `Err()`. +- **Work that must outlive the request:** `context.WithoutCancel(ctx)` (Go 1.21) drops cancellation + but keeps the values (trace, auth, request ID) — reach for it instead of `context.Background()`, + which throws those away. Give the derived context its own timeout, and tie it to a shutdown path; + "outlives the request" must not mean "outlives the process silently". +- **`context.AfterFunc(ctx, f)`** (Go 1.21) instead of a goroutine whose only job is to `select` on + `ctx.Done()` and clean up; the returned `stop` unregisters it if the work finished first. - **Don't copy `sync.Mutex`/`sync.WaitGroup` by value** (`go vet` copylocks). Guard shared maps — a concurrent map write panics; `-race` catches it. - **Channels:** close on the *send* side, never the receive side; a `nil` channel blocks forever (useful for disabling a `select` arm, a bug everywhere else). +- **Prefer synchronous APIs.** Return the result; let the caller decide to run it in a goroutine. A + function that spawns internally and hands back a channel — or takes a completion callback — + imposes its concurrency model on every caller and hides the goroutine's lifetime, which is exactly + where leaks come from. +- **Cleanup is explicit, never finalized.** Release resources in `Close`/`defer`. `runtime.AddCleanup` + (Go 1.24, preferred over the older `runtime.SetFinalizer`: multiple cleanups per object, works on + interior pointers, no leak on reference cycles) is a backstop for OS/native handles only — a + cleanup may never run, so no correctness may depend on it. - **Testing time/concurrency:** use **`testing/synctest`** (stable since Go 1.25) — fake clock + deterministic scheduling. See `go-testing`. ## Sources - synctest — ; Go 1.25/1.26 release notes — - `goroutineleak` profile (Go 1.26, experimental) — -- Code Review Comments (Goroutine Lifetimes, Contexts) — +- `context` (`Cause`, `WithoutCancel`, `AfterFunc`, `WithTimeoutCause`) — ; `runtime.AddCleanup` — +- Code Review Comments (Goroutine Lifetimes, Contexts, Synchronous Functions) — - Uber Go Style Guide (Concurrency) — --- diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index 28de4ae..ab747e6 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -1,6 +1,6 @@ --- name: go-errors -description: Idiomatic Go error handling. This skill should be used when the user writes, reviews, or debugs Go error code — wrapping with `%w`, inspecting via `errors.Is`/`errors.As`, sentinel vs typed errors, `errors.Join`, enum-switch dispatch defaults, keeping payload values out of boundary errors/logs, or chasing a silently-swallowed or context-losing error. Pair with the `errorlint` linter (set it up via `go-linting`). Not for panics-as-control-flow or non-Go languages. +description: Idiomatic Go error handling. This skill should be used when the user writes, reviews, or debugs Go error code — wrapping with `%w`, inspecting via `errors.Is`/`errors.AsType`, sentinel vs typed errors, `errors.Join`, unchecked `Close` errors, when panic is legitimate, enum-switch dispatch defaults, keeping payload values out of boundary errors/logs, or chasing a silently-swallowed or context-losing error. Pair with the `errorlint` linter (set it up via `go-linting`). Not for non-Go languages. --- # go-errors — Go error handling @@ -13,11 +13,18 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch - **Wrap with `%w` when the caller may need to inspect the cause** (Go 1.13): `return fmt.Errorf("read config %s: %w", path, err)`. Use `%v` *only* to deliberately sever the chain (e.g. to avoid leaking an internal error type across an API boundary) — and say so. + Put `%w` **last** so the message reads outside-in; a leading `%w` is right only when the sentinel + *is* the sentence: `fmt.Errorf("%w: %s", ErrNotFound, key)`. - **The `%v`-where-`%w` trap:** formatting a cause with `%v` discards the chain, so downstream `errors.Is`/`errors.As` silently fail. `errorlint` flags it. - **Inspect with `errors.Is` (sentinel) / `errors.As` (typed)** — never `err == ErrX` or a type assertion once any layer wraps, or you get *sentinel breakage* (the comparison silently stops matching). (Go 1.13; `errors.As` target must be a pointer.) +- **Prefer `errors.AsType[E]` over `errors.As`** (Go 1.26): + `if perr, ok := errors.AsType[*fs.PathError](err); ok { … }`. It is the generic form — + compile-time-checked target, no pointer to prepare, no reflection, and it cannot panic on a + mistyped target the way `errors.As` can. `errors.As` is not deprecated, so existing call sites are + not bugs; the `errorsastype` modernizer converts them (`go fix ./...`). - **Sentinel errors** (`var ErrNotFound = errors.New("not found")`) for expected, comparable conditions that are part of your API contract — keep the set small and documented. **Typed errors** (a struct implementing `error`) when callers need fields (`*PathError`). @@ -25,6 +32,16 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch — replaces manual concatenation and most third-party multierror use. - **Never swallow:** no `_ = f()` on an error you care about; no empty `if err != nil {}`. Handle, wrap-and-return, or (deliberately, with a comment) ignore. +- **Check `Close` on anything written to.** `defer f.Close()` discards a failed flush — the write + looks successful and the file is truncated. Capture it into a named result: + `defer func() { err = errors.Join(err, f.Close()) }()`. `errcheck` flags the discarded form; + read-only handles are the one safe place to drop it (say so with `_ =`). +- **Keep the happy path at minimal indentation** — handle the error and return early; no `else` + after a terminating `if`. Error flow goes in the indented branch, business logic does not. +- **Don't panic across a package boundary.** Errors are the mechanism for anything a caller can + plausibly hit; panic is for programmer error, API misuse, and genuinely unreachable states. If a + package uses panic internally for unwinding, `recover` it inside that package and return an error + — a panic must never escape into a caller. - **Fail loudly on impossible dispatch:** a `switch` over an internal enum/kind gets a `default` that returns an error (panic only for the genuinely unreachable) — never a silent pass-through that lets a later-added member ride the weakest arm. Pin exhaustiveness with the `exhaustive` @@ -38,9 +55,11 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch - **Error strings:** lowercase, no trailing punctuation (they get wrapped): `"cannot parse %q"`. ## Sources -- Go 1.13 errors — -- Code Review Comments (Error Strings, Handle Errors) — +- Go 1.13 errors — ; `errors.AsType` (Go 1.26) — +- Code Review Comments (Error Strings, Handle Errors, Indent Error Flow, Don't Panic) — +- Google Go Style Guide (Error Handling, Panics, `%w` placement) — - Uber Go Style Guide (Errors) — +- `os.File.Close` returns write errors — --- *Decomposition inspired by [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe); rules grounded in the sources above.* diff --git a/skills/go-testing/SKILL.md b/skills/go-testing/SKILL.md index deb4327..dfb1ee3 100644 --- a/skills/go-testing/SKILL.md +++ b/skills/go-testing/SKILL.md @@ -1,6 +1,6 @@ --- name: go-testing -description: Idiomatic Go testing. This skill should be used when the user writes or reviews Go tests, benchmarks, or fuzz targets — table-driven tests, `t.Parallel`, `testing.B.Loop`, the race detector, goroutine-leak detection, `testing/synctest` for time/concurrency, fuzzing, or golden files. Pair with `go test -race`. Not for non-Go test frameworks; error-wrapping belongs to `go-errors`. +description: Idiomatic Go testing. This skill should be used when the user writes or reviews Go tests, benchmarks, or fuzz targets — table-driven tests, `t.Parallel` (and what cannot run under it), `t.Context`, `t.Chdir`/`t.Setenv`, `t.TempDir` vs `t.ArtifactDir`, `t.Output`, `testing.B.Loop`, the race detector, goroutine-leak detection, `testing/synctest` for time/concurrency, fuzzing, golden files, or writing failure messages that actually diagnose. Pair with `go test -race`. Not for non-Go test frameworks; error-wrapping belongs to `go-errors`. --- # go-testing — Go testing @@ -13,6 +13,24 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, 1.22 the `tc := tc` copy is unnecessary — drop it (`modernize`/`copyloopvar` flag it). - **`t.Parallel()`** on independent tests to cut wall-clock; watch for shared mutable state and loop-var capture in the parallel body. +- **Process-global helpers are incompatible with `t.Parallel()`** — `t.Setenv` (Go 1.17), `t.Chdir` + (1.24), and `cryptotest.SetGlobalRandom` (1.26) all mutate process state, so they fail in a + parallel test *or one with a parallel ancestor*. A table whose cases need env or cwd stays serial; + pass config explicitly instead where you can. The `usetesting` linter pushes `os.Setenv`/`os.Chdir` + in tests towards the `t.*` forms (which restore state via `Cleanup`). +- **`t.Context()`** (Go 1.24) for any test needing a `ctx` — it is cancelled just before the test's + `Cleanup` functions run, so goroutines under test shut down before teardown asserts on them. Use + it over `context.Background()`; the `testingcontext` modernizer rewrites the old form. Do *not* + use it for a fixture whose lifetime spans tests (a shared server or container started in + `TestMain`) — that needs its own context. +- **`t.TempDir()` for scratch, `t.ArtifactDir()` (Go 1.26) for evidence.** `TempDir` is removed at + test end; `ArtifactDir` gives each test a unique directory for output files worth keeping — + rendered output, protocol dumps, failure snapshots — retained when `go test -artifacts` is passed. + Don't hand-roll paths under `os.TempDir()`. +- **`t.Output()` (Go 1.25) is an `io.Writer` into the test log** — wire a `slog` handler or a + subprocess's stdout into it so output interleaves correctly with `t.Log` under `-race` and + parallel tests, instead of `fmt.Println` escaping to raw stdout. `t.Attr` (1.25) emits structured + key/value metadata into `go test -json` output. - **Benchmarks: `for b.Loop() { … }`** (Go 1.24) — it handles timer reset and run scaling; replaces `for i := 0; i < b.N; i++` plus manual `b.ResetTimer()`. - **`-race` is non-negotiable** for any code touching goroutines; wire it into CI. @@ -24,6 +42,8 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, `synctest.Test(t, func(t *testing.T){ … })`; `synctest.Wait()` blocks until every goroutine in the bubble is durably blocked. Reach for it instead of `time.Sleep`-based polling. (The pre-1.25 `GOEXPERIMENT=synctest` API — `synctest.Run` — was removed in Go 1.26; use the stable `synctest.Test`.) + *Go 1.27 (draft, expected Aug 2026) adds `synctest.Sleep` (`time.Sleep` + `Wait` in one) and + `httptest.NewTestServer`, an in-memory server usable inside a bubble.* - **Fuzzing** (`func FuzzX(f *testing.F)`) for parsers, codecs, and anything consuming untrusted bytes. **Golden files** (an `-update` flag writing `testdata/*.golden`) for large structured output. A golden pins *shape*, not behaviour — when it records something another system executes (SQL, @@ -33,12 +53,19 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, deterministic randomness source for the test's duration — reach for it instead of hand-injecting a custom `io.Reader` when testing code that draws from `crypto/rand`. It's process-global, so it can't run inside a `t.Parallel()` test (or one with a parallel ancestor). -- **Assertions:** stdlib + small helpers (`t.Helper()`) is often enough; `testify` is fine — match - the repo, don't mix styles. +- **Failure messages must diagnose without a debugger:** name the call, the input, the result, and + the expectation — `t.Errorf("Parse(%q) = %v, want %v", in, got, want)` — never a bare + `t.Error("failed")`. For structs and slices print a diff (`cmp.Diff(want, got)`), not two blobs. +- **Helpers set up; the test body asserts.** Call `t.Helper()` so a failure points at the caller's + line, and prefer a helper that *returns* a value or `error` over one that fails internally — + assertion logic belongs where the case's context is visible. `t.Fatal` in a setup helper is fine; + in a goroutine use `t.Error` (only the test's own goroutine may call `Fatal`). Stdlib plus small + helpers is usually enough; `testify` is fine — match the repo, don't mix styles. ## Sources - synctest — ; `testing.B.Loop` — -- `testing` package — ; Go 1.22/1.24/1.25/1.26 release notes — +- `testing` package (`T.Context`, `T.Chdir`, `T.Output`, `T.Attr`, `T.ArtifactDir`) — ; Go 1.22/1.24/1.25/1.26 release notes — +- Code Review Comments (Useful Test Failures) — ; Google Go Style Guide (Tests) — - `testing/cryptotest` (Go 1.26) — - `go.uber.org/goleak` — From ebfde268d4706c70daf22375b1a113c5c0d726d3 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:22:29 +0300 Subject: [PATCH 03/15] feat(skills): name the modernize fixer behind every go-idioms rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill claimed "advice == tooling" but never said which analyzer owns which rewrite, so a reader could not attribute a change, look up its behaviour, or tell the auto-fixed rules from the ones only review catches. Adds a Fixer column naming the `modernize` analyzer per row, sourced from the per-fixer docs in x/tools, with `—` where no fixer exists. `go tool fix help` is the way to see what the installed toolchain actually ships (in golangci-lint the whole set is the single `modernize` linter). New rows for idioms the table was missing: `any`, `errorsastype`, `omitzero` (which corrects a real trap — `omitempty` does nothing for a struct-typed field, so a zero `time.Time` still marshals), `testingcontext`, `stringsseq`, `slicesbackward`, `reflecttypefor`. Adds a "no fixer will do it for you" section for the modern-but-unfixable: `os.OpenRoot` for caller-supplied paths (replacing the `filepath.Join` plus manual `..` checks that traversal bugs keep coming from), `crypto/rand.Text` for tokens, nil slices over empty literals, and sorted map iteration for stable output. Co-Authored-By: Claude Opus 5 (1M context) --- skills/go-idioms/SKILL.md | 75 ++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index f0e835e..c0f9fc3 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -1,13 +1,14 @@ --- name: go-idioms -description: Modern idiomatic Go (the `modernize` analyzer set). This skill should be used when the user writes, reviews, or modernizes Go and wants current-version idioms — range-over-int, `min`/`max`, `slices`/`maps`, `strings.Cut`, `cmp.Or`, `sync.OnceFunc`, iterators, `slog.LogAttrs`, `new(expr)` (Go 1.26), dropped loop-var copies. Framed so advice equals tooling (`go fix ./...` on Go 1.26, or `golangci-lint --enable-only=modernize`). Errors→`go-errors`, concurrency→`go-concurrency`, tests→`go-testing`. Not for golangci-lint configuration (use `go-linting`) or non-Go languages. +description: Modern idiomatic Go (the `modernize` analyzer set). This skill should be used when the user writes, reviews, or modernizes Go and wants current-version idioms — range-over-int, `min`/`max`, `slices`/`maps`, `strings.Cut`, `any` over `interface{}`, iterators, `omitzero` json tags, `os.Root`, `new(expr)` and `errors.AsType` (Go 1.26), dropped loop-var copies — or asks which modernize fixer owns a rewrite. Framed so advice equals tooling (`go fix ./...`, or `golangci-lint --enable-only=modernize`). Not for golangci-lint configuration (use `go-linting`) or non-Go languages. --- # go-idioms — modern Go (modernize) -**Advice == tooling.** The `modernize` analyzers flag and usually auto-fix everything below. Run -them, don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runner — it ships -the full modernizer suite in the toolchain itself: +**Advice == tooling.** The `modernize` analyzers flag and usually auto-fix most of what follows — the +**Fixer** column says which, and the last section covers what no fixer will do for you. Run the tool, +don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runner — it ships the +modernizer suite in the toolchain itself: ``` go fix ./... # Go 1.26+: applies the built-in modernizers @@ -21,22 +22,34 @@ a repo pinned to 1.25 or older. ## Prefer → over (since) -| Prefer | Over | Since | -|---|---|---| -| `new(expr)` — e.g. `Field: new(30)`, `new(int64(req.Limit))` | a `ptr[T](v)` helper (auto-rewritten by the `newexpr` fixer) or a hand-written `tmp := v; &tmp`, for optional/pointer fields | 1.26 | -| `for i := range n` | `for i := 0; i < n; i++` | 1.22 | -| `min(a, b)` / `max(a, b)` builtins | hand-rolled helpers | 1.21 | -| *(drop)* `x := x` loop-var copy | pre-1.22 capture workaround | 1.22 | -| `slices.Sort/Contains/Equal`, `slices.Collect`, `maps.Keys` | hand-rolled sort/contains/dedup | 1.21–1.23 | -| `strings.Cut` / `CutPrefix` / `CutSuffix` | `Index` + manual slicing | 1.18/1.20 | -| `cmp.Or(a, b, …)` | nested `if x == "" { x = y }` | 1.22 | -| `sync.OnceFunc` / `OnceValue` | `sync.Once` + a captured var | 1.21 | -| `iter.Seq[V]` / range-over-func | `Visit(callback)` patterns, exposing slices | 1.23 | -| `slog.LogAttrs(ctx, lvl, msg, attrs…)` on hot paths | key-value variadic `slog` (allocates) | 1.21 | -| `errors.Join` | manual multi-error concat → `go-errors` | 1.20 | -| `wg.Go(...)` | `wg.Add(1)`/`defer wg.Done()` → `go-concurrency` | 1.25 | -| `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | -| typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | +The **Fixer** column names the `modernize` analyzer that owns each rewrite — cite it when explaining +or attributing a change, and use `go tool fix help` to see which analyzers the installed toolchain +actually ships (in golangci-lint the whole set is the single `modernize` linter). `—` means no fixer +exists: review has to catch it. + +| Prefer | Over | Since | Fixer | +|---|---|---|---| +| `new(expr)` — e.g. `Field: new(30)`, `new(int64(req.Limit))` | a `ptr[T](v)` helper or a hand-written `tmp := v; &tmp`, for optional/pointer fields | 1.26 | `newexpr` | +| `errors.AsType[E](err)` | `errors.As(err, &target)` → `go-errors` | 1.26 | `errorsastype` | +| `for i := range n` | `for i := 0; i < n; i++` | 1.22 | `rangeint` | +| `min(a, b)` / `max(a, b)` builtins | hand-rolled helpers | 1.21 | `minmax` | +| *(drop)* `x := x` loop-var copy | pre-1.22 capture workaround | 1.22 | `forvar` | +| `any` | `interface{}` | 1.18 | `any` | +| `slices.Sort/Contains/Equal`, `slices.Collect`, `maps.Keys`, `slices.Concat` | hand-rolled sort/contains/dedup/append chains | 1.21–1.23 | `slices*`, `mapsloop`, `appendclipped` | +| `for i, v := range slices.Backward(s)` | `for i := len(s)-1; i >= 0; i--` | 1.23 | `slicesbackward` | +| `strings.Cut` / `CutPrefix` / `CutSuffix` | `Index` + manual slicing | 1.18/1.20 | `stringscut`, `stringscutprefix` | +| `strings.SplitSeq` / `FieldsSeq` | ranging over `strings.Split`/`Fields` (allocates a slice) | 1.24 | `stringsseq` | +| `omitzero` on a struct-typed json field | `omitempty`, which does **nothing** for struct fields — a zero `time.Time` still marshals | 1.24 | `omitzero` | +| `t.Context()` in tests | `context.WithCancel(context.Background())` → `go-testing` | 1.24 | `testingcontext` | +| `reflect.TypeFor[T]()` | `reflect.TypeOf((*T)(nil)).Elem()` | 1.22 | `reflecttypefor` | +| `cmp.Or(a, b, …)` | nested `if x == "" { x = y }` | 1.22 | — | +| `sync.OnceFunc` / `OnceValue` | `sync.Once` + a captured var | 1.21 | — | +| `iter.Seq[V]` / range-over-func | `Visit(callback)` patterns, exposing slices | 1.23 | `stditerators` | +| `slog.LogAttrs(ctx, lvl, msg, attrs…)` on hot paths | key-value variadic `slog` (allocates) | 1.21 | — | +| `errors.Join` | manual multi-error concat → `go-errors` | 1.20 | — | +| `wg.Go(...)` | `wg.Add(1)`/`defer wg.Done()` → `go-concurrency` | 1.25 | `waitgroupgo` | +| `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | `bloop` | +| typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | `atomictypes` | Idioms are a moving target — let the tool (pinned to the repo's toolchain) be the source of truth so advice never drifts from the user's `go fix`. Go 1.26 also lifts the ban on a generic type @@ -44,10 +57,30 @@ referencing itself in its own type-parameter list (e.g. `type Adder[A Adder[A]] so self-referential constraints no longer need a workaround — but that's a hand-written pattern, not something a modernizer rewrites. +## Modern, but no fixer will do it for you + +- **`os.OpenRoot(dir)` → `*os.Root`** (1.24) for anything that opens a caller-supplied path: its + methods cannot escape the directory, including via symlink. Replaces `filepath.Join` plus + hand-written `..`/prefix checks — the traversal-bug pattern those checks keep getting wrong. +- **`rand.Text()`** from `crypto/rand` (1.24) for tokens, nonces, and IDs — not `math/rand`, and not + a hand-rolled base64 of `rand.Read`. Security-sensitive randomness always comes from `crypto/rand`. +- **`var s []T`, not `s := []T{}`** — the nil slice is the idiomatic empty slice (append works, + `len` is 0). Reach for the non-nil literal only when something genuinely distinguishes them + (e.g. marshalling `[]` vs `null`). +- **`slices.Sorted(maps.Keys(m))`** (1.23) when iterating a map for output — map order is random, and + unstable output is a flaky-test and noisy-diff source. + +*Go 1.27 (draft, expected Aug 2026) adds the `atomictypes`, `embedlit`, `slicesbackward`, and +`unsafefuncs` fixers to `go fix`, renames `waitgroup` → `waitgroupgo`, and drops `fmtappendf`; it also +lands `encoding/json/v2` + `encoding/json/jsontext` (v1 is reimplemented on v2, opt out with +`GOEXPERIMENT=nojsonv2`), `strings.CutLast`/`bytes.CutLast`, and a stdlib `uuid` package. A +golangci-lint built against newer `x/tools` may carry those fixers before the toolchain does.* + ## Sources -- `modernize` — +- `modernize` (per-fixer docs, the Fixer column) — - `go fix` (rewritten in 1.26) — ; range-over-func — - `slog` — ; Go 1.21–1.26 release notes (`new(expr)`, self-ref generics — ) +- `os.Root` / `omitzero` / `rand.Text` — ; Code Review Comments (Declaring Empty Slices, Crypto Rand) — --- *Decomposition inspired by [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe); rules grounded in the sources above.* From bb88bc3004b7cba422843b7291c75ffdd044d7ec Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:23:00 +0300 Subject: [PATCH 04/15] refactor: make Go 1.26.4+ a hard floor, not a hedged baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The set claimed "Go 1.26, works with 1.25+" and then hedged every modern rule against it: "on Go 1.26+ modules prefer errors.AsType", "check go.mod first — don't apply a Go 1.26 idiom to a repo pinned to 1.25 or older", "or golangci-lint on older toolchains". That fallback branch costs a clause in almost every rule and buys nothing for anyone actually on 1.26.4+. Drops the hedging from go-coding, go-idioms, go-errors (in the preceding commit), go-explain, the go-reviewer baseline and its modernization-debt dimension, rules/go-context.mdc, README.md and docs/install.md. Modern forms are now stated flat. Keeps the version annotations — the `Since` column and the inline "(Go 1.24)" tags. Those are provenance, not gates: they explain why older code looks different and what an older module would have to bump to. go-idioms says so explicitly where the go.mod-gating instruction used to be, and go-explain still names the version an idiom landed in without hedging the recommendation. Two single-line spillovers ride along because they share a line with a floor edit: the go-coding and go-context.mdc router tables gain their layout/naming/API-surface row here, which the next commit is about. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- README.md | 2 +- agents/go-reviewer.md | 8 ++++---- docs/install.md | 4 ++-- rules/go-context.mdc | 8 ++++---- skills/go-coding/SKILL.md | 8 ++++---- skills/go-explain/SKILL.md | 5 +++-- skills/go-idioms/SKILL.md | 6 +++--- 8 files changed, 22 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 795aab5..f8fa3f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ This file provides guidance to AI coding assistants (Claude Code, Cursor, and co The **Go Coding Plugin** is an AI plugin by Cadasto B.V. that teaches AI coding assistants **idiomatic Go coding standards** — formatting, naming, error handling, concurrency, testing, and project layout — through skills, commands, agents, hooks, and Cursor rules. It targets **both Claude Code and Cursor** from a single shared component set. -> **Current status — v0.3.0.** A complete dual-host (Claude Code + Cursor) Go-standards set, baselined on **Go 1.26** (1.26.4+) with version-gated guidance that still holds for 1.25 modules, that validates clean (`./scripts/validate.sh` + `claude plugin validate .`): the auto-invoked `go-coding` **router** skill; the focused standards skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout`; the read-only `go-reviewer` agent; the user-invoked `/go-explain` and `/go-lint-setup` skills; a shipped `references/golangci.v2.yml`; the `rules/go-context.mdc` Cursor rule; and host-agnostic `session-start` + `format-on-save` hooks. Do not assume a file is present because it is documented here — check first. +> **Current status — v0.3.0.** A complete dual-host (Claude Code + Cursor) Go-standards set, baselined on **Go 1.26.4+** as a hard floor (no fallback guidance for 1.25 or older; version annotations remain as provenance), that validates clean (`./scripts/validate.sh` + `claude plugin validate .`): the auto-invoked `go-coding` **router** skill; the focused standards skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout`; the read-only `go-reviewer` agent; the user-invoked `/go-explain` and `/go-lint-setup` skills; a shipped `references/golangci.v2.yml`; the `rules/go-context.mdc` Cursor rule; and host-agnostic `session-start` + `format-on-save` hooks. Do not assume a file is present because it is documented here — check first. ## Domain Context diff --git a/README.md b/README.md index ac91ca9..476ca28 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Or from a local working copy: `claude plugin add /path/to/go-coding-plugin`. **Cursor**: add this repository as a plugin (Settings → Plugins). See [`docs/install.md`](docs/install.md) for both hosts. -**Prerequisites** — the plugin installs without a Go toolchain, but its hooks and enforcement guidance expect **Go 1.26.x** (minimum 1.26.4; still works against 1.25 modules) plus `gofmt`, `gofumpt`, `goimports`, and `gopls` on the host `PATH`. See [Host toolchain (minimal requirements)](docs/install.md#host-toolchain-minimal-requirements) for what each tool drives and copy-paste install commands. +**Prerequisites** — the plugin installs without a Go toolchain, but its hooks and enforcement guidance expect **Go 1.26.4+** plus `gofmt`, `gofumpt`, `goimports`, and `gopls` on the host `PATH`. See [Host toolchain (minimal requirements)](docs/install.md#host-toolchain-minimal-requirements) for what each tool drives and copy-paste install commands. ## Component surface diff --git a/agents/go-reviewer.md b/agents/go-reviewer.md index 0299d9d..edb2b1f 100644 --- a/agents/go-reviewer.md +++ b/agents/go-reviewer.md @@ -44,7 +44,7 @@ tools: - Bash --- -You are **go-reviewer**, a reviewer of idiomatic, correct Go (Go 1.26, works with 1.25+; golangci-lint v2). You supply +You are **go-reviewer**, a reviewer of idiomatic, correct Go (Go 1.26.4+; golangci-lint v2). You supply the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go vet`, and `golangci-lint`. You are **read-only**: you report findings, you never edit code. @@ -109,9 +109,9 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v non-atomic reads of those fields); `sync.Mutex`/`WaitGroup` copied by value; a map written concurrently without a lock; check-then-act races. - **Stale modernization debt** — code `modernize`/`go fix` would rewrite (range-int, `min`/`max`, - `slices`/`maps`, `strings.Cut`, `cmp.Or`, pre-1.22 loop-var copies; on Go 1.26 modules also - pointer-helper temps that `new(expr)` replaces). Low severity; point at `go fix ./...` (Go 1.26) or - `golangci-lint run --enable-only=modernize`. Gate suggestions on the module's `go.mod` version. + `slices`/`maps`, `strings.Cut`, `cmp.Or`, leftover loop-var copies, pointer-helper temps that + `new(expr)` replaces, `errors.As` where `errors.AsType` fits). Low severity; point at + `go fix ./...` or `golangci-lint run --enable-only=modernize`. - **slog hot-path waste** — building a per-call logger instead of `logger.With(...)`; formatting or allocating before a level check; key-value variadic on a hot path instead of `slog.LogAttrs`. diff --git a/docs/install.md b/docs/install.md index 9ad021c..00238fc 100644 --- a/docs/install.md +++ b/docs/install.md @@ -41,13 +41,13 @@ Add this repository as a plugin (Cursor **Settings → Plugins**, via Git URL or ## Host toolchain (minimal requirements) -Installing the plugin itself needs no Go toolchain — it is pure Markdown + JSON. But its **enforcement** layer only delivers value when the standard Go tools are on the host `PATH`: the `format-on-save` hook shells out to a formatter, the golangci-lint v2 reference config lists `gofumpt`/`goimports` as formatters, and the recommended official `gopls-lsp` plugin (`@claude-plugins-official`) drives `gopls`. The plugin targets **Go 1.26** + golangci-lint v2, and its version-gated idiom guidance still works against 1.25 modules. +Installing the plugin itself needs no Go toolchain — it is pure Markdown + JSON. But its **enforcement** layer only delivers value when the standard Go tools are on the host `PATH`: the `format-on-save` hook shells out to a formatter, the golangci-lint v2 reference config lists `gofumpt`/`goimports` as formatters, and the recommended official `gopls-lsp` plugin (`@claude-plugins-official`) drives `gopls`. The plugin targets **Go 1.26.4+** + golangci-lint v2 as a hard floor — it does not carry fallback guidance for 1.25 or older modules. At minimum the host should provide: | Tool | Provided by | Used for | If missing | |------|-------------|----------|------------| -| **Go 1.26.x** (min 1.26.4) | [go.dev/dl](https://go.dev/dl/) / package manager | everything; satisfies `go.mod` `go 1.26.x` (and 1.25 modules); `go fix ./...` runs the modernizers | no toolchain at all | +| **Go 1.26.x** (min 1.26.4) | [go.dev/dl](https://go.dev/dl/) / package manager | everything; satisfies `go.mod` `go 1.26.x`; `go fix ./...` runs the modernizers | no toolchain at all | | **`gofmt`** | the Go distribution | `format-on-save.sh` fallback (`gofmt -w -s`) | n/a — always ships with Go | | **`gofumpt`** | `go install` | `format-on-save.sh` primary (`gofumpt -w`), stricter gofmt superset | hook degrades to `gofmt` | | **`goimports`** | `go install` | `goimports` formatter in the golangci-lint v2 config (import grouping/pruning) | import-group formatting skipped | diff --git a/rules/go-context.mdc b/rules/go-context.mdc index fc4fd5e..df35a0b 100644 --- a/rules/go-context.mdc +++ b/rules/go-context.mdc @@ -1,5 +1,5 @@ --- -description: Go coding standards — idiomatic Go (Go 1.26, works with 1.25+; golangci-lint v2) for .go files. Mirrors the go-coding router skill for Cursor. +description: Go coding standards — idiomatic Go (Go 1.26.4+; golangci-lint v2) for .go files. Mirrors the go-coding router skill for Cursor. globs: ["**/*.go"] alwaysApply: false --- @@ -17,11 +17,11 @@ This Cursor rule mirrors the `go-coding` router skill — apply it when editing |---|---|---| | Formatting | `gofmt`/`gofumpt` (+ `goimports`) — machine-enforced | — | | Static analysis / bugs | `go vet ./...`, `golangci-lint run` | `go-linting` | -| Modern idioms | `go fix ./...` (Go 1.26 runs the modernizers natively), or `golangci-lint run --enable-only=modernize` | `go-idioms` | +| Modern idioms | `go fix ./...` (the toolchain's modernizers), or `golangci-lint run --enable-only=modernize` | `go-idioms` | | Errors | `golangci-lint run --enable-only=errorlint` | `go-errors` | | Concurrency | `go test -race ./...`, `go vet ./...` | `go-concurrency` | -| Testing | `go test -race ./...`; `testing/synctest` (since 1.25) for time/concurrency | `go-testing` | -| Layout | judgment | `go-layout` | +| Testing | `go test -race ./...`; `testing/synctest` for time/concurrency | `go-testing` | +| Layout, naming & API surface | `golangci-lint run --enable-only=revive`; rest is judgment | `go-layout` | Ground every judgment call in a cited source — Effective Go, Go Code Review Comments, the Google and Uber Go style guides, `pkg.go.dev`. Don't invent rules. Adopt the shipped golangci-lint v2 diff --git a/skills/go-coding/SKILL.md b/skills/go-coding/SKILL.md index 749a9fc..290e1b7 100644 --- a/skills/go-coding/SKILL.md +++ b/skills/go-coding/SKILL.md @@ -1,6 +1,6 @@ --- name: go-coding -description: Go coding-standards router and entry point for idiomatic Go (Go 1.26, works with 1.25+; golangci-lint v2). This skill should be used when a Go task spans multiple areas, is unspecified, or the question is which tool or standard applies — it routes each topic to the deterministic tool (gofmt/gofumpt, go vet, go fix / golangci-lint v2 modernize, go test -race), to the gopls-lsp plugin for code intelligence, and then to the focused go-* skill that owns it. For a single, already-identified topic prefer that skill directly (errors → go-errors, concurrency → go-concurrency, testing → go-testing, idioms/modernization → go-idioms, linter config → go-linting, layout → go-layout). Not for non-Go languages or domain/business rules. +description: Go coding-standards router and entry point for idiomatic Go (Go 1.26.4+; golangci-lint v2). This skill should be used when a Go task spans multiple areas, is unspecified, or the question is which tool or standard applies — it routes each topic to the deterministic tool (gofmt/gofumpt, go vet, go fix / golangci-lint v2 modernize, go test -race), to the gopls-lsp plugin for code intelligence, and then to the focused go-* skill that owns it. For a single, already-identified topic prefer that skill directly (errors → go-errors, concurrency → go-concurrency, testing → go-testing, idioms/modernization → go-idioms, linter config → go-linting, layout/naming/API design → go-layout). Not for non-Go languages or domain/business rules. --- # go-coding — Go standards router @@ -19,11 +19,11 @@ Two principles from the project research drive it: |---|---|---| | Formatting | `gofmt -l` / `gofumpt -l` (+ `goimports`) — machine-enforced, non-negotiable | — | | Static analysis / likely bugs | `go vet ./...`, `golangci-lint run` | `go-linting` | -| Modern idioms (range-int, `min`/`max`, `slices`/`maps`, `wg.Go`, `strings.Cut`, `new(expr)`) | `go fix ./...` (Go 1.26 applies the full modernizer suite natively), or `golangci-lint run --enable-only=modernize` on older toolchains | `go-idioms` | +| Modern idioms (range-int, `min`/`max`, `slices`/`maps`, `wg.Go`, `strings.Cut`, `new(expr)`, `errors.AsType`) | `go fix ./...` (the toolchain's modernizer suite), or `golangci-lint run --enable-only=modernize` for CI reproducibility | `go-idioms` | | Errors (`%w`, `errors.Is`/`As`, `errors.Join`, sentinel/typed) | `golangci-lint run --enable-only=errorlint` | `go-errors` | | Concurrency (goroutine leaks, ctx lifecycle, atomics) | `go test -race ./...`, `go vet ./...` | `go-concurrency` | -| Testing (table-driven, `t.Parallel`, `B.Loop`, `testing/synctest`) | `go test -race ./...`; use `testing/synctest` (stable since 1.25) for time/concurrency tests | `go-testing` | -| Project layout (`internal/`, start-flat) | judgment — see sources below | `go-layout` | +| Testing (table-driven, `t.Parallel`, `t.Context`, `B.Loop`, `testing/synctest`) | `go test -race ./...`; use `testing/synctest` for time/concurrency tests | `go-testing` | +| Layout, naming & API surface (`internal/`, initialisms, receiver type, in-band errors, doc comments) | `golangci-lint run --enable-only=revive` (`var-naming`, `receiver-naming`, `exported`), `gofmt` for doc-comment layout; the rest is judgment | `go-layout` | | Code intelligence (defs/refs/diagnostics/rename/vulncheck) | install the **`gopls-lsp`** plugin | — | Open the focused `go-*` skill for the topic — it carries the cited rules and the judgment; run the diff --git a/skills/go-explain/SKILL.md b/skills/go-explain/SKILL.md index 70422f5..bc3f1d2 100644 --- a/skills/go-explain/SKILL.md +++ b/skills/go-explain/SKILL.md @@ -18,8 +18,9 @@ Cover, in a few lines: 4. **Source** — cite one authoritative reference: Effective Go, Go Code Review Comments, the Google or Uber Go style guide, a `go.dev/blog` post, or `pkg.go.dev`. -Tailor to the repo's Go version when a `go.mod` is present — don't recommend a Go 1.26 idiom (e.g. -`new(expr)`) for a module pinned to 1.25 or older. For a fuller treatment, route to the matching +Answer against the **Go 1.26.4+** baseline. Name the version an idiom landed in (that's step 1) so +it's clear what an older module would need, but don't hedge the recommendation — only tailor down if +the repo's `go.mod` actually says older. For a fuller treatment, route to the matching skill: `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, or `go-layout`. diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index c0f9fc3..db66ab3 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -16,9 +16,9 @@ golangci-lint run --enable-only=modernize --fix # any toolchain (same analyz ``` Both draw on the same `golang.org/x/tools` engine as gopls, so their fixes agree. This skill -explains *why* and catches what review notices before the tool runs. **Check `go.mod` first** — -gate each idiom on the module's Go version (the `Since` column below); don't apply a Go 1.26 idiom to -a repo pinned to 1.25 or older. +explains *why* and catches what review notices before the tool runs. The **baseline is Go 1.26.4+**, +so every row below applies as written — the `Since` column is provenance: it explains why older code +looks different, and what an older module would have to bump to before adopting the idiom. ## Prefer → over (since) From 4aa556da2c304bba290dbdcec479b9f9fc7fdfee Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:23:35 +0300 Subject: [PATCH 05/15] feat(skills): fold naming, doc comments and API design into go-layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in the set owned the bulk of Go Code Review Comments: naming, doc comments, and the shape of an exported signature. go-idioms is scoped to the `modernize` analyzers, go-layout to directory structure, and the reviewer had no dimension for it — so an initialism like `userId`, a `GetName()` accessor, or an in-band `-1` error passed without comment. go-layout widens from "project layout" to layout + naming + API surface: - Naming: initialism casing, MixedCaps over MAX_LENGTH, name length tracking scope, receiver names consistent across a type, no `Get` prefix, `test` doubles named for behaviour. - Signatures: receiver type (pointer when it mutates, is large, or holds a sync field), pass small values directly, no in-band errors, named results only when they add information, option struct vs variadic options chosen by how often callers pass options, accept interfaces / return concrete types, useful zero value. - Doc comments: full sentence starting with the name, package comment placement, the gofmt-formatted syntax, documenting what the signature can't say (concurrency safety, ownership, cancellation), `Deprecated:` over deletion. go-reviewer gains the matching "exported-surface & naming slips" dimension and names `revive` for the mechanically-checkable half; the discarded `Close` on a written file joins the resource-leak dimension. The router and Cursor rule route there (their table rows landed with the preceding commit). Also drops two `1.26+ modules` hedges the hard-floor commit missed. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- agents/go-reviewer.md | 22 ++++++++--- skills/go-idioms/SKILL.md | 2 +- skills/go-layout/SKILL.md | 78 +++++++++++++++++++++++++++++++++------ 4 files changed, 85 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 476ca28..668abea 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Or from a local working copy: `claude plugin add /path/to/go-coding-plugin`. | Skill `go-coding` | shipped | Auto-invoked router: sends each Go topic to the enforcing tool and the focused skill below; recommends `gopls-lsp`. | | Session-start hook | shipped | Detects a Go workspace (`go.mod`/`*.go`) and prints one standards line; dual-host. | | Format-on-save hook | shipped | After each `Write`/`Edit` of a `*.go` file, runs `gofumpt -w` (or `gofmt -w -s`) on it; dual-host, host-only, silent no-op if no formatter is installed. | -| Skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout` | shipped | Load-on-use standards — each rule cited, framed around the enforcing linter (`modernize`, `errorlint`, `-race`, …). | +| Skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout` | shipped | Load-on-use standards — each rule cited, framed around the enforcing linter (`modernize`, `errorlint`, `-race`, …). `go-layout` also owns naming, doc comments, and exported-API shape. | | Agent `go-reviewer` | shipped | Read-only, context-isolated Go reviewer for what linters miss; severity-ranked findings, no sub-agent dispatch. | | Skills `/go-explain`, `/go-lint-setup` (user-invoked) | shipped | Slash-command skills — idiom/standard lookup; scaffold the golangci-lint v2 config into a repo. | | Lint config `references/golangci.v2.yml` | shipped | Reference golangci-lint v2 config (`modernize` + stack linters). | diff --git a/agents/go-reviewer.md b/agents/go-reviewer.md index edb2b1f..e526cf5 100644 --- a/agents/go-reviewer.md +++ b/agents/go-reviewer.md @@ -2,8 +2,9 @@ name: go-reviewer description: > Use this agent to review Go (`.go`) diffs or files for the bugs and smells that linters miss — - silent error swallowing, goroutine leaks, context misuse, resource leaks, sentinel-error breakage, - silent dispatch defaults, sensitive-value echo in errors/logs, comment–code drift, unsafe atomics, + silent error swallowing, goroutine leaks, context misuse, resource leaks (including a discarded + `Close` on a written file), sentinel-error breakage, silent dispatch defaults, sensitive-value echo + in errors/logs, comment–code drift, unsafe atomics, exported-surface and naming slips, stale modernization debt, and slog hot-path waste. Invoke it after writing or changing Go code, before opening a PR, or whenever the user asks for a Go code review. It is read-only, works alone, and returns severity-ranked findings; it does not edit code or dispatch @@ -88,9 +89,12 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v (`noctx`); missing client timeout; ignored cancellation. - **Resource leaks** — unclosed `http.Response.Body` (`bodyclose`), `sql.Rows`/`Stmt` (`sqlclosecheck`), unchecked `rows.Err()` (`rowserrcheck`); files/listeners not closed; `defer` - inside a loop accumulating handles. + inside a loop accumulating handles. Also the *silent* one: `defer f.Close()` on a file that was + **written** discards a failed flush — the caller sees success over a truncated file. Expect + `defer func() { err = errors.Join(err, f.Close()) }()` on write paths. - **Sentinel / typed-error breakage** — `err == ErrX` or a type assertion where wrapping is in play - (use `errors.Is`/`errors.As`); a documented sentinel removed, or its wrapping changed (an API break). + (use `errors.Is`, or `errors.AsType[E]` for a typed error); a documented sentinel + removed, or its wrapping changed (an API break). - **Silent dispatch defaults** — a `switch` over an internal enum/kind tag whose `default` arm silently passes through, returns a zero value, or picks the weakest behaviour: a member added later rides the wrong arm with no error. Expect a loud `default` (error, or panic only for the @@ -108,6 +112,12 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v - **Concurrency hazards** — bare-int `atomic.Add*` instead of typed `atomic.Int64`/`Bool` (and non-atomic reads of those fields); `sync.Mutex`/`WaitGroup` copied by value; a map written concurrently without a lock; check-then-act races. +- **Exported-surface & naming slips** — a newly exported identifier with no doc comment, or one that + doesn't start with the name it documents; mixed initialism casing (`userId`, `HttpClient`); a `Get` + prefix on an accessor; an in-band error (`-1`, `""`, or a meaningful `nil`) where `(T, error)` or + `(T, bool)` belongs; an interface returned where the concrete type would serve; pointer and value + receivers mixed on one type. `revive` catches the naming and missing-doc-comment cases — name it; + the signature-shape ones are judgment. See `go-layout`. - **Stale modernization debt** — code `modernize`/`go fix` would rewrite (range-int, `min`/`max`, `slices`/`maps`, `strings.Cut`, `cmp.Or`, leftover loop-var copies, pointer-helper temps that `new(expr)` replaces, `errors.As` where `errors.AsType` fits). Low severity; point at @@ -116,8 +126,8 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v allocating before a level check; key-value variadic on a hot path instead of `slog.LogAttrs`. For the *why* and citations behind any dimension, the `go-errors`, `go-concurrency`, `go-testing`, -`go-idioms`, and `go-linting` skills carry the grounded rules — reference them rather than -re-deriving from memory. +`go-idioms`, `go-linting`, and `go-layout` skills carry the grounded rules — reference them rather +than re-deriving from memory. ## Output format diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index db66ab3..d9f3e0e 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -11,7 +11,7 @@ don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runn modernizer suite in the toolchain itself: ``` -go fix ./... # Go 1.26+: applies the built-in modernizers +go fix ./... # applies the toolchain's built-in modernizers golangci-lint run --enable-only=modernize --fix # any toolchain (same analyzers, via golangci-lint) ``` diff --git a/skills/go-layout/SKILL.md b/skills/go-layout/SKILL.md index aec2893..64e6568 100644 --- a/skills/go-layout/SKILL.md +++ b/skills/go-layout/SKILL.md @@ -1,13 +1,14 @@ --- name: go-layout -description: Go project layout and package design. This skill should be used when the user structures a Go module or names packages — `internal/`, `cmd/`, start-flat-then-grow, package naming, avoiding `util`/`common` grab-bags, or deciding whether hexagonal/DDD ceremony is warranted. Counters imported Java/C# structure. Not for build tooling or for non-layout idioms (→ `go-idioms`). +description: Go project layout, naming, and API-surface design. This skill should be used when the user structures a Go module, names things, or shapes an exported API — `internal/`, `cmd/`, start-flat-then-grow, package/variable/receiver naming, initialism casing (`userID`, `HTTPServer`), pointer vs value receivers, in-band errors, named results, option structs vs variadic options, returning concrete types, or writing doc comments. Counters imported Java/C# structure. Not for build tooling or non-layout idioms (→ `go-idioms`). --- -# go-layout — project & package structure +# go-layout — layout, naming & API surface -The Go community consensus is *minimalism*; resist imported ceremony. +The Go community consensus is *minimalism*; resist imported ceremony. Naming and the shape of an +exported signature are part of the API — they are as reviewable as the code. -## Rules +## Layout - **`internal/` is the one true consensus.** Packages under `internal/` cannot be imported from outside the module subtree — use it to keep implementation private while exporting a small surface. @@ -15,21 +16,76 @@ The Go community consensus is *minimalism*; resist imported ceremony. `cmd//main.go` when you have multiple binaries and `internal//` when you need privacy — not before. `golang-standards/project-layout` is community-made, **explicitly not official and contested**; don't treat its deep tree as a starting requirement. -- **Package names are part of the API:** short, lowercase, single word, no underscores or - camelCase. The caller writes `chi.NewRouter()`, so don't stutter (`chi.ChiRouter`). Avoid - `util`, `common`, `helpers`, `base` grab-bags — name by what the package *provides*. - **No Java/C# transplants:** no `*Manager`/`*Impl`/`*Factory` reflexes, no one-type-per-file rule, - no interface-for-everything. Define interfaces *where they're consumed*, keep them small, and - return concrete types. + no interface-for-everything. - **Hexagonal / ports-and-adapters / DDD is a tool, not a default** — justified for larger services with real external-boundary complexity, overkill for a CLI or a small service. - **Files:** one package per directory; `package foo` for `foo.go` + `foo_test.go`; use `package foo_test` for black-box tests that exercise only the exported API. +## Naming + +- **Package names are part of the call site:** short, lowercase, single word, no underscores or + camelCase. The caller writes `chi.NewRouter()`, so don't stutter (`chi.ChiRouter`, + `bytes.BufferWrite`). Avoid `util`, `common`, `helpers`, `base` grab-bags — name by what the + package *provides*. +- **`MixedCaps`, never `MAX_LENGTH` or `snake_case`** — including constants, whatever the convention + was in the language this code came from. +- **Initialisms keep a single case throughout:** `userID`, `parseURL`, `HTTPServer`, `ServeHTTP` — + never `userId`, `HttpServer`. Mixed casing within one identifier is the tell of a translated name. +- **Name length tracks scope.** `i`, `r`, `buf` are correct in a five-line body; anything + package-level, long-lived, or used far from its declaration earns a descriptive name. Longer is not + better — `idx` beats `theCurrentIndexIntoTheSlice`. +- **Receiver names are a one- or two-letter abbreviation of the type** (`c *Client`, `srv *Server`), + identical across every method on that type. Never `self`, `this`, or `me`. +- **No `Get` prefix on accessors:** `u.Name()`, paired with `u.SetName(…)`. A verb-like name is for + something that acts; a noun-like name for something that returns a value. +- **Test doubles live in a `test` package** and are named for behaviour, not mechanism — + `AlwaysDeclines`, not `MockCardProcessorImpl2`. + +## Signatures & API surface + +- **Receiver type:** pointer when the method mutates, when the receiver is large, or when the type + holds a `sync` field (copying a lock is a bug — `go vet` copylocks). Value receivers for small + immutable types. **Be consistent within a type** — don't mix pointer and value receivers. +- **Pass small fixed-size values directly.** `*int` to "avoid a copy" trades a machine word for an + indirection plus aliasing risk. +- **No in-band errors.** Return `(T, error)` or `(T, bool)` — never `-1`, `""`, or a `nil` that means + failure. A caller can forget to compare against a magic value; a second return value is harder to + ignore, and `errcheck` sees it. +- **Named results only when they add information** the types don't (`(n int, err error)`), or when a + deferred closure must assign to them (the `Close`-into-`err` idiom in `go-errors`). Bare `return` + belongs only in short functions. +- **Two option styles, chosen by how often callers pass options:** an **option struct** as the final + parameter when most callers set at least one field (self-documenting, grows compatibly); **variadic + functional options** when most callers pass none. Don't erect a functional-options framework around + two booleans. +- **Accept interfaces, return concrete types.** Define an interface in the package that *consumes* + it, keep it to a method or three, and return the concrete type so callers get the full surface and + you can add methods without breaking them. +- **Prefer synchronous signatures** — let the caller add concurrency (→ `go-concurrency`). +- **Make the zero value useful where you can** (`bytes.Buffer`, `sync.Mutex` need no constructor). If + a type genuinely requires a `New…`, the doc comment must say so. + +## Doc comments + +- **Every exported identifier gets one, as a full sentence starting with the name:** + `// Serve accepts incoming connections on the listener.` That phrasing is what makes `go doc` + output and grep both work. +- **Package comment sits directly above `package x`** with no blank line, exactly one per package, + opening `// Package x …`. +- **`gofmt` formats doc comments** (since Go 1.19) — lists, headings, indented code blocks, and + `[Name]`/`[pkg.Name]` doc links. Write that syntax and let the tool lay it out. +- **Document what the signature can't say:** concurrency safety, who owns and must close a returned + resource, whether cancellation leaves partial work behind, and which errors callers can branch on. + Don't restate parameter names. +- **Retire an exported name with a `Deprecated:` paragraph**, not by deleting it. + ## Sources - Effective Go — -- Code Review Comments (Package Names, Interfaces) — -- Google Go Style Guide — ; `internal/` — +- Code Review Comments (Package/Variable/Receiver Names, Initialisms, Mixed Caps, In-Band Errors, Named Result Parameters, Pass Values, Interfaces, Doc Comments) — +- Google Go Style Guide (naming, option structs, documentation, test doubles) — +- Doc comment syntax — ; `internal/` — --- *Decomposition inspired by [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe); rules grounded in the sources above.* From 1a0fe9fc3703f8498977777338515d0589611bd8 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:23:47 +0300 Subject: [PATCH 06/15] feat(skills): retool go-linting for the v2 schema and a maintainable version pin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps. The v2 coverage stopped at the schema headline, so the mechanics a migration actually needs were missing; and the pin advice named `v2.11.4` as a bare fact with no rationale, which had already gone stale against the v2.12.x line and gave nobody a way to decide when to move. v2 mechanics: `golangci-lint migrate` (in-place, keeps a `.bck` backup, drops comments) instead of hand-porting; the `issues.exclude-rules` → `linters.exclusions.rules` and `linters-settings` → `linters.settings` / `formatters.settings` moves; `golangci-lint fmt` for the formatters section; and `//nolint: // reason` discipline over a bare `//nolint`. Version pin, replacing the hardcoded release with a policy: pin an exact version in exactly one source of truth (the action's `version:` input, which also caches), never `latest` — upstream's reasoning is that a release can retune linters and redden every build at once with nothing in the diff to blame — and let Renovate/Dependabot raise the bump as its own reviewable PR, so the pin stays current without drifting silently. On bump: `--fix` first, then land the leftovers or add an exclusion with a reason. Also records upstream's warning that `go install`/`go get` and `tool` directives "aren't guaranteed to work", since they compile against whatever local Go version is around. Enables `usetesting` (pushes `os.Setenv`/`os.Chdir`/`context.Background` in tests to the `t.*` forms, pairing with the go-testing rules) and `nolintlint` (enforces the suppression discipline above) across all three copies of the reference config. A build too old to know a linter name is a signal to bump the pin, not to delete the line — the config header says so. Co-Authored-By: Claude Opus 5 (1M context) --- references/golangci.v2.yml | 9 +++++-- skills/go-lint-setup/SKILL.md | 10 +++++--- skills/go-linting/SKILL.md | 44 +++++++++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/references/golangci.v2.yml b/references/golangci.v2.yml index 4f93abb..2beaf7e 100644 --- a/references/golangci.v2.yml +++ b/references/golangci.v2.yml @@ -1,8 +1,11 @@ # Reference golangci-lint v2 config for the go-coding plugin. # # Scaffold it into a repo with the `/go-lint-setup` command, or copy it to `.golangci.yml`. -# Schema is golangci-lint v2 — a v1 config will NOT parse. See the `go-linting` skill for -# what each linter does and why. Keep this in sync with the block inlined in +# Schema is golangci-lint v2 — a v1 config will NOT parse (run `golangci-lint migrate` on a v1 +# config). Needs a golangci-lint v2 release recent enough to know every linter named below; if it +# rejects one, bump the pin rather than dropping the line. Pin an exact version in CI in one place +# (see `go-linting` — no version is blessed here on purpose). See the `go-linting` skill for what +# each linter does and why. Keep this in sync with the block inlined in # `skills/go-lint-setup/SKILL.md`. version: "2" @@ -18,6 +21,8 @@ linters: - contextcheck # context not propagated through the call chain - containedctx # context.Context stored in a struct field - perfsprint # fmt.Sprintf where a cheaper call exists + - usetesting # os.Setenv/os.Chdir/context.Background in tests → t.Setenv/t.Chdir/t.Context + - nolintlint # a //nolint must name a linter and carry a reason - revive # configurable golint successor formatters: diff --git a/skills/go-lint-setup/SKILL.md b/skills/go-lint-setup/SKILL.md index 3e8fb2b..3a15272 100644 --- a/skills/go-lint-setup/SKILL.md +++ b/skills/go-lint-setup/SKILL.md @@ -16,11 +16,13 @@ Steps: `.golangci.json`. If one exists, do **not** overwrite it: show how it differs from the reference and ask before changing anything. If it's a **v1** config (no `version` key and/or an `enable-all`/top-level `linters:` list), warn that v1 will not parse under golangci-lint v2 and - offer to migrate. + offer to migrate — the supported path is `golangci-lint migrate` (in-place, keeps a `.bck` backup, + drops comments), not a hand-port. 2. **Write** the config below to `.golangci.yml` (or the path given in `$ARGUMENTS`). 3. **Report how to run it:** `golangci-lint run`, and `golangci-lint run --fix` for the auto-fixable - findings (`modernize` + the formatters). Suggest pinning the `golangci-lint` version in CI so the - rule set is reproducible. + findings (`modernize` + the formatters). Suggest pinning an exact `golangci-lint` version in CI in + one place (the action's `version:` input) with an automated bump PR — see `go-linting`; don't + invent a version number here, point at the releases page. Config to write (mirrors `references/golangci.v2.yml` — keep the two in sync): @@ -38,6 +40,8 @@ linters: - contextcheck - containedctx - perfsprint + - usetesting + - nolintlint - revive formatters: enable: diff --git a/skills/go-linting/SKILL.md b/skills/go-linting/SKILL.md index 887b95e..0b3f9a2 100644 --- a/skills/go-linting/SKILL.md +++ b/skills/go-linting/SKILL.md @@ -1,6 +1,6 @@ --- name: go-linting -description: golangci-lint v2 setup and adoption for Go. This skill should be used when the user configures, upgrades, or debugs Go linting — the `.golangci.yml` file, the golangci-lint v2 schema (the versioned config, the `linters.default` set, the separate formatters section), the `modernize` linter, or stack linters (errorlint, bodyclose, rowserrcheck, sqlclosecheck, noctx, contextcheck). Not for what individual idioms mean (use the `go-idioms` skill) or writing rules by hand. +description: golangci-lint v2 setup and adoption for Go. This skill should be used when the user configures, upgrades, or debugs Go linting — the `.golangci.yml` file, the v2 schema (versioned config, `linters.default` set, separate formatters section, `linters.exclusions`), migrating a v1 config, `golangci-lint fmt`, suppressing a finding with `//nolint`, the `modernize` linter, or stack linters (errorlint, bodyclose, noctx, usetesting, …). Not for what individual idioms mean (use `go-idioms`) or writing rules by hand. --- # go-linting — golangci-lint v2 @@ -14,7 +14,13 @@ plugin. Its schema changed from v1 — **v1 config will not parse**: - `linters.default: standard | all | none | fast` selects the base set (no more `enable-all`). `standard` = errcheck, govet, ineffassign, staticcheck, unused. - **Formatters moved to their own `formatters:` section** (gofmt/gofumpt/goimports are no longer - "linters"). + "linters"), with their settings under `formatters.settings`. `golangci-lint fmt` runs that section. +- **Exclusions moved under `linters`**: v1's `issues.exclude-rules` → `linters.exclusions.rules`, + and `issues.exclude-dirs`/`exclude-files` → `linters.exclusions.paths`. `linters-settings` split + into `linters.settings` + `formatters.settings`. +- **Don't hand-port a v1 config — run `golangci-lint migrate`.** It rewrites in place, keeps a + `.golangci.bck.yml` backup, and takes `--format {yml,yaml,toml,json}`. It drops comments and + unknown/deprecated keys, so re-add comments and diff the result. ## Reference config (v2) @@ -32,6 +38,8 @@ linters: - contextcheck # context not propagated - containedctx # context.Context stored in a struct - perfsprint # fmt.Sprintf where a cheaper call exists + - usetesting # os.Setenv/os.Chdir/context.Background in tests → the t.* forms + - nolintlint # a //nolint must name a linter and carry a reason - revive # configurable golint successor formatters: enable: [gofumpt, goimports] @@ -40,21 +48,37 @@ formatters: ## Adoption - Run: `golangci-lint run`; auto-fix what's fixable (incl. `modernize` and formatters): - `golangci-lint run --fix`. -- **Pin the version in CI** for reproducibility (the Cadasto Go repos pin `v2.11.4`); a version - drift silently changes the rule set. + `golangci-lint run --fix`. `golangci-lint fmt` applies only the `formatters:` section. +- **Pin an exact version in CI, in exactly one place — and keep the pin moving.** Upstream's own + recommendation is a specific release, not `latest`: a new release can add or retune linters and + turn every build red at once, with no code change to blame. Put the version in a single source of + truth — the `golangci/golangci-lint-action` `version:` input (it also caches, and beats a plain + binary install) or the install script's tag — never copied across several workflows and Makefiles. + Then let Renovate/Dependabot raise the bump as its own PR, so the version stays current *and* + every rule-set change arrives reviewable. This skill deliberately names no blessed version; read + the changelog for the current line. +- **Install the release binary, not from source.** Upstream states that `go install`/`go get`, the + tools pattern, and `tool` directives "aren't guaranteed to work" — they compile golangci-lint with + whatever local Go version is around. Use the binary, the action, or the Docker image, from a + release built with Go ≥ your module's toolchain (1.26+) so it can parse the language version. +- **Bumping the pin:** run `--fix` first, then either land the leftover findings or add an explicit + `linters.exclusions.rules` entry with a reason. If the pinned build rejects a linter name from the + reference config, the pin is too old — bump it rather than deleting the linter. +- **Suppress narrowly, and say why.** `//nolint:errcheck // best-effort close on a read-only handle` + — never a bare `//nolint` (it disables every linter on that line) and never a blanket + file-level disable where a `linters.exclusions.rules` entry with a path pattern is the honest + answer. `nolintlint` enforces the specific-and-explained form. - `modernize` is the single highest-leverage linter — it operationalizes most `go-idioms` rules on the same engine as gopls/`go fix`, so the plugin's advice stays consistent with the toolchain. As of **Go 1.26** the rewritten `go fix ./...` runs that same modernizer suite from the toolchain - itself; keep `modernize` in golangci-lint for CI reproducibility and to cover toolchains older - than 1.26. Use golangci-lint built with a Go version ≥ the module's toolchain (Go 1.26 support - landed in the golangci-lint releases built with 1.26). + itself; keep `modernize` in golangci-lint so CI enforces it reproducibly against the pinned + version rather than whatever toolchain a developer happens to have. - Adopt the shipped reference config `references/golangci.v2.yml`, or run `/go-lint-setup` to scaffold it into a repo (it won't overwrite an existing config without asking). ## Sources -- golangci-lint docs — -- v2 migration — +- golangci-lint docs — ; v1→v2 migration guide (`migrate`, key moves) — +- v2 announcement (`fmt`, `formatters`) — - `modernize` — --- From 073ac3efc7ffbecdbde3582efd5b5f96bb768b93 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:23:57 +0300 Subject: [PATCH 07/15] docs: record the source registry for refreshing the standards baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-grounding the skills against current Go practice took a full pass over go.dev, pkg.go.dev, Code Review Comments, the Google style guide and the modernize/golangci-lint docs — none of which was written down, so the next refresh would have to rediscover both the sources and the traps. docs/authoring.md gains "Refreshing the standards baseline (source registry)": the sources in three tiers (normative → style guides → enforcing tools), each with what it settles, plus a procedure. The rules that cost the most to learn: - The released Go version is not whatever `go.dev/doc/go1.NN` renders — that page exists in draft for months. Check the release history first; unreleased guidance goes in as one italic, explicitly labelled sentence, never as a rule. - The baseline is a hard floor: state the modern form flat, keep version annotations as provenance, delete guidance below the floor when it moves. - Verify every version gate against pkg.go.dev's "added in go1.NN" marker before writing a `Since` cell. - Re-check tool names — a renamed fixer turns a rule into a wrong command (`waitgroup` → `waitgroupgo`). - Never hardcode a tool version in a component; carry the pin policy instead. - Keep the three copies of the reference lint config in sync. AGENTS.md points at it for refresh requests, so "refresh the Go skills" is enough to re-enter the procedure. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 ++ docs/authoring.md | 56 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index f8fa3f6..e3a6617 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,6 +23,8 @@ This plugin encodes **Go (golang) coding standards**. Guidance must be grounded When a recommendation derives from one of the above, attribute it explicitly and distinguish cited rules from inference. +**Refreshing the skills against current Go practice** (e.g. "check current Go best practices and update the skills"): follow the **source registry and procedure** in [docs/authoring.md](docs/authoring.md#refreshing-the-standards-baseline-source-registry) — it lists every source to re-read, in order, plus the version-gating rules. Re-read them; never refresh from memory. Two recurring traps: the *released* Go version is not whatever `go.dev/doc/go1.NN` renders (check the release history), and the stdlib APIs a skill is missing are usually the ones that landed *after* it was written. + ## Repository Layout This repo supports **both Claude Code and Cursor**. Shared assets (skills, commands, agents) are consumed by both hosts; host-specific manifests and hook configs are kept separate. diff --git a/docs/authoring.md b/docs/authoring.md index 5322706..c069bdb 100644 --- a/docs/authoring.md +++ b/docs/authoring.md @@ -47,6 +47,62 @@ with *empty* metadata (every field silently dropped). `claude plugin validate` c - Imperative voice; explain *why* a rule matters rather than relying on bare MUST/NEVER. Keep skill bodies focused — the always-on cost is the `description`; the body loads on use. +## Refreshing the standards baseline (source registry) + +When asked to *refresh the skills against current Go practice*, re-read these sources in this order +and update the affected skill bodies — do not refresh from memory, and do not add a rule without a +citation. Everything the skills assert should be traceable to one of these. + +**Tier 1 — normative, always check first** + +| Source | URL | What it settles | +|---|---|---| +| Release notes for the baseline version | `https://go.dev/doc/go1.NN` | new APIs/idioms, experiments, removals | +| Release history | | what is actually *released* vs. draft — the baseline claim in AGENTS.md depends on this | +| Package docs | `https://pkg.go.dev/` | exact signatures + the "added in go1.NN" annotation for every version gate | +| Effective Go | | foundational idiom | +| Go Code Review Comments | | the review-rule catalogue (naming, errors, concurrency, API shape) | +| Doc comment syntax | | `gofmt`-formatted doc comments, doc links | + +**Tier 2 — style guides (attribute when a rule comes from one)** + +- Google Go Style Guide — (esp. `/best-practices`: naming, + error handling, panics, option structs, documentation, test structure) +- Uber Go Style Guide — + +**Tier 3 — the enforcing tools (this is what keeps "advice == tooling" true)** + +- `modernize` per-fixer docs — + — the authority for the `go-idioms` **Fixer** column; new fixers land here before the toolchain +- `go fix` (rewritten in Go 1.26) — ; `go tool fix help` lists what the + *installed* toolchain ships +- golangci-lint docs — · v1→v2 migration — + · changelog (for the CI pin) — + +- `go.dev/blog` for feature-specific posts (`synctest`, `testing-b-loop`, `slog`, `range-functions`) + +**Procedure** + +1. Confirm the current *released* Go version (release history) — a draft `go1.NN` page is not a + baseline. Guidance for an unreleased version goes in as one *italic, explicitly labelled* + sentence (`*Go 1.NN (draft, expected …)*`), never as a rule. + **The baseline is a hard floor** (currently **Go 1.26.4+**): recommend the modern form flat, with + no "on 1.NN+ modules prefer…" hedging and no fallback branch for older toolchains. Keep the + version annotation (`Since`, "(Go 1.24)") — that is provenance, and it tells a reader on an older + module what a bump would buy. When the floor moves, delete the guidance below it. +2. Diff each `go-*` skill against Tier 1 for the baseline and the two prior versions — the common + miss is a stdlib API that landed *after* a skill was written (`errors.AsType`, `t.ArtifactDir`). +3. Verify every version gate in `pkg.go.dev`'s "added in" annotation before writing a `Since` cell. +4. Re-check the Tier 3 tool names — a renamed or dropped fixer/linter turns a rule into a wrong + command (`waitgroup` → `waitgroupgo`). + **Never hardcode a tool version in a component.** A named `golangci-lint` release rots within + weeks and nobody remembers why it was chosen; the skills carry the *pin policy* (pin exactly, one + source of truth, automated bump PR) plus the changelog URL, and let the consuming repo own the + number. The same goes for `gopls`/`gofumpt` versions outside `docs/install.md`. +5. Keep the three copies of the reference lint config in sync: `references/golangci.v2.yml`, the + block in `go-linting`, and the block in `go-lint-setup`. +6. Record the refresh in **CHANGELOG.md** under `## [Unreleased]`. + ## Dual-host parity Skills, commands, and agents are shared by both hosts. The **Cursor** manifest From 2e9db171b6adfe28d5eed2e9209d1d57d36b21e8 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:24:06 +0300 Subject: [PATCH 08/15] docs(changelog): record the standards refresh under Unreleased Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f186270..008d3b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,23 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Agents: `go-reviewer` — three review dimensions: **silent dispatch defaults** (pass-through `default` over an internal enum; paired dispatch sites maintained as independent switches), **sensitive-value echo in errors/logs** at a boundary (driver messages quoting stored values, request bodies in wrapped errors), and **comment–code drift** in the diff. - Skills: `go-errors` — fail-loudly-on-impossible-dispatch rule (loud `default` + `exhaustive` linter or enum-completeness test) and boundary-errors-carry-classification-not-payload rule (pass the class/code, keep the raw message internal). - Skills: `go-testing` — golden files pin *shape*, not behaviour: pair a golden of an externally executed artefact (SQL, wire requests, rendered configs) with at least one live-execution test. +- Skills: `go-errors` — `errors.AsType[E]` (Go 1.26) preferred over `errors.As` on 1.26+ modules (`errorsastype` fixer); `%w` goes last unless the sentinel is the sentence; check `Close` on written files (`errors.Join` into a named result); keep the happy path at minimal indentation; never let a panic cross a package boundary. +- Skills: `go-testing` — `t.Context` (1.24), `t.ArtifactDir` (1.26) vs `t.TempDir`, `t.Output`/`t.Attr` (1.25); `t.Setenv`/`t.Chdir`/`cryptotest.SetGlobalRandom` are process-global and unusable under `t.Parallel`; failure messages must carry call/input/got/want; helpers set up, the test body asserts. +- Skills: `go-concurrency` — cancellation causes (`WithCancelCause` + `context.Cause`, `WithTimeoutCause`), `context.WithoutCancel` for work outliving a request, `context.AfterFunc`, prefer-synchronous-APIs, and explicit cleanup over `runtime.AddCleanup`/`SetFinalizer`. +- Skills: `go-idioms` — **Fixer** column naming the owning `modernize` analyzer per row; rows for `any`, `errorsastype`, `omitzero`, `testingcontext`, `stringsseq`, `slicesbackward`, `reflecttypefor`; new "no fixer will do it for you" section (`os.OpenRoot`, `crypto/rand.Text`, nil slices, sorted map iteration). +- Skills: `go-layout` — naming (initialism casing, `MixedCaps`, name-length-tracks-scope, receiver names, no `Get` prefix, `test` doubles), signatures/API surface (receiver type, in-band errors, named results, option struct vs variadic options, accept interfaces/return concrete types, useful zero value), and doc-comment conventions. +- Skills: `go-linting` — `golangci-lint migrate` for v1 configs, `golangci-lint fmt`, the `linters.exclusions`/`formatters.settings` moves, and `//nolint: // reason` discipline. +- Lint config: `usetesting` + `nolintlint` in `references/golangci.v2.yml`, the `go-linting` block, and `/go-lint-setup`. +- Agents: `go-reviewer` — **exported-surface & naming slips** dimension; discarded-`Close`-on-a-written-file folded into resource leaks; `errors.AsType` in sentinel/typed-error breakage. +- Docs: `docs/authoring.md` — "Refreshing the standards baseline (source registry)": tiered source list plus the procedure for re-grounding the skills against current Go practice; **AGENTS.md** points at it for refresh requests. + +### Changed +- Skills / agent / Cursor rule / docs: **Go 1.26.4+ is now a hard floor** — the "works with 1.25+" framing and the per-idiom "check `go.mod` before applying" hedging are gone from `go-coding`, `go-errors`, `go-idioms`, `go-explain`, `go-reviewer`, `rules/go-context.mdc`, `README.md` and `docs/install.md`. Version annotations (`Since`, "(Go 1.24)") stay as provenance. +- Skills: `go-linting`, `/go-lint-setup`, `references/golangci.v2.yml` — no blessed golangci-lint version anywhere; replaced with a pin *policy* (exact version, one source of truth, `golangci-lint-action` `version:` input, automated bump PR, `--fix`-then-triage on bump) and upstream's warning against `go install`/`tool`-directive installs. +- Docs: `docs/authoring.md` — refresh procedure gains the hard-floor rule and "never hardcode a tool version in a component". +- Skills / Cursor rule: `go-layout` widens from project layout to layout **+ naming + API surface**; the `go-coding` router and `rules/go-context.mdc` route naming/doc-comment/API-shape questions there and name `revive` as the deterministic backstop. +- Skills: labelled Go 1.27 forward notes (draft, expected Aug 2026) only where they change advice — `goroutineleak` on by default, `synctest.Sleep` + `httptest.NewTestServer`, new/renamed `go fix` modernizers, `encoding/json/v2`. +- Skills: `go-linting` — note that upstream golangci-lint is on the `v2.12.x` line while the Cadasto repos pin `v2.11.4`. ## [0.3.0] - 2026-07-01 From 80e6db9dcad0be6362088925c065ee9be9d895c6 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:33:33 +0300 Subject: [PATCH 09/15] feat(skills): exit-in-main, init() discipline, terse wrap context; route slash skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two grounded rules the set was missing, plus a routing gap: go-layout: `main` owns process exit — `os.Exit`/`log.Fatal` only in `main`, ideally once on the error from a `run() error` function; a deep `log.Fatal` skips deferred cleanup and makes the path untestable. `init()` is restricted to cheap, deterministic setup — no I/O, no environment reads, no global mutation; anything more is an explicit constructor called from `main`. (Uber: Exit in Main, Avoid init(); Google: program initialization.) go-errors: keep wrap context terse — `"new store: %w"`, not `"failed to create new store: %w"`; "failed to" states the obvious and piles up as the error climbs the stack. (Uber: Error Wrapping.) go-coding: the router now names `/go-explain` and `/go-lint-setup` — the two user-invoked skills were unreachable from the routing surface. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 ++++ skills/go-coding/SKILL.md | 3 +++ skills/go-errors/SKILL.md | 4 +++- skills/go-layout/SKILL.md | 8 +++++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 008d3b1..38038ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Agents: `go-reviewer` — **exported-surface & naming slips** dimension; discarded-`Close`-on-a-written-file folded into resource leaks; `errors.AsType` in sentinel/typed-error breakage. - Docs: `docs/authoring.md` — "Refreshing the standards baseline (source registry)": tiered source list plus the procedure for re-grounding the skills against current Go practice; **AGENTS.md** points at it for refresh requests. +### Added +- Skills: `go-errors` — terse wrap context (`"new store: %w"`, no `"failed to"` pile-up); `go-layout` — `main` owns process exit (`os.Exit`/`log.Fatal` only in `main`, `run() error` pattern) and `init()` restricted to cheap deterministic setup. +- Skills: `go-coding` router — routes to the `/go-explain` and `/go-lint-setup` user-invoked skills. + ### Changed - Skills / agent / Cursor rule / docs: **Go 1.26.4+ is now a hard floor** — the "works with 1.25+" framing and the per-idiom "check `go.mod` before applying" hedging are gone from `go-coding`, `go-errors`, `go-idioms`, `go-explain`, `go-reviewer`, `rules/go-context.mdc`, `README.md` and `docs/install.md`. Version annotations (`Since`, "(Go 1.24)") stay as provenance. - Skills: `go-linting`, `/go-lint-setup`, `references/golangci.v2.yml` — no blessed golangci-lint version anywhere; replaced with a pin *policy* (exact version, one source of truth, `golangci-lint-action` `version:` input, automated bump PR, `--fix`-then-triage on bump) and upstream's warning against `go install`/`tool`-directive installs. diff --git a/skills/go-coding/SKILL.md b/skills/go-coding/SKILL.md index 290e1b7..86410e3 100644 --- a/skills/go-coding/SKILL.md +++ b/skills/go-coding/SKILL.md @@ -42,5 +42,8 @@ tool in the middle column to enforce them. Don't invent rules: each skill cites Dispatch the `go-reviewer` agent — a read-only, context-isolated reviewer that applies the review-heuristics catalog and returns severity-ranked findings on a diff or file. +Two user-invoked skills round out the surface: `/go-explain ` for a one-shot idiom lookup, +and `/go-lint-setup` to scaffold the reference golangci-lint v2 config into a repo. + --- *Top-level structure adapted from [`samber/cc-skills-golang`](https://github.com/samber/cc-skills-golang) (MIT © 2026 Samuel Berthe).* diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index ab747e6..6699090 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -51,7 +51,9 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch logging or API boundary, pass the stable class/code (e.g. SQLSTATE) and keep the raw message internal — the message-content analogue of severing an internal error *type* with `%v`. - **Add context at each layer, log once at the boundary.** Wrapping at every level *and* logging at - every level produces duplicate noise — return wrapped, log at the top. + every level produces duplicate noise — return wrapped, log at the top. Keep the added context + terse: `"new store: %w"`, not `"failed to create new store: %w"` — "failed to" states the obvious + and piles up (`failed to x: failed to y: …`) as the error climbs the stack. - **Error strings:** lowercase, no trailing punctuation (they get wrapped): `"cannot parse %q"`. ## Sources diff --git a/skills/go-layout/SKILL.md b/skills/go-layout/SKILL.md index 64e6568..b7cc3cb 100644 --- a/skills/go-layout/SKILL.md +++ b/skills/go-layout/SKILL.md @@ -20,6 +20,11 @@ exported signature are part of the API — they are as reviewable as the code. no interface-for-everything. - **Hexagonal / ports-and-adapters / DDD is a tool, not a default** — justified for larger services with real external-boundary complexity, overkill for a CLI or a small service. +- **`main` owns process exit.** Call `os.Exit`/`log.Fatal` only in `main` (ideally once, on the + error from a `run() error` function); everything else returns errors. A deep `log.Fatal` skips + deferred cleanup and makes the code path untestable. Same discipline for `init()`: only cheap, + deterministic setup — no I/O, no environment reads, no mutating global state; anything more is an + explicit constructor called from `main`. - **Files:** one package per directory; `package foo` for `foo.go` + `foo_test.go`; use `package foo_test` for black-box tests that exercise only the exported API. @@ -84,7 +89,8 @@ exported signature are part of the API — they are as reviewable as the code. ## Sources - Effective Go — - Code Review Comments (Package/Variable/Receiver Names, Initialisms, Mixed Caps, In-Band Errors, Named Result Parameters, Pass Values, Interfaces, Doc Comments) — -- Google Go Style Guide (naming, option structs, documentation, test doubles) — +- Google Go Style Guide (naming, option structs, documentation, test doubles, program initialization) — +- Uber Go Style Guide (Exit in Main, Avoid init()) — - Doc comment syntax — ; `internal/` — --- From d14b776e8e81f9b2b0e78d5f00c39167d350569c Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:33:56 +0300 Subject: [PATCH 10/15] refactor(skills,agents): apply plugin-dev authoring conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit against the plugin-dev skill-development and agent-development checklists; three classes of drift, none behavioural: go-reviewer description: 313 words of blocks living in always-on context. Converted to the prose-trigger format — conditions, typical triggers, "not for", pointer to the body — with the worked scenarios moved to a "When to invoke" section that only loads when the agent is dispatched. Always-on cost drops by more than half; no trigger scenario is lost. Skill descriptions: go-coding had grown to 111 words and go-explain to 80 against the repo's ~50-75-word always-on budget (docs/authoring.md). Trimmed to 77 each — the router keeps its skill list, since that is the routing map the harness dispatches on, and drops the tool enumeration the body already owns. Imperative form: swept the second-person phrasing out of six skill bodies ("or you get sentinel breakage" → "or the result is sentinel breakage", "where you can" → "where possible", "your API contract" → "the API contract", heading "no fixer will do it for you" → "no fixer automates it"). Agent bodies correctly remain second person — that is the system-prompt convention. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 ++ agents/go-reviewer.md | 50 +++++++++++++++----------------------- skills/go-coding/SKILL.md | 2 +- skills/go-errors/SKILL.md | 6 ++--- skills/go-explain/SKILL.md | 2 +- skills/go-idioms/SKILL.md | 4 +-- skills/go-layout/SKILL.md | 6 ++--- skills/go-linting/SKILL.md | 2 +- skills/go-testing/SKILL.md | 2 +- 9 files changed, 33 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38038ec..d882063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Skills: `go-coding` router — routes to the `/go-explain` and `/go-lint-setup` user-invoked skills. ### Changed +- Agents: `go-reviewer` — frontmatter `description` converted to prose triggers (was ~313 words of `` blocks); worked scenarios moved to a "When to invoke" body section. +- Skills: `go-coding`/`go-explain` descriptions trimmed to the ~50–75-word always-on budget; skill bodies swept to imperative form (second-person phrasing removed). - Skills / agent / Cursor rule / docs: **Go 1.26.4+ is now a hard floor** — the "works with 1.25+" framing and the per-idiom "check `go.mod` before applying" hedging are gone from `go-coding`, `go-errors`, `go-idioms`, `go-explain`, `go-reviewer`, `rules/go-context.mdc`, `README.md` and `docs/install.md`. Version annotations (`Since`, "(Go 1.24)") stay as provenance. - Skills: `go-linting`, `/go-lint-setup`, `references/golangci.v2.yml` — no blessed golangci-lint version anywhere; replaced with a pin *policy* (exact version, one source of truth, `golangci-lint-action` `version:` input, automated bump PR, `--fix`-then-triage on bump) and upstream's warning against `go install`/`tool`-directive installs. - Docs: `docs/authoring.md` — refresh procedure gains the hard-floor rule and "never hardcode a tool version in a component". diff --git a/agents/go-reviewer.md b/agents/go-reviewer.md index e526cf5..9d3c2ac 100644 --- a/agents/go-reviewer.md +++ b/agents/go-reviewer.md @@ -5,37 +5,13 @@ description: > silent error swallowing, goroutine leaks, context misuse, resource leaks (including a discarded `Close` on a written file), sentinel-error breakage, silent dispatch defaults, sensitive-value echo in errors/logs, comment–code drift, unsafe atomics, exported-surface and naming slips, - stale modernization debt, and slog hot-path waste. Invoke it after writing or - changing Go code, before opening a PR, or whenever the user asks for a Go code review. It is - read-only, works alone, and returns severity-ranked findings; it does not edit code or dispatch - other agents. Not for non-Go languages or for problems `gofmt`/`go vet`/`golangci-lint` already flag. - - - Context: The user just finished a Go change and wants it reviewed. - user: "I refactored the worker pool in scheduler.go — can you review it?" - assistant: "I'll dispatch the go-reviewer agent to check the diff for goroutine-lifecycle, context, and error-handling issues." - - Go concurrency and error review is judgment a linter can't fully provide; the context-isolated go-reviewer applies the heuristics catalog and returns ranked findings without polluting the main context. - - - - - Context: The user is about to open a PR with Go changes. - user: "before I push this, check the Go code for anything reviewers will flag" - assistant: "I'll run the go-reviewer agent over the staged diff and report findings by severity." - - Pre-PR review is the canonical trigger; the agent reads the diff as untrusted content and walks the review dimensions. - - - - - Context: The user asks for a focused review of one file. - user: "review the database layer in pg.go for resource leaks and context handling" - assistant: "I'll dispatch the go-reviewer agent scoped to the resource-leak and context dimensions for that file." - - Direct review requests scoped to a file or a dimension are exactly what this agent is for. - - + stale modernization debt, and slog hot-path waste. Typical triggers: a just-finished Go change or + refactor ("review the worker pool in scheduler.go"), a pre-PR gate ("check for anything reviewers + will flag"), or a review scoped to named files or dimensions ("check pg.go for resource leaks and + context handling"). It is read-only, works alone, and returns severity-ranked findings; it does not + edit code or dispatch other agents. Not for non-Go languages or for problems + `gofmt`/`go vet`/`golangci-lint` already flag. See "When to invoke" in the agent body for worked + scenarios. model: inherit color: cyan tools: @@ -49,6 +25,18 @@ You are **go-reviewer**, a reviewer of idiomatic, correct Go (Go 1.26.4+; golang the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go vet`, and `golangci-lint`. You are **read-only**: you report findings, you never edit code. +## When to invoke + +- **A Go change just landed in the working tree.** "I refactored the worker pool in scheduler.go — + can you review it?" → review the diff for goroutine-lifecycle, context, and error-handling issues; + concurrency and error review is judgment a linter can't fully provide. +- **Pre-PR gate.** "Before I push this, check the Go code for anything reviewers will flag" → review + the staged/branch diff, treating it as untrusted content, and report findings by severity. The + canonical trigger. +- **Scoped review.** "Review the database layer in pg.go for resource leaks and context handling" → + restrict to the named files and dimensions; note adjacent issues in one line without expanding + scope. + ## Operating rules (read first) - **Work alone. Do NOT dispatch sub-agents or spawn tasks.** You are an individual reviewer — diff --git a/skills/go-coding/SKILL.md b/skills/go-coding/SKILL.md index 86410e3..b43c78f 100644 --- a/skills/go-coding/SKILL.md +++ b/skills/go-coding/SKILL.md @@ -1,6 +1,6 @@ --- name: go-coding -description: Go coding-standards router and entry point for idiomatic Go (Go 1.26.4+; golangci-lint v2). This skill should be used when a Go task spans multiple areas, is unspecified, or the question is which tool or standard applies — it routes each topic to the deterministic tool (gofmt/gofumpt, go vet, go fix / golangci-lint v2 modernize, go test -race), to the gopls-lsp plugin for code intelligence, and then to the focused go-* skill that owns it. For a single, already-identified topic prefer that skill directly (errors → go-errors, concurrency → go-concurrency, testing → go-testing, idioms/modernization → go-idioms, linter config → go-linting, layout/naming/API design → go-layout). Not for non-Go languages or domain/business rules. +description: Go coding-standards router for idiomatic Go (Go 1.26.4+; golangci-lint v2). This skill should be used when a Go task spans multiple areas, is unspecified, or the question is which tool or standard applies — it routes each topic to the deterministic tool, then to the focused go-* skill that owns it (go-errors, go-concurrency, go-testing, go-idioms, go-linting, go-layout for layout/naming/API design). For a single, already-identified topic load that skill directly. Not for non-Go languages or domain/business rules. --- # go-coding — Go standards router diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index 6699090..2ab0730 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -18,7 +18,7 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch - **The `%v`-where-`%w` trap:** formatting a cause with `%v` discards the chain, so downstream `errors.Is`/`errors.As` silently fail. `errorlint` flags it. - **Inspect with `errors.Is` (sentinel) / `errors.As` (typed)** — never `err == ErrX` or a type - assertion once any layer wraps, or you get *sentinel breakage* (the comparison silently stops + assertion once any layer wraps, or the result is *sentinel breakage* (the comparison silently stops matching). (Go 1.13; `errors.As` target must be a pointer.) - **Prefer `errors.AsType[E]` over `errors.As`** (Go 1.26): `if perr, ok := errors.AsType[*fs.PathError](err); ok { … }`. It is the generic form — @@ -26,11 +26,11 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch mistyped target the way `errors.As` can. `errors.As` is not deprecated, so existing call sites are not bugs; the `errorsastype` modernizer converts them (`go fix ./...`). - **Sentinel errors** (`var ErrNotFound = errors.New("not found")`) for expected, comparable - conditions that are part of your API contract — keep the set small and documented. + conditions that are part of the API contract — keep the set small and documented. **Typed errors** (a struct implementing `error`) when callers need fields (`*PathError`). - **`errors.Join(err1, err2)`** (Go 1.20) to aggregate independent failures (cleanup, validation) — replaces manual concatenation and most third-party multierror use. -- **Never swallow:** no `_ = f()` on an error you care about; no empty `if err != nil {}`. Handle, +- **Never swallow:** no `_ = f()` on an error that matters; no empty `if err != nil {}`. Handle, wrap-and-return, or (deliberately, with a comment) ignore. - **Check `Close` on anything written to.** `defer f.Close()` discards a failed flush — the write looks successful and the file is truncated. Capture it into a named result: diff --git a/skills/go-explain/SKILL.md b/skills/go-explain/SKILL.md index bc3f1d2..09010c6 100644 --- a/skills/go-explain/SKILL.md +++ b/skills/go-explain/SKILL.md @@ -1,6 +1,6 @@ --- name: go-explain -description: One-shot lookup/explanation of a single Go idiom, standard, or tool. This skill should be used when the user runs `/go-explain ` or asks to "explain", "look up", or "what's the modern way to do" a specific Go construct (e.g. error wrapping, `synctest`, `wg.Go`, `min`/`max`, `internal/` layout) — returning the modern form, the enforcing linter, and a cited source. For applying a standard while writing or reviewing code, use the focused go-* skill (`go-errors`, `go-concurrency`, …). Not for non-Go languages. +description: One-shot lookup/explanation of a single Go idiom, standard, or tool. This skill should be used when the user runs `/go-explain ` or asks to "explain", "look up", or "what's the modern way to do" a specific Go construct (e.g. error wrapping, `synctest`, `wg.Go`, `internal/` layout) — returning the modern form, the enforcing linter, and a cited source. For applying a standard while writing or reviewing code, use the focused go-* skill instead. Not for non-Go languages. argument-hint: a Go topic (e.g. error wrapping, synctest, wg.Go, min/max, internal layout) allowed-tools: Read, Grep, Glob --- diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index d9f3e0e..55b72e3 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -6,7 +6,7 @@ description: Modern idiomatic Go (the `modernize` analyzer set). This skill shou # go-idioms — modern Go (modernize) **Advice == tooling.** The `modernize` analyzers flag and usually auto-fix most of what follows — the -**Fixer** column says which, and the last section covers what no fixer will do for you. Run the tool, +**Fixer** column says which, and the last section covers what no fixer automates. Run the tool, don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runner — it ships the modernizer suite in the toolchain itself: @@ -57,7 +57,7 @@ referencing itself in its own type-parameter list (e.g. `type Adder[A Adder[A]] so self-referential constraints no longer need a workaround — but that's a hand-written pattern, not something a modernizer rewrites. -## Modern, but no fixer will do it for you +## Modern, but no fixer automates it - **`os.OpenRoot(dir)` → `*os.Root`** (1.24) for anything that opens a caller-supplied path: its methods cannot escape the directory, including via symlink. Replaces `filepath.Join` plus diff --git a/skills/go-layout/SKILL.md b/skills/go-layout/SKILL.md index b7cc3cb..52095aa 100644 --- a/skills/go-layout/SKILL.md +++ b/skills/go-layout/SKILL.md @@ -13,7 +13,7 @@ exported signature are part of the API — they are as reviewable as the code. - **`internal/` is the one true consensus.** Packages under `internal/` cannot be imported from outside the module subtree — use it to keep implementation private while exporting a small surface. - **Start flat; grow as needed.** A new module is often one package at the root. Add - `cmd//main.go` when you have multiple binaries and `internal//` when you need privacy + `cmd//main.go` when there are multiple binaries and `internal//` when privacy is needed — not before. `golang-standards/project-layout` is community-made, **explicitly not official and contested**; don't treat its deep tree as a starting requirement. - **No Java/C# transplants:** no `*Manager`/`*Impl`/`*Factory` reflexes, no one-type-per-file rule, @@ -67,9 +67,9 @@ exported signature are part of the API — they are as reviewable as the code. two booleans. - **Accept interfaces, return concrete types.** Define an interface in the package that *consumes* it, keep it to a method or three, and return the concrete type so callers get the full surface and - you can add methods without breaking them. + new methods don't break them. - **Prefer synchronous signatures** — let the caller add concurrency (→ `go-concurrency`). -- **Make the zero value useful where you can** (`bytes.Buffer`, `sync.Mutex` need no constructor). If +- **Make the zero value useful where possible** (`bytes.Buffer`, `sync.Mutex` need no constructor). If a type genuinely requires a `New…`, the doc comment must say so. ## Doc comments diff --git a/skills/go-linting/SKILL.md b/skills/go-linting/SKILL.md index 0b3f9a2..bdc3412 100644 --- a/skills/go-linting/SKILL.md +++ b/skills/go-linting/SKILL.md @@ -60,7 +60,7 @@ formatters: - **Install the release binary, not from source.** Upstream states that `go install`/`go get`, the tools pattern, and `tool` directives "aren't guaranteed to work" — they compile golangci-lint with whatever local Go version is around. Use the binary, the action, or the Docker image, from a - release built with Go ≥ your module's toolchain (1.26+) so it can parse the language version. + release built with Go ≥ the module's toolchain (1.26+) so it can parse the language version. - **Bumping the pin:** run `--fix` first, then either land the leftover findings or add an explicit `linters.exclusions.rules` entry with a reason. If the pinned build rejects a linter name from the reference config, the pin is too old — bump it rather than deleting the linter. diff --git a/skills/go-testing/SKILL.md b/skills/go-testing/SKILL.md index dfb1ee3..cabfc9c 100644 --- a/skills/go-testing/SKILL.md +++ b/skills/go-testing/SKILL.md @@ -16,7 +16,7 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, - **Process-global helpers are incompatible with `t.Parallel()`** — `t.Setenv` (Go 1.17), `t.Chdir` (1.24), and `cryptotest.SetGlobalRandom` (1.26) all mutate process state, so they fail in a parallel test *or one with a parallel ancestor*. A table whose cases need env or cwd stays serial; - pass config explicitly instead where you can. The `usetesting` linter pushes `os.Setenv`/`os.Chdir` + pass config explicitly instead where possible. The `usetesting` linter pushes `os.Setenv`/`os.Chdir` in tests towards the `t.*` forms (which restore state via `Cleanup`). - **`t.Context()`** (Go 1.24) for any test needing a `ctx` — it is cancelled just before the test's `Cleanup` functions run, so goroutines under test shut down before teardown asserts on them. Use From 2384684dd1116c220005591d0ea80fb3768c8ecb Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:52:58 +0300 Subject: [PATCH 11/15] fix(skills): re-audit the Fixer column against go1.26.4; ship exhaustive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two must-fix items from the PR #2 review — both verified against the actual go1.26.4 toolchain on this host, not the review text alone. Fixer column vs `go tool fix help` (go1.26.4): the table taught five names the declared minimum toolchain does not register — `waitgroupgo` (1.26 name is `waitgroup`; the rename is 1.27), `errorsastype`, `bloop`, `atomictypes`, `slicesbackward` — plus `appendclipped`, which the review did not flag. New notation: plain = registered in 1.26.4 `go fix`; † = only in the newer x/tools modernize suite (golangci-lint/gopls) so far; `—` = review-only. The 1.27 forward note now reads as "† fixers graduate into the toolchain". Added the `fmt.Appendf`/`fmtappendf` row (ships in 1.26.4, fixer dropped in 1.27) and a verified `go fix` recipe: `-diff` preview on a clean tree, then apply, with per-fixer `-` / `-=false` selection (flags confirmed via `go help fix` — the review's `-name` spelling was close but not exact). exhaustive: taught by go-errors and go-reviewer, shipped by none of the three config copies — enabled in references/golangci.v2.yml, the go-linting block, and /go-lint-setup, restoring advice == tooling. Also from the review: typed-error teaching now leads with `errors.AsType` (single merged bullet in go-errors; `errors.As` kept as not-deprecated with the † tooling caveat), the router/Cursor-rule errors cell says `Is`/`AsType` and runs `--enable-only=errorlint,exhaustive`, and the yml header names both inlined copies it must stay in sync with. Co-Authored-By: Claude Opus 5 (1M context) --- references/golangci.v2.yml | 5 ++-- rules/go-context.mdc | 2 +- skills/go-coding/SKILL.md | 2 +- skills/go-errors/SKILL.md | 17 +++++++------ skills/go-idioms/SKILL.md | 45 +++++++++++++++++++---------------- skills/go-lint-setup/SKILL.md | 1 + skills/go-linting/SKILL.md | 1 + 7 files changed, 39 insertions(+), 34 deletions(-) diff --git a/references/golangci.v2.yml b/references/golangci.v2.yml index 2beaf7e..015cf44 100644 --- a/references/golangci.v2.yml +++ b/references/golangci.v2.yml @@ -5,8 +5,8 @@ # config). Needs a golangci-lint v2 release recent enough to know every linter named below; if it # rejects one, bump the pin rather than dropping the line. Pin an exact version in CI in one place # (see `go-linting` — no version is blessed here on purpose). See the `go-linting` skill for what -# each linter does and why. Keep this in sync with the block inlined in -# `skills/go-lint-setup/SKILL.md`. +# each linter does and why. Keep this in sync with BOTH inlined copies: the reference block in +# `skills/go-linting/SKILL.md` and the scaffold block in `skills/go-lint-setup/SKILL.md`. version: "2" linters: @@ -14,6 +14,7 @@ linters: enable: - modernize # range-int, min/max, slices/maps, wg.Go, strings.Cut … (same engine as go fix) - errorlint # %w + errors.Is/As discipline + - exhaustive # a switch over an enum names every member (go-errors' loud-default rule) - bodyclose # unclosed http.Response.Body - rowserrcheck # unchecked sql.Rows.Err - sqlclosecheck # unclosed sql.Rows/Stmt diff --git a/rules/go-context.mdc b/rules/go-context.mdc index df35a0b..3fa9428 100644 --- a/rules/go-context.mdc +++ b/rules/go-context.mdc @@ -18,7 +18,7 @@ This Cursor rule mirrors the `go-coding` router skill — apply it when editing | Formatting | `gofmt`/`gofumpt` (+ `goimports`) — machine-enforced | — | | Static analysis / bugs | `go vet ./...`, `golangci-lint run` | `go-linting` | | Modern idioms | `go fix ./...` (the toolchain's modernizers), or `golangci-lint run --enable-only=modernize` | `go-idioms` | -| Errors | `golangci-lint run --enable-only=errorlint` | `go-errors` | +| Errors | `golangci-lint run --enable-only=errorlint,exhaustive` | `go-errors` | | Concurrency | `go test -race ./...`, `go vet ./...` | `go-concurrency` | | Testing | `go test -race ./...`; `testing/synctest` for time/concurrency | `go-testing` | | Layout, naming & API surface | `golangci-lint run --enable-only=revive`; rest is judgment | `go-layout` | diff --git a/skills/go-coding/SKILL.md b/skills/go-coding/SKILL.md index b43c78f..855d334 100644 --- a/skills/go-coding/SKILL.md +++ b/skills/go-coding/SKILL.md @@ -20,7 +20,7 @@ Two principles from the project research drive it: | Formatting | `gofmt -l` / `gofumpt -l` (+ `goimports`) — machine-enforced, non-negotiable | — | | Static analysis / likely bugs | `go vet ./...`, `golangci-lint run` | `go-linting` | | Modern idioms (range-int, `min`/`max`, `slices`/`maps`, `wg.Go`, `strings.Cut`, `new(expr)`, `errors.AsType`) | `go fix ./...` (the toolchain's modernizer suite), or `golangci-lint run --enable-only=modernize` for CI reproducibility | `go-idioms` | -| Errors (`%w`, `errors.Is`/`As`, `errors.Join`, sentinel/typed) | `golangci-lint run --enable-only=errorlint` | `go-errors` | +| Errors (`%w`, `errors.Is`/`AsType`, `errors.Join`, sentinel/typed, enum dispatch) | `golangci-lint run --enable-only=errorlint,exhaustive` | `go-errors` | | Concurrency (goroutine leaks, ctx lifecycle, atomics) | `go test -race ./...`, `go vet ./...` | `go-concurrency` | | Testing (table-driven, `t.Parallel`, `t.Context`, `B.Loop`, `testing/synctest`) | `go test -race ./...`; use `testing/synctest` for time/concurrency tests | `go-testing` | | Layout, naming & API surface (`internal/`, initialisms, receiver type, in-band errors, doc comments) | `golangci-lint run --enable-only=revive` (`var-naming`, `receiver-naming`, `exported`), `gofmt` for doc-comment layout; the rest is judgment | `go-layout` | diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index 2ab0730..ffb90cc 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -17,14 +17,13 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch *is* the sentence: `fmt.Errorf("%w: %s", ErrNotFound, key)`. - **The `%v`-where-`%w` trap:** formatting a cause with `%v` discards the chain, so downstream `errors.Is`/`errors.As` silently fail. `errorlint` flags it. -- **Inspect with `errors.Is` (sentinel) / `errors.As` (typed)** — never `err == ErrX` or a type - assertion once any layer wraps, or the result is *sentinel breakage* (the comparison silently stops - matching). (Go 1.13; `errors.As` target must be a pointer.) -- **Prefer `errors.AsType[E]` over `errors.As`** (Go 1.26): - `if perr, ok := errors.AsType[*fs.PathError](err); ok { … }`. It is the generic form — - compile-time-checked target, no pointer to prepare, no reflection, and it cannot panic on a - mistyped target the way `errors.As` can. `errors.As` is not deprecated, so existing call sites are - not bugs; the `errorsastype` modernizer converts them (`go fix ./...`). +- **Inspect with `errors.Is` (sentinel) / `errors.AsType[E]` (typed)** — never `err == ErrX` or a + type assertion once any layer wraps, or the result is *sentinel breakage* (the comparison silently + stops matching). `if perr, ok := errors.AsType[*fs.PathError](err); ok { … }` (Go 1.26): the + generic form — compile-time-checked target, no pointer to prepare, no reflection, cannot panic on + a mistyped target. `errors.As` (Go 1.13) is not deprecated and existing call sites are not bugs — + the `errorsastype` modernizer converts them (via golangci-lint's `modernize`; not yet in the + 1.26.4 toolchain's `go fix`). - **Sentinel errors** (`var ErrNotFound = errors.New("not found")`) for expected, comparable conditions that are part of the API contract — keep the set small and documented. **Typed errors** (a struct implementing `error`) when callers need fields (`*PathError`). @@ -45,7 +44,7 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch - **Fail loudly on impossible dispatch:** a `switch` over an internal enum/kind gets a `default` that returns an error (panic only for the genuinely unreachable) — never a silent pass-through that lets a later-added member ride the weakest arm. Pin exhaustiveness with the `exhaustive` - linter or a completeness test that iterates the enum. + linter (enabled in the reference config) or a completeness test that iterates the enum. - **Boundary errors carry classification, not payload:** upstream messages can embed data values — a database driver quoting the offending stored value, a validator echoing the request body. At a logging or API boundary, pass the stable class/code (e.g. SQLSTATE) and keep the raw message diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index 55b72e3..12eb5bb 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -11,34 +11,37 @@ don't hand-audit. As of **Go 1.26** the rewritten `go fix` is the canonical runn modernizer suite in the toolchain itself: ``` -go fix ./... # applies the toolchain's built-in modernizers -golangci-lint run --enable-only=modernize --fix # any toolchain (same analyzers, via golangci-lint) +go fix -diff ./... # preview the rewrite as a unified diff (clean tree first) +go fix ./... # apply; - runs one, -=false excludes one +golangci-lint run --enable-only=modernize --fix # the x/tools modernize suite — includes the † fixers below ``` -Both draw on the same `golang.org/x/tools` engine as gopls, so their fixes agree. This skill -explains *why* and catches what review notices before the tool runs. The **baseline is Go 1.26.4+**, -so every row below applies as written — the `Since` column is provenance: it explains why older code -looks different, and what an older module would have to bump to before adopting the idiom. +Both draw on the same `golang.org/x/tools` engine as gopls, but golangci-lint pins its own (usually +newer) snapshot of it — that gap is what the **†** marker below tracks. This skill explains *why* +and catches what review notices before the tool runs. The **baseline is Go 1.26.4+**, so every row +below applies as written — the `Since` column is provenance: it explains why older code looks +different, and what an older module would have to bump to before adopting the idiom. ## Prefer → over (since) -The **Fixer** column names the `modernize` analyzer that owns each rewrite — cite it when explaining -or attributing a change, and use `go tool fix help` to see which analyzers the installed toolchain -actually ships (in golangci-lint the whole set is the single `modernize` linter). `—` means no fixer -exists: review has to catch it. +The **Fixer** column names the analyzer that owns each rewrite. Plain = registered in the Go 1.26.4 +toolchain's `go fix` (ground truth: `go tool fix help`; per-fixer docs: `go tool fix help `). +**†** = only in the newer `x/tools` suite so far — golangci-lint's `modernize` and gopls run it, the +1.26.4 toolchain's `go fix` does not. `—` = no fixer exists: review has to catch it. | Prefer | Over | Since | Fixer | |---|---|---|---| | `new(expr)` — e.g. `Field: new(30)`, `new(int64(req.Limit))` | a `ptr[T](v)` helper or a hand-written `tmp := v; &tmp`, for optional/pointer fields | 1.26 | `newexpr` | -| `errors.AsType[E](err)` | `errors.As(err, &target)` → `go-errors` | 1.26 | `errorsastype` | +| `errors.AsType[E](err)` | `errors.As(err, &target)` → `go-errors` | 1.26 | `errorsastype` † | | `for i := range n` | `for i := 0; i < n; i++` | 1.22 | `rangeint` | | `min(a, b)` / `max(a, b)` builtins | hand-rolled helpers | 1.21 | `minmax` | | *(drop)* `x := x` loop-var copy | pre-1.22 capture workaround | 1.22 | `forvar` | | `any` | `interface{}` | 1.18 | `any` | -| `slices.Sort/Contains/Equal`, `slices.Collect`, `maps.Keys`, `slices.Concat` | hand-rolled sort/contains/dedup/append chains | 1.21–1.23 | `slices*`, `mapsloop`, `appendclipped` | -| `for i, v := range slices.Backward(s)` | `for i := len(s)-1; i >= 0; i--` | 1.23 | `slicesbackward` | +| `slices.Sort/Contains`, `slices.Collect`, `maps.Keys` | hand-rolled sort/contains/map loops | 1.21–1.23 | `slicescontains`, `slicessort`, `mapsloop` | +| `for i, v := range slices.Backward(s)` | `for i := len(s)-1; i >= 0; i--` | 1.23 | `slicesbackward` † | | `strings.Cut` / `CutPrefix` / `CutSuffix` | `Index` + manual slicing | 1.18/1.20 | `stringscut`, `stringscutprefix` | | `strings.SplitSeq` / `FieldsSeq` | ranging over `strings.Split`/`Fields` (allocates a slice) | 1.24 | `stringsseq` | +| `fmt.Appendf(b, …)` | `append(b, fmt.Sprintf(…)...)` / `[]byte(fmt.Sprintf(…))` | 1.19 | `fmtappendf` | | `omitzero` on a struct-typed json field | `omitempty`, which does **nothing** for struct fields — a zero `time.Time` still marshals | 1.24 | `omitzero` | | `t.Context()` in tests | `context.WithCancel(context.Background())` → `go-testing` | 1.24 | `testingcontext` | | `reflect.TypeFor[T]()` | `reflect.TypeOf((*T)(nil)).Elem()` | 1.22 | `reflecttypefor` | @@ -47,9 +50,9 @@ exists: review has to catch it. | `iter.Seq[V]` / range-over-func | `Visit(callback)` patterns, exposing slices | 1.23 | `stditerators` | | `slog.LogAttrs(ctx, lvl, msg, attrs…)` on hot paths | key-value variadic `slog` (allocates) | 1.21 | — | | `errors.Join` | manual multi-error concat → `go-errors` | 1.20 | — | -| `wg.Go(...)` | `wg.Add(1)`/`defer wg.Done()` → `go-concurrency` | 1.25 | `waitgroupgo` | -| `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | `bloop` | -| typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | `atomictypes` | +| `wg.Go(...)` | `wg.Add(1)`/`defer wg.Done()` → `go-concurrency` | 1.25 | `waitgroup` | +| `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | `bloop` † | +| typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | `atomictypes` † | Idioms are a moving target — let the tool (pinned to the repo's toolchain) be the source of truth so advice never drifts from the user's `go fix`. Go 1.26 also lifts the ban on a generic type @@ -70,11 +73,11 @@ something a modernizer rewrites. - **`slices.Sorted(maps.Keys(m))`** (1.23) when iterating a map for output — map order is random, and unstable output is a flaky-test and noisy-diff source. -*Go 1.27 (draft, expected Aug 2026) adds the `atomictypes`, `embedlit`, `slicesbackward`, and -`unsafefuncs` fixers to `go fix`, renames `waitgroup` → `waitgroupgo`, and drops `fmtappendf`; it also -lands `encoding/json/v2` + `encoding/json/jsontext` (v1 is reimplemented on v2, opt out with -`GOEXPERIMENT=nojsonv2`), `strings.CutLast`/`bytes.CutLast`, and a stdlib `uuid` package. A -golangci-lint built against newer `x/tools` may carry those fixers before the toolchain does.* +*Go 1.27 (draft, expected Aug 2026) graduates several † fixers into the toolchain's `go fix` +(`atomictypes`, `slicesbackward`, plus new `embedlit` and `unsafefuncs`), renames `waitgroup` → +`waitgroupgo`, and drops `fmtappendf`; it also lands `encoding/json/v2` + `encoding/json/jsontext` +(v1 is reimplemented on v2, opt out with `GOEXPERIMENT=nojsonv2`), `strings.CutLast`/`bytes.CutLast`, +and a stdlib `uuid` package.* ## Sources - `modernize` (per-fixer docs, the Fixer column) — diff --git a/skills/go-lint-setup/SKILL.md b/skills/go-lint-setup/SKILL.md index 3a15272..8957a8b 100644 --- a/skills/go-lint-setup/SKILL.md +++ b/skills/go-lint-setup/SKILL.md @@ -33,6 +33,7 @@ linters: enable: - modernize - errorlint + - exhaustive - bodyclose - rowserrcheck - sqlclosecheck diff --git a/skills/go-linting/SKILL.md b/skills/go-linting/SKILL.md index bdc3412..3e9e970 100644 --- a/skills/go-linting/SKILL.md +++ b/skills/go-linting/SKILL.md @@ -31,6 +31,7 @@ linters: enable: - modernize # highest value: range-int, min/max, slices/maps, wg.Go, strings.Cut… - errorlint # %w + errors.Is/As discipline + - exhaustive # a switch over an enum names every member - bodyclose # unclosed http.Response.Body - rowserrcheck # unchecked sql.Rows.Err - sqlclosecheck # unclosed sql.Rows/Stmt From 4d4316aa05d87c0991d7c9ea4ceac868a3bc1080 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 19:53:12 +0300 Subject: [PATCH 12/15] fix(skills,docs): hard-floor consistency and changelog hygiene per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit go-explain: drop the "tailor down if go.mod says older" branch — the last per-idiom hedge, contradicting the hard-floor commit's own claim. Landing versions stay as provenance for the reader, not a gate on the recommendation. go-testing: trim the synctest.Run digression to a one-line guard ("always synctest.Test — the pre-stable synctest.Run no longer exists"); the guard stays because models trained on pre-1.25 code still emit the removed API. CHANGELOG [Unreleased]: merge the duplicate "### Added" groups; delete the stale bullet claiming go-linting notes the v2.11.4-vs-v2.12.x pin (the pin policy replaced that note before it ever shipped); "preferred on 1.26+ modules" → preferred flat; section title updated to "no fixer automates it"; the go-idioms bullet now describes the †-audited Fixer column and recipe, and the lint-config bullet includes exhaustive. Not taken from the review (explicitly): slog.NewMultiHandler / cryptotest io.Reader note / channel-leak worked example (nice-to-have 9) and further router-description slimming (10) — left for a follow-up if wanted. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 ++++------- skills/go-explain/SKILL.md | 7 +++---- skills/go-testing/SKILL.md | 4 ++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d882063..4c38ab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,17 +13,15 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Agents: `go-reviewer` — three review dimensions: **silent dispatch defaults** (pass-through `default` over an internal enum; paired dispatch sites maintained as independent switches), **sensitive-value echo in errors/logs** at a boundary (driver messages quoting stored values, request bodies in wrapped errors), and **comment–code drift** in the diff. - Skills: `go-errors` — fail-loudly-on-impossible-dispatch rule (loud `default` + `exhaustive` linter or enum-completeness test) and boundary-errors-carry-classification-not-payload rule (pass the class/code, keep the raw message internal). - Skills: `go-testing` — golden files pin *shape*, not behaviour: pair a golden of an externally executed artefact (SQL, wire requests, rendered configs) with at least one live-execution test. -- Skills: `go-errors` — `errors.AsType[E]` (Go 1.26) preferred over `errors.As` on 1.26+ modules (`errorsastype` fixer); `%w` goes last unless the sentinel is the sentence; check `Close` on written files (`errors.Join` into a named result); keep the happy path at minimal indentation; never let a panic cross a package boundary. +- Skills: `go-errors` — `errors.AsType[E]` (Go 1.26) leads typed-error inspection, preferred over `errors.As` (`errorsastype` modernizer converts call sites); `%w` goes last unless the sentinel is the sentence; check `Close` on written files (`errors.Join` into a named result); keep the happy path at minimal indentation; never let a panic cross a package boundary. - Skills: `go-testing` — `t.Context` (1.24), `t.ArtifactDir` (1.26) vs `t.TempDir`, `t.Output`/`t.Attr` (1.25); `t.Setenv`/`t.Chdir`/`cryptotest.SetGlobalRandom` are process-global and unusable under `t.Parallel`; failure messages must carry call/input/got/want; helpers set up, the test body asserts. - Skills: `go-concurrency` — cancellation causes (`WithCancelCause` + `context.Cause`, `WithTimeoutCause`), `context.WithoutCancel` for work outliving a request, `context.AfterFunc`, prefer-synchronous-APIs, and explicit cleanup over `runtime.AddCleanup`/`SetFinalizer`. -- Skills: `go-idioms` — **Fixer** column naming the owning `modernize` analyzer per row; rows for `any`, `errorsastype`, `omitzero`, `testingcontext`, `stringsseq`, `slicesbackward`, `reflecttypefor`; new "no fixer will do it for you" section (`os.OpenRoot`, `crypto/rand.Text`, nil slices, sorted map iteration). +- Skills: `go-idioms` — **Fixer** column naming the owning analyzer per row, audited against Go 1.26.4 `go tool fix help` (plain = in the toolchain's `go fix`; **†** = x/tools/golangci-lint `modernize` only, e.g. `errorsastype`, `bloop`, `atomictypes`, `slicesbackward`); rows for `any`, `omitzero`, `testingcontext`, `stringsseq`, `reflecttypefor`, `fmt.Appendf`; a `go fix` recipe (`-diff` preview, per-fixer `-`/`-=false` selection); new "no fixer automates it" section (`os.OpenRoot`, `crypto/rand.Text`, nil slices, sorted map iteration). - Skills: `go-layout` — naming (initialism casing, `MixedCaps`, name-length-tracks-scope, receiver names, no `Get` prefix, `test` doubles), signatures/API surface (receiver type, in-band errors, named results, option struct vs variadic options, accept interfaces/return concrete types, useful zero value), and doc-comment conventions. - Skills: `go-linting` — `golangci-lint migrate` for v1 configs, `golangci-lint fmt`, the `linters.exclusions`/`formatters.settings` moves, and `//nolint: // reason` discipline. -- Lint config: `usetesting` + `nolintlint` in `references/golangci.v2.yml`, the `go-linting` block, and `/go-lint-setup`. +- Lint config: `usetesting`, `nolintlint` + `exhaustive` in `references/golangci.v2.yml`, the `go-linting` block, and `/go-lint-setup` (`exhaustive` was taught by `go-errors`/`go-reviewer` but not shipped). - Agents: `go-reviewer` — **exported-surface & naming slips** dimension; discarded-`Close`-on-a-written-file folded into resource leaks; `errors.AsType` in sentinel/typed-error breakage. - Docs: `docs/authoring.md` — "Refreshing the standards baseline (source registry)": tiered source list plus the procedure for re-grounding the skills against current Go practice; **AGENTS.md** points at it for refresh requests. - -### Added - Skills: `go-errors` — terse wrap context (`"new store: %w"`, no `"failed to"` pile-up); `go-layout` — `main` owns process exit (`os.Exit`/`log.Fatal` only in `main`, `run() error` pattern) and `init()` restricted to cheap deterministic setup. - Skills: `go-coding` router — routes to the `/go-explain` and `/go-lint-setup` user-invoked skills. @@ -34,8 +32,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Skills: `go-linting`, `/go-lint-setup`, `references/golangci.v2.yml` — no blessed golangci-lint version anywhere; replaced with a pin *policy* (exact version, one source of truth, `golangci-lint-action` `version:` input, automated bump PR, `--fix`-then-triage on bump) and upstream's warning against `go install`/`tool`-directive installs. - Docs: `docs/authoring.md` — refresh procedure gains the hard-floor rule and "never hardcode a tool version in a component". - Skills / Cursor rule: `go-layout` widens from project layout to layout **+ naming + API surface**; the `go-coding` router and `rules/go-context.mdc` route naming/doc-comment/API-shape questions there and name `revive` as the deterministic backstop. -- Skills: labelled Go 1.27 forward notes (draft, expected Aug 2026) only where they change advice — `goroutineleak` on by default, `synctest.Sleep` + `httptest.NewTestServer`, new/renamed `go fix` modernizers, `encoding/json/v2`. -- Skills: `go-linting` — note that upstream golangci-lint is on the `v2.12.x` line while the Cadasto repos pin `v2.11.4`. +- Skills: labelled Go 1.27 forward notes (draft, expected Aug 2026) only where they change advice — `goroutineleak` on by default, `synctest.Sleep` + `httptest.NewTestServer`, † fixers graduating into `go fix`, `encoding/json/v2`. ## [0.3.0] - 2026-07-01 diff --git a/skills/go-explain/SKILL.md b/skills/go-explain/SKILL.md index 09010c6..9fdb3e5 100644 --- a/skills/go-explain/SKILL.md +++ b/skills/go-explain/SKILL.md @@ -18,10 +18,9 @@ Cover, in a few lines: 4. **Source** — cite one authoritative reference: Effective Go, Go Code Review Comments, the Google or Uber Go style guide, a `go.dev/blog` post, or `pkg.go.dev`. -Answer against the **Go 1.26.4+** baseline. Name the version an idiom landed in (that's step 1) so -it's clear what an older module would need, but don't hedge the recommendation — only tailor down if -the repo's `go.mod` actually says older. For a fuller treatment, route to the matching -skill: `go-errors`, `go-concurrency`, +Answer against the **Go 1.26.4+** baseline. Name the version an idiom landed in (that's step 1) — +that is provenance for the reader, not a gate on the recommendation. For a fuller treatment, route +to the matching skill: `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, or `go-layout`. Keep it tight — this is a lookup, not a lecture. If `$ARGUMENTS` is empty, ask what to explain. diff --git a/skills/go-testing/SKILL.md b/skills/go-testing/SKILL.md index cabfc9c..b71bab7 100644 --- a/skills/go-testing/SKILL.md +++ b/skills/go-testing/SKILL.md @@ -40,8 +40,8 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, tickers, retries, `context` cancellation. It runs the bubble on a *fake clock* with deterministic scheduling, so "5-second" waits complete in microseconds and flakiness disappears. Wrap with `synctest.Test(t, func(t *testing.T){ … })`; `synctest.Wait()` blocks until every goroutine in the - bubble is durably blocked. Reach for it instead of `time.Sleep`-based polling. (The pre-1.25 - `GOEXPERIMENT=synctest` API — `synctest.Run` — was removed in Go 1.26; use the stable `synctest.Test`.) + bubble is durably blocked. Reach for it instead of `time.Sleep`-based polling. (Always + `synctest.Test` — the pre-stable `synctest.Run` no longer exists.) *Go 1.27 (draft, expected Aug 2026) adds `synctest.Sleep` (`time.Sleep` + `Wait` in one) and `httptest.NewTestServer`, an in-memory server usable inside a bubble.* - **Fuzzing** (`func FuzzX(f *testing.F)`) for parsers, codecs, and anything consuming untrusted From 62723257f568922d85363b17f760410e37952301 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 20:01:50 +0300 Subject: [PATCH 13/15] feat: validate taught linters against the reference config; verify tools by running them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two guardrails so the PR #2 review findings become structural checks instead of reviewer vigilance. scripts/validate.py gains an advice == tooling cross-check: every linter a component teaches — named in a `--enable-only=...` command or in the phrase "the `` linter" (patterns kept deliberately narrow so a passing mention is not flagged; the phrase regex tolerates hard-wrapped lines) — must be enabled in references/golangci.v2.yml or belong to the v2 `standard` set. Verified both ways: the current tree passes, and re-creating the exact `exhaustive` drift the review caught fails with all four teaching sites named. docs/authoring.md: the refresh procedure gains "run the tool, don't read about it" — on a floor-version toolchain, `go tool fix help` settles fixer names in one command, and `golangci-lint help linters` on the pinned build does the same for linters. The Tier 3 registry row that called pkg.go.dev's modernize page "the authority for the Fixer column" is corrected to what the review proved: that page tracks x/tools tip, so it sources the † rows and is never evidence that a fixer ships in `go fix`; the floor toolchain's own listing is the authority for plain rows. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 3 ++- docs/authoring.md | 14 +++++++++++--- scripts/validate.py | 43 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c38ab7..9fce495 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Lint config: `usetesting`, `nolintlint` + `exhaustive` in `references/golangci.v2.yml`, the `go-linting` block, and `/go-lint-setup` (`exhaustive` was taught by `go-errors`/`go-reviewer` but not shipped). - Agents: `go-reviewer` — **exported-surface & naming slips** dimension; discarded-`Close`-on-a-written-file folded into resource leaks; `errors.AsType` in sentinel/typed-error breakage. - Docs: `docs/authoring.md` — "Refreshing the standards baseline (source registry)": tiered source list plus the procedure for re-grounding the skills against current Go practice; **AGENTS.md** points at it for refresh requests. +- Validation: `scripts/validate.py` — advice == tooling cross-check: every linter a component teaches (`--enable-only=…` or "the `` linter") must be enabled in `references/golangci.v2.yml`; a taught-but-not-shipped linter now fails CI. - Skills: `go-errors` — terse wrap context (`"new store: %w"`, no `"failed to"` pile-up); `go-layout` — `main` owns process exit (`os.Exit`/`log.Fatal` only in `main`, `run() error` pattern) and `init()` restricted to cheap deterministic setup. - Skills: `go-coding` router — routes to the `/go-explain` and `/go-lint-setup` user-invoked skills. @@ -30,7 +31,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Skills: `go-coding`/`go-explain` descriptions trimmed to the ~50–75-word always-on budget; skill bodies swept to imperative form (second-person phrasing removed). - Skills / agent / Cursor rule / docs: **Go 1.26.4+ is now a hard floor** — the "works with 1.25+" framing and the per-idiom "check `go.mod` before applying" hedging are gone from `go-coding`, `go-errors`, `go-idioms`, `go-explain`, `go-reviewer`, `rules/go-context.mdc`, `README.md` and `docs/install.md`. Version annotations (`Since`, "(Go 1.24)") stay as provenance. - Skills: `go-linting`, `/go-lint-setup`, `references/golangci.v2.yml` — no blessed golangci-lint version anywhere; replaced with a pin *policy* (exact version, one source of truth, `golangci-lint-action` `version:` input, automated bump PR, `--fix`-then-triage on bump) and upstream's warning against `go install`/`tool`-directive installs. -- Docs: `docs/authoring.md` — refresh procedure gains the hard-floor rule and "never hardcode a tool version in a component". +- Docs: `docs/authoring.md` — refresh procedure gains the hard-floor rule, "never hardcode a tool version in a component", and "run the tool, don't read about it" (floor-toolchain `go tool fix help` is the authority for shipped fixers; the Tier 3 registry now marks pkg.go.dev's modernize page as x/tools *tip* — source for † rows only). - Skills / Cursor rule: `go-layout` widens from project layout to layout **+ naming + API surface**; the `go-coding` router and `rules/go-context.mdc` route naming/doc-comment/API-shape questions there and name `revive` as the deterministic backstop. - Skills: labelled Go 1.27 forward notes (draft, expected Aug 2026) only where they change advice — `goroutineleak` on by default, `synctest.Sleep` + `httptest.NewTestServer`, † fixers graduating into `go fix`, `encoding/json/v2`. diff --git a/docs/authoring.md b/docs/authoring.md index c069bdb..6368517 100644 --- a/docs/authoring.md +++ b/docs/authoring.md @@ -72,10 +72,12 @@ citation. Everything the skills assert should be traceable to one of these. **Tier 3 — the enforcing tools (this is what keeps "advice == tooling" true)** +- **`go tool fix help` on the floor-version toolchain** — the authority for which fixers `go fix` + actually ships (the plain rows in the `go-idioms` **Fixer** column); `go fix` blog — + - `modernize` per-fixer docs — - — the authority for the `go-idioms` **Fixer** column; new fixers land here before the toolchain -- `go fix` (rewritten in Go 1.26) — ; `go tool fix help` lists what the - *installed* toolchain ships + — tracks x/tools **tip**, usually ahead of the toolchain: the source for **†** rows, never + evidence that a fixer ships in `go fix` - golangci-lint docs — · v1→v2 migration — · changelog (for the CI pin) — @@ -95,6 +97,12 @@ citation. Everything the skills assert should be traceable to one of these. 3. Verify every version gate in `pkg.go.dev`'s "added in" annotation before writing a `Since` cell. 4. Re-check the Tier 3 tool names — a renamed or dropped fixer/linter turns a rule into a wrong command (`waitgroup` → `waitgroupgo`). + **Run the tool, don't read about it:** when a floor-version toolchain is available, + `go tool fix help` settles fixer names in one command — pkg.go.dev's modernize page tracks + x/tools tip, which is usually ahead of what `go fix` ships; same idea for linters + (`golangci-lint help linters` on the pinned build). `scripts/validate.py` cross-checks every + linter taught in components against `references/golangci.v2.yml`, so a + taught-but-not-shipped linter fails CI. **Never hardcode a tool version in a component.** A named `golangci-lint` release rots within weeks and nobody remembers why it was chosen; the skills carry the *pin policy* (pin exactly, one source of truth, automated bump PR) plus the changelog URL, and let the consuming repo own the diff --git a/scripts/validate.py b/scripts/validate.py index cb25190..3fab24b 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -11,7 +11,10 @@ * hook-config JSON validity when present; * SKILL.md / agent / command frontmatter — required keys, and ``name`` matching the directory/filename. Agents MUST declare ``tools:`` (never ``allowed-tools:``, which - Claude Code silently ignores so the agent inherits *all* tools — flagged as an error). + Claude Code silently ignores so the agent inherits *all* tools — flagged as an error); + * advice == tooling: every linter a component *teaches* (via ``--enable-only=...`` or + "the `` linter") must be enabled in ``references/golangci.v2.yml`` — a skill must + not tell agents to rely on a linter no config copy ships. This plugin has no MCP backend, so there is intentionally no ``.mcp.json`` check. @@ -33,6 +36,9 @@ MANIFEST_PATH_FIELDS = ("logo", "rules", "skills", "agents", "commands", "hooks") # Fields that must agree across the Claude and Cursor manifests. SYNCED_FIELDS = ("name", "version", "description", "author") +# The golangci-lint v2 `linters.default: standard` set — enforced without an explicit +# `enable:` entry (see references/golangci.v2.yml). +STANDARD_LINTERS = {"errcheck", "govet", "ineffassign", "staticcheck", "unused"} def err(msg): @@ -143,6 +149,38 @@ def validate_rules(): err(f"{rel}: frontmatter missing 'description'") +def validate_linter_references(): + """Advice == tooling: every linter a component *teaches* must be shipped by the reference + lint config. A linter counts as taught when a component names it as the enforcing tool — + in a ``--enable-only=...`` command, or in the phrase "the `` linter". Guards against + the drift where a skill tells agents to lean on a linter (e.g. ``exhaustive``) that no + config copy actually enables. Deliberately narrow patterns: a passing mention of a linter + name without either signal is not flagged.""" + ref = ROOT / "references" / "golangci.v2.yml" + if not ref.is_file(): + return + # Only the `linters:` section — formatters are a different contract. + linters_section = ref.read_text().split("formatters:")[0] + allowed = set(re.findall(r"^\s+-\s+([a-z0-9-]+)", linters_section, re.MULTILINE)) + allowed |= STANDARD_LINTERS + components = ( + sorted((ROOT / "skills").glob("*/SKILL.md")) + + sorted((ROOT / "agents").glob("*.md")) + + sorted((ROOT / "rules").glob("*.mdc")) + ) + for md in components: + body = md.read_text() + taught = set() + for group in re.findall(r"--enable-only=([a-z0-9,-]+)", body): + taught.update(group.split(",")) + # \s+ so the phrase still matches when hard-wrapped across a line break. + taught.update(re.findall(r"[Tt]he\s+`([a-z0-9-]+)`\s+linter", body)) + for name in sorted(taught - allowed): + err(f"{md.relative_to(ROOT)}: teaches the '{name}' linter but " + f"references/golangci.v2.yml does not enable it — enable it in all three " + f"config copies or stop naming it (advice == tooling)") + + def validate_manifest_paths(manifest: dict, label: str): for field in MANIFEST_PATH_FIELDS: value = manifest.get(field) @@ -203,6 +241,7 @@ def main(): validate_md_components("agents", require_name=True, is_agent=True) validate_md_components("commands", require_name=False) validate_rules() + validate_linter_references() if __name__ == "__main__": @@ -213,4 +252,4 @@ def main(): print(f" - {e}") sys.exit(1) print("OK: manifests, dual-host parity, component paths, kebab-case names, " - "hook configs, skills, agents, commands, and rules are valid") + "hook configs, skills, agents, commands, rules, and taught-linter references are valid") From 01b677b2a2ea1958e4bd232a4b8c298694b3eab4 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 20:07:24 +0300 Subject: [PATCH 14/15] feat: verify the go-idioms Fixer column against the floor toolchain in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the guardrail pair from the PR #2 review response: the linter half was text-vs-text and already enforced; the fixer half needed the toolchain itself, so validate.py now runs `go tool fix help` when a Go toolchain at the floor minor (GO_FLOOR_MINOR = 1.26) is on PATH and checks the go-idioms Fixer column both ways — plain names must be registered in `go fix`, † names must NOT be (a registered † fixer means the marker went stale after a toolchain bump). Any other Go minor soft-skips with a note: another minor's fixer list proves nothing about the floor. Locally the check is opportunistic (no Go → note + skip, keeping the stdlib-only soft-skip philosophy); CI installs Go 1.26.x via setup-go so it runs strictly there. The floor minor now lives in three pinned places — the script constant, the workflow's setup-go version, and the documented baseline — and docs/authoring.md says to move them together. Tested all four paths on go1.26.4: clean pass (21 fixer cells verified), plain-name-not-registered fails (re-created the original `waitgroupgo` bug), stale-† fails (marked shipped `newexpr` as †), and the no-Go skip path notes itself. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/validate.yml | 7 ++++ AGENTS.md | 2 +- CHANGELOG.md | 2 +- docs/authoring.md | 10 +++-- scripts/validate.py | 74 +++++++++++++++++++++++++++++++++- 5 files changed, 89 insertions(+), 6 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index d722d05..45a79b3 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -13,6 +13,13 @@ jobs: - uses: actions/setup-python@v5 with: python-version: '3.x' + # The floor-minor Go toolchain activates the validator's Fixer-column check + # (go tool fix help is the authority for what `go fix` ships). Keep this minor + # in step with GO_FLOOR_MINOR in scripts/validate.py and the documented baseline. + - uses: actions/setup-go@v5 + with: + go-version: '1.26.x' + cache: false # no go.mod — nothing to cache # CI is strict and deterministic: Python is guaranteed here, so run the # validator directly (the scripts/validate.sh graceful skip is for local use). - run: python3 scripts/validate.py diff --git a/AGENTS.md b/AGENTS.md index e3a6617..06a6540 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,7 @@ This repo supports **both Claude Code and Cursor**. Shared assets (skills, comma - **Cursor hooks**: `hooks/cursor-hooks.json` — object `{ "hooks": { "sessionStart": [...], "afterFileEdit": [...] } }`; the command runs from the plugin root (a **workspace-relative** path, **not** `${CLAUDE_PLUGIN_ROOT}`). Present — wires `session-start.sh` (`sessionStart`) and `format-on-save.sh` (`afterFileEdit`). - **Shared hook scripts**: `hooks/session-start.sh` — detects `go.mod` / `*.go`, prints one Go-standards context line, exits 0 always. `hooks/format-on-save.sh` — after a `*.go` Write/Edit, runs `gofumpt -w` (or `gofmt -w -s`) on that single file; resolves the path from `$CLAUDE_FILE_PATH` or the stdin tool-payload JSON, host-only, silent no-op if no formatter is installed, exits 0 always. Both host-agnostic so either manifest can invoke them. Present. - **MCP config** *(optional, not present)*: `.mcp.json` — only if the plugin later integrates an MCP server. There is no companion MCP server today; do not reference one. -- **Validation**: `scripts/validate.sh` wraps `scripts/validate.py` to check both manifests, dual-host parity, declared component paths, kebab-case names, hook-config JSON, and skill/command/agent frontmatter (**agents must use `tools:` not `allowed-tools:`** — flagged as an error). The Python is stdlib-only. `.github/workflows/validate.yml` pins Python and runs the validator strictly. +- **Validation**: `scripts/validate.sh` wraps `scripts/validate.py` to check both manifests, dual-host parity, declared component paths, kebab-case names, hook-config JSON, skill/command/agent frontmatter (**agents must use `tools:` not `allowed-tools:`** — flagged as an error), and two *advice == tooling* invariants: every linter taught in a component is enabled in `references/golangci.v2.yml`, and (when a floor-minor Go toolchain is on PATH — CI installs `1.26.x`, locally it soft-skips) the `go-idioms` Fixer column matches `go tool fix help`. The Python is stdlib-only. `.github/workflows/validate.yml` pins Python + Go and runs the validator strictly. - **Contributor docs**: `docs/` for human-facing references — `install.md`, `testing.md`, `versioning.md`, `authoring.md`. `.github/` holds issue + PR templates, `copilot-instructions.md`, and the CI workflow. (Planning and research working notes are kept locally under `docs/`, **gitignored** — not part of the published plugin.) ### Component surface diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fce495..fedaa09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Lint config: `usetesting`, `nolintlint` + `exhaustive` in `references/golangci.v2.yml`, the `go-linting` block, and `/go-lint-setup` (`exhaustive` was taught by `go-errors`/`go-reviewer` but not shipped). - Agents: `go-reviewer` — **exported-surface & naming slips** dimension; discarded-`Close`-on-a-written-file folded into resource leaks; `errors.AsType` in sentinel/typed-error breakage. - Docs: `docs/authoring.md` — "Refreshing the standards baseline (source registry)": tiered source list plus the procedure for re-grounding the skills against current Go practice; **AGENTS.md** points at it for refresh requests. -- Validation: `scripts/validate.py` — advice == tooling cross-check: every linter a component teaches (`--enable-only=…` or "the `` linter") must be enabled in `references/golangci.v2.yml`; a taught-but-not-shipped linter now fails CI. +- Validation: `scripts/validate.py` — two advice == tooling cross-checks: every linter a component teaches (`--enable-only=…` or "the `` linter") must be enabled in `references/golangci.v2.yml`, and the `go-idioms` **Fixer** column is verified against the floor toolchain's `go tool fix help` (plain names must be registered, † names must not be; soft-skips locally without Go). CI (`validate.yml`) now pins Go `1.26.x` alongside Python to run the Fixer check strictly. - Skills: `go-errors` — terse wrap context (`"new store: %w"`, no `"failed to"` pile-up); `go-layout` — `main` owns process exit (`os.Exit`/`log.Fatal` only in `main`, `run() error` pattern) and `init()` restricted to cheap deterministic setup. - Skills: `go-coding` router — routes to the `/go-explain` and `/go-lint-setup` user-invoked skills. diff --git a/docs/authoring.md b/docs/authoring.md index 6368517..0c6ad07 100644 --- a/docs/authoring.md +++ b/docs/authoring.md @@ -100,9 +100,13 @@ citation. Everything the skills assert should be traceable to one of these. **Run the tool, don't read about it:** when a floor-version toolchain is available, `go tool fix help` settles fixer names in one command — pkg.go.dev's modernize page tracks x/tools tip, which is usually ahead of what `go fix` ships; same idea for linters - (`golangci-lint help linters` on the pinned build). `scripts/validate.py` cross-checks every - linter taught in components against `references/golangci.v2.yml`, so a - taught-but-not-shipped linter fails CI. + (`golangci-lint help linters` on the pinned build). `scripts/validate.py` enforces both + halves: every linter taught in components must be enabled in `references/golangci.v2.yml`, + and — when a floor-minor Go toolchain is on PATH (CI installs `1.26.x`; locally it + soft-skips with a note) — the `go-idioms` Fixer column is verified against + `go tool fix help`: plain names must be registered, † names must not be. The floor minor + lives in `GO_FLOOR_MINOR` in the script and in the workflow's `setup-go` pin — move all + three (docs baseline included) together. **Never hardcode a tool version in a component.** A named `golangci-lint` release rots within weeks and nobody remembers why it was chosen; the skills carry the *pin policy* (pin exactly, one source of truth, automated bump PR) plus the changelog URL, and let the consuming repo own the diff --git a/scripts/validate.py b/scripts/validate.py index 3fab24b..ab4f8a7 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -14,7 +14,10 @@ Claude Code silently ignores so the agent inherits *all* tools — flagged as an error); * advice == tooling: every linter a component *teaches* (via ``--enable-only=...`` or "the `` linter") must be enabled in ``references/golangci.v2.yml`` — a skill must - not tell agents to rely on a linter no config copy ships. + not tell agents to rely on a linter no config copy ships; + * when a Go toolchain at the floor minor (``GO_FLOOR_MINOR``) is on PATH, the go-idioms + **Fixer** column is verified against ``go tool fix help``: plain names must be registered, + † names must not be. Soft-skips locally without Go; CI installs the floor toolchain. This plugin has no MCP backend, so there is intentionally no ``.mcp.json`` check. @@ -23,11 +26,14 @@ """ import json import re +import shutil +import subprocess import sys from pathlib import Path ROOT = Path(__file__).resolve().parent.parent errors = [] +notes = [] PLUGIN_NAME_RE = re.compile(r"^[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?$") KEBAB_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") @@ -39,6 +45,10 @@ # The golangci-lint v2 `linters.default: standard` set — enforced without an explicit # `enable:` entry (see references/golangci.v2.yml). STANDARD_LINTERS = {"errcheck", "govet", "ineffassign", "staticcheck", "unused"} +# The plugin's Go hard floor (minor). The go-idioms Fixer column is verified against THIS +# minor's `go tool fix help` — a newer/older toolchain's list would prove nothing about the +# floor, so the check skips on any other minor. Bump together with the documented baseline. +GO_FLOOR_MINOR = "1.26" def err(msg): @@ -181,6 +191,65 @@ def validate_linter_references(): f"config copies or stop naming it (advice == tooling)") +def validate_fixer_column(): + """Verify the go-idioms **Fixer** column against `go tool fix help` — the authority for + which fixers the floor toolchain's `go fix` ships. Plain fixer names must be registered; + names marked † (x/tools-only) must NOT be — a registered † fixer means the marker went + stale after a toolchain bump. Soft-skips (with a note) when `go` is absent or is not the + floor minor: another minor's list proves nothing about the floor. CI installs Go + {GO_FLOOR_MINOR}.x so the check is strict there.""" + skill = ROOT / "skills" / "go-idioms" / "SKILL.md" + if not skill.is_file(): + return + gobin = shutil.which("go") + if not gobin: + notes.append("Fixer-column check skipped: no `go` on PATH (CI runs it strictly)") + return + try: + ver_out = subprocess.run([gobin, "version"], capture_output=True, text=True, + timeout=30).stdout + except Exception as e: + err(f"`go version` failed: {e}") + return + ver = re.search(r"go(\d+\.\d+)", ver_out) + if not ver or ver.group(1) != GO_FLOOR_MINOR: + notes.append(f"Fixer-column check skipped: toolchain is go{ver.group(1) if ver else '?'}" + f", floor is go{GO_FLOOR_MINOR} (another minor's list proves nothing)") + return + try: + help_out = subprocess.run([gobin, "tool", "fix", "help"], capture_output=True, + text=True, timeout=60) + except Exception as e: + err(f"`go tool fix help` failed: {e}") + return + section = help_out.stdout.split("Registered analyzers:") + if len(section) < 2: + err("`go tool fix help` output has no 'Registered analyzers:' section — " + "cannot verify the go-idioms Fixer column") + return + registered = set(re.findall(r"^\s+([a-z][a-z0-9]*)\b", section[1].split("By default")[0], + re.MULTILINE)) + # Fixer cells are the 4th column of the go-idioms table. + checked = 0 + for line in skill.read_text().splitlines(): + cells = line.split("|") + if len(cells) != 6 or "---" in cells[3] or cells[4].strip() == "Fixer": + continue + cell = cells[4] + daggered = "†" in cell + for name in re.findall(r"`([a-z][a-z0-9]*)`", cell): + checked += 1 + if daggered and name in registered: + err(f"skills/go-idioms/SKILL.md: '{name} †' is stale — " + f"go{ver.group(1)}'s `go fix` registers it; drop the †") + elif not daggered and name not in registered: + err(f"skills/go-idioms/SKILL.md: Fixer column names '{name}' as shipping in " + f"`go fix`, but go{ver.group(1)} `go tool fix help` does not register it — " + f"mark it † (x/tools only) or fix the name") + notes.append(f"Fixer column verified against go{ver.group(1)} `go tool fix help` " + f"({checked} fixer cells)") + + def validate_manifest_paths(manifest: dict, label: str): for field in MANIFEST_PATH_FIELDS: value = manifest.get(field) @@ -242,6 +311,7 @@ def main(): validate_md_components("commands", require_name=False) validate_rules() validate_linter_references() + validate_fixer_column() if __name__ == "__main__": @@ -253,3 +323,5 @@ def main(): sys.exit(1) print("OK: manifests, dual-host parity, component paths, kebab-case names, " "hook configs, skills, agents, commands, rules, and taught-linter references are valid") + for note in notes: + print(f" note: {note}") From 1959db6c23b07203da07579805a61333e7768247 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Wed, 5 Aug 2026 20:07:58 +0300 Subject: [PATCH 15/15] chore(release): v0.4.0 Both manifests to 0.4.0, AGENTS.md status line synced, and the accumulated [Unreleased] notes folded into `## [0.4.0] - 2026-08-05`. The v0.4.0 tag goes on the merge commit once PR #2 lands. Co-Authored-By: Claude Opus 5 (1M context) --- .claude-plugin/plugin.json | 2 +- .cursor-plugin/plugin.json | 2 +- AGENTS.md | 2 +- CHANGELOG.md | 4 ++++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 01bd009..11e4ea4 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "go-coding", - "version": "0.3.0", + "version": "0.4.0", "description": "Idiomatic Go coding standards for AI assistants — formatting, errors, concurrency, testing, layout.", "author": { "name": "Cadasto B.V.", diff --git a/.cursor-plugin/plugin.json b/.cursor-plugin/plugin.json index 56068c8..d02e490 100644 --- a/.cursor-plugin/plugin.json +++ b/.cursor-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "go-coding", - "version": "0.3.0", + "version": "0.4.0", "description": "Idiomatic Go coding standards for AI assistants — formatting, errors, concurrency, testing, layout.", "author": { "name": "Cadasto B.V.", diff --git a/AGENTS.md b/AGENTS.md index 06a6540..2c440fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ This file provides guidance to AI coding assistants (Claude Code, Cursor, and co The **Go Coding Plugin** is an AI plugin by Cadasto B.V. that teaches AI coding assistants **idiomatic Go coding standards** — formatting, naming, error handling, concurrency, testing, and project layout — through skills, commands, agents, hooks, and Cursor rules. It targets **both Claude Code and Cursor** from a single shared component set. -> **Current status — v0.3.0.** A complete dual-host (Claude Code + Cursor) Go-standards set, baselined on **Go 1.26.4+** as a hard floor (no fallback guidance for 1.25 or older; version annotations remain as provenance), that validates clean (`./scripts/validate.sh` + `claude plugin validate .`): the auto-invoked `go-coding` **router** skill; the focused standards skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout`; the read-only `go-reviewer` agent; the user-invoked `/go-explain` and `/go-lint-setup` skills; a shipped `references/golangci.v2.yml`; the `rules/go-context.mdc` Cursor rule; and host-agnostic `session-start` + `format-on-save` hooks. Do not assume a file is present because it is documented here — check first. +> **Current status — v0.4.0.** A complete dual-host (Claude Code + Cursor) Go-standards set, baselined on **Go 1.26.4+** as a hard floor (no fallback guidance for 1.25 or older; version annotations remain as provenance), that validates clean (`./scripts/validate.sh` + `claude plugin validate .`): the auto-invoked `go-coding` **router** skill; the focused standards skills `go-errors`, `go-concurrency`, `go-testing`, `go-idioms`, `go-linting`, `go-layout`; the read-only `go-reviewer` agent; the user-invoked `/go-explain` and `/go-lint-setup` skills; a shipped `references/golangci.v2.yml`; the `rules/go-context.mdc` Cursor rule; and host-agnostic `session-start` + `format-on-save` hooks. Do not assume a file is present because it is documented here — check first. ## Domain Context diff --git a/CHANGELOG.md b/CHANGELOG.md index fedaa09..b7d4478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve ## [Unreleased] +## [0.4.0] - 2026-08-05 + +Grounds the standards set on a **Go 1.26.4+ hard floor**, widens `go-layout` to naming + API surface, replaces the pinned golangci-lint version with a pin *policy*, and makes "advice == tooling" machine-checked (taught linters vs the reference config; the `go-idioms` Fixer column vs the floor toolchain's `go tool fix help`). + ### Added - Agents: `go-reviewer` — three review dimensions: **silent dispatch defaults** (pass-through `default` over an internal enum; paired dispatch sites maintained as independent switches), **sensitive-value echo in errors/logs** at a boundary (driver messages quoting stored values, request bodies in wrapped errors), and **comment–code drift** in the diff. - Skills: `go-errors` — fail-loudly-on-impossible-dispatch rule (loud `default` + `exhaustive` linter or enum-completeness test) and boundary-errors-carry-classification-not-payload rule (pass the class/code, keep the raw message internal).