Write Go

Go Conventions (common)

This is the base layer for Go in every a-novel / a-novel-kit repository. The rules hold whatever the repo kind — a backend service, a shared library, or a one-off tool. Repo-kind rules live in two companion skills; load the one matching where you work in addition to this one:

• write-go-service — clean-architecture services under a-novel (app/service-*, app/platform-*): the cmd/internal/{config,lib,dao,core,handlers,models}/pkg layout, the interface+implementation pattern, transactions, OpenTelemetry instrumentation, REST/gRPC handlers, and the layer-specific test patterns.
• write-go-kit — shared libraries under a-novel-kit (kit/golib, kit/jwt, …): the golib-stays-minimal philosophy, when something earns its own package, and the public/community API obligations that come with it.

When work spans repos and needs versions kept in sync to merge cleanly, also load manage-versions.

Before touching any file, read it. Before touching a package, read its siblings and the interfaces it depends on and exposes. These codebases are deliberately consistent — coherence with the surrounding code outranks personal preference. When two files disagree, the newer one and the one closest to your change usually win; when it is genuinely unclear, ask.

Look up the API before you use it. Before writing code that touches an external package or a non-trivial stdlib API, check the official pkg.go.dev docs and the package's README / release notes. APIs evolve, and the first approach that comes to mind is often not the idiomatic one. Seconds of reading prevent subtle misuse.

After every edit

Run, in this order, after any change to Go files:

pnpm generate:go   # ONLY when you changed an interface or a .proto file — regenerates mocks / proto stubs
pnpm format:go     # always
pnpm lint:go       # always

pnpm format:go / pnpm lint:go exist in every Go repo (they wrap go mod tidy and a pinned golangci-lint invoked through golangci-lint.mod); never skip them. If lint flags something you did not introduce, fix it anyway while you are in the file.

When local lint fails and CI is green on the same commit, clean the cache before you believe it. golangci-lint.mod pins the version, so local and CI run the identical linter and a disagreement is environmental — chasing version drift is wasted effort. A stale analyzer cache is the usual cause: facts about the standard library go stale across toolchain bumps, and once staticcheck can no longer prove (*testing.T).Fatalf terminates, every if x == nil { t.Fatalf(…) } followed by a deref of x reads as a nil dereference. CI never sees it because its runners start cold.

go tool -modfile=golangci-lint.mod golangci-lint cache clean

Re-run after cleaning. Findings that survive a cold run are real; editing code to appease the rest is churn against a false positive.

Then, before the change is done:

1. Invoke write-go-tests — write or update tests for every file you created or modified, and run the narrowest test target that covers the change (a-novel test --type=go -y, or raw go test ./<pkg>/... for one package) until it is green. Tests are part of the change, not a follow-up.
2. Invoke document-code — doc comments for every symbol you added or changed. Also part of the change.

(Repo-kind skills add steps: write-go-service adds the write-openapi step when a REST contract changes; write-go-kit adds the public-API and docs-site obligations for a graduated package.)

Dependencies

The standing goal across both orgs: the fewest libraries doing the most. Every dependency is a maintenance liability, a supply-chain surface, and a constraint on future choices.

