Coding Standards

  # Coding Standards: <Team or repository>

| Field | Value |
|---|---|
| Applies to | |
| Owner | |
| Last reviewed | |


## 1. Automated rules
Everything here is enforced in CI; reviewers must not spend time on it.

| Rule | Tool | Config location | Failing behaviour |
|---|---|---|---|


## 2. Language conventions
Naming, file layout, package structure, public surface.

## 3. Error handling
When to wrap, when to return, what must never be swallowed.

## 4. Logging and observability
Levels, structure, required fields, what must never be logged.

## 5. Testing expectations
What must have a test, what kind, and what coverage means here.

## 6. Dependencies
Adding, pinning, updating, licence policy, and who approves a new one.

## 7. Security rules
Input handling, secrets, crypto, authorisation checks.

## 8. Exceptions
How to deviate: marker, justification, approver, review date.

## 9. Changing this document
  

  # Coding Standards: Platform Identity (Go services)

## 1. Automated rules
| Rule | Tool | Config | Failing behaviour |
|---|---|---|---|
| Formatting | gofumpt | Makefile `fmt` | CI fails |
| Static analysis | golangci-lint (errcheck, ineffassign, gosec) | .golangci.yml | CI fails |
| Dependency vulnerabilities | govulncheck | CI workflow | CI fails on High/Critical |
| Licence policy | go-licenses | CI workflow | CI fails on GPL family |
| Test coverage on changed lines | CI script | ci/coverage.sh | Warning below 70%, fail below 50% |


Reviewers must not comment on anything in this table. If a rule is wrong,
change the config, not the review comment.

## 2. Language conventions
- One package per bounded concept (`approval`, `bundles`, `audit`), not per
  layer. `util` and `helpers` packages do not pass review.
- Exported identifiers are a deliberate surface. Default to unexported;
  export when a second package genuinely needs it, not in anticipation.
- Constructors return concrete types; consumers declare the interfaces they
  need. An interface with one implementation and one caller should be the
  concrete type.
- `context.Context` is the first parameter and is never stored in a struct.
- No `init()` outside `main`. Wiring goes in an explicit constructor so tests
  can build the thing under test.
- File names match the primary type: `resolver.go` holds `Resolver`, with
  `resolver_test.go` beside it.

## 3. Error handling
- Wrap with context at boundaries: `fmt.Errorf("resolve bundle %s: %w", code, err)`.
  Do not wrap the same error twice in one call chain.
- Never discard an error with `_` except in deferred `Close()` on a read-only
  handle, and then with a comment saying why.
- Adapters are the only layer that retries. Everything else treats an error as
  final for that attempt. This is a safety rule: nested retries multiply and
  turned a 30-second outage into 40 minutes in incident INC-2026-0142.
- Errors crossing the API boundary map to a stable `code` (see the API
  specification); a new code is an API change and needs the same review.

## 4. Logging and observability
- Structured logs only (slog), never `fmt.Println`.
- Every log line inside a request must carry `correlation_id` and, where it
  exists, `request_id`. Get them from the context; do not pass them explicitly.
- Levels: `error` means someone should look; `warn` means it self-healed;
  `info` is a business event; `debug` is off in production.
- Never log: tokens, entitlement justification text (personal free text),
  full HR records. Log the employee ID, not the name.
- New code paths need a metric before they need a log line — logs are for
  detail, metrics are for detection.

## 5. Testing expectations
- Every bug fix starts with a failing test that reproduces it. No exceptions;
  this is the one rule reviewers do block on.
- State machines and adapters need table-driven tests covering every
  transition or error branch.
- Contract tests against recorded provider responses, refreshed quarterly.
- Coverage is a signal, not a target. 100% coverage of trivial getters is not
  worth a reviewer's attention; an untested error branch is.

## 6. Dependencies
- A new direct dependency needs a one-paragraph justification in the pull
  request and an approval from a maintainer. Volume, not any single choice, is
  the risk.
- Prefer the standard library. `time`, `net/http` and `database/sql` cover most
  of what small helper libraries offer.
- Pin exact versions; renovate proposes updates weekly, and security updates
  merge without discussion once CI is green.

## 7. Security rules
- Authorisation is checked in the handler, never in the adapter. One place.
- Secrets come from the secrets manager at startup or on rotation; never from
  environment variables baked into an image, never from a file in the repo.
- SQL through parameterised queries only. String-built SQL fails review even
  if the input is "obviously" safe.

## 8. Exceptions
Deviate with a comment: `// standards-exception(<rule>): <why>. approved:
<name> <date>. review: <date>`. CI lists all exceptions in a weekly report so
they do not accumulate silently.

## 9. Changing this document
Open a pull request against it. Two maintainer approvals. A rule that cannot be
justified in two sentences gets deleted rather than debated.