agentleFS
a-novel-kita-novel-kit/stack/.agents/skills/write-go/SKILL.md

write-go

Skill2 starsChanged 4 days agoMITView on GitHub

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.

The file

Show its rules and text

We split this skill into the rules it gives an agent

20 rules. About 0 are common, 6 are distinctive, and 14 are very specific to this project.

  • ≈0

    Common · estimate

    Many files like this have a rule of this kind.

  • ≈6

    Distinctive · estimate

    This author's own choices. Worth a look.

  • 14

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  1. Distinctive:

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

    line 27
  2. Distinctive:

    Before touching any file, read it: Before touching a package, read its siblings and the interfaces it depends on and exposes.

    line 30
  3. Very specific:

    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 …

    line 35
  4. Distinctive:

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

    line 44
  5. Very specific:

    Stdlib first: If net/http, encoding/json, context, errors, fmt, time, crypto/*, slices, maps, cmp, etc.

    line 90
  6. Very specific:

    Reuse before adding: Prefer a package already in go.mod.

    line 93
  7. Very specific:

    Mine what's already imported: When implementing, don't re-research online practices — the planning phase (plan-feature) already did that.

    line 95
  8. Very specific:

    Every type with a constructor uses New<TypeName>(deps...) *<TypeName>.

    line 161
  9. Very specific:

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

    line 176
  10. Distinctive:

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

    line 182
  11. Very specific:

    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 …

    line 218
  12. Very specific:

    Never silently discard an error: If one truly can be dropped, write _ = ...

    line 247
  13. Very specific:

    Success records nothing: Leave the status Unset and return the value directly.

    line 265
  14. Distinctive:

    A handled error is not a span error: When an operation recovers and completes, such as a best-effort cleanup that failed, it records …

    line 266
  15. Very specific:

    When an operation derives more than one timestamp from "now" — created_at + expires_at, start + deadline, any paired audit fields — capture …

    line 298
  16. Very specific:

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

    line 310
  17. Distinctive:

    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 …

    line 423
  18. Very specific:

    context.Context stored in a struct: Always a method parameter.

    line 432
  19. Very specific:

    Suppressing span reporting for an error: Every span'd layer reports every error it returns — no identity-keyed reportUnexpected helper, no …

    line 436
  20. Very specific:

    new(T) in a constructor: Use &T{}.

    line 438

≈ “Common” is an estimate: at least 1 in 5 of the 350,495 skill files we read have a rule of the same kind, not the same words. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

Go Conventions (common)

Load develop-feature 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.

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

// 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:

// 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: the label belongs where the call happens.

    // 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. 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:

// 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 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:

    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:

    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. 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.

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.