• Stdlib first. If net/http, encoding/json, context, errors, fmt, time, crypto/*, slices, maps, cmp, etc. cover the need, use them. Never pull a package for what the standard library already does.
• Reuse before adding. Prefer a package already in go.mod. A second library that overlaps one already present is almost never worth it.
• Mine what's already imported. When implementing, don't re-research online practices — the planning phase (plan-feature) already did that. Do read the documentation of the libraries already in go.mod for the task at hand: a helper, option, or whole subsystem an imported dependency already provides is code you neither write nor maintain.
• Vet candidates. A new dependency must be: well-maintained (recent commits, responsive issue tracker, real test coverage); owned by an organization, not a single personal account — org ownership survives a maintainer losing interest; and broad — pick the library covering the most of the surrounding problem space, so one dependency replaces three. A narrow utility from an individual's account is the worst combination.
• Ask first — always. Introducing a package not already in go.mod requires explicit developer approval. No exceptions, not even a small utility. (a-novel-kit repos hold this bar even higher — see write-go-kit; a-novel services keep their internal/lib/ as close to empty as possible.)
• Remove, don't accumulate. During maintenance, actively look for dependencies — and hand-rolled helpers — that a newer upstream now subsumes, and delete the duplicate.

Package naming

One directory = one package. Package names are short, lowercase, single words — no underscores, no camelCase: package dao, package jwk, package httpf. The name is what every caller types in imports; keep it unambiguous, collision-free, and non-stuttering with the types inside it (jwk.Key, not jwk.JwkKey).

File naming

Multi-word file names are camelCase — never snake_case (master_key.go ✗), never run-together when the name is two words (masterkey.go ✗): masterKeyContext.go, userSearch.go. A test file mirrors the production file it covers with a _test.go suffix (userSearch.go → userSearch_test.go) — see write-go-tests for why the underscore (not a dot) matters and how to share fixtures across packages. The repo-kind skills define the layer/role prefixes (pg.*, rest.*, grpc.*, *.config.go, common.go, …).

Naming conventions

Variables and fields

• Be explicit. userDao, not repo. orderCreateService, not svc. Length is not the cost; ambiguity is.
• Short names are fine for conventional roles only: w / r for HTTP handler params, ctx for context.Context, err for errors, i / k / v in range loops, t for *testing.T.
• Acronyms keep ecosystem casing: ID, URL, JSON, JWT, JWK, HTTP, REST, gRPC, SQL, TTL. An acronym that starts an unexported identifier goes all-lowercase: id, url, grpc.
• Don't shadow imported package names (json, http, context, errors, time, …). Rename the variable.

Constructors

Every type with a constructor uses New<TypeName>(deps...) *<TypeName>. Return the concrete pointer, not an interface — unless the concrete type is genuinely an implementation detail hidden by design (e.g. a pkg/go client behind an interface).

Always the composite-literal form, never new(T):

// WRONG — `new(T)` is a second style for the same thing; mixing the two is noise.
func NewPgUserSelect() *PgUserSelect { return new(PgUserSelect) }

// CORRECT — reads identically whether the struct is empty or has fields.
func NewPgUserSelect() *PgUserSelect              { return &PgUserSelect{} }
func NewUserSearch(r UserSearchDao) *UserSearch { return &UserSearch{dao: r} }

Treat any new(T) you find as cleanup-on-sight when the file is already in scope.

Error handling

• Lowercase, no trailing punctuation. Errors get wrapped and read mid-sentence: errors.New("user not found"), not "User not found.".
• Sentinels for expected outcomes: var ErrUserNotFound = errors.New("user not found"), defined in the same file as the type that produces it (or a shared file if reused across the package). Map an upstream/library error onto your own sentinel by joining it — err = errors.Join(err, ErrUserNotFound) — so callers keep both identities.
• One vocabulary per concept. If the type is Jwk, every error message that refers to it says "jwk …" — never "key not found" in one file and "jwk not found" in another. Wording drift is easy to create and hard to dashboard around. Take the term from the type name and use it everywhere the error surfaces.
• Wrap with context and %w: fmt.Errorf("search users: %w", err). Always %w, never %v, so callers keep errors.Is / errors.As identity. The message chain should let a reader trace the call path.
• Never silently discard an error. If one truly can be dropped, write _ = ... with a comment saying why.

Reporting errors on spans / telemetry — the layer-relative rule

When a function instruments itself with a span (or any other per-operation telemetry), one rule governs reporting: every layer that has a span records, on its own span, every error it sees — whether it raises it, propagates it, or maps it to a transport response. Two moves are forbidden: suppressing reporting based on the error's identity at a propagating layer, and a bare return nil, ErrXxx from a layer that has a span. "Expected" is never a property the error value carries, nor something one layer guesses on a caller's behalf.

• A layer that raises an error (a DAO hitting sql.ErrNoRows, a validator producing ErrInvalidRequest, a service detecting a mismatch) → otel.ReportError(span, err).
• A layer that receives an error and returns it upward → still otel.ReportError. Returning upward is propagating; wrapping it (errors.Join, fmt.Errorf("...: %w", err)) changes nothing.
• The handler that maps the error to a transport response → still reports. golib/httpf.HandleError calls otel.ReportError unconditionally before writing the HTTP status, so the REST handler span records the error whatever status it maps to; the gRPC manual mapping should do the same by hand (_ = otel.ReportError(span, err) before status.Error(...)). The handler span exists to show which error a request ended on.
• Anti-pattern: a helper that suppresses reporting based on the error's identity at a layer that still propagates or surfaces it (a reportUnexpected(span, err) keyed on a list of "known" sentinels). It couples the layer to an error registry and silently drops real signal. The layer-local question is just "did I see this error?" — if yes, report it.

Spans are independent — a child span ending Error does not taint the parent. So the DAO, service, and handler spans all say "no row" for a 404, while the "is the service broken" view is built on the HTTP status code (recorded by the otel HTTP instrumentation), which counts a 404 as a 404. Span status answers "did an error occur in processing", which is true even for a deliberate 404, and that is fine. For bulk-anomaly visibility on a specific security sentinel, use a counter, an audit log, or a dedicated event rather than span.status. The helpers otel.ReportError / otel.ReportSuccess / otel.ReportSuccessNoContent live in golib/otel; ReportError only sets RecordError + SetStatus(Error) — it does not end the span (a defer span.End() does), and returning a sentinel with no Report* call leaves the span Unset, which backends treat as "completed, not a failure". write-go-service covers span naming and the span-per-operation rule.

Context rules

• Never store context.Context in a struct. It is request-scoped and must not outlive the call. Pass it as the first parameter of every method that needs it.
• Contexts flow downward only — caller to callee. Never return one.
• Context values are for request-scoped data that crosses API boundaries — DB transactions, trace spans, auth tokens. They are not a back door for optional arguments.

Time capture

When an operation derives more than one timestamp from "now" — created_at + expires_at, start + deadline, any paired audit fields — capture now := time.Now() once and reuse it:

// WRONG — two wall-clock reads; expires_at - created_at is not exactly the TTL, and a test can't freeze it.
repo.Exec(ctx, &dao.InsertRequest{Now: time.Now(), Expiration: time.Now().Add(cfg.TTL)})

// CORRECT — one logical instant.
now := time.Now()
repo.Exec(ctx, &dao.InsertRequest{Now: now, Expiration: now.Add(cfg.TTL)})

Only call time.Now() more than once when you genuinely want distinct measurements (e.g. computing an elapsed duration).

Secrets and sensitive data

• Never log, trace, or put in span attributes: passwords, tokens, API keys, private keys, key ciphertexts, signed JWTs, or any other credential material. Record identifiers only (user.id, key.id). A redacted "*****" of the same length still leaks the input length over every trace — don't do that either.
• Never return secret material in an error message or a transport response. Errors get logged; responses get cached and indexed.
• Compare secrets in constant time — crypto/subtle.ConstantTimeCompare, never == or bytes.Equal — and on the "not found" branch of an authentication path do the equivalent work (e.g. a throwaway hash comparison) so a lookup miss costs the same as a wrong-secret outcome and timing does not reveal whether the subject exists.

Loop variable scope

These codebases target Go 1.22+, where for loop variables are per-iteration; a closure captures the right value with no extra copy. Remove any current := item shadow copy inside a for range loop when you encounter one — it is dead code on this minimum version.

Common pitfalls

• context.Context stored in a struct. Always a method parameter.
• A new dependency added without asking. Explicit developer approval, every time.
• Logging / tracing secret material. Identifiers only — never the secret.
• Multiple time.Now() for timestamps that should share one instant. Capture once, reuse.
• Suppressing span reporting for an error. Every span'd layer reports every error it sees — no identity-keyed reportUnexpected helper, no bare return nil, ErrXxx from a layer with a span.
• new(T) in a constructor. Use &T{}.
• snake_case or run-together multi-word file names. camelCase.
• Discarding an error without _ = and a why-comment.
• Shadowing an imported package name with a local variable. Rename the variable.