go-doc/data/docs/go-bananas-standards.md

Imported from docs/standards.md in the go-bananas repository.

---

Coding standards

This document is normative. Where it says "must", a change that does not follow it will not merge. Where it says "should", there is a reason to deviate and the reason belongs in the pull request.

The standards exist to make the codebase readable by someone who has never seen it. Every rule here is one that, in review, someone would otherwise have to enforce by hand.

---

Contents

1. General principles
2. Go
3. TypeScript and React
4. CSS
5. API design
6. Error handling
7. Security
8. Testing
9. Database
10. Documentation
11. Commits and pull requests
12. Dependencies
13. Enforcement

---

1. General principles

1.1 Comment the decision, not the mechanism

A comment that restates the code is noise and will rot. A comment that explains why something is done that way is one of the most valuable things in a codebase, because the reasoning is the part that is lost first.

// Bad: restates the code
// Increment the counter
count++

// Good: explains why the obvious thing is wrong
// The window is fixed rather than sliding: a fixed window needs one counter per
// client rather than a list of timestamps, so its memory is O(clients) and a
// client cannot make it degrade by sending requests at a high rate. The cost is a
// burst of up to 2x the limit across a boundary, which is acceptable for the abuse
// it defends against.

Every non-obvious constant, every workaround for a library quirk, and every place where the straightforward implementation would be wrong or insecure gets a comment. Everything else stays uncommented.

1.2 Prefer the boring, explicit solution

This codebase does not need to be clever. It needs to be correct and legible. A reader should never have to reconstruct an invariant from a clever trick.

When two implementations are equally correct, prefer the one that a reviewer can verify in one reading.

1.3 Fail loudly, fail early

A silent fallback that produces a plausible-looking wrong result is worse than a crash, because it will not be found. This is why:

• configuration errors are reported in full at start-up, not one per deploy;
• a malformed theme is rejected with the path that failed;
• the token engine never silently drops a token it cannot parse;
• a development authentication bypass refuses to initialise outside development.

1.4 Handle the boring cases exhaustively

Empty input, zero values, single-element slices, a nil map, a closed channel, a context that is already cancelled. Handle them in one place with a clear rule rather than at each call site.

1.5 Do not leak abstractions

A package exposes what it needs to expose. Internal helpers stay unexported. An interface with one implementation, used from one package, is indirection without benefit — unless it exists to make the caller testable, which is a real reason and is called out in a comment when used.

---

2. Go

2.1 Layout

cmd/go-bananas/       the binary. main() must stay trivial.
internal/
  app/                dependency injection and the process lifecycle
  config/             environment parsing and validation
  domain/             entities and invariants. No HTTP, no SQL.
  store/              persistence. The only package that writes SQL.
  tokenengine/        pure logic: colour maths, validation, rendering
  httpapi/            transport: routing, middleware, handlers
  auth/               authentication and authorisation
  logging/            slog configuration
  seed/               built-in themes

The dependency rule, enforced by review:

cmd → app → { httpapi, store, auth, config, logging }
                    ↓         ↓
              tokenengine  domain ← everything may depend on this

domain depends on nothing. tokenengine depends only on domain. store depends only on domain. httpapi may depend on store, tokenengine and domain, never on app. Nothing imports cmd.

2.2 Naming

Kind	Convention	Example
Exported identifier	CamelCase, with the initialism capitalised	HTTPAddr, ParseColor, IDToken
Unexported identifier	camelCase	themeColumns, renderCache
Interface	Noun, no I prefix	Resolver, KeyStore
Error variable	Err prefix	ErrNotFound
Test double	stub/fake prefix, in _test.go	stubResolver
File	snake_case	token_control.go
Package	One word, lowercase, no underscores	tokenengine, httpapi

Initialisms follow the Go convention exactly: ID, URL, HTTP, API, TLS, CSS, JSON, SQL, UUID. Not Id, not Url.

2.3 Comments

Every exported identifier has a doc comment beginning with the identifier itself:

