Go code review standards

# Go code review standards

Follow these standards when writing or changing code in this repository. Code review flags changes that break them.

## Handle and Wrap Every Error with Context in Go

Applies to `**/*.go`.

Review Go error handling for errors that are dropped, masked, or stripped of the context callers need.

Flag changes that:
- Discard an error with `_ =` or `_` in a multi-value assignment, then use the other results as if the call succeeded.
- Check `err != nil` and only log it, then continue with a zero, nil, or partial value instead of returning or handling the failure.
- Return a nil error from a failure branch, such as `if err != nil { return nil, nil }`, or return an outer `err` that is nil because the failing call assigned to a different variable.
- Shadow `err` with `:=` inside an `if`, `for`, or closure, so the error never reaches the outer variable that is checked, returned, or read by a deferred function.
- Wrap with `fmt.Errorf` using `%v`, `%s`, or `err.Error()` where callers need the cause, which breaks `errors.Is` and `errors.As`.
- Compare errors with `==`, type-assert them with `err.(*T)`, or match `err.Error()` text when the error may be wrapped, instead of `errors.Is` or `errors.As`.
- Call `log.Fatal` or `os.Exit` in library or request-handling code, which exits without running deferred functions, or `panic` on an expected failure such as bad input or an unavailable dependency.

Do not flag:
- Ignored errors from calls documented never to fail, such as writes to `bytes.Buffer`, `strings.Builder`, or `hash.Hash`.
- `err == io.EOF` on direct `io.Reader` results, which return `io.EOF` unwrapped by contract.
- `%v` used on purpose at a package boundary to keep an underlying error out of the public API.
- An ignored error from a deferred `tx.Rollback()` that runs after `tx.Commit()`.

## Prevent Goroutine Leaks in Go

Applies to `**/*.go`.

Review goroutines, channels, and tickers for goroutines that can block or run forever after their work is no longer needed.

Flag changes that:
- Start a goroutine whose loop or blocking receive has no exit path: no `ctx.Done()` case, no closed channel to end a `range`, and no return condition.
- Send on an unbuffered channel from a goroutine whose receiver can return first, for example after a `select` timeout, a `ctx.Done()` case, or an early error return, leaving the sender blocked forever.
- `range` over a channel that the producer never closes, or closes only on some paths.
- Create a `time.NewTicker` without `defer ticker.Stop()` in a module whose `go.mod` declares a Go version below 1.23, where an unstopped ticker is never garbage collected.
- Start goroutines per request or per item with no `sync.WaitGroup`, `errgroup.Group`, or result channel the caller waits on, so work outlives the request and its errors are lost.
- Spawn one goroutine per element of an unbounded input without a limit such as `errgroup.Group.SetLimit` or a fixed worker pool.
- Call `wg.Done()` without `defer`, so an early return or panic skips it and `wg.Wait()` blocks forever.

Do not flag:
- Goroutines started once in `main` or server setup that are meant to run for the life of the process.
- Sends on a channel buffered for every sender, such as `make(chan result, 1)` for a single worker.
- Goroutines joined through `errgroup` or `sync.WaitGroup` where every path reaches `Wait` and the work observes the group's context.

## Propagate context.Context Through Go Request Paths

Applies to `**/*.go`.

Review Go request paths for a context.Context that is dropped, replaced, or never given a deadline.

Flag changes that:
- Call `context.Background()` or `context.TODO()` inside an HTTP handler, gRPC method, or any function that already receives a `ctx` or `*http.Request`, instead of passing the incoming context or `r.Context()`.
- Accept a `ctx` parameter and then call downstream I/O without it.
- Use a call that cannot be canceled when a context variant exists: `db.Query`, `db.Exec`, or `db.QueryRow` instead of `QueryContext`, `ExecContext`, or `QueryRowContext`; `http.Get` or `http.NewRequest` instead of `http.NewRequestWithContext`; `exec.Command` instead of `exec.CommandContext`.
- Make outbound calls through `http.DefaultClient` or an `http.Client` with no `Timeout` when no caller deadline bounds the request.
- Retry or poll in a loop with `time.Sleep` that never checks `ctx.Done()` or `ctx.Err()`, so cancellation cannot stop it.
- Start background work from a handler with the request context, which is canceled when the handler returns, instead of `context.WithoutCancel` plus its own timeout.
- Store a `context.Context` in a struct field instead of passing it as the first parameter.

Do not flag:
- `context.Background()` in `main`, `init`, tests, and at the top of workers or CLI commands that create the root context.
- Deliberate detachment with `context.WithoutCancel` or a new root context for work that must outlive the request, when it sets its own timeout.
- Calls to libraries that expose no context-aware API.

## Prevent Data Races on Shared State in Go

Applies to `**/*.go`.

Review Go code that shares state across goroutines for reads and writes without synchronization.

