diff --git a/docs/adr/0001-target-architecture.md b/docs/adr/0001-target-architecture.md new file mode 100644 index 0000000..a091a79 --- /dev/null +++ b/docs/adr/0001-target-architecture.md @@ -0,0 +1,152 @@ +# ADR 0001: Target architecture (layered + hexagonal) + +- **Status**: Accepted +- **Date**: 2026-06-28 +- **Tracks**: epic #645 + +## Context + +codeq grew organically. Today the layout couples public API, internal +services, persistence, transport, and wiring in ways that block +independent change: + +- `pkg/app/Application` is a god-object exposing 11 internal services as + a public surface. +- `pkg/config/Config` is a 689-line struct mixing HTTP, gRPC, Redis, + Pebble, Raft, auth, sharding, and observability. +- `pkg/persistence` ships Redis and in-memory implementations, leaking + infrastructure into the public API. +- Core task algorithms (`Claim`, `Enqueue`, `MoveDueDelayed`) are + duplicated across three places (Redis repo, Pebble repo, cluster + router) with no shared contract. +- `internal/services/scheduler_service.go` is 541 lines / 12 fields and + owns task creation, claiming, retry, webhook coordination, and admin. +- There is no enforced rule that domain logic stays free of HTTP or DB + imports. + +Without a stable target, the planned refactor (epics #645–#656) cannot +converge. + +## Decision + +codeq adopts a **layered architecture with hexagonal boundaries at +storage and transport**. The layout is: + +``` +cmd/ thin entrypoints (≤300 LOC each) + server/ composition root for the service + codeq/ composition root for the CLI + +pkg/ public, stable surface — contracts only + types/ DTOs (Task, ResultRecord, QueueStats, ...) + plugin/ storage and extension interfaces + auth/ Validator interface + Claims + producerclient/ gRPC + HTTP SDK + workerclient/ gRPC + HTTP SDK + +internal/ private implementation + core/ pure domain: model, policy, errors + application/ use-cases (TaskCreator, TaskClaimer, ...) + storage/ + adapter/redis/ Redis adapter + adapter/pebble/ Pebble adapter + cluster/ routing, ring, bloom, raft group coordination + replication/ Raft FSM, log/snapshot stores, mux transport + server/ + http// HTTP handlers grouped by resource + grpc// gRPC stream servers + middleware/ request id, auth, tenant, tracing, ... + config/ loader (flag > env > yaml > default) + bootstrap/server/ wiring helpers invoked by cmd/server + cli/ CLI commands, profile loader, output formatting + observability/ + metrics/ Prometheus registry + collectors + tracing/ OpenTelemetry setup + testutil/wait/ polling helpers for tests +``` + +**Dependency direction is strict and one-way**: + +``` +cmd/ ──▶ internal/bootstrap ──▶ internal/{server,application,storage,cluster} + │ + ▼ + internal/core ──▶ pkg/types + pkg/plugin + pkg/auth +``` + +- `internal/core` imports only stdlib and `pkg/{types,plugin,auth}`. +- `internal/application` imports `internal/core` and consumer-side + interfaces it defines for storage and transport collaborators. +- `internal/storage/adapter/*` and `internal/server/*` implement + interfaces declared by their consumers. +- `pkg/` never imports `internal/`. + +**Interfaces are defined on the consumer side**: the package that calls +a collaborator owns the interface; the package that implements it +satisfies that interface structurally. + +**Errors are typed structs** in `internal/core/errors`. Callers inspect +them with `errors.As`. String matching on error messages is forbidden. + +**Composition happens in `cmd/`**. There is no DI framework. Wiring is +visible, debuggable, and `cmd/server/main.go` is the only file that +knows the full dependency graph. + +**The rules are enforced**, not aspirational: `depguard` in +`.golangci.yml` (PR #713) blocks imports that violate the layering. The +coverage floor in `.testcoverage.yml` (PR #714) enforces a 95% floor on +`internal/core` and 80–85% on storage and transport. + +## Consequences + +What this guarantees: + +- Domain logic is portable. A change to `net/http` or to the database + engine cannot reach `internal/core`. +- The public API (`pkg/`) is small and stable. Internal restructuring + no longer breaks SDK consumers. +- New contributors can navigate by layer: where does *X* live? Answer: + the layer that owns *X*'s responsibility. +- Refactors land as small PRs because the seams are explicit. + +What this costs: + +- More files, more package declarations, more interface definitions. +- A breaking change to `pkg/` (removing `pkg/app`, slimming + `pkg/persistence`) — mitigated by a one-release deprecation window + via strangler-fig aliases (epic #656). +- `cmd/server/main.go` becomes the central wiring file. It must stay + readable; bootstrap helpers absorb the bulk. + +New obligations: + +- Every change consults the layer rules before adding an import. +- Every public API change in `pkg/` lands with a deprecation window and + an ADR. +- Every domain decision (state machine, invariant, error category) + lives in `internal/core`, not in services or handlers. + +## Alternatives considered + +- **Keep the current layout, refactor in place**. Rejected: there is no + rule to anchor refactors against. Drift would continue. +- **Pure DDD with bounded contexts and a context map**. Rejected for + this scope: codeq is one bounded context (task scheduling). The + ceremony of context maps does not pay off. +- **Clean architecture with `usecase/`, `port/`, `adapter/` folders + exactly as in Uncle Bob's book**. Rejected: the layered + hexagonal + hybrid above conveys the same constraints with fewer levels of + indirection and matches the reference implementation that informed + this work. +- **A DI framework (`wire`, `fx`)**. Rejected: constructor injection in + `main()` is sufficient and keeps the dependency graph greppable. + +## References + +- `.golangci.yml` (PR #713) — depguard rules that enforce the layering. +- `.testcoverage.yml` (PR #714) — per-layer coverage floors. +- Epics #645 (foundations), #646 (`internal/core`), #647 (reshape + `pkg/`), #648 (storage adapters), #649 (server), #650 (application), + #651 (composition root), #656 (rollout). diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..277ca6e --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,56 @@ +# Architecture Decision Records + +This directory holds Architecture Decision Records (ADRs) for codeq. + +An ADR captures a single architecturally significant decision: the +context, the choice that was made, the trade-offs accepted, and the +alternatives that were rejected. ADRs are immutable once accepted; a +later decision that supersedes them is recorded as a new ADR with the +older one marked `Superseded`. + +## When to write an ADR + +Write one when the change: + +- alters the dependency direction between layers (domain / application / + storage / transport); +- changes the public surface of `pkg/` (adds, renames, deletes); +- introduces or replaces a runtime dependency (database engine, queue, + consensus library, auth provider); +- redefines a wire-level contract (HTTP route shape, gRPC service, event + schema); +- changes a security boundary (authn, authz, tenant isolation); +- accepts a trade-off that future maintainers would otherwise reverse + (e.g., choosing eventual over strong consistency, or in-memory state + over persistence). + +A one-line fix, a bug patch, or a pure refactor that preserves behavior +does not need an ADR. + +## File naming + +`NNNN-kebab-case-title.md` where `NNNN` is the next zero-padded sequence +number. Sequence is per-repo, not per-area. + +## Template + +Copy `template.md` to start a new ADR. + +## Status lifecycle + +``` +Proposed → Accepted → (Superseded by NNNN | Deprecated) +``` + +- **Proposed**: open for discussion in a PR. +- **Accepted**: merged to `main`; the decision is in effect. +- **Superseded by NNNN**: a later ADR replaces this one. Keep the file; + add the supersession line at the top. +- **Deprecated**: the decision no longer applies but no replacement was + written. Rare. + +## Index + +| # | Title | Status | +|---|---|---| +| [0001](0001-target-architecture.md) | Target architecture (layered + hexagonal) | Accepted | diff --git a/docs/adr/template.md b/docs/adr/template.md new file mode 100644 index 0000000..cad4407 --- /dev/null +++ b/docs/adr/template.md @@ -0,0 +1,36 @@ +# ADR NNNN: Title + +- **Status**: Proposed | Accepted | Superseded by NNNN | Deprecated +- **Date**: YYYY-MM-DD +- **Deciders**: list of names or GitHub handles +- **Tracks**: optional links to epics or issues + +## Context + +What is the problem? What is the current state? What evidence triggered +this decision (incident, perf data, growth, contract breakage)? Keep it +factual; one short paragraph is usually enough. + +## Decision + +What was chosen. State the decision as a present-tense rule, not as +intent. Be specific: name packages, interfaces, file paths where +relevant. + +## Consequences + +The trade-offs accepted. Include: + +- what becomes easier or guaranteed; +- what becomes harder or forbidden; +- new obligations on operators or callers; +- the deprecation timeline if anything is removed. + +## Alternatives considered + +For each rejected alternative, one line on why it lost. If only one +option was viable, state that and why. + +## References + +Links to RFCs, prior ADRs, external docs, benchmarks, incidents.