// ContrastRatio returns the WCAG 2.1 contrast ratio between the two colours,
// always >= 1.
func ContrastRatio(a, b Color) float64 {

Unexported identifiers get a comment when the why is not obvious from the name. A comment restating an unexported function's name is still noise.

2.4 Errors

• Wrap with context and %w so errors.Is works through the chain.
• Error strings are lowercase and do not end in punctuation. They read as part of a sentence: invalid configuration, not Invalid configuration!.
• Sentinel errors live in domain and are matched with errors.Is. Never compare error strings.
• A function returns a bare sentinel when the caller already knows the context, and a wrapped one when it adds information.
• Never discard an error silently. If an error genuinely does not matter, say so in a comment explaining why.

// Good
return nil, fmt.Errorf("open store: %w", err)

// Good: the caller cannot add anything, so return the sentinel unwrapped
return domain.ErrNotFound

// Bad: loses the chain, so errors.Is cannot match it
return nil, fmt.Errorf("failed: %s", err.Error())

2.5 Context

• ctx context.Context is the first parameter of any function that does I/O.
• Never store a context in a struct.
• Every blocking operation takes a context. A network call without one cannot be cancelled, and a request handler that cannot be cancelled is a resource leak on shutdown.

2.6 Concurrency

• Make a type safe for concurrent use, or say clearly that it is not.
• Prefer sync.RWMutex over sync.Mutex when reads dominate.
• Never copy a type containing a mutex.
• Every goroutine must have an owner and a termination condition. A goroutine started in a constructor needs a matching stop path or it leaks on every reload.
• Pre-allocate with a known capacity in hot paths: make([]T, 0, len(src)).

2.7 Zero values

The zero value of a type must be useful or invalid, never subtly wrong. A Cache whose zero value has no map is fine. A RateLimiter whose zero value silently allows everything is not.

2.8 Imports

Standard library first, then third party, then internal, each group separated by a blank line. goimports enforces the grouping; gci enforces the ordering within groups.

Never import a package only to use one constant when a local definition would be clearer. Never import a package in a blank form to check for a build tag without a comment saying why.

2.9 defer

defer immediately after the call that can fail, not at the top of the function. A defer fifty lines below the resource acquisition is a defer that will be attached to the wrong variable.

defer file.Close() and check the error on a read-only close only where it matters. defer rows.Close() needs no error check; defer resp.Body.Close() usually does.

---

3. TypeScript and React

3.1 Strictness

strict, noUncheckedIndexedAccess, noImplicitOverride, noFallthroughCasesInSwitch, noUnusedLocals, noUnusedParameters, verbatimModuleSyntax and isolatedModules are all on. A type error is a build failure, not a warning.

TypeScript is pinned to 6.0.3, not 7.x. TypeScript 7 is the native compiler and ships no JavaScript compiler API, so no type-aware linting tool can use it yet. This is revisited when the ecosystem catches up; see ADR 0004.

What is deliberately not enforced

Two rules are turned off in eslint.config.js, on purpose:

• explicit-function-return-type
• explicit-module-boundary-types

Both demand a written type where tsc --strict already enforces one. A component with a wrongly-typed prop is a compile error either way; what the rules add is annotation churn — one per callback, and (): JSX.Element => on components that legitimately start returning null and then need the annotation widened. That is a maintenance cost with no defect class behind it.

What is enforced instead, and what this means in practice:

Rule	Where
Every component declares a typed props type	tsc
Every exported non-component function has a JSDoc @returns	Review, and the JSDoc the docs are generated from
no-floating-promises, no-misused-promises	ESLint, error
switch-exhaustiveness-check	ESLint, error
no-unnecessary-condition (in web/)	ESLint, error

The one place it is relaxed is the SDK, where no-unnecessary-condition is off because the DOM type definitions describe a fully featured browser and the runtime guards that protect Safari below 14 and embedded webviews are load-bearing. That is recorded in sdk/eslint.config.js next to the rule.

3.2 any is not permitted

any disables checking for everything it touches. Use unknown at a boundary and narrow it. When a genuinely untyped value arrives — a JSON body, a third-party library — declare the shape you expect rather than casting to any.

An any with a comment explaining exactly why it cannot be avoided is acceptable only in a test double or at the boundary of an untyped library.

3.3 Types

• type over interface for unions, mapped types and anything conditional. interface over type for object shapes meant to be extended or implemented.
• Prefer a named union over a union of inline literals. type Appearance = 'light' | 'dark' is a domain concept and deserves a name.
• Model optionality honestly. An optional field means the value may genuinely be absent; a field that is always present should not be marked optional to silence a type error.
• Discriminated unions over optional fields for state that varies together: { status: 'loading' } | { status: 'ready', theme: Theme } cannot be represented wrongly, whereas { status: 'ready', theme?: Theme } can.

3.4 Components

• One component per file unless they are trivially small and only used together.
• Props are typed with an interface or type, never inline.
• A component is a pure function of its props unless it has a stateful reason to exist. If a useMemo is there purely for readability, it is noise.
• Destructure props in the parameter list when more than about three are used.
• Every component that renders user-visible text must be able to render it from a prop or a translation table. Hard-coded English in a leaf component is not acceptable; a page title in the default language is.
• No component over 400 lines. A component that long is doing several jobs and the split is usually along the line of "container" and "presentational".

3.5 Hooks

• Hooks are called at the top level, unconditionally. A hook inside a condition is a bug waiting for the condition to change.
• Every effect that subscribes, times out or fetches returns a cleanup function.
• An effect that only computes derived state is wrong: compute it during render.
• Dependency arrays must be complete. If one looks wrong, the component is probably deriving state that should not be state.
• useMemo and useCallback are for referential stability and genuinely expensive computation, not for clarity. Do not use them to satisfy an exhaustive-deps warning without understanding the consequence.

3.6 State

• Server state lives in TanStack Query, not in useState. A useState holding something a fetcher owns will inevitably disagree with the fetcher.
• Local form state is useState. A form with more than about ten fields, or one that needs validation on blur, belongs in a form library or in a reducer.
• State is declared at the lowest component that needs it and not lifted until two siblings genuinely share it.

3.7 Imports

Alias @/ resolves to src/. Relative imports within a directory, aliased imports across directories. Named imports, never namespace imports, unless the namespace is genuinely used as one.

---

4. CSS

4.1 Tailwind

• Utility classes in JSX. A stylesheet is for things utilities cannot express: keyframes, cascade layers, and generated design tokens.
• Class order follows the Prettier plugin, so formatting is never a review topic.
• Arbitrary values (bg-[#123456]) are a last resort. If the same value appears twice it becomes a theme token.
• Colour is never hard-coded in a component. It comes from a semantic token, which is the entire premise of the project.

4.2 The token layer

The admin UI is itself a consumer of go-bananas. Runtime variables are mapped onto Tailwind's namespaces in index.css and nowhere else. A component that needs a new colour extends the mapping, not its own className.

4.3 Accessibility

• A visible focus indicator on every interactive element. :focus-visible with at least a 2:1 contrast ratio against its background.
• Interactive elements are button, a, input, select or textarea. A div with onClick is not a control: it cannot be focused, does not appear in the accessibility tree, and does not respond to the keyboard.
• Every icon-only control has an accessible name.
• Colour is never the only signal. A status badge has text as well as a colour.
• Motion respects prefers-reduced-motion.
• Interactive targets are at least 44×44 CSS pixels on touch, or have sufficient spacing to the standard.

---

5. API design

5.1 URLs

• Plural nouns for collections, kebab-case for members: /api/v1/themes, /api/v1/themes/{slug}/versions.
• The resource identity is in the path. The query string holds filtering, sorting and pagination.
• PATCH for a partial update, PUT for a full replacement. A partial update must ignore absent fields rather than writing their zero values.

5.2 Status codes

Code	When
200	Success with a body
201	Resource created, with a body
204	Success, no body. A delete or revoke
304	Conditional request matched
400	Malformed or invalid input
401	No usable credential
403	Authenticated but insufficient role
404	Not found, or deliberately not disclosed
409	Uniqueness or optimistic-concurrency conflict
413	Body too large
422	Reserved for a future validation-error shape
429	Rate limited, with Retry-After
500	Unexpected fault

A 404 rather than a 403 is returned when confirming existence would itself leak information. A draft theme returns 404 from the runtime API for exactly this reason.

5.3 Error bodies

One shape everywhere:

{
  "code": "validation_failed",
  "message": "the theme cannot be published because its tokens are invalid",
  "details": [{ "field": "color.semantic.primary", "message": "not a valid colour" }],
  "requestId": "0f90d3cb9ab8e449"
}

code is a stable machine-readable identifier and is part of the contract: clients branch on it, so its meaning must not change between releases. message is for a human. requestId correlates with the server log and is always present on errors.

5.4 Versioning

/api/v1 and /rt/v1 are independent contracts, each with an apiVersion. A breaking change is a new major version served alongside the old one for at least one release cycle. Adding an optional field, a new endpoint or a new query parameter is not a breaking change.

5.5 Pagination

Cursor-based for collections that grow without bound (audit), offset-based for collections a human pages through (themes). Both return { items, total, limit, offset } or the cursor equivalent, so the shape is predictable.

5.6 Idempotency

DELETE and revoke operations are idempotent: repeating them succeeds. A PUT or PATCH that would change nothing still succeeds. Only non-idempotent operations require an idempotency key, and only where a client genuinely cannot retry.

---

6. Error handling

6.1 The layers

Layer	Responsibility
Domain	Returns typed sentinel errors. Knows nothing about HTTP.
Store	Translates driver-specific errors into domain sentinels, once, in one place.
Handlers	Translate domain sentinels into status codes, once, in writeProblem.
Frontend client	Turns a non-2xx into an ApiError carrying the code and the fields.

A handler never chooses a status code itself, and no repository ever matches on an error string.

6.2 Never swallow

A swallowed error is a bug that will be found by a user. If an error genuinely cannot be handled at a call site, log it at the right level and say why in a comment.

// The credentials were already validated, so a failure here means the database is
// unreachable; failing the login would be worse than a missing directory entry.
if err := a.keys.UpsertUser(ctx, identity); err != nil {
	log.Error("record login", "error", err)
}

6.3 Panic

Only for programmer errors: an invariant that cannot be true, or a nil where the type says non-nil. Never for bad input. The HTTP layer recovers from any panic and returns a 500, because a panic on a request goroutine must not take the process down.

---

7. Security

7.1 Secrets

• Secrets come from the environment, injected by Kubernetes from a Secret. Never from a values file, a ConfigMap, a command-line flag or a committed file.
• A secret is never logged, never rendered into a ConfigMap, and never included in an error message. config.Redacted() exists for logging the configuration.
• Anything that generates source for someone else to paste is an output boundary: the value is escaped for the target language, not for HTML, and the language check is on the input rather than trusted.
• crypto/rand for anything security-relevant. Never math/rand.

7.2 Authentication

• Fail closed. An unmapped group is viewer, never admin.
• A development bypass refuses to initialise outside a development environment. This is enforced in code and refused again by the configuration validator, because an accidentally deployed bypass is a total authentication bypass.
• Session cookies are HttpOnly, Secure, SameSite and signed. The signature is compared in constant time.
• The post-login redirect is restricted to a local path. An absolute returnTo is an open redirect that also primes a phishing flow.

7.3 Input

• Every query parameter that reaches SQL goes through an allow-list of column names. Never interpolate a caller's string into a statement.
• LIKE patterns are escaped so a user-supplied % is a literal.
• Request bodies are size-limited. A theme document is a few kilobytes; the limit is 2 MiB.
• JSON with unknown fields is rejected, so a client typo is reported rather than silently ignored.
• Rendered HTML is escaped. Interpolation into a template goes through html.EscapeString even when the value is operator-controlled.

7.4 HTTP

• TLS terminated at the ingress. HSTS advertised only when the deployment is actually served over HTTPS.
• Content-Security-Policy with no unsafe-eval and no wildcard script source.
• X-Content-Type-Options: nosniff and X-Frame-Options: DENY on every response.
• CORS is an explicit allow-list. A wildcard origin with credentials is never valid.
• X-Forwarded-For is believed only from a configured proxy CIDR. Without that, anyone can spoof the client address in the audit log.

7.5 Audit

Every mutation records an actor, an action, a target and a timestamp. Actor comes from the authenticated identity, never from the request body. An audit write failure is logged but does not fail the user's operation: losing an audit row is bad, failing a publish because of it is worse.

---

8. Testing

8.1 What to test

• Domain and tokenengine logic: exhaustively. This is where a subtle mistake is most expensive and most invisible in review.
• Store behaviour: the contract, not the implementation. Uniqueness, cascade behaviour, optimistic concurrency, the error sentinels.
• HTTP handlers: the status code and the error body for each branch, plus the authorisation decision for each role.
• Frontend logic: the pure helpers, exhaustively. Component tests focus on behaviour a user depends on, not on structure.
• Security behaviour: every guard, as a test that fails when the guard is removed. A guard with no test is a guard that will be removed by accident.

8.2 What not to test

Implementation details, private function structure, and anything that would break on a refactor without a behaviour change. A test that fails when a private helper is renamed is a test that will be deleted rather than fixed.

8.3 Style

Table-driven where the cases share a shape. One assertion topic per test, with the assertion message explaining the expectation rather than restating the code. Prefer require over expect for preconditions that make the rest of the test meaningful.

Name the test after the behaviour, not the function: TestContrastRatioMatchesWCAGReferenceValues, not TestContrast.

A test that cannot fail is worse than no test. If a bug is found in production, add the test that would have caught it, in the same change.

8.4 Required coverage

Package	Minimum
tokenengine	90%, including WCAG reference values
domain	90%
store	80%
auth	85%, including every security guard
httpapi	75%, with every authorisation branch
web/src/lib	90%

---

9. Database

• All SQL lives in internal/store. No handler writes SQL.
• Column names are never interpolated from caller input. Sorting and filtering go through an allow-list (domain.SortFields).
• Every foreign key has an explicit ON DELETE behaviour. Cascade is a decision, not an accident.
• Every index exists because a query needs it, and its justification is in the migration comment.
• Timestamps are TEXT in RFC 3339, which is sortable lexicographically, so ORDER BY and range filters need no date parsing.
• Migrations are embedded in the binary and applied at start-up. They must be idempotent: CREATE TABLE IF NOT EXISTS and friends.
• A schema change that is destructive or irreversible gets a snapshot step in the Helm chart and an entry in the runbook.

---

10. Documentation

• A package comment states what the package is for, what it depends on, and what depends on it. Every package has one.
• README.md is for someone who has never seen the project. Its quick start must work as written, from a clean clone.
• docs/standards.md is this document. It changes by the same review as the code.
• An architecture decision that could reasonably have gone another way is recorded in docs/adr/, including the option that was rejected and why. An ADR is never edited to change the decision; it is superseded.
• A comment explaining a workaround names the issue or the version that caused it, so a future reader knows when to remove it.

---

11. Commits and pull requests

11.1 Commit messages

Conventional Commits, because the release notes and the changelog are generated from them.

<type>(<scope>): <subject>

<body>

<footer>

Types: feat, fix, perf, refactor, docs, test, build, ci, chore.

The subject is imperative, under 72 characters, and describes what the change does rather than which files it touches. The body explains why, and what was considered and rejected.

11.2 Pull requests

• One concern per pull request. A refactor and a behaviour change are two pull requests.
• The description states what changed, why, and how it was verified. If a decision was not obvious, it is explained.
• The test plan is concrete: not "tests pass" but "a built-in theme rendering with an unreadable foreground still passes the contrast assertion".
• Screenshots for any visual change.
• Every CI check green. A failing check that is "expected" is not merged.

---

12. Dependencies

• The standard library first. A dependency is a permanent maintenance obligation and a permanent supply-chain surface.
• Every new dependency is justified in the pull request: what it does, why the standard library cannot, and its maintenance status.
• Prefer a dependency already in the tree over a new one for a similar job.
• Pin versions exactly. A caret range in an application is how a build becomes unreproducible.
• go mod tidy and npm ci in CI. Never npm install in a pipeline.
• Renovate or Dependabot handles updates; they merge on a green build.

Current stack, and why each is present, is in docs/architecture.md.

---

13. Enforcement

Rule	Enforced by
Go formatting	gofmt -l, make fmt-check
Go lint	golangci-lint, make lint-go
Go vet	go vet, part of make check
Go import grouping	gci, in .golangci.yml
Go tests, with the race detector	go test -race, make test-go
TypeScript strictness	tsc --build, make lint-web
Frontend lint	eslint --max-warnings 0, make lint-web
SDK lint	eslint, make lint-sdk
Frontend formatting	prettier, make format-web
Frontend tests	vitest, make test-web
Chart validity	helm lint --strict, make helm-lint
Chart values	values.schema.json
Chart configuration	_helpers.tpl guards, at render time
Container build	docker build, make image
Everything	make verify

make verify is the pre-merge gate. It is the same command CI runs.