Flag changes that:
- Read or write a map from more than one goroutine (HTTP handlers, `go` statements, callbacks, package-level caches) without a `sync.Mutex`, `sync.RWMutex`, or `sync.Map`.
- Update counters, flags, or package-level variables shared across goroutines with plain `++`, `+=`, or assignment instead of a mutex or a `sync/atomic` type such as `atomic.Int64`.
- `append` to a shared slice from multiple goroutines, such as `results = append(results, r)` inside `go func()`.
- Guard a field with a mutex in some methods but read or write it without the lock in others, including getters and `len` calls.
- Return or keep a reference to a mutex-guarded map, slice, or pointer after unlocking, so callers read it while another goroutine writes it.
- Check and act in separate critical sections, such as reading under `RLock`, unlocking, and then writing based on the stale read.

Do not flag:
- Closures that capture loop variables in modules whose `go.mod` declares `go 1.22` or later, where each iteration has its own variable.
- Goroutines writing distinct indexes of a pre-sized slice (`results[i] = r`) when the slice is read only after `Wait`.
- State written before the goroutines start and only read afterward.

## Close Resources and Check Deferred Close Errors in Go

Applies to `**/*.go`.

Review Go code that opens connections, files, and writers for resources that are not released and errors lost at close.

Flag changes that:
- Get an `*http.Response` without closing `resp.Body` on every path, including early returns on a non-2xx status placed before `defer resp.Body.Close()`.
- Call `Query` or `QueryContext` without `defer rows.Close()` right after the error check, so an early return before or inside the loop keeps a pool connection checked out.
- Loop over `rows.Next()` without checking `rows.Err()` afterward, so a driver or network error mid-iteration looks like a shorter result.
- Put `defer` inside a `for` loop to close files, rows, or response bodies, or to unlock a mutex, so every iteration holds its resource until the function returns.
- Ignore the error from `Close` on files and writers opened for writing (`os.Create`, `os.OpenFile` with write flags, `gzip.Writer`, `zip.Writer`), from `bufio.Writer.Flush`, or from `csv.Writer.Error()` after `Flush`.
- Begin a `sql.Tx` without `defer tx.Rollback()` or an explicit rollback on every error path.

Do not flag:
- Ignored errors from `resp.Body.Close()` or from `Close` on files opened read-only with `os.Open`.
- `defer tx.Rollback()` followed by `tx.Commit()`; the deferred rollback returns `sql.ErrTxDone` after a commit and is harmless.
- Resources returned to the caller when the function contract makes the caller responsible for closing them.

## Flag Security Vulnerabilities

Scan for security flaws: hardcoded secrets, SQL injection, XSS, weak auth, exposed data, poor crypto, unsafe deserialization, insecure HTTP calls in production, and more. Ensure all network requests use HTTPS/TLS. Identify attack vectors and security violations to maintain robust defenses.

## Require API Auth, Validation, and Tenant Scoping

Review API handlers and controller endpoints for missing authentication, authorization, tenant scoping, and request validation.

Flag new or modified endpoints that:
- Accept request bodies without runtime validation, such as Zod schemas, DTOs, or framework-native validators.
- Trust user-supplied identifiers for tenant, organization, or account access instead of deriving access from the authenticated session.
- Return sensitive internal errors or use inconsistent HTTP status codes for 400, 401, 403, 404, and 500 cases.
- Mix reads and writes in a way that makes side effects hard to reason about.
- Add public or high-volume endpoints without rate limiting or abuse controls.

## Prevent Sensitive Data in Logs

Logging, error reporting, analytics events, and audit trails must not expose sensitive data. Flag code that logs PII, emails, passwords, API keys, auth tokens, session identifiers, payment data, or raw request/response bodies without redaction. Error serialization must preserve enough context to debug while sanitizing secrets and user data.

## Prevent Database Query Performance Regressions

Flag database access patterns that can degrade production performance, including N+1 queries, unbounded list endpoints, missing pagination, full-table scans, missing indexes on selective filters, per-row writes in loops, and queries that load more columns or rows than needed. Prefer bounded reads, batching, selective indexes, and query shapes that scale with table size.

## Block Unsafe Database Migrations

Review database migrations for operations that can lock large tables, rewrite full tables, run long backfills, create blocking indexes, or require full table scans. Prefer phased rollouts, concurrent indexes, nullable columns followed by backfills, batched data changes, and explicit rollback or recovery plans for risky schema changes.

## Enforce Pragmatic Test Coverage

Be reasonable when evaluating tests. Flag changes when important behavior is untested, but do not require tests for every export, type, config, metadata file, or mechanical change.

Flag PRs that:
- Change core business logic without covering the main success and failure paths.
- Add UI behavior without testing the user-visible interaction or state change.
- Add tests that only assert rendering, existence, snapshots, or that a function returns any value.
- Skip, mute, or weaken existing tests without a clear reason.
- Mock so much of the system that the test no longer validates the behavior the PR claims to change.