Write Go

---
name: write-go
description: >
  Apply base Go conventions in either organization: naming, errors, context, time, secrets,
  dependencies, formatting, and lint. Add write-go-service or write-go-kit where applicable.
---

# Go Conventions (common)

Load [develop-feature](../develop-feature/SKILL.md) for drafting, agreement, test timing, and final
cleanup. Use focused checks while the solution is being discussed; complete the relevant regression
suite after issue scope approval. The code-quality rules below apply throughout.

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.

```bash
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`** — after scope approval, complete tests for meaningful changed
   behavior and regression risks, not a quota per file. Run the affected suites
   (`a-novel test --type=go -y`; raw `go test ./<pkg>/...` for focused iteration). Earlier tests
   should be limited to those needed to validate the draft. Coverage is complete before readiness.
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.
- **Fix what govulncheck reports.** A module that pins `govulncheck` in `govulncheck.mod` gets a
  `lint-go` warning for each vulnerability its code reaches, and the daily CI digest collects those
  from the default branch. The report never fails CI. Move the module to its fixed version, or the
  `go` directive for a standard-library finding, and confirm with
  `go tool -modfile=govulncheck.mod govulncheck ./...`.

---

## 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`.
- **Initialisms keep Go's casing**: `ID`, `URL`, `JSON`, `JWT`, `HTTP`, `SQL`, `TTL`, `DSN`, `KID`,
  as in `GcloudProjectID`, `GrpcURL` and `PostgresDSN`. An initialism that _starts_ an unexported
  identifier goes all-lowercase: `id`, `url`.
- **Layer and protocol prefixes are words**: `Rest`, `Grpc`, `Pg`, `Jwk`, `Dao`, as in
  `RestJwkGet`, `GrpcClaimsSign`, `PgJwkSearch` and `JwkSearchDao`. Generated code keeps the casing
  its generator chose, such as protobuf's `GetId`.
- **Don't shadow imported package names** (`json`, `http`, `context`, `errors`, `time`, …).
  Rename the variable.
- **A zero value is a `var` declaration**: `var position int`, not `position := 0`. `:=` introduces
  a value computed on the spot.
- **A default takes the name of the field it fills**, then `Default`: `LayerListLimitDefault` fills
  `LayerListRequest.Limit`. A synonym, as in `LayerListDefaultSize`, gives one value two terms.

### 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)`:

```go
// 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.

---

## Declarations outside control flow

Keep a declaration out of the statement it feeds, so the statement reads as control flow and a name
says what the data is:

- A long literal, such as a table, a slice of structs or a map, written inside a `range`, `if`,
  `switch` or call argument becomes a named variable declared just before the statement.
- A closure longer than three lines becomes a named function or method, unless a closure is the
  idiomatic form there, as for a `t.Run` body, an `errgroup.Go` task or an HTTP handler. In a
  service's dao, core and handlers layers, a closure that takes the context, such as a `WithinTx` body or a
  retried write, stays a closure: the `a-novel-must-open-span` semgrep rule requires a span in every
  named function there, and a wrapper's span only repeats the one below it.

---

## Parameters

A function takes only the parameters it uses. Where a callback signature asks for more, such as
`lo.Map`'s index, wrap the call in a closure instead of adding the parameter to the function:

```go
// WRONG — the index exists only to match lo.Map.
func newCell(entity *dao.Cell, _ int) *Cell { … }

// CORRECT
cells := lo.Map(rows, func(row *dao.CellListRow, _ int) *Cell { return newCell(&row.Cell) })
```

---

## 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.
- **Wrap at the call site.** A helper that runs one call and relabels its error is a
  [pass-through wrapper](../prefer-small-solutions/SKILL.md#shape-the-code-for-a-scanning-reader):
  the label belongs where the call happens.

  ```go
  // WRONG: one more frame to step through, for a label.
  func marshalJSON(v any, cause string) ([]byte, error) {
  	out, err := json.Marshal(v)
  	if err != nil {
  		return nil, fmt.Errorf("marshal(%s): %w", cause, err)
  	}

  	return out, nil
  }

  // CORRECT
  payload, err := json.Marshal(claims)
  if err != nil {
  	return nil, fmt.Errorf("marshal claims: %w", err)
  }
  ```

- **Never silently discard an error.** If one truly can be dropped, write `_ = ...` with a
  comment saying why.

### Reporting errors on spans — once, where they start

Span telemetry follows OpenTelemetry's
[recording-errors guidance](https://opentelemetry.io/docs/specs/semconv/general/recording-errors/).
A trace already links its spans, so each fact is recorded once.

- **Every failed operation reports its error on its own span** with `otel.ReportError(span, err)`,
  whether it raises the error or propagates it. Two moves are forbidden: a bare `return nil, ErrXxx`
  from a function that has a span, and suppressing the report based on the error's _identity_ (a
  `reportUnexpected(span, err)` keyed on "known" sentinels). "Expected" is never a property the error
  value carries.
- **The detail is recorded once.** `ReportError` describes the error on the first span that reports
  it; each span it then propagates through takes the `Error` status alone. Wrapping
  (`fmt.Errorf("...: %w", err)`) keeps that mark, so compare a reported error with `errors.Is`, never
  `==`.
- **Success records nothing.** Leave the status `Unset` and return the value directly.
- **A handled error is not a span error.** When an operation recovers and completes, such as a
  best-effort cleanup that failed, it records nothing: the child span that failed already holds it.
- **A transport handler span fails only on a server fault**, as the OpenTelemetry HTTP and gRPC server
  conventions do. `golib/httpf.HandleError` marks the span for a 5xx and leaves a 4xx unset. A gRPC
  handler answers a server-fault code (`Internal`, `Unavailable`, `Unknown`, `DataLoss`,
  `DeadlineExceeded`) with `grpcf.Fault(code, msg, otel.ReportError(span, err))`, which keeps the
  reported error as the cause the server span's `error.type` names. It returns a client-error code
  (`InvalidArgument`, `NotFound`, …) without a report.
- **No span events.** OpenTelemetry is deprecating them, and a progress marker or exception event
  repeats what the span's status and duration already show.

Spans are independent: a child span ending `Error` does not taint the parent. The DAO and core spans
both end `Error` for a 404 while the handler span stays `Unset`; the "is the service broken" view is
built on the server span's HTTP status code. For bulk-anomaly visibility on a specific security
sentinel, use a counter or an audit log rather than span status. `ReportError` does not end the span;
a `defer span.End()` does. `write-go-service` covers span naming, attributes, 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:

```go
// 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.

