Skip to content

coding-conventions.md

Latest commit

 

History

History
169 lines (131 loc) · 5.78 KB

coding-conventions.md

File metadata and controls

169 lines (131 loc) · 5.78 KB
Raw

Context: this is part of Contribute. Start with the primer if you haven't.

Coding conventions

The human-readable summary. The full agent-facing reference is AGENTS.md at the repo root.

Naming

Idiomatic Go. No Hungarian; no abbreviations beyond the Go idioms (ctx, cfg, req, err, id). Exported identifiers carry doc comments that start with the identifier name. Test files use _test.go; integration tests carry build tag integration (SQLite) or integration_postgres (PostgreSQL).

Services live as flat files under internal/services/ with no _service suffix — authorize.go, broker_issuer.go, client_credentials.go. Don't create per-feature sub-packages there; the layer is intentionally flat.

Error handling

Wrap errors with context, propagate them:

if err != nil {
    return fmt.Errorf("save grant: %w", err)
}

Never log.Fatal inside a library, service, or adapter. The composition root in cmd/authserver/serve.go is the only place that may exit the process.

Domain errors live in internal/domain/errors.go as sentinel error values created via newError("code", "human-readable"). Compare with errors.Is. Cross-boundary translation (sentinel → HTTP status / OAuth error code) happens in api/shared/ and the per-endpoint handlers — never in services.

Factory + Registry over switch x.kind

When you find yourself reaching for:

// Don't do this.
switch p.Protocol {
case "oauth":
    return doOAuth(p)
case "api_key":
    return doAPIKey(p)
case "service_account":
    return doServiceAccount(p)
}

…replace it with the brokerproto Registry pattern. The reference implementation is internal/brokerproto/registry.go: each sibling package under internal/adapters/brokerproto/ implements output.BrokerProtocol, registers itself at startup by Name(), and the runtime dispatches via Registry.Lookup(name).

Why this matters:

  • New kinds add a sibling package; no existing code changes.
  • Test seams are obvious — register a fake adapter into a test registry.
  • The switch form scatters protocol-specific knowledge across the callers; the registry form keeps it inside the adapter.

This pattern applies to any polymorphic dispatch over a small fixed set of "kinds" — upstream protocols today, future grant-engine plug-ins, external KMS drivers. If a design doc says "switch on kind", implement it as Factory+Registry instead.

The pre-push gate

make ci-local runs build + lint + import-boundary + OSS-leak + unit tests + govulncheck. If it's red, you do not push. Adapter or service changes additionally need make test-integration (and test-integration-postgres for postgres-adapter touches). See running-tests.md.

Commits and PRs

Conventional-commit style:

feat: add token exchange grant type
fix: handle expired DPoP nonces correctly
docs: update configuration reference
refactor: extract consent service from auth flow
test: add PostgreSQL adapter integration tests
ci:   add CodeQL security scanning

Title under 70 characters; description explains the why, not the what. Do not add Claude/AI attribution trailers, "Co-Authored-By" bot lines, or "Generated with …" footers — those are an immediate review reject in this repo.

DTOs are ground truth; OpenAPI is downstream

When the wire shape and an OpenAPI YAML disagree, the DTO struct tags win. Specifically:

  • api/admin/dto.go and internal/admin/dto/dto.go define the admin wire shape.
  • api/public/oauth/dto.go defines the public wire shape.
  • docs/reference/http-api.md is the generated human-readable surface (carries <!-- generated by tools/docsgen -->). Regenerate via make docs-gen; never hand-edit.
  • The legacy docs/api/{admin,public}-api.yaml OpenAPI specs have been retired — they drifted from the handlers in practice. The generated docs/reference/http-api.md is the ground-truth surface; verify wire shapes against the DTO struct tags in api/admin/dto.go and internal/admin/dto/dto.go when precision matters.

Imports

Standard Go grouping enforced by goimports:

import (
    "context"
    "fmt"

    "github.com/gofrs/uuid"

    "github.com/authplane/authserver/internal/domain"
    "github.com/authplane/authserver/internal/ports/output"
)

stdlib first, third-party second, in-repo third. golangci-lint catches order violations.

What not to do

  • Don't put business logic in HTTP handlers — handlers parse, dispatch, marshal.
  • Don't log token values, secrets, or key material. Audit events carry hashes only.
  • Don't use == for secret comparison — crypto/subtle.ConstantTimeCompare.
  • Don't call slog.Info(...) without context — slog.InfoContext(ctx, ...) threads the trace ID.
  • Don't import a concrete adapter from a service or a handler — go through the port interface, wire the concrete in cmd/authserver/serve.go.
  • Don't hand-edit docs/reference/{cli,http-api,env-vars,configuration}.md — regenerate via make docs-gen.

Documentation style

When writing comments, docs, or test names, prefer self-contained technical insight over references that require external context:

  • State the rule inline: "Actor MCP is resolved via Resource.policy.runtime.client_ids list-membership, not slug==client_id match."
  • Avoid bare "the spec" / "per spec". Either qualify with the standard (per RFC 7523, per the MCP Authorization spec) or state the rule directly.

The Helm/K8s YAML spec: keyword and Go field names like spec.Slug are fine — those refer to language constructs, not standards.