---

## Performance traps

[prefer-small-solutions](../prefer-small-solutions/SKILL.md#keep-simple-code-fast) lists the traps
every language shares. Go adds these:

- **Recursion on input depth.** A goroutine stack grows by copying itself. Past the maximum size,
  Go stops with a fatal error that `recover` cannot catch, and the whole service exits. Walk a tree
  shaped by input with an explicit stack:

  ```go
  stack := []*Node{root}

  for len(stack) > 0 {
  	node := stack[len(stack)-1]
  	stack = stack[:len(stack)-1]

  	visit(node)

  	stack = append(stack, node.Children...)
  }
  ```

  Walk files with `fs.WalkDir`; the file system bounds its depth.

- **`defer` inside a loop.** A deferred call runs when the function returns, not when the iteration
  ends, so every file, lock, or span opened in the loop stays open until then. Move the loop body
  into a function that defers its own cleanup.
- **Setup repeated per call.** `regexp.MustCompile`, `template.Must(template.New(…).Parse(…))`, and
  `&http.Client{}` belong in a package variable or the constructor. A client built per call also
  loses connection reuse.
- **Expensive work in a package variable.** A package-level initializer runs in every binary that
  links the package, test binaries and consumers of a published client included. Keep it to cheap
  setup such as a compiled `regexp`. Build an expensive value, such as a password hash, on first use
  with `sync.OnceValue`, and warm it at the entrypoint when the first request must not pay for it.
  `GODEBUG=inittrace=1` prints each package's init time and allocations.
- **A goroutine per item.** An unbounded fan-out has no ceiling on memory or open connections. Use
  `errgroup.Group` with `SetLimit`.
- **`append` into a slice without capacity.** Each time the backing array fills, `append`
  allocates a larger one and copies every element. When the final length is known, allocate once:
  `make([]T, len(source))` assigned by index, or `make([]T, 0, len(source))` when a filter may drop
  items.
- **A slice built to be joined.** Collecting parts into a `[]string` for `strings.Join` allocates the
  slice, grows it, and copies every part a second time. Write the parts into a `strings.Builder`,
  with `Grow` first when the length is easy to bound:

  ```go
  var sb strings.Builder

  sb.WriteString(base)

  for _, segment := range segments {
  	sb.WriteByte('/')
  	sb.WriteString(url.PathEscape(segment))
  }

  return sb.String()
  ```

- **A value encoded through an intermediate.** `json.Marshal` then `w.Write` holds the whole payload
  before sending it, and every `[]byte(s)` or `string(b)` conversion copies the data. Encode into the
  destination, as `golib/httpf.SendJSONStatus` does with `json.NewEncoder(w)`, and keep one
  representation from read to write.
- **A body read whole.** `io.ReadAll` on an untrusted request or response body has no ceiling. Wrap
  the body in `http.MaxBytesReader` or `io.LimitReader` first.
- **A pipe abandoned by an early return.** The writer of an `io.Pipe` or a `cmd.StdoutPipe` blocks
  once nobody reads, and `cmd.Wait` blocks with it. `defer` the reader's `Close` right after
  creating it, or kill the process before `Wait` on a failure path.
- **A shared fetch under a lock or a request context.** A cache refresh that fetches while holding
  its write lock stalls every reader for the fetch's latency. One caller's context also cancels the
  fetch for every waiter. Fetch outside the lock under `context.WithoutCancel(ctx)` with its own
  timeout, then swap the result in.
- **`fmt.Sprintf` to join strings on a hot path.** It boxes every argument into an `any`, and each
  box allocates. Join with `+` or `strings.Builder`. Take one segment with `strings.Cut`, not
  `strings.SplitN`.

`time.After` inside a loop is not a leak on Go 1.23 and later: the runtime collects an unreferenced
timer. Do not replace it with a `Ticker` for that reason; a ticker changes the pacing.

Package-level variables, constant tables, and startup code keep `strings.Join`, `fmt.Sprintf`, and
`+`. They run once, so a builder there costs readability and saves nothing.

---

## Correctness traps

Before you write or review code that shares state across goroutines, runs a subprocess, serves a
stream or a unix socket, or builds a URL or DSN from parts, read
[correctness traps](references/correctness-traps.md). It lists the race, cancellation and
stdlib pitfalls that reached `master` once, each with the fix that held.

---

## 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 returns — no
  identity-keyed `reportUnexpected` helper, no bare `return nil, ErrXxx` from a layer with a span.
- **`new(T)` in a constructor.** Use `&T{}`.
- **A parameter added only to fit a callback's signature.** Wrap the call in a closure.
- **snake_case or run-together multi-word file names.** camelCase.
- **Discarding an error without `_ =` and a why-comment.**
- **A helper that only relabels one call's error.** Wrap at the call site.
- **A `[]string` built for one `strings.Join` in per-request code.** Write into a `strings.Builder`.
- **Shadowing an imported package name with a local variable.** Rename the variable.