Initial commit
This commit is contained in:
19
docs/decisions/adr-001-monorepo-tool.md
Normal file
19
docs/decisions/adr-001-monorepo-tool.md
Normal file
@@ -0,0 +1,19 @@
|
||||
# ADR-001: Turborepo + pnpm for Monorepo
|
||||
|
||||
## Status: Accepted
|
||||
|
||||
## Context
|
||||
|
||||
Need a monorepo tool for a clean architecture template with multiple apps and shared packages.
|
||||
|
||||
## Decision
|
||||
|
||||
Turborepo + pnpm workspaces.
|
||||
|
||||
## Rationale
|
||||
|
||||
- Clean architecture already provides organizational structure — Nx's opinions would compete
|
||||
- Minimal config (turbo.json + pnpm-workspace.yaml) means agents understand the setup quickly
|
||||
- Excellent caching — shared packages build once and are reused
|
||||
- Battle-tested pairing, both reference repos use it
|
||||
- Works naturally with Vite for non-Next packages
|
||||
30
docs/decisions/adr-002-di-framework.md
Normal file
30
docs/decisions/adr-002-di-framework.md
Normal file
@@ -0,0 +1,30 @@
|
||||
# ADR-002: InversifyJS for Dependency Injection
|
||||
|
||||
## Status: Accepted
|
||||
|
||||
## Context
|
||||
|
||||
Need DI for clean architecture. Options: InversifyJS, tsyringe, manual composition root.
|
||||
|
||||
## Decision
|
||||
|
||||
InversifyJS with symbol-based resolution + targeted agent documentation.
|
||||
|
||||
## Rationale
|
||||
|
||||
- Scales to 20+ services with automatic dependency chain resolution
|
||||
- Built-in singleton/transient/request scopes
|
||||
- Middleware support for logging/tracing (Sentry integration)
|
||||
- Matches the Clean Architecture reference implementation
|
||||
- Agent readability gap (4/10 → 7/10) mitigated by resolution tables and step-by-step recipes in di/AGENTS.md
|
||||
- Familiar to developers from Java/C# backgrounds
|
||||
|
||||
## Trade-offs
|
||||
|
||||
- Requires reflect-metadata + decorator config in tsconfig
|
||||
- Symbol indirection harder to trace than plain functions
|
||||
- Extra dependency (inversify + reflect-metadata)
|
||||
|
||||
## Update (2026-05-04)
|
||||
|
||||
The vertical-feature refactor preserved InversifyJS but moved from a single shared container in `packages/core/src/di/` to **per-feature containers** in each feature package (`packages/<feature>/src/di/container.ts`). See ADR-008.
|
||||
35
docs/decisions/adr-003-cms-separation.md
Normal file
35
docs/decisions/adr-003-cms-separation.md
Normal file
@@ -0,0 +1,35 @@
|
||||
# ADR-003: CMS Core + CMS Client Separation
|
||||
|
||||
## Status: Accepted
|
||||
|
||||
## Context
|
||||
|
||||
Payload CMS needs to integrate with clean architecture without creating circular dependencies.
|
||||
|
||||
## Decision
|
||||
|
||||
Three packages: `@repo/cms-core` (config + collections), `@repo/cms-client` (standalone typed client), `apps/cms` (thin shell).
|
||||
|
||||
## Rationale
|
||||
|
||||
- `cms-core` is testable independently — collections are just config objects
|
||||
- `cms-client` is standalone (no internal imports) — prevents circular deps
|
||||
- `apps/cms` is almost empty — just boots Next.js with cms-core's config
|
||||
- Payload instance injected into cms-client, not imported — app startup code wires them
|
||||
|
||||
## Circular Dependency Prevention
|
||||
|
||||
```
|
||||
apps/cms → @repo/cms-core → @repo/core/application (hooks)
|
||||
@repo/core/infrastructure → @repo/cms-client (standalone)
|
||||
```
|
||||
|
||||
No cycles because cms-client never imports from cms-core or core.
|
||||
|
||||
## Status: Partially superseded by v2 (2026-05-04)
|
||||
|
||||
v1 advocated `@repo/cms-core` as a single CMS package. v2 splits this into:
|
||||
- `@repo/core-cms` — composition only (assembles feature CMS schemas)
|
||||
- Each feature owns its own collections/globals under `packages/<feature>/src/integrations/cms/`
|
||||
|
||||
Rationale: vertical-feature ownership scales better; CMS schema lives with the business code that needs it. See ADR-006.
|
||||
22
docs/decisions/adr-004-dual-mode-client.md
Normal file
22
docs/decisions/adr-004-dual-mode-client.md
Normal file
@@ -0,0 +1,22 @@
|
||||
# ADR-004: Dual-Mode Payload Client (Local + HTTP)
|
||||
|
||||
## Status: Accepted
|
||||
|
||||
## Context
|
||||
|
||||
Apps need to access Payload CMS data. Payload 3.x offers both Local API (direct) and REST API (HTTP).
|
||||
|
||||
## Decision
|
||||
|
||||
`@repo/cms-client` supports both modes via `createPayloadClient()`.
|
||||
|
||||
## Rationale
|
||||
|
||||
- **Local mode (primary):** Direct Payload instance access — no HTTP overhead, full query capabilities (where, sort, limit, depth, page, populate). All server-side apps use this.
|
||||
- **HTTP mode (fallback):** REST API for external consumers without access to a Payload process.
|
||||
- Payload instance is injected at app startup, not imported — keeps cms-client standalone.
|
||||
- Both modes share the same `PayloadClient` interface — consumers don't know which mode is active.
|
||||
|
||||
## Status: Superseded by ADR-007 (2026-05-04)
|
||||
|
||||
The dual-mode client wrapper was deleted. Feature payload-backed repositories now call `getPayload({ config })` directly with the assembled config injected via constructor. See ADR-007 for rationale.
|
||||
24
docs/decisions/adr-005-atomic-design.md
Normal file
24
docs/decisions/adr-005-atomic-design.md
Normal file
@@ -0,0 +1,24 @@
|
||||
# ADR-005: Atomic Design for UI Components
|
||||
|
||||
## Status: Accepted
|
||||
|
||||
## Context
|
||||
|
||||
Need a scalable component architecture for the shared UI package.
|
||||
|
||||
## Decision
|
||||
|
||||
Atomic Design (atoms/molecules/organisms/templates) + shadcn/ui + Storybook.
|
||||
|
||||
## Rationale
|
||||
|
||||
- Clear hierarchy makes agents know exactly where to place new components
|
||||
- Import rules enforce composition direction (atoms never import molecules)
|
||||
- Co-located stories make components self-documenting
|
||||
- shadcn/ui provides excellent base atoms that map naturally to atomic levels
|
||||
- Storybook sidebar mirrors the hierarchy via story titles
|
||||
- Pages live in apps (not UI package) — they connect to real data
|
||||
|
||||
## Update (2026-05-04)
|
||||
|
||||
Atomic Design now applies to `@repo/core-ui/` only — generic primitives (atoms, molecules, generic organisms, templates). Feature-specific components (e.g., `ArticleCard`, `HeaderNavMenu`) live in the owning feature's `ui/` folder per the vertical-feature architecture. See ADR-006.
|
||||
27
docs/decisions/adr-006-vertical-feature-packages.md
Normal file
27
docs/decisions/adr-006-vertical-feature-packages.md
Normal file
@@ -0,0 +1,27 @@
|
||||
# ADR-006: Vertical Feature Packages
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-04
|
||||
|
||||
## Context
|
||||
|
||||
The original template organized packages by architectural layer: `core` (all domains together), `api` (all routers), `ui` (all components). As features grow, shared code accumulates and cross-feature dependencies become implicit.
|
||||
|
||||
## Decision
|
||||
|
||||
Reorganize by business capability. Each feature owns a vertical slice from entities through UI. `core-*` packages host only non-business concerns (DI, shared types, UI primitives).
|
||||
|
||||
**Result:** 5 feature packages (`auth`, `blog`, `media`, `marketing-pages`, `navigation`) + 5 core packages (`core-shared`, `core-cms`, `core-api`, `core-trpc`, `core-ui`).
|
||||
|
||||
## Consequences
|
||||
|
||||
- Features evolve independently without coordinating with shared code
|
||||
- Cross-feature coupling is visible at the package-graph level (ESLint enforces it)
|
||||
- Per-feature DI containers eliminate symbol collisions
|
||||
- New team members can understand a feature completely by reading one package
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- Horizontal layers (kept): Simpler initially, but scales to implicit hidden dependencies
|
||||
- Monolithic single package: No modularity, impossible to reason about at scale
|
||||
- Feature shells + shared core: Hybrid approach tried by many teams; creates "dump" in core that nobody owns
|
||||
36
docs/decisions/adr-007-drop-cms-client-wrapper.md
Normal file
36
docs/decisions/adr-007-drop-cms-client-wrapper.md
Normal file
@@ -0,0 +1,36 @@
|
||||
# ADR-007: Drop the Dual-Mode CMS Client Wrapper
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-04
|
||||
|
||||
## Context
|
||||
|
||||
ADR-004 introduced `@repo/cms-client` as a standalone wrapper supporting both Local API and HTTP modes. It was never used in production after Payload 3.x solidified its Local API as the primary pattern.
|
||||
|
||||
## Decision
|
||||
|
||||
Delete the wrapper. Feature payload-backed repositories call `getPayload({ config })` directly. The `config` is injected via constructor, avoiding hard coupling to `@repo/cms-core` at import time.
|
||||
|
||||
**Example:**
|
||||
```typescript
|
||||
export class PayloadArticlesRepository implements IArticlesRepository {
|
||||
constructor(private config: Config) {}
|
||||
|
||||
async getById(id: string) {
|
||||
const payload = await getPayload({ config: this.config });
|
||||
return payload.findByID({ collection: "articles", id });
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Consequences
|
||||
|
||||
- One fewer abstraction layer — repositories work directly with Payload's typed API
|
||||
- No "modes" — always use Local API from server code
|
||||
- Package graph stays acyclic: feature packages never import `@repo/cms-client`
|
||||
- `apps/cms` can directly boot Payload with the assembled config
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- Keep the wrapper for "future-proofing": Payload 3.x is stable; wrapper was never activated in practice
|
||||
- Add HTTP mode later if needed: Simple to implement when actually required
|
||||
36
docs/decisions/adr-008-per-feature-di-containers.md
Normal file
36
docs/decisions/adr-008-per-feature-di-containers.md
Normal file
@@ -0,0 +1,36 @@
|
||||
# ADR-008: Per-Feature InversifyJS Containers
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-04
|
||||
|
||||
## Context
|
||||
|
||||
The original template used a single shared InversifyJS Container in `packages/core/src/di/`. As the number of features grows, the container becomes a point of coordination: adding a symbol requires modifying shared code, and tests that mock one feature risk breaking others.
|
||||
|
||||
## Decision
|
||||
|
||||
Each feature owns its own `Container` and symbol table. No shared DI state. Tests rebind per feature in isolation. Apps boot by calling `bindProduction*()` for each feature independently.
|
||||
|
||||
**Example:**
|
||||
```typescript
|
||||
// packages/blog/src/di/container.ts
|
||||
export const container = createContainer();
|
||||
|
||||
// packages/blog/tests/feature.test.ts
|
||||
beforeEach(() => {
|
||||
rebindRepository(new TestRepository());
|
||||
// Only blog's container is affected
|
||||
});
|
||||
```
|
||||
|
||||
## Consequences
|
||||
|
||||
- Zero cross-feature DI coupling — each feature's test can mock its repos without coordination
|
||||
- Symbol collisions impossible — each feature has its own `ARTICLES_REPOSITORY` symbol
|
||||
- Shared services (if needed) are explicitly bound in each feature that uses them
|
||||
- Apps must boot each feature's container on startup
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- Single shared container: Simpler upfront, becomes a bottleneck and test coordination point
|
||||
- Function injection (no DI): Avoids framework overhead, but loses the scaling benefits of DI
|
||||
34
docs/decisions/adr-009-integrations-folder-naming.md
Normal file
34
docs/decisions/adr-009-integrations-folder-naming.md
Normal file
@@ -0,0 +1,34 @@
|
||||
# ADR-009: Rename `adapters/` to `integrations/`
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-04
|
||||
|
||||
## Context
|
||||
|
||||
The source spec (monorepo-architecture-spec-detailed-v5.md) uses `adapters/cms` and `adapters/api` for Payload and tRPC integration points. Clean Architecture also uses `adapters/` (or `interface-adapters/`) for transport-agnostic controller layer. The name collision is confusing.
|
||||
|
||||
## Decision
|
||||
|
||||
Use `integrations/` for role-based plug points (Payload and tRPC). Keep `interface-adapters/` for the Clean Architecture controller layer.
|
||||
|
||||
**Result:**
|
||||
```
|
||||
packages/<feature>/src/
|
||||
interface-adapters/controllers/ ← Clean Architecture layer (transport-agnostic)
|
||||
integrations/
|
||||
cms/ ← Payload collections/globals
|
||||
api/ ← tRPC routers
|
||||
```
|
||||
|
||||
## Consequences
|
||||
|
||||
- No naming collision — intent is unambiguous
|
||||
- `integrations/` semantically captures "external system integration" better than `adapters/`
|
||||
- Minor deviation from source spec, documented here as deliberate choice
|
||||
- All new features follow this naming consistently
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- Keep `adapters/`: Collision with Clean Architecture terminology is confusing
|
||||
- Use `external/`: Less specific; doesn't convey "Payload and tRPC"
|
||||
- Rename Clean Architecture layer to `controllers/`: Could work; less standard
|
||||
148
docs/decisions/adr-010-turbo-boundaries.md
Normal file
148
docs/decisions/adr-010-turbo-boundaries.md
Normal file
@@ -0,0 +1,148 @@
|
||||
# ADR-010: Turborepo boundaries alongside ESLint enforcement
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-04
|
||||
**Supersedes:** ADR-006 (partial refinement; not a contradiction)
|
||||
|
||||
## Context
|
||||
|
||||
ADR-006 established the vertical-feature monorepo with three tags (`app`, `feature`, `core`). ESLint with `eslint-plugin-boundaries` was introduced to enforce direct-import boundaries at lint time, catching violations like:
|
||||
|
||||
- Feature package importing from another feature
|
||||
- Core package importing from a feature
|
||||
- Deep imports past public `exports` map boundaries
|
||||
|
||||
However, ESLint has two intrinsic limitations:
|
||||
|
||||
1. **No transitive enforcement** — If `core-trpc` imports `core-api` (which imports feature routers), ESLint sees only the direct edge `core-trpc` → `core-api`. The transitive reach into features is invisible. A package can declare `@repo/feature-x` as a dependency without ever importing it directly.
|
||||
|
||||
2. **No declared-dep enforcement** — A `package.json` dependency that doesn't match an actual import goes undetected. Conversely, a `package.json` dependency might be missing but the transitive graph still allows the code to work.
|
||||
|
||||
The result: two types of dependency drift escape lint-time checking:
|
||||
|
||||
- A composition package's transitive reach into features (real problem: `core-trpc` → `core-api` → `feature-blog-routers`)
|
||||
- Incorrect or missing `package.json` declarations
|
||||
|
||||
## Decision
|
||||
|
||||
Add Turborepo's `boundaries` feature as a second enforcement layer running at **build-graph time** (before build), parallel to ESLint's lint-time checks.
|
||||
|
||||
### Five-tag model (refined from ADR-006)
|
||||
|
||||
ADR-006 mentioned three tags. This ADR refines the model to five, distinguishing composition packages explicitly:
|
||||
|
||||
- **app** — `apps/web-next`, `apps/web-tanstack`, `apps/cms`, `apps/storybook`
|
||||
- **core** — `packages/core-shared`, `core-ui` (pure foundation, no transitive feature reach)
|
||||
- **core-composition** — `packages/core-api`, `core-cms`, `core-trpc` (composition or transitively reach features)
|
||||
- **feature** — `packages/auth`, `blog`, `media`, `marketing-pages`, `navigation`
|
||||
- **tooling** — `packages/core-eslint`, `core-typescript`
|
||||
|
||||
Why `core-trpc` is `core-composition`:
|
||||
|
||||
- `core-trpc` imports `@repo/core-api` (the tRPC app router)
|
||||
- `core-api` imports feature routers from `@repo/<feature>/api`
|
||||
- Therefore, `core-trpc` transitively depends on features through the `AppRouter` type
|
||||
- This violates ADR-006's "core → NOT feature" rule if we treat `core-trpc` as plain `core`
|
||||
- Solution: tag `core-trpc` as `core-composition` to make the transitive reach explicit and allowed
|
||||
|
||||
### Implementation
|
||||
|
||||
1. **Root `turbo.json`** declares boundary rules as a `boundaries.tags` config:
|
||||
```json
|
||||
{
|
||||
"boundaries": {
|
||||
"tags": {
|
||||
"app": {
|
||||
"dependencies": {
|
||||
"allow": ["app", "core", "core-composition", "feature", "tooling"]
|
||||
}
|
||||
},
|
||||
"core-composition": {
|
||||
"dependencies": {
|
||||
"allow": ["core", "core-composition", "feature", "tooling"]
|
||||
}
|
||||
},
|
||||
"core": {
|
||||
"dependencies": { "allow": ["core", "core-composition", "tooling"] }
|
||||
},
|
||||
"feature": {
|
||||
"dependencies": { "allow": ["core", "feature", "tooling"] }
|
||||
},
|
||||
"tooling": { "dependencies": { "allow": ["tooling"] } }
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
> **Amendment (2026-05-21) — feature → feature.** The `feature` tag's allow-list
|
||||
> includes `feature`. A feature package may depend on another feature's
|
||||
> _published public exports_ — the `@repo/<feature>` contract barrel (types,
|
||||
> schemas, errors, event contracts). This is required by the cross-feature
|
||||
> event system (ADR-015): a consumer must import the publisher's event
|
||||
> contract. Two guardrails remain: each feature's `exports` map seals its
|
||||
> internals (only `.`, `./ui`, `./api`, `./cms`, `./di/bind-*` are reachable),
|
||||
> and cross-feature _behaviour_ still flows through `IEventBus` — a feature
|
||||
> never imports and invokes another feature's use cases directly.
|
||||
|
||||
2. **Per-package `turbo.json`** declares the package's tag:
|
||||
|
||||
```json
|
||||
// packages/blog/turbo.json
|
||||
{ "extends": ["../../../turbo.json"], "tasks": { /* ... */ } }
|
||||
// package.json
|
||||
{ "turbo": { "tasks": { "build": { /* ... */ } }, "tags": ["feature"] } }
|
||||
```
|
||||
|
||||
3. **CLI validation** — `pnpm turbo boundaries` runs the check in <1 second without building anything
|
||||
|
||||
### Two layers, not one
|
||||
|
||||
Both ESLint and Turborepo enforce the same five-tag rules, but for different reasons:
|
||||
|
||||
| Layer | Runs | Sees | Exempts via |
|
||||
| --------------------------------- | ---------------- | ------------------------------------------------------- | ---------------------------------- |
|
||||
| ESLint `eslint-plugin-boundaries` | lint-time | Direct imports per file | `// @boundaries-ignore` comments |
|
||||
| Turborepo `boundaries` | build-graph time | Entire workspace dependency graph including transitives | None (graph-based, not per-import) |
|
||||
|
||||
**Why both?**
|
||||
|
||||
- **ESLint** provides fine-grained control (file-level exemptions) and immediate feedback during development
|
||||
- **Turborepo** catches transitive issues (e.g., feature reach through composition packages) and missing declarations
|
||||
|
||||
**Enforcement** — CI runs both: `pnpm lint` (includes ESLint) and `pnpm turbo boundaries`.
|
||||
|
||||
## Consequences
|
||||
|
||||
1. **Two independent checks** — developers get immediate ESLint feedback and a final Turbo gate in CI
|
||||
2. **Both must stay in sync** — when adding a new tag rule, update both:
|
||||
- `packages/core-eslint/base.js` (ESLint `eslint-plugin-boundaries` config)
|
||||
- Root `turbo.json` (`boundaries.tags` config)
|
||||
- Per-package `package.json` `turbo.tags` declaration
|
||||
3. **Stricter than before** — Turborepo sees transitives; some patterns that pass ESLint may fail Turbo:
|
||||
- Example: `packages/cms` might try to use `@repo/<dep>` indirectly (not importing, but depending)
|
||||
- Solution: declare the dependency explicitly or restructure the import
|
||||
4. **The five-tag model is a refinement, not a contradiction** — ADR-006 mentioned three tags; this ADR adds `core-composition` and `tooling` explicitly and moves them into the boundary enforcement
|
||||
5. **`core-trpc`'s new tag** — previously thought of as `core`, now explicitly `core-composition` to match its transitive reach; any code assuming `core-trpc` is `core` must be updated
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
1. **ESLint only** — Simple, immediate feedback. Downside: misses transitive issues; no build-graph validation.
|
||||
2. **Turborepo only** — Catches everything eventually, but slower feedback cycle; less granular exemptions.
|
||||
3. **Both in series** — ESLint first (fast), Turbo second (thorough). Chosen.
|
||||
4. **Custom graph validator** — Over-engineered; Turborepo's built-in feature is stable and designed for this.
|
||||
|
||||
## Related
|
||||
|
||||
- **ADR-006:** Vertical feature packages (the original three-tag model)
|
||||
- **ADR-009:** Integrations folder naming
|
||||
- **`packages/core-eslint/base.js`** — ESLint configuration
|
||||
- **Root `turbo.json`** — Turborepo configuration with boundaries rules
|
||||
- **`docs/architecture/overview.md`** — Package map and five-tag summary
|
||||
- **`docs/architecture/dependency-flow.md`** — Enforcement layers and rules
|
||||
- **`AGENTS.md`** — Per-package boundary documentation
|
||||
|
||||
## Timeline
|
||||
|
||||
- **2026-05-04** — Turborepo boundaries added alongside existing ESLint enforcement
|
||||
- **CI integration** — `pnpm turbo boundaries` added to lint stage
|
||||
- **Documentation** — Updated AGENTS.md, overview.md, dependency-flow.md, vertical-feature-spec.md
|
||||
69
docs/decisions/adr-011-tdd-foundation.md
Normal file
69
docs/decisions/adr-011-tdd-foundation.md
Normal file
@@ -0,0 +1,69 @@
|
||||
# ADR-011: TDD Foundation
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-05
|
||||
**Supersedes:** none
|
||||
|
||||
## Context
|
||||
|
||||
The vertical-feature monorepo refactor (ADRs 001-010) established
|
||||
clean architecture with per-feature DI containers but did not enforce
|
||||
TDD as the path of least resistance. Agentic workers were producing
|
||||
code-first commits with tests added later, leading to test theatre and
|
||||
mock/real drift.
|
||||
|
||||
## Decision
|
||||
|
||||
1. **New package `@repo/core-testing` (tag: tooling)** — shared test
|
||||
utilities: defineFactory, defineContractSuite, renderWithProviders,
|
||||
mock-payload helpers, jsdom setup file. Tagged `tooling` so any
|
||||
package may depend on it as devDependency without boundary violation.
|
||||
|
||||
2. **Vitest base configs split into node + jsdom** with safety defaults
|
||||
(clearMocks, restoreMocks, mockReset, unstubGlobals, sequence.shuffle)
|
||||
and coverage thresholds (80/75/80/80 baseline; 100% in entities +
|
||||
use-cases + controllers).
|
||||
|
||||
3. **Factories per feature** in `src/__factories__/` replace inline
|
||||
fixtures. Stable date defaults (2026-01-01) so snapshot diffs reflect
|
||||
SUT behavior only.
|
||||
|
||||
4. **Contract suites per repository interface** in `src/__contracts__/`
|
||||
run against every implementation (Mock + Payload). Eliminates the
|
||||
class of bug where the mock and the real impl drift apart.
|
||||
|
||||
5. **Tests in core-\* packages and apps** — composition smoke tests
|
||||
(appRouter, payloadConfig, bind-production, providers).
|
||||
|
||||
6. **Storybook test-runner** — every story executed as a smoke test.
|
||||
|
||||
7. **CI workflow** — typecheck + lint + boundaries + test + build +
|
||||
e2e + storybook on every PR. Coverage uploaded as artifact.
|
||||
|
||||
8. **Two new docs** — tdd-workflow.md (process) + restructured
|
||||
adding-a-feature.md (interleaves tests with impl).
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Fixture files instead of factories** — rejected. Fixtures rot with
|
||||
schema changes and require manual updates per test.
|
||||
- **One shared test file per impl** — rejected. Contract suites give
|
||||
the same coverage in fewer LOC and prevent drift.
|
||||
- **Real Postgres in tests via testcontainers** — rejected for unit
|
||||
tests (slow, complex). Repository contract suites + vi.mock('payload')
|
||||
give equivalent confidence in milliseconds.
|
||||
- **Stryker mutation testing** — deferred. Coverage thresholds + contract
|
||||
suites get us most of the way; mutation testing is incremental.
|
||||
|
||||
## Consequences
|
||||
|
||||
- New package to maintain (small, mostly stable surface).
|
||||
- Coverage thresholds may fail builds initially; we add tests to cross
|
||||
threshold as features land.
|
||||
- Sequence shuffle may surface latent flakes; we fix as found.
|
||||
- Templates for new features now require writing tests first; this is
|
||||
by design.
|
||||
|
||||
## Refines
|
||||
|
||||
- ADR-006 (boundary tags) — adds @repo/core-testing as a tooling package.
|
||||
174
docs/decisions/adr-012-feature-conventions.md
Normal file
174
docs/decisions/adr-012-feature-conventions.md
Normal file
@@ -0,0 +1,174 @@
|
||||
# ADR-012: Feature Conventions
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-05
|
||||
**Supersedes:** none — extends ADR-006 (vertical-feature-packages) and ADR-008 (per-feature DI containers)
|
||||
|
||||
## Context
|
||||
|
||||
The vertical-feature monorepo refactor (ADRs 001-010) and the TDD
|
||||
foundation (ADR-011) established Clean Architecture per feature, but
|
||||
the per-layer code shape was inconsistent across features and needed
|
||||
to be standardized.
|
||||
|
||||
Specifically, before this ADR:
|
||||
|
||||
- Use cases called `<feature>Container.get()` inside their bodies
|
||||
(locator pattern), making them hard to test in isolation.
|
||||
- Controllers were multi-method classes (`articles.controller.ts` with
|
||||
`getBySlug`, `create`, `list` methods on one symbol).
|
||||
- Entities lived flat at `entities/<x>.ts`; errors at `entities/errors.ts`.
|
||||
- Mock implementations used a `mock-` prefix (`mock-articles.repository.ts`),
|
||||
separating them visually from the real impl.
|
||||
- Real Payload-backed implementations used a `payload-` prefix
|
||||
(`payload-articles.repository.ts`).
|
||||
- Repository/service interface filenames used a `-` separator
|
||||
(`articles-repository.interface.ts`) instead of the canonical dot
|
||||
(`articles.repository.interface.ts`).
|
||||
- Tests rebound the DI container in `beforeEach` rather than constructing
|
||||
mocks and injecting directly.
|
||||
|
||||
## Decision
|
||||
|
||||
Bring every feature into structural conformance with the canonical
|
||||
Clean Architecture pattern, with four intentional divergences (§Adaptations below).
|
||||
|
||||
### What we adopted
|
||||
|
||||
1. **Factory-function use cases and controllers** — every use case and
|
||||
every controller is a factory: `(deps) => async (input) => result`.
|
||||
Each file exports `export type I*UseCase = ReturnType<typeof xUseCase>`
|
||||
(or the analogous `I*Controller`) so consumers can depend on the type
|
||||
without depending on the impl.
|
||||
|
||||
2. **Entity layout** — `entities/models/<x>.ts` (Zod schema + type)
|
||||
and `entities/errors/<domain>.ts` (domain-grouped error classes) +
|
||||
`entities/errors/common.ts` (`InputParseError`).
|
||||
|
||||
3. **Naming** — dot-separated qualifiers throughout:
|
||||
- Real repo impl: `<noun>.repository.ts` (no `payload-` prefix)
|
||||
- Mock repo impl: `<noun>.repository.mock.ts` (suffix, not prefix)
|
||||
- Repo interface: `<noun>.repository.interface.ts`
|
||||
- Service variants follow the same pattern.
|
||||
|
||||
4. **One controller per use case** — no multi-method controllers.
|
||||
Each verb-noun pair has its own file: `sign-in.controller.ts`,
|
||||
`get-articles.controller.ts`, `delete-media.controller.ts`.
|
||||
|
||||
5. **InversifyJS `.toDynamicValue()` for factory bindings:**
|
||||
|
||||
```typescript
|
||||
bind<ISignInUseCase>(AUTH_SYMBOLS.ISignInUseCase).toDynamicValue((ctx) =>
|
||||
signInUseCase(
|
||||
ctx.container.get<IUsersRepository>(AUTH_SYMBOLS.IUsersRepository),
|
||||
ctx.container.get<IAuthenticationService>(
|
||||
AUTH_SYMBOLS.IAuthenticationService,
|
||||
),
|
||||
),
|
||||
);
|
||||
```
|
||||
|
||||
6. **Direct injection in tests** — construct mocks and pass them into the
|
||||
factory; no container rebinding for unit/use-case/controller tests:
|
||||
|
||||
```typescript
|
||||
const users = new MockUsersRepository();
|
||||
const auth = new MockAuthenticationService(users);
|
||||
const useCase = signInUseCase(users, auth);
|
||||
```
|
||||
|
||||
7. **Real Payload-backed `UsersRepository` and `AuthenticationService`
|
||||
for `auth`** — previously only mocks existed. Some methods on
|
||||
`AuthenticationService` (session create/validate/invalidate) are
|
||||
deferred behind `NotImplementedError` until the cookie-strategy
|
||||
decision is finalized.
|
||||
|
||||
8. **`media` is now a complete Clean Architecture feature** — entities,
|
||||
application, infrastructure, interface-adapters, DI, integrations/api,
|
||||
factories, contract, feature test. Previously it was just a Payload
|
||||
collection.
|
||||
|
||||
### Intentional divergences (kept from prior ADRs)
|
||||
|
||||
| Aspect | Community default | Ours | Reason |
|
||||
| --------------- | ------------------------------------- | ----------------------------------------------------- | ------------------------------------------------------------------------ |
|
||||
| DI library | `@evyweb/ioctopus` | `inversify` | Already integrated; equivalent expressive power via `.toDynamicValue()`. |
|
||||
| DI scope | One global `ApplicationContainer` | One per feature (`authContainer`, `blogContainer`, …) | Vertical-feature isolation (ADR-008). |
|
||||
| Test placement | `tests/unit/...` mirror | Colocated `*.test.{ts,tsx}` | Established by ADR-011; clearer per-file ownership. |
|
||||
| Instrumentation | Sentry/observability service wrapping | Not adopted | Out of scope; revisit when observability becomes a requirement. |
|
||||
|
||||
`InputParseError` is also duplicated per feature (~6 lines × 5
|
||||
features) instead of sharing a global class — feature independence
|
||||
beats DRY for a class this small.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Trivially testable use cases & controllers.** Factory functions
|
||||
take deps as arguments — tests inject mocks directly, no container
|
||||
involvement, no shared mutable state across tests.
|
||||
- **One reason to change per controller file.** Single-responsibility
|
||||
per file makes diffs and code review cleaner.
|
||||
- **Type aliases (`I*UseCase`/`I*Controller`) decouple consumers.**
|
||||
The tRPC router and any other caller depends on the type, not the
|
||||
factory impl.
|
||||
- **Naming consistency** with widely-shared community convention, lowering
|
||||
ramp-up cost for engineers familiar with Clean Architecture patterns.
|
||||
- **Real auth + media** complete the architectural symmetry — every
|
||||
feature now demonstrates the full layer stack.
|
||||
|
||||
### Negative
|
||||
|
||||
- **More files.** Per-use-case controllers added ~6 controller files
|
||||
across `blog` and `marketing-pages`; `media` added ~30 files from
|
||||
scratch.
|
||||
- **DI bindings are more verbose.** `.toDynamicValue()` blocks are
|
||||
longer than `.to()` for class bindings. Acceptable in exchange for
|
||||
factory-function purity.
|
||||
- **Two AuthenticationService methods remain `NotImplementedError`**
|
||||
pending session-cookie strategy. Documented in refactor log §7;
|
||||
unblocks development that doesn't depend on session lifecycle.
|
||||
- **Doc churn.** All references to old paths/patterns across guides,
|
||||
per-feature AGENTS, and the spec required updating in this follow-up
|
||||
pass.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Adopt `@evyweb/ioctopus`** — rejected. Already on inversify; switching
|
||||
DI libraries is high-risk for low gain.
|
||||
- **Move tests to `tests/unit/` mirror layout** — rejected. Colocation
|
||||
is established (ADR-011); `*.test.ts` next to source is unambiguous
|
||||
per-file ownership.
|
||||
- **Move to a single global container** — rejected. Per-feature
|
||||
containers (ADR-008) are load-bearing for vertical-feature isolation.
|
||||
- **Keep multi-method controllers** — rejected. The single-responsibility
|
||||
controller-per-use-case pattern wins on readability and testability.
|
||||
- **Skip real `AuthenticationService`** — rejected for `UsersRepository`
|
||||
(keep mock only) — partial real impl with documented `NotImplementedError`
|
||||
for session methods is the better trade because it unblocks the
|
||||
`auth` integration without forcing a premature cookie-strategy choice.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- All tests passing.
|
||||
- `pnpm typecheck`, `pnpm lint`, `pnpm turbo boundaries` clean.
|
||||
- No `entities/<x>.ts` files at root level.
|
||||
- No `mock-*.ts` files in feature packages.
|
||||
- No `payload-*.ts` files anywhere.
|
||||
- Every use case has `export type I*UseCase = ReturnType<typeof ...>`.
|
||||
- Every controller has `export type I*Controller = ReturnType<typeof ...>`.
|
||||
|
||||
## References
|
||||
|
||||
- Prior ADRs: ADR-006 (vertical-feature-packages), ADR-008 (per-feature DI containers), ADR-011 (TDD foundation)
|
||||
|
||||
## Update — 2026-05-06
|
||||
|
||||
ADR-013 further unifies the input/output schema story:
|
||||
schemas now live in the use-case file (a refinement of §What we
|
||||
adopted #1's "factory-function use cases"); controllers gain a
|
||||
co-located `function presenter` (extending §What we adopted #4's
|
||||
"one-controller-per-use-case"); domain error → `TRPCError`
|
||||
translation runs through a per-feature middleware factory (a new
|
||||
concern not in this ADR). See `docs/decisions/adr-013-input-output-unification.md`.
|
||||
156
docs/decisions/adr-013-input-output-unification.md
Normal file
156
docs/decisions/adr-013-input-output-unification.md
Normal file
@@ -0,0 +1,156 @@
|
||||
# ADR-013: Use-Case Input/Output Unification + Presenter Pattern + Feature-Scoped Error Mapping
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-06
|
||||
**Supersedes:** none — extends ADR-008 (per-feature DI), ADR-011 (TDD foundation), ADR-012 (feature conventions)
|
||||
|
||||
## Context
|
||||
|
||||
ADR-012 established factory-function use cases and one-controller-
|
||||
per-use-case. But the input contract was still defined three times — once
|
||||
in the tRPC procedure's `.input(z.object({...}))`, once in the controller's
|
||||
local `const inputSchema`, and once implicitly in the use case's TypeScript
|
||||
parameter type. The three definitions drifted: the controller's
|
||||
`z.string().min(3).max(31)` was stricter than the tRPC version's
|
||||
`z.string()`. Output validation was TypeScript-only — repositories could
|
||||
return malformed values and use cases happily passed them through.
|
||||
|
||||
There was also no consistent error-translation between domain errors
|
||||
(`ArticleNotFoundError`, `AuthenticationError`, …) and `TRPCError`,
|
||||
meaning the wire response code was unpredictable per feature.
|
||||
|
||||
Per-feature public-API surfaces conflated UI artifacts (query builders
|
||||
imported React Query) with pure contracts (entity types) on the same
|
||||
top-level export, making "what does this package expose to whom" muddy.
|
||||
|
||||
## Decision
|
||||
|
||||
Adopt four interlocking patterns, codified as 30 RFC-2119 rules in the
|
||||
spec:
|
||||
|
||||
1. **Use-case file is the single source of truth for input AND output
|
||||
contracts.** Every use case exports `xInputSchema` (always a
|
||||
`z.ZodObject`, even for void inputs via `z.object({}).strict()`) and
|
||||
— for non-void use cases — `xOutputSchema`. The use-case body ends
|
||||
with `xOutputSchema.parse(result)` before returning. Type aliases
|
||||
`XInput`/`XOutput`/`IXUseCase` are exported alongside.
|
||||
|
||||
2. **Controllers consume the use-case schema; output passes through a
|
||||
co-located `function presenter`.** Controllers receive `unknown`,
|
||||
`safeParse` against `xInputSchema`, throw `InputParseError` on
|
||||
failure, then call the use case and pass the result through a
|
||||
top-level `function presenter(value: XOutput)` defined in the same
|
||||
file. The controller's return type is `Promise<ReturnType<typeof
|
||||
presenter>>`. Identity presenters are permitted and expected for
|
||||
pass-through cases — the function form must always exist (R11) so
|
||||
adding a transform is a one-line edit. Void-output controllers
|
||||
(e.g., `signOutController`, `deleteMediaController`) skip the
|
||||
presenter and return `Promise<void>` (R12).
|
||||
|
||||
3. **Feature-scoped error→TRPCError middleware.** Each feature's
|
||||
`integrations/api/procedures.ts` exports an `xProcedure` built from
|
||||
`t.procedure.use(defineErrorMiddleware([[ErrorCtor, "TRPC_CODE"],
|
||||
...]))`. The factory `defineErrorMiddleware` lives in
|
||||
`core-shared/trpc/`; it discriminates by `instanceof` and preserves
|
||||
the original error as `TRPCError.cause`. **`core-shared` never
|
||||
enumerates feature-specific error classes** — each feature passes its
|
||||
own constructors in via its own `procedures.ts`. Routers use the
|
||||
feature's `xProcedure` instead of bare `publicProcedure` and
|
||||
`.input(xInputSchema)` instead of redefining input shapes.
|
||||
|
||||
4. **Per-feature public surface split.** Feature root `.` exports only
|
||||
contracts: domain types, domain errors, schemas, `IXUseCase`/`IXController`
|
||||
aliases, router type, constants. UI artifacts (query builders,
|
||||
future React components) move to a new `./ui` subpath
|
||||
(`src/ui/index.ts`). Apps that need queries import from
|
||||
`@repo/<feature>/ui`; apps that need the type-only contract import
|
||||
from `@repo/<feature>`.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Single source of truth for I/O contracts.** Schema drift is no
|
||||
longer possible — there's one definition, imported by everyone.
|
||||
- **Runtime-validated outputs.** `xOutputSchema.parse(...)` catches
|
||||
"repo returned malformed data" bugs at the layer that owns the
|
||||
contract, instead of silently flowing wrong shapes downstream.
|
||||
- **Predictable error responses.** Every domain error maps to a known
|
||||
`TRPCError.code` via the per-feature middleware; clients can rely on
|
||||
status codes.
|
||||
- **Discoverable transforms via presenter.** When a view needs to drop
|
||||
fields, rename them, or serialize dates, the presenter function is
|
||||
already there — change one function body. No structural refactor.
|
||||
- **Clean public surface.** Feature root packages no longer pretend to
|
||||
be UI packages; apps make explicit choices about what they need.
|
||||
- **Frontend gets schemas for free.** Forms can `import { signInInputSchema
|
||||
} from "@repo/auth"` and feed it into `react-hook-form` + `zodResolver`
|
||||
with the same constraints the backend enforces.
|
||||
|
||||
### Negative
|
||||
|
||||
- **More code.** Every use case grows by ~10 lines (input + output
|
||||
schema + parse). Every controller grows by ~5 lines (presenter,
|
||||
even if identity). Acceptable cost for the consistency.
|
||||
- **Per-feature `procedures.ts` boilerplate.** Five new files (~10
|
||||
lines each) — one per feature. Maintaining the error map is one of
|
||||
the few feature-level chores; new error classes need adding to the
|
||||
map.
|
||||
- **Schemas run twice on the tRPC path** (tRPC's `.input()` parse +
|
||||
controller's `safeParse`). Negligible cost; zero behavioral risk
|
||||
because both use the same schema. Defense-in-depth value when the
|
||||
controller is invoked from non-tRPC entry points.
|
||||
- **Apps with existing imports may need updating** — `articleBySlugQuery`,
|
||||
`pageBySlugQuery`, etc. now live behind `@repo/<feature>/ui`.
|
||||
(At the time of this ADR, no apps consume these yet, so the cost is
|
||||
forward-only.)
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Keep schemas in controllers.** The reference pattern has only one
|
||||
validation layer (server actions skip `.input()`), so one schema is
|
||||
sufficient. Our entry point is tRPC, which insists on a schema for
|
||||
type inference — putting the canonical schema in the controller and
|
||||
exporting it for the router was considered. Rejected because the use
|
||||
case is the contract owner; schemas describe the _operation_, not the
|
||||
_transport_.
|
||||
|
||||
- **Centralized error-name → code map in `core-shared`.** Considered
|
||||
using `error.name` discrimination with a small global registry.
|
||||
Rejected because it violates feature ownership — `core-shared` would
|
||||
need to know about every feature's error classes. The
|
||||
`defineErrorMiddleware` factory cleanly inverts the dependency:
|
||||
`core-shared` provides the plumbing, features pass their own
|
||||
constructors.
|
||||
|
||||
- **Validate outputs only in tests.** Considered using TypeScript
|
||||
types alone for output, deferring runtime validation to
|
||||
contract-suite tests. Rejected because the cost of `.parse()` on
|
||||
return is trivial and the bug-catching value at runtime is real
|
||||
(Payload integrations have surprised us before).
|
||||
|
||||
- **Presenters only when reshaping.** Considered limiting presenters to
|
||||
cases with actual transforms. Rejected because the discoverable hook
|
||||
for future shaping is worth the trivial identity-function boilerplate.
|
||||
|
||||
- **Presenters in a separate `presenters/` folder.** Considered as a
|
||||
concession to "controllers = thin orchestration". Rejected because
|
||||
co-locating the presenter with its controller keeps the contract
|
||||
visible in one file.
|
||||
|
||||
- **Shared `./schemas` subpath.** Considered exposing schemas only via
|
||||
a dedicated subpath instead of the feature root. Rejected because
|
||||
schemas ARE feature contracts — they belong with the other contracts
|
||||
(types, errors). Adding a fourth subpath felt like ceremony.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- Tests: 360 total. Coverage: every acceptance rule represented.
|
||||
- `pnpm typecheck && pnpm lint && pnpm test && pnpm turbo boundaries
|
||||
&& pnpm build` green.
|
||||
- Five feature-level router error-mapping tests demonstrate domain
|
||||
error → `TRPCError.code` translation works end-to-end.
|
||||
|
||||
## References
|
||||
|
||||
- Prior ADRs: ADR-008 (per-feature DI), ADR-011 (TDD foundation), ADR-012 (feature conventions)
|
||||
91
docs/decisions/adr-014-instrumentation-sentry.md
Normal file
91
docs/decisions/adr-014-instrumentation-sentry.md
Normal file
@@ -0,0 +1,91 @@
|
||||
# ADR-014 — Instrumentation & Sentry Logging
|
||||
|
||||
**Status:** Accepted
|
||||
**Status (revised):** Superseded by ADR-017 for the implementation layer. The interface decisions remain authoritative.
|
||||
**Date:** 2026-05-06
|
||||
|
||||
## Context
|
||||
|
||||
The monorepo had no distributed tracing or error capture. Production failures surfaced only via stdout / stderr from a Vercel function log. We needed:
|
||||
|
||||
1. End-to-end traces (browser → tRPC → use case → repo → Payload) to diagnose latency without ad-hoc timers.
|
||||
2. Centralized exception capture so silent failures (especially in CMS mutations) get a permanent record with stack + context.
|
||||
3. Privacy posture suitable for production: scrubbing for PII, masked replay, no opaque-vs-named user identifiers.
|
||||
|
||||
The reference Clean Architecture repo demonstrates a Sentry-driven pattern with `Sentry.startSpan` and `Sentry.captureException` calls inline in use cases and repos. We needed to adapt this to:
|
||||
|
||||
- Three apps (web-next, cms, web-tanstack) — not one.
|
||||
- Per-feature DI (ADR-008) — not a single container.
|
||||
- Vendor neutrality — feature packages must not import `@sentry/*` directly.
|
||||
|
||||
## Decision
|
||||
|
||||
**1. Vendor-neutral interfaces in `core-shared/instrumentation/`.** Two interfaces (`ITracer`, `ILogger`) with three implementation pairs (`Noop*`, `Sentry*`, `Recording*`). Feature packages depend only on the interfaces.
|
||||
|
||||
**2. Full-depth instrumentation.** Spans nest at every layer: tRPC procedure (auto) → controller (DI-wrapped) → use case (DI-wrapped) → repository (explicit `startSpan`) → Payload (auto). Use case + controller spans applied via `withSpan` higher-order wrapper at DI binding time so factory code stays unchanged. Repositories pay explicit boilerplate per method — accepted cost for per-method visibility.
|
||||
|
||||
**3. Throw-site capture with double-report guard.** `Sentry.captureException` fires only at the layer that originates the error (repos catch infra; use cases catch their own throws; controllers catch parse failures; middleware does not capture). A non-enumerable `__sentryReported` flag prevents re-capture as errors bubble.
|
||||
|
||||
**4. Hard PII rules (R31–R38).** `sendDefaultPii: false` (CI-grep enforced); replay default-masks all text/inputs/media (allowlist empty by default); `beforeSend` / `beforeSendTransaction` scrubbers strip emails/passwords/tokens/cookies/auth/IPs (substring-matched, including derived names like `userEmail`, `accessToken`, `apiKey`, `ipAddress`); `setUser` accepts only `{ id }`.
|
||||
|
||||
**5. Three Sentry projects, orthogonal binding.** Each app gets its own DSN. `bindAll()`'s Rule 0 (DSN → Sentry vs Noop) is independent of `USE_DEV_SEED` / `NODE_ENV` repo binding. Optional dev-mode Sentry: developer can run `pnpm dev` with `SENTRY_DSN` set to test integration locally.
|
||||
|
||||
**6. ESLint boundary rule (R40).** `no-restricted-imports` blocks `@sentry/*` outside the allowlisted paths: `core-shared/instrumentation/sentry/**`, `instrumentation/di/bind-sentry-instrumentation.{ts,test.ts}`, `core-testing/setup/no-sentry.{ts,test.ts}`, and the apps' `instrumentation*.{ts,mjs}` / `next.config.{mjs}` / `vite.config.{ts}` entries. Allowlist patterns use `**/`-prefix so they match whether ESLint runs from the repo root or from inside a sub-package.
|
||||
|
||||
**7. Test-side `RecordingTracer` / `RecordingLogger`** in `core-testing/instrumentation/`. Tests inject them directly into factory functions (direct-injection, not container manipulation). The `core-testing/setup/no-sentry.ts` setup file mocks `@sentry/nextjs`, `@sentry/node`, and `@sentry/react` at the module level, so any code that imports them gets a no-op surface during vitest runs.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Direct `@sentry/nextjs` imports in features.** Rejected — couples every feature package to a vendor SDK, violating the architecture's vendor-isolation principle.
|
||||
- **Procedure-only spans (no per-use-case or per-repo spans).** Rejected — would lose the breakdown that makes a slow request diagnosable. The middle path (procedure + use case + controller, no per-repo) was rejected for the same reason at a finer granularity.
|
||||
- **Capture in `defineErrorMiddleware` only.** Rejected — would noisily report every input-parse / unauthenticated error as a Sentry event, polluting the inbox.
|
||||
- **Single Sentry project for all apps with environment tags.** Rejected — different alert routing, different quotas. Three projects scale better.
|
||||
- **Always-Sentry in all environments.** Rejected — `pnpm test` and `pnpm dev` should not initialize a real SDK by default. Optional dev (DSN-driven) is the cleanest rule.
|
||||
- **Replay flags as configurable env vars.** Rejected — the privacy posture must be the default; opt-out requires per-selector justification (R34, R51).
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- End-to-end traces in Sentry with full context.
|
||||
- One captured event per error (no double-report, no noise from expected domain errors).
|
||||
- Privacy-by-default replay and scrubbing.
|
||||
- Vendor-swappable: replacing Sentry means writing one new adapter pair in `core-shared/instrumentation/<vendor>/`.
|
||||
- Tests run against `Recording*` for assertions; `Noop*` by default.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Every public repo method gains ~6 lines of `tracer.startSpan(...)` boilerplate. Mitigated by uniform pattern; if it ever proves excessive, a `withRepoSpan` collapse helper can be added.
|
||||
- `__sentryReported` flag mutates errors. Non-enumerable, so JSON / spread are unaffected; flag is checked only inside `SentryLogger`.
|
||||
- Three Sentry projects to administer.
|
||||
- Replay bundle peak (~250KB on errored sessions) — accepted; healthy sessions don't load replay payload.
|
||||
|
||||
## Notes from execution
|
||||
|
||||
- **PII key-substring extension (deviation from spec):** the original spec listed only explicit PII keys (`email`, `password`, `token`, etc.) for substring matching. During Task 25 we added `ipaddress` to `PII_KEY_SUBSTRINGS` so keys like `ipAddress` trigger key-level redaction. This is a tighter privacy posture than the spec's substring list — the spec's intent (no PII in events) is honoured strictly; existing `scrub.test.ts` cases continue to pass.
|
||||
- **Web-tanstack vite.config.ts deferred:** the app currently has no `vite.config.ts` (build is a placeholder per its `package.json`). The `@sentry/vite-plugin` dep is added but unused until the TanStack Start build is wired in a later plan. A minimal `src/vite-env.d.ts` shims `ImportMetaEnv` for the client entry until the full Vite types land.
|
||||
- **Subpath exports:** `core-shared/package.json` gained five new subpath entries (`./instrumentation/sentry/{init-server,init-client,init-server-node,init-client-react,scrub}`) so the apps' `instrumentation*.ts` files can import the helpers via deep paths without pulling the entire barrel.
|
||||
- **`@sentry/node` + `@sentry/react`** added as optional `peerDependencies` of `core-shared` (so feature packages don't transitively pull them) and as `devDependencies` (so typecheck/test runs in `core-shared` resolve them).
|
||||
- **Pre-existing lint nits surfaced when Task 28's restricted-imports rule lit up `pnpm lint`:** added `argsIgnorePattern: "^_"` to the shared eslint config (matches the underscore convention used throughout the repo); added `globals.node` for `*.{mjs,cjs,js}` and `*.config.{ts,tsx}` so `next.config.mjs`'s `process.env` lints clean; cleared two unused-import / unused-disable nits in marketing-pages and core-testing that were unrelated to instrumentation but blocked the lint gate.
|
||||
- **Direct `@repo/core-shared` deps:** `apps/cms` and `apps/web-tanstack` previously had only transitive access to `@repo/core-shared`; both gained explicit `workspace:*` deps so the deep `./instrumentation/sentry/*` subpath imports resolve.
|
||||
|
||||
## Post-merge follow-up — closing the R44 gap
|
||||
|
||||
The initial implementation shipped repository-side capture but **not** use-case or controller capture. The ADR/AGENTS docs described the intended capture-rules table as if it were the as-shipped state; in fact every `captureException` call site lived in `infrastructure/repositories/*.repository.ts`. A grep proved it: zero call sites in any controller or use-case body. The gap was spotted and fixed.
|
||||
|
||||
**Fix (post-merge commit):**
|
||||
|
||||
1. Extracted the `__sentryReported` flag helpers into `core-shared/instrumentation/reported-flag.ts` (`markReported`, `isReported`). `SentryLogger` now imports them; `RecordingLogger` carries an inlined copy (tooling → core import is disallowed by the boundary rule, so duplication is the right tradeoff).
|
||||
2. Added `withCapture(logger, tags, fn)` higher-order wrapper at `core-shared/instrumentation/with-capture.ts`, parallel to `withSpan`. On error: capture-with-tags, mark, re-throw — but bail if the flag is already set (covers the bubbled-from-repo case).
|
||||
3. Applied `withSpan(withCapture(factory))` to every use case and controller in every feature's `bind-production.ts` and `bind-dev-seed.ts`. Span is outermost so the errored span's timing reflects the capture-and-rethrow.
|
||||
4. `RecordingLogger.captureException` now also honours the flag, so test assertions about capture counts stay honest.
|
||||
5. Added `packages/blog/tests/r44-no-double-capture.test.ts` to lock the contract: an error originated in the repo is captured once with repo tags; an error originated in the controller (parse failure) is captured once with controller tags; success paths capture nothing.
|
||||
|
||||
**Why use cases also wrap, even though current bodies mostly delegate to the repo:** R44's intent is that _any_ throw originated locally — output-schema validation, business-rule errors like `AuthenticationError` in `signInUseCase` — gets captured with use-case tags. The wrapper makes the rule uniform; the flag makes it safe.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-008 — per-feature DI containers
|
||||
- ADR-011 — TDD foundation
|
||||
- ADR-012 — feature conventions
|
||||
- ADR-013 — input/output unification
|
||||
114
docs/decisions/adr-015-events-and-jobs.md
Normal file
114
docs/decisions/adr-015-events-and-jobs.md
Normal file
@@ -0,0 +1,114 @@
|
||||
# ADR-015 — Cross-feature events and background jobs
|
||||
|
||||
**Status:** Optional — scaffold via `pnpm turbo gen core-package events`.
|
||||
When absent, `ctx.bus` is undefined and feature binders' `bus?.subscribe/publish`
|
||||
calls are silent no-ops. Cross-feature event fanout does not operate until
|
||||
core-events is scaffolded. `IJobQueue` (in `@repo/core-shared/jobs`) and the
|
||||
`gen event`/`gen job` generators remain fully functional without core-events.
|
||||
|
||||
**Date:** 2026-05-08
|
||||
|
||||
## Context
|
||||
|
||||
Until this ADR the monorepo had no shared mechanism for _cross-feature_ communication or _deferred_ work. Two separate gaps:
|
||||
|
||||
1. **Cross-feature reactions** — when `auth` creates a user, `marketing-pages` wants to send a welcome email. Direct imports between feature packages are blocked by ESLint boundaries (R20). Without a bus, the only options were to merge the features or to leak a use-case import through `core-api`. Both compromise the vertical-slice property.
|
||||
2. **Background jobs** — heavyweight side effects (email send, image processing, periodic cleanups) belong off the request path. The repo had no contract for "enqueue and run later." Payload's job system sits in `apps/cms` but feature packages had no abstraction over it.
|
||||
|
||||
The architecture's vendor-isolation principle (R40) — feature packages must not import vendor SDKs directly — applies to Payload as well. Whatever bus and queue we ship must give features a vendor-neutral interface and route the vendor calls through one boundary layer.
|
||||
|
||||
## Decision
|
||||
|
||||
**1. Two new abstractions, both vendor-neutral.** `IEventBus` lives in `@repo/core-events` (a brand-new package); `IJobQueue` lives in `@repo/core-shared/jobs/` (a new subpath of an existing package). Both are pure TypeScript interfaces. Feature packages depend on the interface, never the implementation.
|
||||
|
||||
**2. Three rules, ESLint-enforced.**
|
||||
|
||||
- **E0 — Events are for cross-feature decoupling, not internal flow control.** In-feature reactions are direct use-case calls. The bus is for crossing feature boundaries.
|
||||
- **E1 — Event contracts are public; handlers are private.** The publisher's `events/<x>.event.ts` is exported from the feature root barrel. The consumer's `events/handlers/on-<publisher>-<event>.handler.ts` is private to the consumer's `bind-*` files and never re-exported. Custom rule `core-eslint/rules/no-handler-reexport` blocks accidental exports.
|
||||
- **J0 — Jobs are for _deferred_ work, not abstraction.** Synchronous code stays synchronous. A job exists only when something must run off the request path (latency, retries, cron).
|
||||
|
||||
A second custom ESLint rule, `no-direct-payload-jobs`, blocks `payload.jobs.queue(...)` outside `core-shared/jobs/`. Feature packages enqueue through `IJobQueue` only.
|
||||
|
||||
**3. Two bus implementations, two queue implementations, swapped by `bindAll()`.**
|
||||
|
||||
- `InMemoryEventBus` — synchronous fan-out for dev / test; respects an optional `failFast` mode.
|
||||
- `PayloadJobsEventBus` — production; each `publish()` enqueues `__events.<publisher>.<event>.<consumer>` Payload tasks for every subscribed consumer. Uses the per-feature container to resolve the wrapped handler at task-handler time.
|
||||
- `InMemoryJobQueue` — `setImmediate`-based for dev / test; supports `register(slug, handler)` so feature binders wire dispatch at boot.
|
||||
- `PayloadJobQueue` — production; thin wrapper over `payload.jobs.queue`.
|
||||
|
||||
The `apps/web-next/src/server/bind-production.ts` `bindAll()` dispatcher gains `resolveEventsAndJobsProduction()` and `resolveEventsAndJobsDevSeed()`. Selection follows the same rule order as repository binding: `USE_DEV_SEED=true` → in-memory; `NODE_ENV=production` → Payload-backed; otherwise → in-memory (developer default). Selection is orthogonal to instrumentation Rule 0.
|
||||
|
||||
**4. Subscribe takes a `consumerFeature` string.** `IEventBus.subscribe(descriptor, consumerFeature, handler)` — three arguments. The middle argument lets `PayloadJobsEventBus` enumerate concrete `__events.*.task.<consumer>` slugs at fan-out time, and lets `InMemoryEventBus.failFast` produce useful error messages. The spec's original two-arg form was widened during plan self-review so `PayloadJobsEventBus` could be directly assignable to `IEventBus`.
|
||||
|
||||
**5. Per-feature folder layout (all optional).**
|
||||
|
||||
```
|
||||
packages/<feature>/src/
|
||||
events/
|
||||
<event-kebab>.event.ts (publisher)
|
||||
handlers/on-<publisher>-<event>.handler.ts (consumer)
|
||||
jobs/
|
||||
<job-kebab>.job.ts (factory + Zod schema + ITypedJob)
|
||||
integrations/cms/jobs/
|
||||
<job-kebab>.task.ts (Payload TaskConfig)
|
||||
__events-<publisher>-<event>.task.ts (auto-generated by gen event consume)
|
||||
```
|
||||
|
||||
The Payload `TaskConfig` for an event-task uses `TaskConfig<{ input; output }>` shape (not `TaskConfig<"slug">`) because runtime-generated event slugs are not keys of `TypedJobs['tasks']`.
|
||||
|
||||
**6. Six anchor-comment slots in every feature.** Generators inject at fixed `// <gen:*>` anchors (`<gen:events>` in `src/index.ts`, `<gen:event-handler-symbols>` and `<gen:job-symbols>` in `src/di/symbols.ts`, `<gen:event-handlers>` and `<gen:jobs>` in both `bind-*.ts` files, `<gen:job-tasks>` in `src/integrations/cms/index.ts`). All five existing features were retrofitted; the `feature` generator template emits the four anchors that fall inside generated files (the `<gen:job-tasks>` location is in CMS-index, which is manually authored). A CI guard at `packages/core-eslint/anchors.test.js` asserts the anchors stay present.
|
||||
|
||||
**7. Three generators.**
|
||||
|
||||
- `pnpm turbo gen event publish <feature> <slug>` — scaffolds the contract + test, threads through `<gen:events>`.
|
||||
- `pnpm turbo gen event consume <consumer> <slug> <publisher>` — scaffolds the handler + test, plus a Payload event-task; threads through `<gen:event-handler-symbols>`, both `<gen:event-handlers>`, and `<gen:job-tasks>`. The Payload task closes the production-bus loop end-to-end.
|
||||
- `pnpm turbo gen job <feature> <slug> <void|typed>` — scaffolds the factory + test + Payload TaskConfig; threads through `<gen:job-symbols>`, both `<gen:jobs>`, and `<gen:job-tasks>`.
|
||||
|
||||
A shared `assertAnchors(repoRoot, relPath, anchors[])` helper at `turbo/generators/lib/anchor-validate.ts` is the first action of every generator path.
|
||||
|
||||
**8. `IEventBus` lives in a new package, `IJobQueue` lives in `core-shared`.** `IEventBus` is built on top of `IJobQueue` (`PayloadJobsEventBus.publish` calls `queue.enqueue`), so they cannot live in the same package without a circular import — hence the split. `IJobQueue` is closer to a system primitive (Redis-style enqueue/dequeue), `IEventBus` is application-layer pubsub.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Single package containing both interfaces.** Rejected — `PayloadJobsEventBus` depends on `IJobQueue`. If `IJobQueue` lived in `core-events`, every feature that uses _only_ jobs (no events) would still pull `core-events` transitively. The split keeps the dependency graph minimal.
|
||||
- **Synchronous in-process events without a queue layer.** Rejected for production — Payload's job system gives durability, retries, and observability for free; events that flow through it gain those properties at no extra cost.
|
||||
- **Vendor-coupled events (e.g., direct `payload.jobs.queue`).** Rejected — would re-couple feature packages to Payload, violating R40's vendor-isolation principle.
|
||||
- **Event contracts as ad-hoc TypeScript types instead of `EventDescriptor` + Zod.** Rejected — the descriptor's `name` field is the wire format the production bus uses to route to `__events.*` task slugs. Without a single source of truth, publisher and consumer can disagree at runtime. Zod gives runtime payload validation cheaply.
|
||||
- **No anchor protocol; generators target file-end positions.** Rejected — feature files evolve, file-end positions are unstable, and the ESLint config is the only place we can lock structure. Anchors give explicit injection points; the CI guard locks them.
|
||||
- **One `event` generator with mode-as-prompt vs. two separate generators (`event-publish`, `event-consume`).** The single-generator form ships, but Plop's `--args` cannot bypass conditional prompts, so the publisher prompt's `when` clause was dropped — publish mode silently ignores the field. Future revision may split the generators if the UX cost is significant.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- Cross-feature event flows that span vertical slices without violating boundaries.
|
||||
- Background work has a single contract (`IJobQueue`) that swaps from in-memory to Payload-durable per environment.
|
||||
- Vendor-swappable: replacing Payload means writing one new `IJobQueue` adapter.
|
||||
- Generators eliminate boilerplate for the most repetitive parts (handler scaffold, Payload task glue, DI bindings).
|
||||
- The proof-of-life flow (sign-up → welcome email) ships green in both dev-seed and production wiring; the dev-seed path is fully exercised by `apps/web-next/src/__tests__/sign-up-welcome-email.test.ts`.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Two queue implementations means dev-seed handlers register via `queue.register(slug, ...)` while production relies on Payload tasks resolving from the per-feature container. The dispatch story differs by environment; the abstraction hides it but it's a real surface.
|
||||
- `InMemoryEventBus` is synchronous; `PayloadJobsEventBus` is asynchronous and at-least-once. Subscribers must be idempotent.
|
||||
- Six anchor comments in every feature is more visual noise than the average reader expects. Mitigated by the CI guard (so they can't drift accidentally) and the generators (so contributors don't need to know they exist).
|
||||
- `void bus; void queue;` lines linger in feature binders that haven't yet wired any event/job (placeholder so `no-unused-vars` passes). Cosmetic; removed naturally as features adopt the system.
|
||||
|
||||
## Implementation notes
|
||||
|
||||
- **Auth is username-based, not email-based.** The spec's example contract had `email`; this ADR's `userSignedUpEvent` schema does keep `email`, but `signUpUseCase` synthesizes `${username}@example.local` to satisfy the contract. The proof-of-life flows record this synthesized email — the realism of the address is incidental to the cross-feature plumbing being verified.
|
||||
- **`apps/auth/src/di/module.ts` (the default-fallback DI module) gains `new InMemoryEventBus()` per `.toDynamicValue()` resolution.** Real cross-feature wiring runs through `bindProductionAuth` / `bindDevSeedAuth` where the bindAll-resolved bus is shared; the module's per-resolution bus is acceptable because the module is a default-mock fallback, not a runtime path.
|
||||
- **`@repo/auth` and `@repo/marketing-pages` exports were extended** for the e2e test: `./di/container`, `./di/symbols`, plus `marketing-pages` exposes `./services/mailer` and `./services/recording-mailer`. Containers and symbols being public is consistent with the binders already being public.
|
||||
- **Generator-level fixes:** dropped publisher prompt's `when` clause (Plop `--args` cannot bypass conditional prompts); switched event-task template to `TaskConfig<{ input; output }>` shape (runtime slugs aren't keys of `TypedJobs['tasks']`); registered a custom Handlebars `eq` helper for the void/typed branch in `gen job`'s template.
|
||||
|
||||
## Out of scope (deferred)
|
||||
|
||||
1. **Production-mode e2e test against Payload.** The proof-of-life test runs in dev-seed (`InMemoryEventBus` + `InMemoryJobQueue`). A parallel test that exercises `PayloadJobsEventBus` against a real Payload test database would prove the `__events.*.task.ts` slug-to-handler chain end-to-end. Skipped for v1 — requires Payload test fixtures.
|
||||
2. **Cron schedules for jobs.** Job cron schedules live in `core-cms`'s `buildConfig({ jobs: { ... } })`, not in the feature's job file or generator output. v2 may add a `--cron` prompt to `gen job`.
|
||||
3. **Event contract evolution / versioning.** The current spec has no migration story for breaking-change schema updates. v2 may introduce versioned descriptors (`auth.user.signed-up.v2`) and dual-publish helpers.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-008 — per-feature DI containers
|
||||
- ADR-010 — Turborepo boundaries
|
||||
- ADR-014 — Instrumentation & Sentry logging
|
||||
103
docs/decisions/adr-016-realtime-layer.md
Normal file
103
docs/decisions/adr-016-realtime-layer.md
Normal file
@@ -0,0 +1,103 @@
|
||||
# ADR-016 — Realtime layer (Socket.IO)
|
||||
|
||||
**Status:** Optional — scaffold via `pnpm turbo gen core-package realtime`. The package is not in the default template; this ADR documents the design that the generator emits.
|
||||
**Date:** 2026-05-08
|
||||
|
||||
## Context
|
||||
|
||||
Until this ADR the monorepo had no mechanism for the server to push state to a connected browser without polling. tRPC covers request/response; the event bus from ADR-015 covers in-process cross-feature publish/subscribe. Neither delivers a server-side state change to an open browser tab without polling.
|
||||
|
||||
Two concerns drove this work. First, establishing the abstraction seam — the same logic that motivated `IEventBus` and `IJobQueue` in ADR-015: define the interface once so individual features can adopt realtime on demand without each designing its own socket integration. Second, an admin live-observability dashboard that streams event/job traffic is the first concrete consumer of the seam, but its UI work is large enough for a separate PR. v1 therefore ships a built-in `realtime-ping` channel as the proof-of-life and defers the dashboard.
|
||||
|
||||
The vendor-isolation principle from ADR-014 and ADR-015 carries over without modification: the wire-protocol library (`socket.io`) is hidden behind `IRealtimeBroadcaster` / `IRealtimeServer` interfaces. Feature packages MUST NOT import `socket.io` directly.
|
||||
|
||||
## Decision
|
||||
|
||||
**1. Three rules, ESLint-enforced.**
|
||||
|
||||
- **R0 — Realtime is for state delivery, not for replacing tRPC.** Persistent operations with request/response semantics belong on tRPC procedures. Use realtime when (a) the server needs to push without a request, or (b) the data is too high-frequency for HTTP.
|
||||
- **R1 — Channel descriptors are exported; handlers are private.** A feature's `realtime/<name>.channel.ts` is re-exported from the package root barrel. A feature's `realtime/handlers/<name>.handler.ts` is wired only inside that feature's own `bind-production` / `bind-dev-seed` and is never re-exported from any subpath. Custom rule `no-realtime-handler-reexport` enforces this — parallel to ADR-015's `no-handler-reexport` for event handlers.
|
||||
- **R2 — `socket.io` lives in one package only.** Feature packages MUST NOT `import "socket.io"` or `import "socket.io-client"`. The only allowlist entries are `packages/core-realtime/src/socket-io-*.ts` and `apps/*/server.ts`. ESLint rule `no-direct-socket-io` enforces this — parallel to ADR-015's `no-direct-payload-jobs`.
|
||||
|
||||
**2. New package `@repo/core-realtime`.** Parallel in shape to `@repo/core-events` from ADR-015. Tagged `core`. Exports pure TypeScript interfaces (`IRealtimeBroadcaster`, `IRealtimeServer`, `IRealtimeAuthenticator`, `IRealtimeHandlerRegistry`), the `defineRealtimeChannel` factory, four scope kinds (`"public"`, `"authenticated"`, `{ role }`, `{ userScoped }`), and Socket.IO adapter classes (`SocketIORealtimeBroadcaster`, `SocketIORealtimeServer`). Also exports `InMemoryRealtimeBroadcaster` for dev/test use — no Socket.IO dependency, stores broadcasts in memory. Feature packages depend on interfaces only; the Socket.IO adapter classes are consumed exclusively by the app's custom Node server.
|
||||
|
||||
**3. Four auth checkpoints, pure function authorization.** The connect handler (gate 1) reads the session cookie, calls `IRealtimeAuthenticator.authenticate()`, and attaches `{ userId, roles } | null` to `socket.data.user`. Channel subscribe (gate 2) matches the requested name against registered descriptors (template-aware for `"notifications.user.{userId}"`-style channels), calls `authorize(descriptor, params, user)`, and on success calls `socket.join("ch:<name>")`. Inbound message (gate 3) re-validates schema and re-applies `authorize` as defense-in-depth, then invokes the wrapped handler with `ctx = { userId, roles }`. Broadcast (gate 4) has no gate — `io.to("ch:<name>").emit(...)` fans out to whoever cleared gate 2; subscribe is the single source of truth. `authorize` is a pure function with no DB hit.
|
||||
|
||||
**4. Hybrid bus-bridge / direct-broadcast model.** The existing `IEventBus` from ADR-015 is reused as a _third consumer_ in a bridge pattern: a `bindRealtimeBridge(bus, broadcaster, allowlist)` step in `bindAll()` subscribes to allowlisted bus events and forwards them onto realtime channels. The bridge allowlist ships empty in v1; the first entries land with the dashboard PR. Direct broadcast (feature use case adds `realtime: IRealtimeBroadcaster` to its factory deps and calls `realtime.broadcast(channel, payload)`) is the primary path; the bridge is for cases where bus events already exist and realtime is additive.
|
||||
|
||||
**5. Custom Node server for `apps/web-next`.** `apps/web-next/server.ts` replaces `next start` / `next dev` as the boot entry. Both Next.js and Socket.IO share one http server on port 3000. The `bindAll()` dispatcher gains two new resolution steps: `resolveRealtime()` (picks `InMemoryRealtimeBroadcaster` vs `SocketIORealtimeBroadcaster` by env) and the bridge wiring call. `bindAll(deps?)` is optional — callers may pass pre-constructed broadcaster/registry instances (the server does) or omit them and receive `InMemoryRealtimeBroadcaster` defaults (page-handler callers, existing tests). A `bound` guard ensures the Noop defaults are never silently accepted in production.
|
||||
|
||||
**6. Per-feature folder layout (all optional).**
|
||||
|
||||
```
|
||||
packages/<feature>/src/
|
||||
realtime/
|
||||
<name>.channel.ts ← channel descriptor (re-exported from root barrel)
|
||||
handlers/
|
||||
on-<name>.handler.ts ← inbound handler (private, never re-exported)
|
||||
```
|
||||
|
||||
Top-level `realtime/`, not under `integrations/`. Channel descriptors are descriptor-shaped (same shape as event descriptors from ADR-015, which also live at top level in `events/`). There is no per-feature transport code — the Socket.IO server lives once in `core-realtime`.
|
||||
|
||||
**7. Three new anchors per feature, two new generators.** All five existing features plus the `feature` generator template gain three new `// <gen:*>` anchor comments (`// <gen:realtime-channels>` in `src/index.ts`, `// <gen:realtime-handler-symbols>` in `src/di/symbols.ts`, `// <gen:realtime-handlers>` in both `bind-*.ts` files). The CI anchor guard in `packages/core-eslint/anchors.test.js` extends to assert these stay present.
|
||||
|
||||
- `pnpm turbo gen realtime --args channel <feature> <slug> <scope>` — scaffolds `<slug>.channel.ts` + test; threads the re-export through `<gen:realtime-channels>`.
|
||||
- `pnpm turbo gen realtime --args handler <feature> <channel-slug>` — scaffolds `on-<channel>.handler.ts` + test; threads the wrapped registration through `<gen:realtime-handler-symbols>` and both `<gen:realtime-handlers>` anchors.
|
||||
|
||||
Handlers are wrapped in the same `withSpan(tracer, { op: "realtime-handler" }, withCapture(logger, { layer: "realtime-handler" }, handlerFactory(deps)))` sandwich as use cases and controllers (ADR-014 R41–R44).
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **WebSockets without Socket.IO.** Rejected — Socket.IO's room-based fan-out, built-in reconnect, and namespace support cover the subscription / user-scoped-channel use cases cleanly. The vendor-isolation interface means swapping later is one adapter rewrite.
|
||||
- **Server-Sent Events (SSE).** Considered for server-push-only scenarios. Rejected — the inbound handler (client → server) use case rules it out, and maintaining two separate realtime primitives for push vs bidirectional adds protocol surface without architectural benefit.
|
||||
- **Merge realtime into `core-shared`.** Rejected — `core-realtime` depends on `socket.io` for the production adapter. Adding that dep to `core-shared` would force every feature to transitively pull the SDK, violating the vendor-isolation principle that ADR-014 and ADR-015 both enforce. A new package keeps the dep graph minimal.
|
||||
- **No bridge; direct broadcast only.** Considered and partially adopted (the bridge ships empty in v1). Rejected as the permanent answer — cases where a bus event should fan out to multiple durable consumers AND push to live clients are real (article published → search index + live feed). Two APIs, mutually independent, cover the matrix without forcing one shape on the other.
|
||||
- **Per-feature transport code (each feature owns its socket event names).** Rejected — channel descriptors and `SocketIORealtimeServer` handle the dispatch table centrally. Spreading socket event name strings across feature packages recreates the problem that `EventDescriptor.name` solved in ADR-015.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- Server-push to connected browser tabs without polling, across any feature on demand.
|
||||
- Vendor-swappable: replacing Socket.IO means writing one new adapter pair (`IRealtimeBroadcaster` + `IRealtimeServer`).
|
||||
- The existing `IEventBus` is reused as the bridge source; features that already publish events get realtime fan-out for free via one `allowlist` entry.
|
||||
- Four auth checkpoints are centrally managed — features don't each implement cookie parsing or scope authorization.
|
||||
- `RecordingRealtimeBroadcaster` in `core-testing` gives use-case tests a drop-in broadcaster that records calls, mirroring `RecordingEventBus`.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- `bindProductionX` / `bindDevSeedX` now take seven arguments `(config, tracer, logger, bus, queue, realtime, realtimeRegistry)`. Future expansion may warrant collapsing to a single `BindContext` object; deferred.
|
||||
- The custom Node server for `apps/web-next` means `next start` / `next dev` are no longer sufficient entry points. The CMS and TanStack apps still use their existing runtimes until they need realtime.
|
||||
- `InMemoryRealtimeBroadcaster` has no room/socket model — it stores all broadcasts in a flat array. Sufficient for unit testing; insufficient for integration tests that assert specific sockets received a broadcast. The `realtime-ping` integration test uses a real `SocketIORealtimeServer` in-process.
|
||||
|
||||
## Notes from execution
|
||||
|
||||
- **`RecordingRealtimeBroadcaster` scope-type alias widened.** The spec's local type alias `RealtimeChannelDescriptor<TName, TSchema>` in `core-testing` was widened to handle the discriminated-union shape of the actual descriptor correctly. The recorded-broadcast entries use `{ channel: string; payload: unknown }` to avoid tying the recording type to the exact generic parameters.
|
||||
- **`IRealtimeHandlerRegistry` gained `registerChannel` / `listChannels`.** The original spec had only `register` / `getInboundDescriptor` / `list`. `registerChannel` and `listChannels` were added to support outbound-only channel subscription: gate 2 (subscribe authorization) iterates `listChannels` + `register`ed descriptors independently of inbound handler registration, separating the "is this a known channel?" check from "does this channel have an inbound handler?"
|
||||
- **`bindAll(deps?)` is optional with `InMemoryRealtimeBroadcaster` defaults.** Existing page-handler callers that invoke `bindAll()` with no args continue to work without modification. A `bound` guard ensures that in production, where `bindAll()` is always called from `server.ts` with explicit `SocketIORealtimeBroadcaster` / `RealtimeHandlerRegistry` args, the in-memory fallback is never silently wired.
|
||||
|
||||
## Out of scope (deferred)
|
||||
|
||||
1. **Live observability dashboard.** The first concrete realtime consumer. Bridge allowlist entries land in that PR; v1 ships the allowlist empty.
|
||||
2. **DB-backed roles / permissions.** `authorize` today reads whatever `IRealtimeAuthenticator` returns. When the auth feature gains a `RolesRepository`, `authenticate()` will return populated `roles` / `permissions` arrays with no changes to `core-realtime`.
|
||||
3. **Multi-instance fanout.** Redis adapter / sticky sessions for horizontally scaled deployments. The `IRealtimeBroadcaster` interface is the seam.
|
||||
4. **Custom Node server for `cms` and `web-tanstack`.** Both apps continue on their existing runtimes until they need realtime.
|
||||
5. **Production-mode e2e test.** The `realtime-ping` integration test exercises the four checkpoints in-process. A multi-socket test that verifies fan-out across N connected clients is deferred to v2.
|
||||
|
||||
## Known follow-ups (post-merge polish)
|
||||
|
||||
Items surfaced by the final branch review that were intentionally not landed in v1:
|
||||
|
||||
1. **`bindRealtimeBridge` stub has no test scaffolding.** v1 ships `apps/web-next/src/server/bind-production.ts:bindRealtimeBridge` as a no-op (`_`-prefixed args). The dashboard PR adds the first allowlist entries; a minimal contract-shaped test (e.g. accept `allowlist: BridgeEntry[]` and assert subscribe wiring) should land alongside the first entry, not before.
|
||||
2. **`IRealtimeHandlerRegistry.register` + `registerChannel` precedence is implicit.** Calling both for the same channel name silently overwrites the channel-map descriptor while leaving the entry-map intact. Behaviour is correct for current callers; document the precedence in the interface JSDoc or reject conflicting re-registration once the second outbound channel ships.
|
||||
3. **`matchChannelTemplate` placeholders cannot contain dots** (`packages/core-realtime/src/channel-template.ts:14-17` uses `([^.]+)`). Fine for UUID-style identifiers; document the constraint in `defineRealtimeChannel`'s JSDoc when the first non-UUID key shape arrives.
|
||||
4. **`SocketIORealtimeServer` swallows handler errors with bare `catch {}`** (`packages/core-realtime/src/socket-io-realtime-server.ts:108-117`). Wrapped handlers (`withCapture`) already record the error; unwrapped handlers lose it. Adding a server-injected logger that records "handler_error for channel X" would help debug connection-level issues — defer until a debugging incident actually motivates it.
|
||||
5. **`bindAll(deps?: Partial<BindAllDeps>)` permits a half-populated deps object** that mixes a real broadcaster with a fresh registry (or vice versa). In practice no caller does this, but the type doesn't enforce all-or-nothing semantics. Tighten to `deps?: BindAllDeps` (full or absent) when the next consumer lands.
|
||||
6. **AGENTS.md anchor count phrasing.** AGENTS.md says "three fixed `// <gen:realtime-*>` anchor comments per feature." There are three _kinds_ but four placements (the handlers anchor lives in both `bind-production.ts` and `bind-dev-seed.ts`). Tighten to "three anchor kinds across both bind files" when the AGENTS.md is next touched.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-008 — per-feature DI containers
|
||||
- ADR-010 — Turborepo boundaries
|
||||
- ADR-014 — Instrumentation & Sentry logging (span+capture sandwich reused for realtime handlers)
|
||||
- ADR-015 — Cross-feature events and background jobs (`IEventBus` reused as the bridge source)
|
||||
53
docs/decisions/adr-017-opentelemetry-migration.md
Normal file
53
docs/decisions/adr-017-opentelemetry-migration.md
Normal file
@@ -0,0 +1,53 @@
|
||||
# ADR-017 — OpenTelemetry Migration
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-11
|
||||
**Supersedes (impl section):** ADR-014
|
||||
|
||||
## Context
|
||||
|
||||
ADR-014 established vendor-neutral `ITracer` + `ILogger` interfaces with Sentry as the active backend. The interface decisions (R31–R51) have held up; what coupled to a vendor was the **substrate**: `SentryTracer` and `SentryLogger` called Sentry SDK methods directly. Swapping vendors required rewriting every `*Tracer`/`*Logger` pair.
|
||||
|
||||
This ADR migrates the substrate to OpenTelemetry: code emits OTel spans, logs, and metrics; exporters route to one or more backends. Sentry is wired as the (initially only) exporter via `@sentry/opentelemetry`. Swapping vendors becomes an exporter swap.
|
||||
|
||||
## Decision
|
||||
|
||||
1. **OTel SDK as substrate.** Server-side `ITracer` and `ILogger` impls use `@opentelemetry/api` and `@opentelemetry/api-logs` respectively. New `IMetrics` signal added via OTel metrics API.
|
||||
2. **Sentry-as-exporter.** `@sentry/opentelemetry` provides `SentrySpanProcessor` + `SentryLogRecordProcessor`. They consume OTel signals and forward to Sentry. Sentry's UI experience is preserved (minus some browser-side richness, addressed below).
|
||||
3. **Server-only scope.** Browser keeps Sentry SDK directly. Replay + session-error correlation stay native. Future spec extends OTel to browser when warranted.
|
||||
4. **Pure OTel Logs API for the logger.** `OtelLogger` emits via `@opentelemetry/api-logs`. Trade-off: slightly degraded Sentry-native error UX (stack normalization, breadcrumb buffer) in exchange for swap-by-exporter vendor neutrality.
|
||||
5. **Breadcrumbs → span events.** `ILogger.addBreadcrumb` attaches to the active OTel span as an event. Native OTel pattern.
|
||||
6. **`setUser` per-span.** Sets `user.id` as a span attribute on the active span. R36 preserved (id only; no email/username).
|
||||
7. **PII scrubbing migrated.** From Sentry's `beforeSend`/`beforeSendTransaction` hooks to OTel `SpanProcessor` + `LogRecordProcessor` impls (`PiiScrubSpanProcessor`, `PiiScrubLogRecordProcessor`). Processors run BEFORE the Sentry exporter, so PII is stripped at the OTel layer regardless of downstream exporter. Browser init files (`init-client.ts`, `init-client-react.ts`) retain `beforeSend`/`beforeSendTransaction` hooks because they do not use the OTel pipeline.
|
||||
8. **R52 new ESLint rule.** `@opentelemetry/sdk-*`, `@opentelemetry/exporter-*`, `@opentelemetry/instrumentation-*`, `@opentelemetry/resources`, `@opentelemetry/semantic-conventions` restricted to `**/instrumentation/otel/**` and app init paths. `@opentelemetry/api` and `@opentelemetry/api-logs` are unrestricted within `core-shared/instrumentation/`.
|
||||
9. **`bindSentryInstrumentation` renamed to `bindOtelInstrumentation`** with a deprecation alias for one release cycle.
|
||||
10. **`IMetrics` synchronous-only.** Three methods: `counter`, `histogram`, `gauge`. `gauge` uses `UpDownCounter` under the hood; true "set" gauge semantics require an `ObservableGauge` with a periodic callback, deferred to a v2 metrics interface.
|
||||
11. **Auto-instrumentations enabled.** HTTP (`@opentelemetry/instrumentation-http`), undici (`instrumentation-undici`), pg (`instrumentation-pg`) registered in `initOtelServerNode`. HTTP instrumentation strips query strings from `http.url.path` attribute and ignores `/_health` and `/_otel-export` paths. PgInstrumentation has `enhancedDatabaseReporting: false` to avoid SQL statement capture (R32 — SQL often contains PII in WHERE clauses).
|
||||
12. **`no-sentry.ts` → `no-instrumentation.ts` in `core-testing`.** Renamed with backward-compat alias for one release. Mocks both Sentry SDK and OTel SDK modules to prevent real init in vitest runs.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Keep Sentry SDK directly.** Rejected — couples impl to Sentry forever.
|
||||
- **OTel SDK + keep Sentry-direct for `captureException`.** Rejected — partial vendor swap re-introduces lock-in for the error path.
|
||||
- **Migrate browser too.** Rejected — OTel-Browser maturity in 2026 is good for traces but Sentry's browser SDK has features (replay, native error correlation) that don't yet have OTel equivalents.
|
||||
- **Put PII scrub in Sentry exporter config.** Rejected — `beforeSend` hooks run inside the Sentry SDK after OTel signals are converted; the OTel processor layer is earlier and vendor-agnostic. Scrubbing at the processor layer means any future exporter added alongside Sentry also sees clean data.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- Vendor swaps are exporter swaps. Adding Honeycomb / Datadog / Grafana Cloud / Tempo is just adding their exporter alongside Sentry's.
|
||||
- Auto-instrumentations (HTTP, undici, pg) reduce manual span boilerplate.
|
||||
- New `IMetrics` signal available; metrics call sites can land per-feature opportunistically.
|
||||
- PII scrubbing is vendor-neutral — applies before any exporter sees the data.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Sentry-native error UX is slightly degraded (errors arrive as OTel log records instead of native Sentry events). Acceptable per vendor-neutrality goal.
|
||||
- Breadcrumb semantics shift from buffered cross-span to per-span events. Acceptable.
|
||||
- Browser is still Sentry-direct — observability stack is asymmetric server vs. browser until a future browser migration.
|
||||
- OTel SDK adds dependency surface (~12 new packages in `core-shared`).
|
||||
|
||||
## Relationship to ADR-014
|
||||
|
||||
ADR-014's interface decisions (R31–R51) remain authoritative. This ADR supersedes only the implementation section (Sentry SDK direct calls → OTel SDK). ADR-014 keeps a "Status: Superseded for impl by ADR-017" header.
|
||||
102
docs/decisions/adr-018-audit-and-compliance.md
Normal file
102
docs/decisions/adr-018-audit-and-compliance.md
Normal file
@@ -0,0 +1,102 @@
|
||||
# ADR-018 — Audit Logging & DPA Compliance
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-11
|
||||
**Companion guide:** docs/guides/audit-and-compliance.md
|
||||
|
||||
## Context
|
||||
|
||||
DPA compliance mandates audit logging for every personal-data access event:
|
||||
VIEW/CREATE/UPDATE/DELETE/EXPORT/PERMISSION_CHANGE, with immutable storage,
|
||||
GDPR-deletable path, centralized aggregation, and strict "what NOT to log"
|
||||
boundaries. The interface decisions from ADR-014 (R31-R51) carry over but
|
||||
audit needs its own channel — observability data is sampled and short-retention,
|
||||
audit data is lossless and long-retention with privileged erasure.
|
||||
|
||||
## Decision (12 points)
|
||||
|
||||
1. **`AuditLogProtocol` in `core-shared`** — must-have universal surface.
|
||||
Features call `ctx.auditLog?.record(entry)` without importing the optional package.
|
||||
2. **`AuditEntry` type with closed action enum** — VIEW/CREATE/UPDATE/DELETE/
|
||||
EXPORT/PERMISSION_CHANGE/CONSENT_GRANT/CONSENT_WITHDRAW/RESTRICT/UNRESTRICT;
|
||||
new actions require explicit type bump. No payload/body/oldValue/newValue
|
||||
fields — type enforces "what NOT to log".
|
||||
3. **`@repo/core-audit` as 5th optional package** — joins realtime, events,
|
||||
trpc, ui. Scaffolded via `pnpm turbo gen core-package audit`.
|
||||
4. **Four impls + Recording test double**: NoopAuditLog, PayloadAuditLog
|
||||
(local cache), StdoutJsonAuditLog (operator ships via Vector/Fluent Bit),
|
||||
MultiSinkAuditLog (fan-out), RecordingAuditLog (core-testing).
|
||||
5. **Append-only Payload collection** — `update: () => false` access rule
|
||||
is the compliance backbone; erasure path uses `overrideAccess: true`.
|
||||
6. **GDPR erasure** — sha256-salted pseudonymization (`erased-{hash[0:16]}`)
|
||||
or hard delete. AUDIT_PSEUDONYM_SALT env REQUIRED in production; bind-time
|
||||
validation fails fast.
|
||||
7. **Erasure trigger surface** — admin tRPC procedure (`audit.eraseSubject`),
|
||||
Payload `afterDelete` hook factory (`createAuditErasureHook`), auth
|
||||
integration via printed generator next-steps (NOT auto-installed).
|
||||
8. **OTel correlation bridge** — `currentTraceId()` helper in core-shared;
|
||||
`TraceIdEnrichingAuditLog` decorator at bind time auto-populates
|
||||
`AuditEntry.correlationId` from active OTel span. Explicit caller wins.
|
||||
9. **VIEW capture via BOTH patterns** — use-case `record()` calls (developer
|
||||
decides per-read-path) AND `createAuditAfterReadHook` factory (opt-in
|
||||
per-collection automatic capture). Fire-and-forget for hooks.
|
||||
10. **IP/UA explicit at call sites** — no AsyncLocalStorage. Callers use
|
||||
`truncateIp(raw)` (/24 IPv4, /48 IPv6) and pass into `record({ from: { ... } })`.
|
||||
Sentinels for non-HTTP context: `"system"` / `"background-job"`.
|
||||
11. **Multi-tenancy: tenant field required** — `AuditEntry.scope.tenant`
|
||||
non-optional; single-tenant projects pass `"default"`. Forces multi-tenant
|
||||
thinking from day one.
|
||||
12. **Six-phase delivery** matching established cadence.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Vendor-coupled SDK (Datadog/Grafana direct)** — rejected; couples to vendor.
|
||||
- **Payload-only sink** — fails compliance (hostile-actor immutability).
|
||||
- **Aggregator-only sink** — fails dev ergonomics. Fan-out is the balance.
|
||||
- **AsyncLocalStorage for request context** — rejected per user preference;
|
||||
explicit > implicit.
|
||||
- **Optional tenant field** — rejected; DPA-aligned scope discipline benefits
|
||||
from forcing the question on every call.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- DPA-compliant baseline ships with the optional package.
|
||||
- Vendor-neutral via stdout JSON + log shipper; any aggregator works.
|
||||
- OTel correlation gives compliance auditors one-click pivot to traces.
|
||||
- Type-enforced exclusion of "what not to log" prevents categories of mistakes.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Boilerplate at every record() call site (IP/UA explicit).
|
||||
- core-audit ↔ auth coupling for the user-collection hook is awkward
|
||||
(manual install via generator next-steps).
|
||||
- StdoutJsonAuditLog's eraseSubject is best-effort (tombstone only; past
|
||||
stdout lines can't be retroactively removed).
|
||||
|
||||
## Relationship to other ADRs
|
||||
|
||||
- ADR-014 (instrumentation interfaces): audit is a parallel channel, not a
|
||||
signal flowing through OTel. The correlationId field is the bridge.
|
||||
- ADR-015 (events/jobs): no overlap; audit is observational, events are reactive.
|
||||
- ADR-017 (OTel migration): provides currentTraceId() helper.
|
||||
|
||||
## Amendments
|
||||
|
||||
### 2026-05-19 — Consent and restriction action types (Epic B, ADR-025)
|
||||
|
||||
Added four new `AuditAction` values to `core-shared/audit/audit-entry.ts`:
|
||||
|
||||
| Action | Article | Description |
|
||||
| ------------------ | ------------ | ------------------------------------------------- |
|
||||
| `CONSENT_GRANT` | GDPR Art. 7 | Subject granted consent for a processing purpose |
|
||||
| `CONSENT_WITHDRAW` | GDPR Art. 7 | Subject withdrew consent for a processing purpose |
|
||||
| `RESTRICT` | GDPR Art. 18 | Subject requested restriction of processing |
|
||||
| `UNRESTRICT` | GDPR Art. 18 | Restriction lifted (controller or subject action) |
|
||||
|
||||
**Reason:** `core-consent` and `core-dsr` optional packages (Story 03 and 06
|
||||
of Epic B) emit these action types via `core-audit`'s existing `IAuditLog`
|
||||
channel. The values must exist in `core-shared`'s closed enum before either
|
||||
optional core can be implemented. No change to `IAuditLog`'s interface surface —
|
||||
the new values flow through `AuditEntry.action` automatically.
|
||||
134
docs/decisions/adr-019-sandcastle-for-agent-orchestration.md
Normal file
134
docs/decisions/adr-019-sandcastle-for-agent-orchestration.md
Normal file
@@ -0,0 +1,134 @@
|
||||
# ADR-019 — Sandcastle for Agent Orchestration
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-13
|
||||
**Spec:** docs/architecture/agent-first-workflow-and-conformance.md
|
||||
**Companion guide:** docs/guides/runbook.md ("Using Sandcastle for agent dispatch")
|
||||
**Related:** ADR-011 (TDD foundation), ADR-012 (feature conventions), ADR-015 (events and jobs)
|
||||
|
||||
## Context
|
||||
|
||||
This template is designed for **agent-driven feature development**. The conformance
|
||||
system (ADR-012 + the post-ADR conformance-system-v1 epic) gives agents a tight,
|
||||
layered feedback loop — type errors in 0s, lint in <1s, boot assertion in ~3s, CI
|
||||
gates in ~120s. The remaining substrate question is: how does an agent actually
|
||||
get dispatched against a task?
|
||||
|
||||
Three pieces are needed:
|
||||
|
||||
1. **A way to invoke an agent** (Claude / Codex) with a task description,
|
||||
inside a sandbox so the agent can't break the host while iterating.
|
||||
2. **A way to capture the agent's commits** so a reviewer agent can inspect
|
||||
the diff and approve or reject.
|
||||
3. **A way to compose the above into a per-task dispatch loop** with retry
|
||||
semantics, branch management, and integration into the existing
|
||||
docs/work/ task system.
|
||||
|
||||
Without a substrate that handles all three, agentic development falls back to
|
||||
copy-paste-prompt-by-hand, which is slow and error-prone.
|
||||
|
||||
## Decision
|
||||
|
||||
Adopt [Sandcastle](https://github.com/mattpocock/sandcastle) (`@ai-hero/sandcastle`)
|
||||
as the agent-orchestration substrate. `pnpm work dispatch` is the entry point.
|
||||
|
||||
Concretely:
|
||||
|
||||
1. **`@ai-hero/sandcastle` is a workspace-root devDependency.** Pinned at
|
||||
`^0.5.10` at adoption; pnpm resolves later patches automatically.
|
||||
2. **`.sandcastle/` holds the canonical prompt templates.** Five role-specific
|
||||
prompts: PRD eliciter, ADR eliciter, decomposer, implementer, reviewer.
|
||||
Each enforces the **generator-first** rule (prefer `pnpm turbo gen <kind>`
|
||||
over hand-rolling — see saved memory `generator-first-for-agents`).
|
||||
3. **`.sandcastle/Dockerfile`** is the sandbox baseline (node:22-bookworm-slim
|
||||
- pnpm via corepack). The agent runs `pnpm install --frozen-lockfile` as
|
||||
its first step per the implementer prompt.
|
||||
4. **`scripts/work/dispatch.mjs` is the orchestrator.** It reads `_state.json`,
|
||||
finds the first ready story's first unchecked AC bullet, builds a task spec,
|
||||
and calls `sandcastle.run({ promptFile, promptArgs: { TASK_FILE_CONTENT } })`
|
||||
for the implementer, then again for the reviewer with `{{DIFF}}`. The
|
||||
orchestrator does NOT mutate state in v1 — it prints suggested mutations
|
||||
for the human to apply.
|
||||
5. **Two modes:** `pnpm work dispatch` (planning, no agent invoked) and
|
||||
`pnpm work dispatch --execute` (real sandcastle call, requires auth — see
|
||||
point 7).
|
||||
6. **Reviewer agent verifies generator-first.** Hand-rolled output that should
|
||||
have been a `pnpm turbo gen <kind>` invocation is grounds for rejection.
|
||||
7. **Bring-your-own-auth.** Two paths are supported, in priority order:
|
||||
- **Subscription (primary)** — bind-mount the host's `~/.claude/` into the
|
||||
sandbox. Claude Code CLI inside the sandbox uses the host's logged-in
|
||||
subscription session. Zero per-task token spend for Pro/Max subscribers.
|
||||
Path overridable via `SANDCASTLE_CLAUDE_CREDS_DIR` env var.
|
||||
- **API key (fallback)** — `ANTHROPIC_API_KEY` or `OPENAI_API_KEY` passed
|
||||
through to the sandbox env. Used when no host creds directory exists.
|
||||
- The resolver (`resolveClaudeAuth` in `scripts/work/dispatch.mjs`) picks
|
||||
automatically with subscription always preferred. Sandcastle's own issue
|
||||
#191 documents that subscription support won't be added natively;
|
||||
this mount-based pattern is our workaround promoted to first-class.
|
||||
8. **Per-task max-attempts honoured (v2).** Each task's frontmatter may carry
|
||||
`max-attempts: N` to bound the implementer↔reviewer retry loop. Default 3.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Bare Claude Code / Codex CLI invocation per task** — rejected. No sandbox
|
||||
isolation; no consistent prompt template surface; no built-in branch
|
||||
management; no reviewer-loop primitive.
|
||||
- **GitHub Copilot Workspace / native CI agent** — rejected. Vendor lock-in;
|
||||
workflow lives outside the repo; no local equivalent for development time.
|
||||
- **Custom orchestrator built from scratch on the Anthropic SDK** — rejected.
|
||||
Sandcastle already solves sandbox + branch + structured-output extraction;
|
||||
rebuilding it is not the leverage point.
|
||||
- **No orchestrator — humans dispatch each task manually via copy-paste** —
|
||||
rejected as the steady-state mode, but supported as a fallback via planning
|
||||
mode (`pnpm work dispatch` without `--execute`).
|
||||
- **A different sandbox provider (Vercel sandboxes, Daytona, native fly.io)**
|
||||
— sandcastle is provider-agnostic; the choice of provider sits behind the
|
||||
`SANDCASTLE_PROVIDER` env var and can change without disrupting prompts or
|
||||
orchestrator code. Default is Docker.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Per-task isolation.** Each implementer dispatch runs in its own Docker
|
||||
sandbox + sandbox branch. Bad agent output stays in the branch; merge to
|
||||
`main` is gated by the reviewer agent + the full 5-gate stack.
|
||||
- **Provider-agnostic.** Switching from Claude to Codex (or to a future
|
||||
agent runtime) is a one-line change to the prompt's `agent` parameter.
|
||||
- **Composable with existing workflow.** `pnpm work` CLI already reads
|
||||
`_state.json` and the docs/work/ markdown; dispatch is one more subcommand
|
||||
layered on top.
|
||||
- **Cost-aware default.** Planning mode invokes no agent; only `--execute`
|
||||
spends tokens. Operators choose when to escalate from plan to execute.
|
||||
- **Recoverable failure modes.** If an implementer goes off-rails, its diff
|
||||
lives on a sandbox branch — review, reject, re-dispatch with notes.
|
||||
|
||||
### Negative / accepted trade-offs
|
||||
|
||||
- **External dependency on sandcastle.** If the project stalls, we either pin
|
||||
- maintain a fork or migrate to another orchestrator. Sandcastle is small
|
||||
enough (~3KLOC) that a fork is manageable.
|
||||
- **Token cost is real.** A complex task can use 100K-200K tokens per
|
||||
implementer + reviewer round-trip. Operators budget per-dispatch; the
|
||||
planning mode + the optional `max-attempts` frontmatter cap exposure.
|
||||
- **Docker dependency for the default sandbox.** Without Docker (or a
|
||||
provider swap), `--execute` won't run. Documented in the runbook.
|
||||
- **State mutation is manual in v1.** The orchestrator prints suggested
|
||||
state mutations; a human ticks the AC bullet + commits. Auto-mutation is
|
||||
v2 work, gated on confidence that the reviewer's decision can be trusted
|
||||
without human inspection.
|
||||
|
||||
### Follow-up work
|
||||
|
||||
- **Auto state mutation** — when the reviewer agent's decision is approve,
|
||||
the orchestrator could automatically tick the AC bullet + commit. Currently
|
||||
manual; promote when reviewer confidence is established empirically.
|
||||
- **Multi-task batch dispatch** — `pnpm work dispatch --all-ready` would
|
||||
fan out across all ready stories. Requires DAG-aware concurrency
|
||||
(no two implementers touching the same files).
|
||||
- **Sandcastle CI image alignment** — the `.sandcastle/Dockerfile` is
|
||||
minimal; once we identify the CI base image, the sandbox should extend it
|
||||
to match the CI environment exactly.
|
||||
- **Cost telemetry** — `sandcastle.run()` returns iteration usage stats; the
|
||||
orchestrator could log these to `_state.json` per-task so operators see
|
||||
cumulative spend.
|
||||
116
docs/decisions/adr-020-coverage-architecture.md
Normal file
116
docs/decisions/adr-020-coverage-architecture.md
Normal file
@@ -0,0 +1,116 @@
|
||||
# ADR-020 — Agent-first coverage architecture (4 layers + manifest-driven thresholds)
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-13
|
||||
**Builds on:** ADR-006 (vertical-feature-packages), ADR-011 (TDD foundation)
|
||||
**PRD:** docs/work/prds/coverage-architecture.prd.md
|
||||
|
||||
## Context
|
||||
|
||||
ADR-011 established the TDD foundation: per-package vitest configs with V8 coverage, a coverage baseline (80/75/80/80) and stricter per-layer bands (100% on `entities/`, `application/use-cases/`, `interface-adapters/controllers/`). The ESLint conformance rule `usecase-must-have-test-file` enforces _that a test file exists_. CI runs `pnpm test -- --coverage` and uploads `**/coverage/lcov.info` as an artifact.
|
||||
|
||||
This leaves five gaps that matter especially in an agent-first repo:
|
||||
|
||||
1. **No diff-coverage gate.** A slice can ship without exercising its new lines. The ESLint rule only checks file presence; the per-package thresholds catch only large drops.
|
||||
2. **No aggregate visibility.** N separate lcov files; no merged view, no trend over time.
|
||||
3. **Threshold declarations are duplicated** across 5+ `vitest.config.ts` files. Drift is mechanical to spot (we did it during the 2026-05-13 brainstorm: `@repo/media` had no `coverage:` block at all; `@repo/navigation` failed its declared layer thresholds in entities + controllers).
|
||||
4. **100% coverage with weak assertions is invisible.** Coverage doesn't measure whether tests would catch real regressions. Mutation testing — explicitly deferred in ADR-011 — is the next signal.
|
||||
5. **Coverage data isn't agent-readable.** The HTML report serves humans; the dispatch loop has no way to ask "did my slice cover its diff?".
|
||||
|
||||
## Decision
|
||||
|
||||
**1. Adopt a 4-layer coverage architecture** mirroring the 5-gate conformance philosophy (multi-latency, machine-readable, agent-first):
|
||||
|
||||
| Layer | Catches | Latency | Surface |
|
||||
| ---------------------------------- | ---------------------------------------------------- | ------------------ | ------------------------------------------------------------ |
|
||||
| **L0** Per-layer vitest thresholds | Drift below declared bands | ~5–30s per package | `pnpm test --coverage` (existing) |
|
||||
| **L1** Diff coverage | Changed line not exercised | ~5s after L0 | `pnpm coverage:diff`; CI gate; dispatch post-task |
|
||||
| **L2** Aggregate trend | Drift across the codebase over time | ~10s | `pnpm coverage:aggregate`; committed `coverage/summary.json` |
|
||||
| **L3** Mutation testing | Tests that exist + execute the code + assert nothing | Minutes | `pnpm mutate`; on-demand, not default `pnpm test` |
|
||||
|
||||
Each layer answers a distinct question; none replaces the others.
|
||||
|
||||
**2. Make `feature.manifest.ts` the single source of truth for coverage expectations.** A new `coverage:` section per feature:
|
||||
|
||||
```ts
|
||||
coverage: {
|
||||
bands: {
|
||||
"entities": { statements: 100, branches: 100, functions: 100, lines: 100 },
|
||||
"use-cases": { statements: 100, branches: 95, functions: 100, lines: 100 },
|
||||
"controllers": { statements: 100, branches: 95, functions: 100, lines: 100 },
|
||||
baseline: { statements: 80, branches: 75, functions: 80, lines: 80 },
|
||||
},
|
||||
mutationTargets: ["entities", "use-cases"],
|
||||
}
|
||||
```
|
||||
|
||||
Three readers consume the manifest:
|
||||
|
||||
- **Vitest** — `vitest.config.ts` imports the manifest's `coverage` and emits its `thresholds`. The duplicated block in 5 per-feature vitest configs goes away.
|
||||
- **`assertFeatureConformance`** — reads `coverage/lcov.info` for the package at boot and asserts each band. Graceful degradation in `USE_DEV_SEED=true` (warns rather than throws when lcov is absent).
|
||||
- **`pnpm coverage:diff`** — uses `baseline` for uncategorized files; stricter layer bands override per matching path glob.
|
||||
|
||||
This eliminates the duplication that caused the `@repo/media` drift and centralizes one decision in one place per feature.
|
||||
|
||||
**3. Diff coverage is cover-the-diff, not cover-the-new-code.** Every changed _executable_ line must have execution-count > 0 against the merged lcov. Modified-but-not-new lines count too — catches "agent edited code, didn't update the test." Allowlist: `*.test.ts`, `*.config.*`, `*.md`, `*.json`, `*.mjs`, plus the per-package exclude lists.
|
||||
|
||||
**4. Aggregate trend ships in-tree, not via SaaS.** `coverage/summary.json` is committed on merge to main. Trend readable via `git log -- coverage/summary.json`. No external service dependency; the dispatch loop can read history without a network call.
|
||||
|
||||
**5. Mutation testing is opt-in and narrowly scoped.** Stryker with `@stryker-mutator/vitest-runner`, runs on `entities/` + `application/use-cases/` only. Default mutation-score threshold 80% per feature (tunable per-manifest). Not part of `pnpm test`. Nightly GH Action surfaces score drift > 5%.
|
||||
|
||||
**6. Output format is machine-first.** `pnpm coverage:diff` emits JSON to stdout; human-readable summary to stderr. The dispatch loop reads stdout; humans read stderr or the HTML report.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Codecov / Coveralls SaaS.** Polished PR comments and trend dashboards, free for OSS. Rejected as the _primary_ L2 store — adds an external dep, makes the dispatch loop dependent on a network call, and the PR-comment UX targets humans (not the primary consumer of this signal). Can be added later as gold-plating without disturbing the architecture.
|
||||
- **Cover-the-new-code instead of cover-the-diff.** Lighter touch; ignores modified lines. Rejected — catches less drift. A slice that edits a use case without updating its test should fail, and cover-the-new-code wouldn't notice.
|
||||
- **Keep thresholds in per-package vitest configs.** Status quo. Rejected — the 2026-05-13 audit found drift in 2 of 5 features (media had no block at all; navigation's block diverged subtly from the canonical). Manifest centralization is the only durable fix.
|
||||
- **Run mutation testing in default `pnpm test`.** Rejected — Stryker on entities + use-cases takes minutes. Adding minutes to the default loop violates the constraint ("new gates must not add more than ~30s wall time"). On-demand is the right cadence; nightly catches drift.
|
||||
- **Mutation testing across all layers.** Rejected for v1 — repository/controller/integration code has too many environmental dependencies to mutate cleanly. Start narrow; expand if signal is high.
|
||||
- **Use ESLint or fallow for diff coverage.** Rejected — diff coverage needs runtime data (which lines actually executed), not AST or filesystem state. It belongs alongside `pnpm test`, not in `pnpm lint` or `pnpm fallow`.
|
||||
- **Boot-time coverage assertion is too heavy.** Considered. Counter-argument: the assertion is `O(features × lcov-file-size)` — small numbers, ~200ms. The graceful-degradation in dev mode means contributors aren't blocked. The payoff — coverage drift caught at the same latency as TypeScript brands — justifies the machinery.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- Every PR/task is gated on covering its own diff. Agent shipping an untested slice becomes mechanically impossible at the CI step.
|
||||
- One source of truth per feature for coverage expectations. The `@repo/media`-style "no coverage block" drift can't recur.
|
||||
- Trend history lives in the repo. `git log -- coverage/summary.json` answers "how has coverage moved over the last quarter?" without leaving the editor.
|
||||
- Mutation testing on the highest-leverage layers (entities + use-cases — the pure-business-logic surface) raises the floor on test quality without slowing the dispatch loop.
|
||||
- Machine-readable diff-coverage output integrates directly with the dispatch loop's post-task verification, completing the agent-first observability story.
|
||||
- Coverage joins the 5-gate conformance philosophy as a first-class signal; ADR-020 becomes the row alongside TS brands / ESLint / boot / `pnpm conformance` / fallow / coverage.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Implementation surface is non-trivial: 6–8 stories spanning manifest schema, vitest auto-derive, two new scripts, boot-time assertion, mutation tooling, ADR + guide + glossary + generator + hook updates.
|
||||
- The boot-time assertion adds a small dependency on `coverage/lcov.info` existing. Graceful degradation in dev mode handles this, but the implementation needs care.
|
||||
- `coverage/summary.json` committed on merge introduces a small CI permissions surface (`contents: write`) gated to the main-branch workflow.
|
||||
- Mutation testing is slow. The nightly cadence is the compromise; on-demand `pnpm mutate` is opt-in but rare in practice.
|
||||
|
||||
## Implementation phasing
|
||||
|
||||
Shipped as a single epic over 10 commits on 2026-05-13. Per-step state:
|
||||
|
||||
| # | Step | Commit | Status |
|
||||
| --- | ----------------------------------------------------------------- | --------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| 1 | Manifest schema + helper + auth proof-of-concept | `f7baa8b` | ✅ Shipped |
|
||||
| 2 | Vitest auto-derive (helper + `DEFAULT_COVERAGE_BANDS`) | `f7baa8b` + rollouts | ✅ Shipped (all 5 features wired) |
|
||||
| 3 | L1 diff coverage (`scripts/coverage/diff.mjs`) | `412d994` | ✅ Shipped |
|
||||
| 4 | L2 aggregate (`scripts/coverage/aggregate.mjs` + `summary.json`) | `bd5a077` | ✅ Shipped |
|
||||
| 5 | CI integration (validate gate + snapshot workflow) | `39e33eb` | ✅ Shipped |
|
||||
| 6 | Helper rollout to blog + marketing-pages | `15db9c4` | ✅ Shipped |
|
||||
| 7 | Docs + generator + hook rollout | `4dce1df` + `f4254aa` | ✅ Shipped |
|
||||
| 8 | L3 mutation testing (Stryker + nightly Action) | `6428f10` | ✅ Shipped (auth proof-of-concept; other features can add `stryker.config.json` by `extends: "@repo/core-testing/stryker.base.json"`) |
|
||||
| 9 | L0 unification (close test gaps in nav + media + marketing-pages) | `bf0b049` | ✅ Shipped — all 5 features hit declared bands |
|
||||
| 10 | Boot-time `assertFeatureConformance` coverage check | — | ⏸ Deferred. Duplicates L0's structural enforcement when both readers derive from the same manifest source of truth; the drift it was supposed to catch is mechanically impossible. Revisit if a concrete need emerges. |
|
||||
|
||||
Repo-wide state at shipping (`coverage/summary.json`): statements 95.87% / branches 88.91% / functions 100% / lines 95.87%. All five features pass their declared 100%/100%/95%/100% bands on entities/use-cases/controllers.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-006 — vertical-feature-packages
|
||||
- ADR-011 — TDD foundation
|
||||
- ADR-018 — audit-and-compliance (similar manifest-declared shape pattern)
|
||||
- ADR-019 — sandcastle agent orchestration (the dispatch loop that reads `pnpm coverage:diff`)
|
||||
- PRD `coverage-architecture` — implementation seed
|
||||
111
docs/decisions/adr-021-versioning-and-changelog.md
Normal file
111
docs/decisions/adr-021-versioning-and-changelog.md
Normal file
@@ -0,0 +1,111 @@
|
||||
# ADR-021 — Hybrid versioning + automated changelog via release-please
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-13
|
||||
**Builds on:** Conventional Commits convention (CLAUDE.md Key Conventions), ADR-019 (sandcastle agent orchestration)
|
||||
|
||||
## Context
|
||||
|
||||
Until this ADR the template had no versioning + no changelog. Every package shipped at `0.0.0`; there was no tag history, no "what changed since I forked this" answer, and no formal cadence for surfacing meaningful state changes.
|
||||
|
||||
Two pressures motivated wiring this up now:
|
||||
|
||||
1. **Conventional Commits had just been mandated** (visible across CLAUDE.md, AGENTS.md, session-start, and prompt-context hooks). Conventional commits are the substrate that automated versioning tools consume — leaving them unused would waste a free signal.
|
||||
2. **The template is a fork target.** Downstream consumers need a version they can pin against ("I forked at v0.3.0, what's changed?") and a changelog they can diff.
|
||||
|
||||
The available tools fall in three families:
|
||||
|
||||
- **Changesets** (`@changesets/cli`) — contributors run `pnpm changeset` per PR; explicit semver intent. Higher friction; not needed when conventional commits are already mandated.
|
||||
- **semantic-release** — fully automated from conventional commits; publishes on merge. Less control; monorepo support requires extra plumbing.
|
||||
- **release-please** (Google) — parses conventional commits, opens a rolling release PR with bumps + CHANGELOG entries; merging the PR cuts tags + GitHub releases.
|
||||
|
||||
`release-please` is the natural fit: conventional commits are already enforced, and the "release PR" model gives a human approval gate without requiring per-commit changeset files.
|
||||
|
||||
The remaining design choice is **versioning scope**:
|
||||
|
||||
- **Single root version** — one CHANGELOG.md at the root; the whole template moves together. Simplest.
|
||||
- **Per-package versions** — every `@repo/*` package versions independently; one CHANGELOG.md per package. Useful for published packages; we publish nothing today.
|
||||
- **Hybrid** — root template version (for cross-cutting changes: docs, scripts, ci, generators, core packages) + per-feature versions (for `packages/<feature>/**` changes). One root CHANGELOG.md + one per feature. Decision boundary follows commit-path scoping.
|
||||
|
||||
## Decision
|
||||
|
||||
**1. Adopt `release-please` (Google) as the versioning + changelog substrate.** Configuration in `release-please-config.json` and `.release-please-manifest.json` at the repo root. The GitHub Action lives at `.github/workflows/release-please.yml` and runs on every push to `main`.
|
||||
|
||||
**2. Hybrid versioning scope.** Six tracked packages:
|
||||
|
||||
| Path | Package name | Component (tag prefix) | Initial version |
|
||||
| -------------------------- | ----------------------- | ---------------------- | --------------- |
|
||||
| `.` | `template-vertical` | `template` | `0.1.0` |
|
||||
| `packages/auth` | `@repo/auth` | `auth` | `0.1.0` |
|
||||
| `packages/blog` | `@repo/blog` | `blog` | `0.1.0` |
|
||||
| `packages/media` | `@repo/media` | `media` | `0.1.0` |
|
||||
| `packages/marketing-pages` | `@repo/marketing-pages` | `marketing-pages` | `0.1.0` |
|
||||
| `packages/navigation` | `@repo/navigation` | `navigation` | `0.1.0` |
|
||||
|
||||
Each gets its own `CHANGELOG.md`. Tags use the per-package component prefix to avoid collisions: `template-v0.2.0`, `auth-v0.1.1`, etc.
|
||||
|
||||
Core packages (`core-shared`, `core-cms`, `core-api`, `core-eslint`, `core-typescript`, `core-testing`) and optional cores (`core-events`, `core-realtime`, etc., when scaffolded) are **NOT** independently versioned. Cross-cutting changes to those land in the root template version. Rationale: those packages cascade — bumping `core-shared` would functionally invalidate every feature anyway; surfacing them as separate versions creates noise without information. If a future consumer publishes individual core packages downstream, they can add per-package tracking then.
|
||||
|
||||
Apps (`web-next`, `web-tanstack`, `cms`, `storybook`) stay at `0.0.0`. They're not consumable artifacts.
|
||||
|
||||
**3. Pre-1.0 bump policy.** While each tracked package is `<1.0.0`:
|
||||
|
||||
- `feat:` commits bump **patch** (not minor), per `bump-patch-for-minor-pre-major: true`
|
||||
- `fix:` commits bump patch
|
||||
- `feat!:` (or any `BREAKING CHANGE:` footer) bumps **minor** (not major)
|
||||
- `chore`, `ci`, `build`, `style`, `test` commits don't bump
|
||||
|
||||
Rationale: the conservative pre-1.0 default means surface area can change without exhausting the version space. When a package crosses `1.0.0`, standard semver kicks in.
|
||||
|
||||
**4. Conventional-commit type → changelog section mapping.** From `release-please-config.json`:
|
||||
|
||||
| Type | Section | Hidden |
|
||||
| --------------------------------------- | ------------- | ------ |
|
||||
| `feat` | Features | no |
|
||||
| `fix` | Bug Fixes | no |
|
||||
| `perf` | Performance | no |
|
||||
| `refactor` | Refactoring | no |
|
||||
| `docs` | Documentation | no |
|
||||
| `revert` | Reverts | no |
|
||||
| `test`, `chore`, `ci`, `build`, `style` | (omitted) | yes |
|
||||
|
||||
Hidden sections still drive version bumps where applicable (none do, by current policy) but don't clutter the changelog.
|
||||
|
||||
**5. Bump targeting is by commit-path, not commit-scope.** release-please decides which package(s) to bump based on the _files changed_ in a commit, NOT the conventional-commit `scope`. A commit changing `packages/auth/**` bumps `@repo/auth`; a commit changing `docs/**` or `scripts/**` or `CLAUDE.md` bumps the root template; a commit changing both bumps both. The conventional-commit `scope` field is for human readability in the changelog — it does not drive routing.
|
||||
|
||||
**6. Release PR is a rolling document.** Every push to main re-evaluates the open release PR. Merging it cuts tags + creates GitHub releases for each affected package. No manual edits to the PR — the changelog content is reproducible from the commit history.
|
||||
|
||||
**7. CHANGELOG files are committed and edited only by release-please.** Manual edits are discouraged because they will be overwritten the next time release-please assembles the rolling PR. Initial baseline content for each `0.1.0` entry is the only exception.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Changesets (`@changesets/cli`)** — rejected primarily because Conventional Commits already capture the necessary intent. Per-PR changeset files would duplicate information. Could revisit if release-please ever fails to handle a release-shape edge case (e.g. needing a manual bump that's larger than commits imply).
|
||||
- **semantic-release** — rejected for monorepo friction. Per-package support requires `semantic-release-monorepo` or `multi-semantic-release`; release-please handles this natively.
|
||||
- **Single root version** — rejected because cross-cutting commits ARE a different kind of change from feature commits. A `fix(media): off-by-one in upload` shouldn't churn the root template version; a `refactor(coverage): unify L0 thresholds` shouldn't churn `@repo/auth`'s version. Separating gives consumers a finer-grained "what changed for me" signal.
|
||||
- **Per-package for every workspace member (apps, core, tooling)** — rejected. Core packages cascade; bumping `core-shared` is effectively a template-wide change. Apps aren't consumable. Tooling churn is mostly mechanical. Adding versions for these adds bookkeeping without information value.
|
||||
- **Tag prefix `template-vertical-v...` vs `template-v...`** — chose `template-v` for tag economy (shorter; consistent length with `auth-v`, `blog-v`, etc.). The package name `template-vertical` is still authoritative in `package.json`.
|
||||
- **Auto-merge the release PR** — rejected. Human approval is the gate that catches the rare case where a commit's content doesn't match its conventional type (e.g. a `chore:` commit that actually shipped a feature).
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- Every merge to main produces a tracked, dated record of what changed for downstream consumers.
|
||||
- Conventional commits become load-bearing: their type + body shape the changelog directly.
|
||||
- The "what version did I fork at" question has a real answer per tracked package.
|
||||
- Tagged releases enable `git diff vX.Y.Z..HEAD -- <path>` for narrow "what changed in feature X since I last looked" questions.
|
||||
- Release cadence is implicit (merge the PR when ready); no separate release planning required.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Six packages × independent versions means six CHANGELOG.md files to navigate. Mitigated by the `## Cross-references` section in each pointing at the relevant ADRs + this one.
|
||||
- release-please-action is GitHub-Actions-coupled. A migration to a different CI provider would need a different runner (the release-please core CLI runs anywhere, but the orchestration around the rolling PR is Action-specific).
|
||||
- The first release PR after the initial baseline could be large (covers everything merged after `0.1.0`). This is one-time; subsequent PRs are scoped to the work between releases.
|
||||
- Apps stuck at `0.0.0` is mildly confusing if a contributor expects every package to be versioned. Mitigated by documentation here + in `docs/guides/releasing.md`.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-019 — sandcastle agent orchestration (the dispatch loop that ships these commits)
|
||||
- ADR-020 — coverage architecture (one of the systems shipped at the 0.1.0 baseline)
|
||||
- CLAUDE.md Key Conventions — Conventional Commits requirement (the substrate this ADR consumes)
|
||||
- `docs/guides/releasing.md` — day-to-day cookbook
|
||||
318
docs/decisions/adr-022-library-evaluation-policy.md
Normal file
318
docs/decisions/adr-022-library-evaluation-policy.md
Normal file
@@ -0,0 +1,318 @@
|
||||
# ADR-022 — Library evaluation policy
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-14
|
||||
**Related:** ADR-006 (vertical feature packages), ADR-010 (turbo boundaries), ADR-014 (Sentry observability), ADR-017 (OpenTelemetry + vendor isolation), ADR-019 (sandcastle agent orchestration), ADR-021 (release-please versioning)
|
||||
**Companion guide:** `docs/guides/adding-a-library.md` (human reading-room)
|
||||
**Companion skill:** `.claude/skills/evaluate-library/SKILL.md` (authoritative agent runbook)
|
||||
|
||||
## Context
|
||||
|
||||
This template ships with a deliberately narrow third-party surface. Every feature
|
||||
package today carries the same six runtime dependencies — `@repo/core-shared`,
|
||||
`@trpc/server`, `inversify`, `payload`, `reflect-metadata`, `zod` — and nothing
|
||||
else. That uniformity isn't an accident; it's the result of unstated discipline
|
||||
that the boundary-tag system (ADR-006, ADR-010) and the manifest-first ordering
|
||||
(ADR-012) silently reward.
|
||||
|
||||
The discipline is not codified. New dependencies get added by anyone — human or
|
||||
agent — running `pnpm add <pkg>`, with no checkpoint between intent and lockfile.
|
||||
Three recent signals show the gap:
|
||||
|
||||
1. An exploratory grill session on **2026-05-14** nearly added `trpc-to-openapi`
|
||||
plus `zod-to-json-schema` plus a build-time generator to the repo before
|
||||
stopping to ask "who calls this code path?" The honest answer was "nobody —
|
||||
all callers are TypeScript via `createCaller`." The library would have shipped
|
||||
~30 lines of `.meta({...})` annotations per router and a `superjson`-incompatible
|
||||
HTTP handler in exchange for zero downstream consumers. Pure carrying cost,
|
||||
caught by a chance question, not by a system.
|
||||
2. **Three existing ADRs already record post-hoc library decisions** —
|
||||
ADR-002 (Inversify), ADR-014 (Sentry), ADR-017 (OpenTelemetry). Each notes
|
||||
"we picked X over Y" but the records were written after adoption. By the time
|
||||
the ADR existed the lockfile already held the dep. No mechanism existed to
|
||||
catch a _bad_ choice before it became a migration project.
|
||||
3. The repo's automation depends on the lockfile staying small and predictable.
|
||||
`pnpm fallow` audits for dead code; `pnpm conformance` audits manifest drift;
|
||||
`pnpm coverage:diff` audits change coverage. There is no equivalent audit for
|
||||
"did we just adopt a library nobody asked for?"
|
||||
|
||||
A fourth pressure comes from this template being EU-resident and GDPR-bound.
|
||||
A library that defaults to a US-only SaaS endpoint (telemetry, analytics, AI,
|
||||
log aggregation) silently moves user data out of the EU as soon as it's imported
|
||||
and configured with defaults. The current process has no point where that gets
|
||||
caught.
|
||||
|
||||
The decision below codifies the de-facto discipline, formalizes the agent-loop
|
||||
hook that prevents drift, and makes rejection records first-class so future
|
||||
agents don't re-evaluate libraries that were already rejected for known reasons.
|
||||
|
||||
## Decision
|
||||
|
||||
Adopt a **tiered library-evaluation policy** with eight hard auto-reject filters,
|
||||
three discussion prompts, a per-decision trace artifact, and a four-layer
|
||||
enforcement stack.
|
||||
|
||||
### 1. Tiered trigger (by boundary tag)
|
||||
|
||||
| Tier the dep lands in | Process | Companion record |
|
||||
| ------------------------------------ | ------------------------ | ---------------- |
|
||||
| `apps/<x>` | Author's call; no policy | – |
|
||||
| `feature` (e.g. `packages/auth`) | Trace required | – |
|
||||
| `core` (e.g. `packages/core-shared`) | Trace required | ADR required |
|
||||
| New optional-core category | Trace required | ADR required |
|
||||
|
||||
The trigger maps onto the workspace-tag boundary already enforced by ESLint
|
||||
(`eslint-plugin-boundaries`) and Turborepo (`turbo boundaries`). No new mental
|
||||
model — the policy is a corollary of an existing one.
|
||||
|
||||
### 2. Eight hard auto-reject filters
|
||||
|
||||
Failing any single filter is an automatic reject. Trace records the failure.
|
||||
|
||||
1. **License allowlist.** Only `MIT`, `Apache-2.0`, `BSD-*`, `ISC`, `MPL-2.0`.
|
||||
Anything else (GPL family, AGPL, CC-BY-NC, custom EULAs) is a no.
|
||||
2. **TypeScript-native or `@types/*` available.** The repo is strict TS;
|
||||
un-typed JS libraries shift maintenance cost to the integrating feature.
|
||||
3. **Not abandoned.** Last release < 18 months **AND** PR/issue activity
|
||||
< 12 months. The triple-AND avoids killing finished-but-stable libraries
|
||||
(`reflect-metadata`-style).
|
||||
4. **Boundary-tag fit.** A dep added to a feature package cannot require
|
||||
imports the boundary rules forbid (e.g. a Sentry SDK in a feature —
|
||||
ADR-017 §4 forbids this).
|
||||
5. **Doesn't shadow an existing must-have.** Proposing `valibot` when `zod`
|
||||
is locked, or `tsyringe` when Inversify is locked by ADR-002, is an
|
||||
auto-reject; the replacement must be a separate ADR with consequences
|
||||
analysis, not a parallel adoption.
|
||||
6. **EU data residency for hosted/SaaS components.** If the package transmits
|
||||
user data, telemetry, or business state to a vendor-controlled endpoint
|
||||
by default, that vendor must offer an EU data region, and the integration
|
||||
must be configured to use it. Self-hostable packages, on-device libraries,
|
||||
and build-time-only tools are exempt.
|
||||
7. **CVE scan clean.** `pnpm audit --audit-level=moderate` clean at adoption
|
||||
time. Documented allowlist mechanism for accepted-risk advisories.
|
||||
8. **Named consumer exists now, not hypothetically.** The integration must
|
||||
answer "who calls this code path today, or who is blocked waiting for it?"
|
||||
"Future code that might exist" is not a consumer. This filter is the
|
||||
direct response to the 2026-05-14 OpenAPI near-miss.
|
||||
|
||||
### 3. Three discussion prompts
|
||||
|
||||
Filters that don't auto-reject but must be answered in the trace, either
|
||||
direction acceptable with justification.
|
||||
|
||||
- **What does it replace?** New-and-old running in parallel is a smell.
|
||||
- **Migration cost out.** What does ripping this back out look like 18 months
|
||||
from now? Mechanical, hard, or impossible?
|
||||
- **Alternatives considered.** Two named alternatives at minimum (or "none
|
||||
with explanation"). Required for `feature`-tier; required _and_ duplicated
|
||||
into the ADR for `core`-tier.
|
||||
|
||||
### 4. Trace artifact
|
||||
|
||||
Every decision — approved or rejected — emits a trace file at
|
||||
`docs/library-decisions/<YYYY-MM-DD>-<package-name>.md`. Always written,
|
||||
regardless of whether an accompanying ADR exists. Shape:
|
||||
|
||||
```markdown
|
||||
---
|
||||
package: <name>
|
||||
version: "<semver range>"
|
||||
tier: app | feature | core
|
||||
decision: approved | rejected
|
||||
date: <YYYY-MM-DD>
|
||||
deciders: [<author>, ...]
|
||||
adr: adr-NNN | null
|
||||
filter-results:
|
||||
license: <SPDX id>
|
||||
types: native | "@types/<x>" | none
|
||||
maintenance: active | dormant | abandoned
|
||||
boundary-fit: pass | fail
|
||||
shadow-check: pass | fail | "shadows <x>"
|
||||
eu-residency: ok | n/a | self-hostable | fail
|
||||
cve-scan: clean | "<advisory-id>" | fail
|
||||
named-consumer: pass | fail
|
||||
verification-commands:
|
||||
- <literal command that produced each filter result>
|
||||
---
|
||||
|
||||
## Filter: <name>
|
||||
|
||||
<prose>
|
||||
|
||||
## Prompt: <name>
|
||||
|
||||
<prose>
|
||||
```
|
||||
|
||||
Frontmatter is the machine surface (greppable, schema-stable). Headings are
|
||||
the human surface. Rejection traces are first-class — the OpenAPI scenario,
|
||||
had this policy existed, would have produced a permanent record so the next
|
||||
agent considering `trpc-to-openapi` finds the prior reasoning in <1s of
|
||||
`ls docs/library-decisions/`.
|
||||
|
||||
### 5. Four-layer enforcement stack
|
||||
|
||||
Mirrors the latency-tiered shape of the conformance system (ADR-012).
|
||||
|
||||
| Layer | Latency | Catches |
|
||||
| -------------------------------------- | ---------- | ---------------------------------------------------------------- |
|
||||
| Claude `PreToolUse`/`PostToolUse` hook | inline | Agent skipping the skill before `pnpm add` / `package.json` edit |
|
||||
| `evaluate-library` skill | seconds | The decision itself + writes the trace |
|
||||
| Git pre-commit hook | pre-commit | Humans or agents bypassing the skill |
|
||||
| Sandcastle reviewer prompt | per-slice | Bypasses that slipped past pre-commit |
|
||||
|
||||
The Claude hook injects a `<system-reminder>` directing the agent to the skill
|
||||
but does **not** auto-deny (false-positive paths like dev-deps and app-tier
|
||||
additions are common). The pre-commit hook is the deterministic gate.
|
||||
|
||||
### 6. Composition with `pnpm turbo gen core-package`
|
||||
|
||||
The optional-cores generator emits **pre-shipped traces** — one per direct
|
||||
runtime dep of the new core — pre-marked `decision: approved` and cited
|
||||
against the relevant ADR (ADR-015 for events, ADR-016 for realtime,
|
||||
ADR-018 for audit). Same frozen-snapshot discipline that the optional cores
|
||||
already follow (`turbo/generators/__snapshots__/core-package/`). New optional
|
||||
cores added later inherit this requirement.
|
||||
|
||||
### 7. Skill invocation
|
||||
|
||||
```
|
||||
/evaluate-library <package-name> --tier <feature|core|app> --target <package-path>
|
||||
```
|
||||
|
||||
The skill walks the eight filters in **collect-cheap-skip-expensive** order:
|
||||
cheap structural filters (license, types, shadow-check, boundary-fit) run to
|
||||
completion regardless of failure; expensive filters (CVE scan, EU residency
|
||||
probe, maintenance signals) short-circuit after the first reject. The trace
|
||||
records which filters ran and which were skipped, so a partial trace is still
|
||||
useful evidence.
|
||||
|
||||
### 8. Backfill at policy adoption
|
||||
|
||||
Every existing runtime dependency in feature- and core-tier packages
|
||||
(~10 deps at this writing — `payload`, `inversify`, `zod`, `@trpc/server`,
|
||||
`reflect-metadata`, `superjson`, `@sentry/*`, `@opentelemetry/*` family,
|
||||
`socket.io`, etc.) gets a backfilled trace dated 2026-05-14 (adoption day).
|
||||
ADR-002, ADR-014, ADR-017 are cited via the `adr:` frontmatter field;
|
||||
verification-command output is captured at backfill time.
|
||||
|
||||
### 9. Sub-processor discriminated union (amendment: 2026-05-18)
|
||||
|
||||
Every trace carries two top-level frontmatter fields classifying the library
|
||||
from a GDPR sub-processor perspective:
|
||||
|
||||
```yaml
|
||||
is-sub-processor: false # boolean — true when the vendor receives personal data on the operator's behalf
|
||||
processes-pii: false # boolean — true when the library processes PII in-process (even without transmitting it)
|
||||
```
|
||||
|
||||
When `is-sub-processor: true`, five additional fields are **required**:
|
||||
|
||||
```yaml
|
||||
data-sent: "<what personal data the library transmits to the vendor>"
|
||||
region: "<vendor data region, e.g. eu-west-1>"
|
||||
dpa-signed: true | false
|
||||
sccs-required: true | false
|
||||
contact: "<vendor DPO or privacy contact email/URL>"
|
||||
```
|
||||
|
||||
**Discriminated-union rules:**
|
||||
|
||||
| `is-sub-processor` | `processes-pii` | Conditional fields required? |
|
||||
| ------------------ | --------------- | --------------------------------------------------------------------------------- |
|
||||
| `false` | `false` | No — pure library, no data involvement |
|
||||
| `false` | `true` | No — in-process only, no vendor data flow |
|
||||
| `true` | `true` | Yes — all five conditional fields required |
|
||||
| `true` | `false` | Technically possible but very unusual; still requires all five conditional fields |
|
||||
|
||||
**Baseline for backfill:** pure in-process libraries (no network calls to
|
||||
vendor-controlled endpoints) get `is-sub-processor: false` + `processes-pii: false`.
|
||||
Self-hosted software that stores PII but transmits nothing to the vendor (e.g.
|
||||
`payload`) gets `is-sub-processor: false` + `processes-pii: true`.
|
||||
|
||||
These fields are the machine surface consumed by `scripts/emit-sub-processors.mjs`
|
||||
(see Story 06 of the compliance-manifests-pii-retention-subprocessors epic). A
|
||||
trace missing `is-sub-processor` is treated as `false` by the generator for
|
||||
backward-compatibility; all new traces authored after this amendment must include
|
||||
both fields. The `evaluate-library` skill (§7) prompts for these fields
|
||||
unconditionally and writes the conditional block only when `is-sub-processor: true`.
|
||||
|
||||
The weekly `dpa-signed` staleness check in CI (ADR-023 cross-reference) should
|
||||
flag any `dpa-signed: true` traces where the DPA has not been revalidated within
|
||||
the prior 365 days. Implementation of that cron is deferred to the CI security
|
||||
hardening work.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **No policy, keep relying on instinct.** Rejected. The 2026-05-14 OpenAPI
|
||||
near-miss demonstrated the gap. Instinct catches some adds, misses others;
|
||||
the failure mode is silent.
|
||||
- **Every `package.json` change requires a trace, no tier distinction.**
|
||||
Rejected. Devdeps in tooling packages and ESLint plugin bumps in apps would
|
||||
drown the trace directory in noise. The boundary-tag system already
|
||||
partitions blast radius — the policy reuses that partition.
|
||||
- **Only "category" decisions require process** (new auth provider, new ORM,
|
||||
new queue). Rejected. The OpenAPI scenario was a sub-tool inside an existing
|
||||
category, not a category swap. Category-only triggers miss it.
|
||||
- **A central library-evaluation service / Linear queue / Slack bot.**
|
||||
Rejected. The repo is agent-first and currently single-developer (+ agents).
|
||||
Human-in-the-loop services don't fit the dispatch loop; the policy must be
|
||||
agent-runnable end-to-end.
|
||||
- **No CVE filter — rely on `npm audit` ambient noise.** Rejected. CVE
|
||||
status is point-in-time; pinning the result to the trace at adoption is the
|
||||
whole value. Re-running the verification commands later detects drift.
|
||||
- **Drop the named-consumer filter to a discussion prompt.** Rejected. The
|
||||
filter is the one that would have stopped the OpenAPI flirtation; demoting
|
||||
it back to a prompt is the same as not adding the filter.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive**
|
||||
|
||||
- New dependencies require deliberate intent. The trace artifact + four-layer
|
||||
enforcement stack make accidental adds detectable at four latencies, mirroring
|
||||
the conformance-system pattern that already works.
|
||||
- Rejection records are permanent. Future agents considering a previously
|
||||
rejected library find the trace in `docs/library-decisions/` and don't
|
||||
re-litigate.
|
||||
- EU data residency becomes a binary, machine-readable filter result, not
|
||||
an afterthought.
|
||||
- Per-decision verification commands give future agents a single source of
|
||||
truth they can re-run to verify the trace is still valid.
|
||||
- The policy composes with `pnpm turbo gen core-package`: optional cores
|
||||
remain a one-command scaffold without bypassing the rule.
|
||||
|
||||
**Negative**
|
||||
|
||||
- New feature-tier deps take longer to land. Walking eight filters + writing
|
||||
the trace is ~5 minutes of agent work per addition.
|
||||
- Backfilling ~10 existing deps is one-time work; expected ~half a day of
|
||||
agent dispatch.
|
||||
- The Claude `PreToolUse` hook adds latency to every `pnpm add` invocation.
|
||||
Mitigated by the hook being a reminder-injector, not a blocker.
|
||||
- The pre-commit hook adds a new failure mode ("you added a dep but forgot
|
||||
the trace"). Mitigated by the skill being the natural path the hook
|
||||
reminders point to.
|
||||
- The CVE filter creates a maintenance obligation: when `pnpm audit` finds
|
||||
a new advisory in an already-approved dep, the trace becomes stale. Acceptable
|
||||
trade-off — staleness detection is exactly what `pnpm audit` already does;
|
||||
the trace adds a per-dep anchor for the conversation that follows.
|
||||
|
||||
**Neutral**
|
||||
|
||||
- ADR-002, ADR-014, and ADR-017 remain authoritative for their respective
|
||||
libraries. Backfilled traces cite them rather than duplicating their reasoning.
|
||||
- The policy doesn't constrain transitive dependencies; `pnpm audit` and license
|
||||
scanning already handle those recursively. Only direct deps require a trace.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-006 — Vertical feature packages (the tag system the trigger maps to)
|
||||
- ADR-010 — Turbo boundaries (the enforcement substrate)
|
||||
- ADR-012 — Feature conventions (the conformance-system shape this policy mirrors)
|
||||
- ADR-017 — OpenTelemetry migration (vendor-isolation pattern the policy extends)
|
||||
- ADR-019 — Sandcastle agent orchestration (reviewer-prompt layer of enforcement)
|
||||
- ADR-021 — release-please versioning (where dep additions show up in release notes)
|
||||
- Companion guide: `docs/guides/adding-a-library.md`
|
||||
- Companion skill: `.claude/skills/evaluate-library/SKILL.md`
|
||||
- Glossary: `docs/glossary.md` entries for **Library trace** and **Pre-shipped trace**
|
||||
471
docs/decisions/adr-023-ci-security-and-supply-chain.md
Normal file
471
docs/decisions/adr-023-ci-security-and-supply-chain.md
Normal file
@@ -0,0 +1,471 @@
|
||||
# ADR-023 — CI security + supply-chain enforcement stack
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-14
|
||||
**Builds on:** ADR-022 (library evaluation policy)
|
||||
**Related:** ADR-006 (vertical feature packages), ADR-010 (turbo boundaries), ADR-019 (sandcastle agent orchestration), ADR-021 (release-please versioning)
|
||||
**Companion guide:** `docs/guides/ci-security.md` (to be written; human reading-room)
|
||||
|
||||
## Context
|
||||
|
||||
ADR-022 codified the library-evaluation policy: at adoption time, every new
|
||||
runtime dependency in a feature- or core-tier package is gated by 8 hard filters
|
||||
|
||||
- 3 prompts and produces a permanent trace at `docs/library-decisions/`. That
|
||||
closes the **decision** gate. It does not close the **drift** gate. Once a
|
||||
library is in the lockfile, ADR-022 has nothing to say about:
|
||||
|
||||
1. **CVE disclosures against the current pinned version.** A library that
|
||||
passes `pnpm audit --audit-level=moderate` clean at adoption can have a
|
||||
critical CVE published against it the next day. The trace's
|
||||
`verification-commands` snapshot goes stale silently.
|
||||
2. **Supply-chain _behavior_ compromise.** The disclosed-CVE world only
|
||||
catches vulnerabilities that someone has filed. Packages with malicious
|
||||
behavior — `event-stream` (2018), `ua-parser-js` (2021), `tj-actions/changed-files`
|
||||
(2025), `xz-utils` (2024) — shipped malware that no CVE database had
|
||||
seen at the moment of compromise. CVE scanning is a lagging indicator.
|
||||
3. **Maintainer-account compromise.** A trusted upstream maintainer's npm
|
||||
account gets phished. The next `1.2.4` patch publishes a malicious
|
||||
post-install script. Every consumer pulling `^1.2.0` inherits it.
|
||||
Renovate or Dependabot will happily open a bump PR.
|
||||
4. **GitHub Actions supply chain.** This repo's 5 existing workflows pin
|
||||
their actions to **major-version tags** (`actions/checkout@v4`,
|
||||
`pnpm/action-setup@v4`, `googleapis/release-please-action@v4`). The
|
||||
`tj-actions/changed-files` incident demonstrated that a compromised
|
||||
maintainer can push a malicious tag and everyone pinned to `@v4`
|
||||
silently inherits it. Major-tag pinning is documented insecure.
|
||||
5. **License drift.** Upstream packages occasionally relicense (Sentry
|
||||
went BSL on a major; Elasticsearch went SSPL). A `1.x → 2.x` Renovate
|
||||
PR might silently move a previously MIT-licensed dep to a copyleft or
|
||||
source-available license that violates ADR-022's filter #1.
|
||||
6. **EU-residency drift.** A vendor (Sentry, PostHog, etc.) announces
|
||||
US-only changes mid-flight. The trace's `eu-residency: ok` snapshot
|
||||
becomes false; ADR-022's filter #6 has no way to detect this.
|
||||
|
||||
The repo's current security posture, audited 2026-05-14: **zero security
|
||||
tooling**. No Dependabot config, no Renovate, no CodeQL, no Snyk, no Trivy,
|
||||
no OSV-Scanner, no Socket, no gitleaks, no `pnpm audit signatures` step.
|
||||
GitHub Actions are pinned to major-version tags. The 5 existing workflows
|
||||
(`ci.yml`, `coverage-snapshot.yml`, `mutation-nightly.yml`, `release-please.yml`,
|
||||
`sentry-pii-guard.yml`) cover functional CI, but nothing surfaces a
|
||||
post-adoption supply-chain signal.
|
||||
|
||||
For a GDPR-bound EU-resident template that ships as agent-friendly
|
||||
infrastructure, this is the load-bearing gap that ADR-022 cannot close
|
||||
alone. The decision below extends ADR-022 with a continuous-validation
|
||||
counterpart and adds five orthogonal layers that catch the threat surface
|
||||
ADR-022's adoption-time gate doesn't see.
|
||||
|
||||
## Decision
|
||||
|
||||
Adopt a **four-pillar CI security and supply-chain enforcement stack**:
|
||||
(1) Renovate-managed bumps + Action SHA pinning, (2) Socket-based
|
||||
supply-chain-behavior detection, (3) continuous trace revalidation
|
||||
extending ADR-022, (4) baseline GitHub-native gates (CodeQL + secret
|
||||
scanning + sigstore provenance). Each pillar composes with the existing
|
||||
5-gate conformance pattern from ADR-012 — layered enforcement at
|
||||
declining latencies.
|
||||
|
||||
### 1. Renovate adoption (bumps + Action SHA pinning)
|
||||
|
||||
`.github/renovate.json` configures Renovate to manage all runtime + dev
|
||||
dependency bumps and to SHA-pin every GitHub Action invocation:
|
||||
|
||||
- **npm bumps** — per-workspace package.json updates honored. Minor +
|
||||
patch bumps grouped by ecosystem cluster (e.g. one weekly PR for all
|
||||
`@sentry/*`, one for all `@opentelemetry/*`). Auto-merge enabled for
|
||||
green minor + patch PRs. Major bumps require human (or agent) review
|
||||
— see §3.
|
||||
- **Dockerfile bumps** — `.sandcastle/Dockerfile`'s `node:22-bookworm-slim`
|
||||
base image gets the same treatment as npm.
|
||||
- **Action SHA pinning** — `pinGitHubActionDigests` rewrites every
|
||||
`uses: <owner>/<repo>@<tag>` to `uses: <owner>/<repo>@<40-char-sha> # <tag>`
|
||||
on first run. Subsequent Action releases produce bump PRs that update
|
||||
the SHA + the trailing comment in one diff.
|
||||
- **Vulnerability alerts** stay on the GitHub-native server-side surface
|
||||
(no Dependabot bump PRs). Server-side alerts compose with Renovate
|
||||
bumps: an alert may trigger a manual Renovate `:rebase` to accelerate
|
||||
a particular bump.
|
||||
|
||||
Renovate over Dependabot for this repo specifically because:
|
||||
|
||||
- **pnpm-workspace support** is mature and per-workspace updates work
|
||||
from one config file (Dependabot requires verbose per-workspace blocks).
|
||||
- **`pinGitHubActionDigests` is native** (Dependabot SHA-pinning requires
|
||||
manual config).
|
||||
- **PR grouping rules** are more granular — one PR per ecosystem cluster
|
||||
instead of per-package noise.
|
||||
- **Major/minor split + automerge** is one-liner config (Dependabot
|
||||
requires a separate GitHub Action for automerge).
|
||||
|
||||
### 2. Socket.dev integration (supply-chain _behavior_ detection)
|
||||
|
||||
Layered free-tier integration; no paid plan required:
|
||||
|
||||
- **Socket GitHub App** installed on the repo. Posts risk-score comments
|
||||
on every PR that touches `package.json` / `pnpm-lock.yaml`. Free
|
||||
for OSS use.
|
||||
- **`socket-cli` CI step** in `ci.yml`'s `validate` job. Runs
|
||||
`socket-cli scan` against the lockfile and fails the job on
|
||||
configurable severity. Configuration in `.socket.json`:
|
||||
```json
|
||||
{ "issueRules": { "critical": "error", "high": "warn", "medium": "ignore" } }
|
||||
```
|
||||
Default: critical → block the PR; lower severities → comment only.
|
||||
- **Sandcastle reviewer prompt** reads Socket CI output via the GitHub
|
||||
CLI and rejects the agent slice when a `critical` finding is present.
|
||||
Adds machine-readable enforcement to the agent dispatch loop.
|
||||
|
||||
Socket adds a **9th hard filter** to `evaluate-library` (ADR-022's filter
|
||||
set). New trace frontmatter field:
|
||||
|
||||
```yaml
|
||||
filter-results:
|
||||
socket-risk: clean | flagged | "<finding-summary>"
|
||||
```
|
||||
|
||||
At adoption time the skill runs `socket-cli scan <package>` and records
|
||||
the result. The continuous monitor surface is §3 (trace revalidation),
|
||||
which re-runs the same command on schedule.
|
||||
|
||||
### 3. Trace revalidation cron (ADR-022 continuous-validation counterpart)
|
||||
|
||||
New workflow at `.github/workflows/trace-revalidation-weekly.yml`. Runs
|
||||
weekly via cron + on-demand via `workflow_dispatch`. Mirrors the cadence
|
||||
shape of `mutation-nightly.yml`.
|
||||
|
||||
**Scope:** every approved + pre-shipped trace under `docs/library-decisions/`.
|
||||
Rejection traces skipped (no signal value in re-validating an already-rejected
|
||||
library).
|
||||
|
||||
**Action — for each in-scope trace:**
|
||||
|
||||
1. Read the trace's `verification-commands:` block.
|
||||
2. Re-run each command, capture stdout/stderr.
|
||||
3. Compare against the trace's `filter-results:` snapshot.
|
||||
4. Classify divergence:
|
||||
- **Soft** — CVE count changed without crossing severity threshold;
|
||||
maintenance signal still active but downgraded one level; transitive
|
||||
dep count changed.
|
||||
- **Hard** — license changed; named-consumer no longer present;
|
||||
critical CVE disclosed; EU residency flipped to `fail`; Socket
|
||||
flag escalated to `critical`.
|
||||
|
||||
**Issue management:**
|
||||
|
||||
- **Soft divergence** appends to a single rolling **"library-trace
|
||||
dashboard" GitHub issue** kept open continuously. One issue total,
|
||||
updated each run with the latest comparison diff. Labeled
|
||||
`library-policy/dashboard`.
|
||||
- **Hard divergence** opens a fresh per-dep GitHub issue labeled
|
||||
`library-policy/re-evaluation`. Title format:
|
||||
`[trace-revalidation] <package> — <reason>`. The issue body cites the
|
||||
trace path + the verification-command output + the diff.
|
||||
|
||||
**Auto-edit policy:** trace revalidation NEVER edits a trace file. The
|
||||
re-walk needs the `evaluate-library` skill (8 filters + 3 prompts, with
|
||||
agent judgement). CI catches divergence; the dispatch loop fixes it.
|
||||
|
||||
**Auto-dispatch policy:** `library-policy/re-evaluation` issues are
|
||||
**not** auto-picked up by the dispatch loop. Human triage required.
|
||||
Auto-dispatch on CI-opened issues would create a feedback loop where
|
||||
the agent loop spends nights re-evaluating libraries based on rotating
|
||||
CVE data. Issues are a queue; humans drain them via `pnpm work dispatch`.
|
||||
|
||||
**Main-CI gating policy:** hard divergence does NOT fail CI on main.
|
||||
Main can keep deploying while the trace gets re-walked. Gating on main
|
||||
would block release-please PRs every time a CVE drops upstream.
|
||||
|
||||
### 4. Baseline GitHub-native gates
|
||||
|
||||
- **CodeQL** at `.github/workflows/codeql.yml`. Language config
|
||||
`javascript-typescript` covers everything this repo is. Runs on push to
|
||||
main + PRs + weekly schedule. Free for public repos and on
|
||||
Pro/Team/Enterprise plans for private repos; consumers without a
|
||||
CodeQL-eligible plan get a no-op + a clear error message from GitHub.
|
||||
- **`pnpm audit signatures --audit-level=high`** added as one step in
|
||||
`ci.yml`'s existing `validate` job. Verifies npm sigstore attestations.
|
||||
~40% of the registry is signed today and climbing.
|
||||
- **Secret scanning — two layers:**
|
||||
- GitHub-native **push protection** (server-side, free, blocks pushes
|
||||
containing known token patterns at the GitHub edge). Consumer toggles
|
||||
in repo settings. Documented in `docs/guides/ci-security.md`.
|
||||
- **`gitleaks` pre-commit hook** wired into `.husky/pre-commit` as a
|
||||
step alongside the existing state-sync guard. Catches custom token
|
||||
patterns the GitHub allowlist doesn't know about. Local; free.
|
||||
|
||||
### 5. Failure-mode hierarchy
|
||||
|
||||
Two principles govern what blocks vs. what comments:
|
||||
|
||||
- **Boolean checks** (compiles, schema valid, signature verifies, secret
|
||||
present, trace file present) hard-block. They have a definite right
|
||||
answer.
|
||||
- **Judgment checks** (Socket risk score, CodeQL semantic finding) are
|
||||
advisory unless severity reaches `critical` / `error`. They can have
|
||||
false positives.
|
||||
|
||||
Concrete table (the source of truth referenced by reviewer-prompt and
|
||||
documentation):
|
||||
|
||||
| Gate | Layer | Hard block? |
|
||||
| ---------------------------------------------------------------- | ----------- | --------------------------------------------------- |
|
||||
| `pnpm typecheck && test && lint && conformance && coverage:diff` | CI | Yes |
|
||||
| State-sync guard | pre-commit | Yes |
|
||||
| `gitleaks` (custom patterns) | pre-commit | Yes |
|
||||
| Library-trace presence check (ADR-022) | pre-commit | Yes |
|
||||
| GitHub native push protection | server-side | Yes (GitHub edge) |
|
||||
| Renovate minor/patch bump PRs | CI | Auto-merge if green |
|
||||
| Renovate major bump PRs | CI | Block until evaluate-library re-run + trace refresh |
|
||||
| Socket CI step — `critical` | CI | Yes |
|
||||
| Socket CI step — `high` or below | CI | Advisory |
|
||||
| Socket GitHub App PR comments | server-side | Advisory |
|
||||
| CodeQL — `error` severity | CI | Yes |
|
||||
| CodeQL — `warning` / `note` | CI | Advisory |
|
||||
| `pnpm audit signatures` failure | CI | Yes |
|
||||
| GitHub Dependabot vuln alerts | server-side | Advisory (post-merge) |
|
||||
| Trace revalidation — soft divergence | weekly cron | Dashboard issue |
|
||||
| Trace revalidation — hard divergence | weekly cron | Per-dep issue |
|
||||
|
||||
### 6. Amendments to ADR-022
|
||||
|
||||
This ADR amends ADR-022 in three places. ADR-022 itself stays unedited
|
||||
(its `Status: Accepted` is preserved for provenance); the amendments are
|
||||
recorded here and the new behavior is what the implementation honors.
|
||||
|
||||
**§6.1 — Major-bump re-evaluation trigger.** ADR-022 §1 and §8 spoke of
|
||||
"new runtime dependencies" but did not address bumps to existing deps.
|
||||
When Renovate (§1 above) bumps a runtime dep in a feature- or core-tier
|
||||
package and the bump crosses a semver-major boundary, the
|
||||
`evaluate-library` skill re-runs against the upgraded version. Minor +
|
||||
patch bumps do **not** trigger re-evaluation. The existing trace file is
|
||||
updated in-place: `version`, `filter-results`, `verification-commands`,
|
||||
and `last-revalidated` (new field — see §6.2) are refreshed; the original
|
||||
`date` field is preserved as the adoption-provenance marker.
|
||||
|
||||
**§6.2 — `last-revalidated` frontmatter field.** Trace schema gains
|
||||
`last-revalidated: <YYYY-MM-DD>`, set by both major-bump re-eval (§6.1)
|
||||
and trace revalidation (§3). Separate from the original `date` field
|
||||
which is immutable post-adoption.
|
||||
|
||||
**§6.3 — Socket as 9th hard filter.** ADR-022's 8 hard filters gain a
|
||||
9th: `socket-risk`. Trace frontmatter's `filter-results:` block adds
|
||||
`socket-risk: clean | flagged | "<finding-summary>"`. At adoption time
|
||||
the `evaluate-library` skill runs `socket-cli scan <package>` as part of
|
||||
the cheap-structural filter block; `critical` findings auto-reject.
|
||||
Verification-commands gains the Socket scan command.
|
||||
|
||||
### 7. Composition with the sandcastle reviewer prompt
|
||||
|
||||
The reviewer prompt at `.sandcastle/reviewer.prompt.md` is extended with
|
||||
two new responsibilities (bundled into the library-evaluation epic's
|
||||
existing story 06, not split into a separate story):
|
||||
|
||||
- Read Socket CI output (via `gh run view` or PR API) and reject the
|
||||
slice if any `critical` finding is present.
|
||||
- Read CodeQL findings and reject the slice if any `error` severity is
|
||||
present.
|
||||
|
||||
These compose with the reviewer's existing responsibilities (library-
|
||||
trace presence check from ADR-022's PRD, `pnpm coverage:diff` from
|
||||
ADR-020).
|
||||
|
||||
### 8. Template-vs-consumer framing
|
||||
|
||||
This stack ships as **template artifacts** that work in any consumer's
|
||||
GitHub repo. Configurations (`renovate.json`, `.socket.json`, `codeql.yml`,
|
||||
`trace-revalidation-weekly.yml`) are written generically:
|
||||
|
||||
- No project-name-specific paths.
|
||||
- All workflows use `ubuntu-latest`.
|
||||
- Plan-gated tools (CodeQL on private repos) include a clear error
|
||||
message when the consumer's plan doesn't cover them, rather than
|
||||
no-op-ing silently.
|
||||
- `docs/guides/ci-security.md` documents what each consumer toggles
|
||||
(GitHub push protection, Socket App install, branch protection rules
|
||||
for `library-policy/re-evaluation` labels).
|
||||
|
||||
This template's own consumption of the stack — when it's eventually
|
||||
pushed to a GitHub remote — uses the same configurations unchanged.
|
||||
|
||||
### 9. Amendment — SBOM release artifact (CycloneDX)
|
||||
|
||||
**Added:** 2026-05-20 (story `10-sbom-ci-workflow`)
|
||||
|
||||
`.github/workflows/release-please.yml` is amended to generate a
|
||||
[CycloneDX](https://cyclonedx.org/) SBOM and attach it to every GitHub
|
||||
release cut by release-please.
|
||||
|
||||
**Concrete step shape:**
|
||||
|
||||
```yaml
|
||||
- name: Generate CycloneDX SBOM
|
||||
if: ${{ steps.release.outputs.releases_created == 'true' }}
|
||||
run: >
|
||||
pnpm dlx @cyclonedx/cyclonedx-npm
|
||||
--output-file sbom-${{ steps.release.outputs.tag_name }}.cdx.json
|
||||
--output-format json
|
||||
--ignore-npm-errors
|
||||
|
||||
- name: Attach SBOM to GitHub release
|
||||
if: ${{ steps.release.outputs.releases_created == 'true' }}
|
||||
uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda # v3.0.0
|
||||
with:
|
||||
tag_name: ${{ steps.release.outputs.tag_name }}
|
||||
files: sbom-${{ steps.release.outputs.tag_name }}.cdx.json
|
||||
```
|
||||
|
||||
**Prerequisites** (also conditional on `releases_created == 'true'`):
|
||||
`actions/checkout@v4` → `pnpm/action-setup@v4` → `actions/setup-node@v4`
|
||||
→ `pnpm install --frozen-lockfile`, providing the installed workspace
|
||||
graph that `@cyclonedx/cyclonedx-npm` analyses.
|
||||
|
||||
**Rationale:**
|
||||
|
||||
- **Compliance surface.** Consumers pursuing SOC 2 / ISO 27001 /
|
||||
FedRAMP / EU CRA must produce an inventory of every version that
|
||||
shipped. A CycloneDX JSON SBOM attached to each GitHub release gives
|
||||
auditors a machine-readable, per-release artifact without requiring
|
||||
them to inspect or re-run the repo.
|
||||
- **`pnpm dlx`, not `package.json`.** `@cyclonedx/cyclonedx-npm` is
|
||||
a release-time audit tool, not a runtime or build dependency; adding
|
||||
it to the lockfile would violate ADR-022's spirit (library evaluation
|
||||
required for runtime deps in feature/core packages). `pnpm dlx`
|
||||
fetches and discards it within the CI step.
|
||||
- **`--ignore-npm-errors`.** `@cyclonedx/cyclonedx-npm` internally
|
||||
invokes `npm ls` to traverse the dependency graph. In a pnpm
|
||||
workspace, `npm ls` exits non-zero when it encounters dev-deps-of-
|
||||
dev-deps that pnpm correctly elides from the install tree; the SBOM
|
||||
content is unaffected. Without this flag the step exits 254 and
|
||||
produces no file. `--ignore-npm-errors` instructs the tool to treat
|
||||
those `npm ls` warnings as non-fatal and emit the SBOM regardless.
|
||||
- **SHA-pinned action.** `softprops/action-gh-release` is pinned to a
|
||||
40-character commit SHA (`# v3.0.0` trailing comment) per §1's
|
||||
Renovate `pinGitHubActionDigests` preset. Renovate will open a bump
|
||||
PR when a newer release is available.
|
||||
- **Conditional execution.** The SBOM steps run only when
|
||||
`releases_created == 'true'` — every non-release push to `main`
|
||||
skips them entirely. This keeps the workflow fast for the common case
|
||||
(release-please just updating its rolling PR).
|
||||
- **Root SBOM covers all workspace packages.** Running
|
||||
`@cyclonedx/cyclonedx-npm` from the workspace root after
|
||||
`pnpm install --frozen-lockfile` captures the full resolved
|
||||
dependency graph across all packages. Per-package SBOMs are out of
|
||||
scope (see story `10-sbom-ci-workflow` §Out of scope).
|
||||
|
||||
**Failure-mode table row** (extends §5):
|
||||
|
||||
| Gate | Layer | Hard block? |
|
||||
| ------------------------------------------- | ------- | ----------------------- |
|
||||
| SBOM generation (`@cyclonedx/...`) | release | Yes — release job fails |
|
||||
| SBOM upload (`softprops/action-gh-release`) | release | Yes — release job fails |
|
||||
|
||||
SBOM absence blocks the release job; it does **not** gate main CI
|
||||
(the `ci.yml` `validate` job is unaffected). This matches the
|
||||
principle that release assets are part of the release job, not part
|
||||
of the per-PR validation loop.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Dependabot for everything instead of Renovate.** Rejected. Less
|
||||
granular monorepo handling, requires verbose per-workspace config,
|
||||
Action SHA pinning needs manual setup. Renovate's `pnpm-workspace`
|
||||
- `pinGitHubActionDigests` presets do this declaratively.
|
||||
- **No bump tool, manual bumps only.** Rejected. Deps go stale; CVE
|
||||
patches land late; the named-consumer-cares-now signal becomes "who
|
||||
even remembers."
|
||||
- **Paid Socket Team plan for hard PR blocks.** Rejected (default).
|
||||
Free App + self-hosted `socket-cli` in CI achieves equivalent
|
||||
enforcement at $0. Consumers who want the server-side branch-protection
|
||||
integration can upgrade per their own threat model.
|
||||
- **Nightly trace revalidation instead of weekly.** Rejected. License /
|
||||
maintenance / EU-residency signals don't change daily; nightly burns
|
||||
CI minutes on noise. CVE batches publish ~weekly.
|
||||
- **Auto-dispatch on `library-policy/re-evaluation` issues.** Rejected.
|
||||
Creates a feedback loop where the agent loop runs nightly re-evals on
|
||||
rotating CVE data. The issue queue stays human-triaged; the dispatch
|
||||
loop drains it on demand.
|
||||
- **CI gating on `library-policy/re-evaluation` (block main).** Rejected.
|
||||
Main can keep deploying while the trace gets re-walked. CI gating
|
||||
would block release-please PRs every time a CVE drops upstream,
|
||||
conflating release flow with policy maintenance.
|
||||
- **Splitting into two ADRs (CI security + ADR-022 extensions).**
|
||||
Rejected. The bump-trigger rule only makes sense once Renovate is in
|
||||
place; trace revalidation only makes sense alongside Socket; the
|
||||
failure-mode hierarchy spans both. One coherent decision, one ADR.
|
||||
- **Editing ADR-022 in-place to add the bump rule + Socket filter + new
|
||||
field.** Rejected. ADR-022's `Status: Accepted` is provenance — what
|
||||
we believed when it was signed. Amendments live here in §6 and the
|
||||
implementation honors the composed picture. This is the repo's first
|
||||
amendment-style ADR; if it works, future ADR drift gets the same
|
||||
pattern.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive**
|
||||
|
||||
- Closes the post-adoption rot ADR-022 cannot reach alone — CVE drift,
|
||||
supply-chain behavior compromise, license drift, EU-residency drift,
|
||||
Action supply-chain attacks.
|
||||
- The trace artifact becomes continuously validated, not point-in-time.
|
||||
Approved libraries carry a `last-revalidated` freshness signal.
|
||||
- Renovate's major/minor split + automerge keeps the lockfile current
|
||||
with minimal human attention while still gating real decision moments.
|
||||
- Action SHA pinning closes the `tj-actions/changed-files` class of
|
||||
attack permanently.
|
||||
- Layered Socket integration (App + CLI + reviewer prompt) gives
|
||||
consumers a $0 baseline that's stricter than most paid offerings'
|
||||
defaults.
|
||||
- Failure-mode hierarchy is machine-readable: the sandcastle reviewer
|
||||
prompt becomes the single composable gate for agent-driven PRs.
|
||||
- Template-vs-consumer framing means downstream repos inherit the
|
||||
stack on day one without per-project setup.
|
||||
|
||||
**Negative**
|
||||
|
||||
- Six new artifacts ship (`renovate.json`, `.socket.json`, `codeql.yml`,
|
||||
`trace-revalidation-weekly.yml`, `gitleaks` pre-commit step, updates
|
||||
to `ci.yml` + reviewer prompt). Each is a maintenance surface.
|
||||
- Renovate config is dense; consumers extending it past defaults need
|
||||
to learn its rules.
|
||||
- Socket GitHub App requires a per-consumer install (one click); the
|
||||
CLI step in CI works regardless.
|
||||
- Trace revalidation produces a steady stream of dashboard-issue
|
||||
updates that humans/agents must skim periodically. Most are
|
||||
no-action.
|
||||
- ADR-022 + ADR-023 together are the repo's first amendment chain.
|
||||
Future agents must read both to get the current policy picture.
|
||||
- Major-bump trigger means every semver-major Renovate PR blocks on
|
||||
agent walk-through of `evaluate-library` (~5 min agent work per
|
||||
major-bump per package).
|
||||
|
||||
**Neutral**
|
||||
|
||||
- The 6 amendments to ADR-022 don't change ADR-022's `Status: Accepted`.
|
||||
Future archaeology finds both ADRs and the implementation honors
|
||||
the composed picture.
|
||||
- Existing 5 workflows are untouched except `ci.yml` gaining 1 step
|
||||
(`pnpm audit signatures`) and 1 step (`socket-cli scan`).
|
||||
- Glossary entries for **Trace revalidation** and **Major-bump
|
||||
re-evaluation** landed inline during the grill session that produced
|
||||
this ADR.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-022 — Library evaluation policy (the foundation this builds on
|
||||
and amends in §6)
|
||||
- ADR-019 — Sandcastle agent orchestration (the reviewer prompt is the
|
||||
agent-loop enforcement surface, see §7)
|
||||
- ADR-021 — release-please versioning (Renovate's bump PRs interact
|
||||
with release-please's release PRs; both are managed automatically)
|
||||
- ADR-012 — Feature conventions (the conformance system shape this
|
||||
stack mirrors — layered enforcement at declining latencies)
|
||||
- ADR-017 — OpenTelemetry migration (vendor-isolation pattern; Socket
|
||||
integration follows the same shape — `core-shared` doesn't import
|
||||
Socket SDK)
|
||||
- Glossary entries for **Library trace**, **Pre-shipped trace**,
|
||||
**Trace revalidation**, **Major-bump re-evaluation**
|
||||
- PRD: `docs/work/prds/ci-security-and-supply-chain.prd.md`
|
||||
(to be written, materialized via `/to-prd`)
|
||||
- Companion guide: `docs/guides/ci-security.md` (to be written; human
|
||||
reading-room with worked examples)
|
||||
225
docs/decisions/adr-024-product-analytics-channel.md
Normal file
225
docs/decisions/adr-024-product-analytics-channel.md
Normal file
@@ -0,0 +1,225 @@
|
||||
## ADR-024 — Product analytics as a fourth capture channel
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-18
|
||||
**Related:** ADR-006 (vertical feature packages), ADR-014 (Sentry observability), ADR-015 (events + jobs), ADR-017 (OpenTelemetry + PII boundary), ADR-018 (audit + compliance), ADR-022 (library evaluation policy)
|
||||
**Companion PRD:** `docs/work/prds/product-analytics-channel.prd.md` (decomposition seed)
|
||||
**Companion guide:** `docs/guides/analytics.md` (human reading-room — emitted by epic story 09)
|
||||
|
||||
## Context
|
||||
|
||||
The repo today has five capture channels that signal "something happened in a use case":
|
||||
|
||||
| Channel | Interface | Purpose | Backend |
|
||||
| ------------- | ------------------------------------------ | -------------------------------- | --------------------------------- |
|
||||
| Tracing | `ITracer` (`core-shared`) | distributed request tracing | OTel → Sentry / Datadog |
|
||||
| Error capture | `ILogger.captureException` (`core-shared`) | error reporting + crash messages | Sentry |
|
||||
| Breadcrumbs | `ILogger.addBreadcrumb` (`core-shared`) | trail attached to next exception | Sentry |
|
||||
| Metrics | `IMetrics` (`core-shared`) | aggregate numeric signals | OTel meter |
|
||||
| Audit | `IAuditLog.record` (`core-audit`) | DPA-compliant who-did-what trail | Payload `audit_events` collection |
|
||||
|
||||
None of these is product analytics. The Sentry surface is intentionally PII-stripped (`sendDefaultPii: false`, `setUser({ id })` only — ADR-017 §7). The audit surface is compliance-driven (state changes with actor/subject/IP). The metrics surface is pre-aggregated. **There is no path for funnel analysis, cohort tracking, conversion measurement, or any other identified-user event stream that a typical product needs.**
|
||||
|
||||
The gap is concrete: every consumer of this template who builds something user-facing eventually has the same conversation — "how do I add PostHog / Segment / Mixpanel?" — and ends up bolting it on at the React component layer, bypassing the conformance system entirely. The use case never knows it's emitting an analytics event. The manifest never records it. The ESLint rule that catches `audits` / `publishes` drift has no analog for analytics. Five gates of conformance protection collapse to zero the moment the consumer adds an SDK directly.
|
||||
|
||||
Two pressures motivate codifying analytics as a first-class channel:
|
||||
|
||||
1. **Symmetry with audit.** Audit and analytics are structurally near-identical from the manifest's perspective: both are call-site driven (use case emits specific named events at specific moments), both have a manifest field listing event slugs, both have a dependency injected into the use case factory, both have an ESLint rule cross-checking call literals against the manifest. The repo already accepted this shape for audit; treating analytics differently is special-pleading.
|
||||
|
||||
2. **PII boundary distinct from observability.** ADR-017 §7 locks the observability surface to id-only user context. That policy is deliberate and load-bearing for Sentry's PII posture. Analytics has a fundamentally different job (funnel and retention analysis), needs identified users with traits (email, plan, signup date), and serves a different stakeholder (product, not engineering). Conflating the two surfaces — either by stretching `ILogger.setUser` to accept traits or by adding analytics methods to `ILogger` — would compromise both. They need separate interfaces with separate policies.
|
||||
|
||||
A third pressure is downstream: consumers building real products will pick a vendor. ADR-022 (library evaluation policy) requires that choice to go through a trace with EU-residency, license, and Socket.dev filters. The template can either offer a contract surface those vendor choices plug into, or force every consumer to invent their own contract. The audit precedent shows that providing the contract is the right move — `IAuditLog` doesn't mandate Payload, but every consumer wires Payload by default and the contract makes that wiring uniform.
|
||||
|
||||
## Decision
|
||||
|
||||
Introduce **`IAnalytics`** as a fourth capture channel, living in a new optional core package `@repo/core-analytics`, scaffolded via `pnpm turbo gen core-package analytics`. Mirror the audit channel's structural shape with three deliberate divergences.
|
||||
|
||||
### Package layout
|
||||
|
||||
`@repo/core-analytics` is an **optional core package**, not part of `@repo/core-shared`. Same tier as `@repo/core-audit`. Consumers add it when they want analytics; the template ships it as a scaffoldable but uninstalled option. The `template-tiers.md` table grows by one optional core.
|
||||
|
||||
Why optional rather than must-have: analytics is a product-policy decision (do you track? to where? with what consent?), not infrastructure. The audit precedent established that product-policy capture channels live in optional cores. Bundling analytics into `core-shared` would force every feature in every consumer to carry an analytics dep regardless of need.
|
||||
|
||||
### Interface
|
||||
|
||||
```ts
|
||||
// @repo/core-analytics/src/analytics.interface.ts
|
||||
|
||||
export type AnalyticsAttributeValue = string | number | boolean;
|
||||
|
||||
export type AnalyticsUser = {
|
||||
/** Stable identifier for this user. Joins events across sessions + devices. */
|
||||
id: string;
|
||||
};
|
||||
|
||||
export interface IAnalytics {
|
||||
/** Emit a named event. The conformance gate cross-checks `event` against the manifest's analyticsEvents. */
|
||||
track(
|
||||
event: string,
|
||||
attributes?: Record<string, AnalyticsAttributeValue>,
|
||||
): void;
|
||||
|
||||
/**
|
||||
* Associate the current analytics session with a user. The optional
|
||||
* `attributes` carry user traits (plan, signup date, etc.) — see
|
||||
* ADR-024 § "PII boundary". Not gated by the manifest — it's identity
|
||||
* establishment, not an event.
|
||||
*/
|
||||
identify(
|
||||
user: AnalyticsUser,
|
||||
attributes?: Record<string, AnalyticsAttributeValue>,
|
||||
): void;
|
||||
|
||||
/** Client-side route change. Server impls no-op by convention. */
|
||||
pageView(
|
||||
path: string,
|
||||
attributes?: Record<string, AnalyticsAttributeValue>,
|
||||
): void;
|
||||
|
||||
/** Drain the in-memory batch. Wire into SIGTERM / beforeExit / serverless-response-finish handlers. */
|
||||
flush(): Promise<void>;
|
||||
}
|
||||
```
|
||||
|
||||
Four methods. Mirrors Segment Spec's core minus the rarely-needed (`group`, `alias`, `screen`). Events are attributed to the user established by the most recent `identify()` call — the standard SDK model — so `track()` carries only the event name and its attributes, and `AnalyticsUser` stays minimal (`id` only); traits ride the `attributes` argument of `identify()`. `flush()` is on the interface because every meaningful server-side SDK batches by default and event loss on shutdown is the most common analytics bug in serverless deployments.
|
||||
|
||||
### Manifest field
|
||||
|
||||
Each use case in `feature.manifest.ts` grows a fourth event-channel field:
|
||||
|
||||
```ts
|
||||
useCases: {
|
||||
signUp: {
|
||||
mutates: true,
|
||||
audits: ["user.created"],
|
||||
publishes: ["auth.user-signed-up"],
|
||||
consumes: [],
|
||||
analyticsEvents: ["user.signed-up"], // NEW
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
`analyticsEvents` is an array of event slug literals. Same syntax as `audits` / `publishes` / `consumes`. The use case body emits via `analytics.track(slug, attributes?)` calls.
|
||||
|
||||
### Brand + wrapper
|
||||
|
||||
Parallel to `withAudit` + `Audited`:
|
||||
|
||||
- **Wrapper**: `withAnalytics(analytics, factory(deps))` attaches the `Analyzed` brand to the wrapped function at DI bind time.
|
||||
- **Brand**: `Analyzed` — non-enumerable property `__analyzed: true`, identical implementation to `Audited`.
|
||||
- **Conformance check**: `assertFeatureConformance` requires the `Analyzed` brand when `manifest.useCases[name].analyticsEvents.length > 0`. No `mutates` gate (divergence from audit — see below).
|
||||
|
||||
`wireUseCase({ ... })` extends to accept an optional `analytics` arg; when present plus manifest has `analyticsEvents`, it composes `withAnalytics` into the wrapper chain. Composition order (innermost → outermost): `factory(deps)` → `withAnalytics` → `withAudit` → `withCapture` → `withSpan`.
|
||||
|
||||
### ESLint rule
|
||||
|
||||
New conformance rule `no-undeclared-analytics-event` at warn severity:
|
||||
|
||||
- Applies to `*.use-case.ts` files
|
||||
- Finds `analytics.track("X", ...)` calls with a string-literal first argument
|
||||
- Cross-checks `X` against the sibling manifest's `useCases[<name>].analyticsEvents`
|
||||
- Reports on undeclared event slugs
|
||||
|
||||
Mirror of `no-undeclared-audit` and `no-undeclared-event-publish`. Severity matches them (`warn`) because product event sprawl is common during exploration and the warn level catches drift without blocking iteration.
|
||||
|
||||
### React provider (client side)
|
||||
|
||||
`@repo/core-analytics/react` exports `<AnalyticsProvider value={...}>` + `useAnalytics()`. Both sides see `IAnalytics`. The consumer's app boundary constructs the impl (vendor-specific) and passes it into the provider; the rest of the React tree calls `useAnalytics().track(...)` / `.pageView(...)` against the same contract the server uses.
|
||||
|
||||
The provider does NOT auto-wire framework-specific route events (Next App Router events, TanStack Router events, etc.). Those vary per consumer; the provider exposes `pageView()` and the consumer wires their router's "route-changed" hook to call it. A future `pnpm turbo gen` could scaffold per-framework router adapters; out of scope for the initial epic.
|
||||
|
||||
### Three deliberate divergences from audit
|
||||
|
||||
1. **No `mutates` gate on the brand check.** The audit channel only requires `Audited` when `mutates: true && audits.length > 0` because compliance audits exist only for state changes (reads are tracked via a different DPA mechanism). Analytics events are equally legitimate for reads (article viewed, search performed, page visited) and writes (signup, purchase). The brand check is therefore conditioned on `analyticsEvents.length > 0` alone.
|
||||
|
||||
2. **`flush()` on the interface.** Audit writes are synchronous to a DB collection; metrics are pushed on a meter cadence. Analytics is the first capture channel where async batching is the default vendor behavior, so it's the first one where `flush()` belongs on the interface — wired into the app's graceful-shutdown hook by the consumer.
|
||||
|
||||
3. **React provider scaffold.** Audit is server-only by nature. Analytics has a meaningful client side (button clicks, pageviews) that benefits from sharing the same typed contract. The `@repo/core-analytics/react` subpath provides the provider so consumers don't reinvent it. Server-side conformance scope is unchanged — the manifest-gated path is use-case-level only.
|
||||
|
||||
### PII boundary
|
||||
|
||||
This is the only place the analytics channel structurally diverges from `ILogger` policy.
|
||||
|
||||
The observability surface (`ILogger`, `ITracer`) is bound to ADR-017 §7: id-only user context, `sendDefaultPii: false` everywhere, CI grep gate, server-side PII scrubbing at the OTel processor layer. That policy is deliberate and remains untouched.
|
||||
|
||||
The analytics surface is **structurally permissive**. The `attributes` argument of `identify()` accepts arbitrary `Record<string, AnalyticsAttributeValue>` user traits. The template makes no claim about what's allowed in it. Consumer is responsible for:
|
||||
|
||||
- Cookie consent and legal basis (e.g. GDPR Art. 6)
|
||||
- Retention policy in the analytics backend
|
||||
- Trait allowlist enforcement in their application code (if they want stricter than "permissive")
|
||||
- DSAR / right-to-erasure plumbing
|
||||
|
||||
This is documented in the interface's doc-comment on `identify()` and in `docs/guides/analytics.md`. No CI guardrail enforces it — the cross-cutting CI gates (ADR-022 library trace covering the backend, ADR-023 supply-chain scan) cover the actionable surface.
|
||||
|
||||
The reason this divergence is explicit rather than emergent: analytics PII is product/legal scope, not template scope. Conflating it with observability PII would either cripple analytics (id-only is unworkable for funnel analysis) or weaken observability (allowing traits in `ILogger.setUser` would erode ADR-017's grep gate).
|
||||
|
||||
### Channel orthogonality
|
||||
|
||||
The fourth channel does NOT merge with existing channels. `analyticsEvents`, `audits`, `publishes` remain three independent manifest fields, each with its own dep, its own call site, its own ESLint cross-check. A use case may emit to one, two, three, or all four event channels (counting `consumes`); each declaration is local to its purpose.
|
||||
|
||||
Why three independent fields rather than a unified `events: [{ slug, channels: [...] }]` shape: unification would refactor `defineFeature`, every conformance rule, every binder, every consumer in the codebase — a much larger scope than analytics itself, with payoff only when the same slug overlaps across channels (common but not universal). A future ADR can revisit when a real consumer feels the pain.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
### A. Stretch `ILogger` to do analytics
|
||||
|
||||
Add `track()` / `identify()` to `ILogger`. Sentry has `captureMessage` already; arguments could carry user traits.
|
||||
|
||||
**Rejected.** ADR-017 §7's PII policy is load-bearing — relaxing `setUser({ id })` to allow traits would either weaken Sentry's PII posture or require a parallel `setUserForAnalytics` method, which is the same divergence wearing a smaller hat. Two surfaces with two policies is cleaner than one surface with conditional policies.
|
||||
|
||||
### B. Single unified `events: [...]` manifest field
|
||||
|
||||
Replace `audits` + `publishes` + `consumes` + `analyticsEvents` with a single `events: [{ slug, channels: ["audit", "bus", "analytics"] }]` declaration. Use case fires once, channels fan out.
|
||||
|
||||
**Rejected.** Touches every existing feature manifest, every conformance rule, every binder, the ESLint rule set, and `defineFeature` itself. Payoff is real (single source of truth for slug overlap) but scope is enormous for a refactor that's adjacent to the analytics work, not central to it. Defer to a future ADR if real consumers feel the pain.
|
||||
|
||||
### C. Bus-as-bridge
|
||||
|
||||
Every analytics event is published to `IEventBus` as a normal event; an `AnalyticsBusHandler` listens to all events and forwards them to `IAnalytics`. Use case never knows about analytics.
|
||||
|
||||
**Rejected.** Couples analytics latency + retries to bus semantics. Forces analytics-only events to be wrapped in bus publishes that have no other consumer (wasted publishes). And the manifest gate becomes harder to express (the analytics handler subscribes to slugs declared in `publishes`, but only some bus events are also analytics events — the cross-check becomes two-step instead of one-step).
|
||||
|
||||
### D. Skip the template channel, document `/evaluate-library` route
|
||||
|
||||
Don't ship `IAnalytics`. Consumers who want analytics walk through `/evaluate-library`, pick PostHog / Segment / etc., wire it directly at the React + Node boundary, and never benefit from the conformance gates.
|
||||
|
||||
**Rejected.** This is the status quo, and it produces the exact problem the ADR opens with — every product ends up bolting analytics on at the wrong layer, bypassing the gate set, and reinventing the contract per consumer. The template's job is to make the right shape the easy shape. `IAuditLog` proves the model works.
|
||||
|
||||
### E. Bake in a default backend (e.g. PostHog)
|
||||
|
||||
Ship `@repo/core-analytics` with a PostHog impl included. Consumers who don't override get PostHog by default.
|
||||
|
||||
**Rejected.** Vendor lock without consumer signoff. The whole point of vendor-neutral interfaces (ADR-017's OTel + neutral `ITracer`) is that backend choices are downstream decisions, gated by ADR-022's library evaluation. Bundling PostHog (or any vendor) bypasses that gate.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Fourth capture channel codified as first-class.** Use cases now declare their analytics events in the manifest; the conformance system applies the same five-latency drift detection that already protects audit + bus + tracing.
|
||||
- **PII boundary made explicit.** ADR-017 §7's id-only observability policy stops being implicitly the analytics policy too. Two policies, two surfaces, no ambiguity.
|
||||
- **Vendor-neutral contract for downstream products.** Consumers picking PostHog / Segment / Mixpanel implement `IAnalytics` against the existing contract — no contract reinvention, no wrong-layer bolting. The library evaluation gate (ADR-022) applies to the vendor choice.
|
||||
- **Symmetric server + client surface.** Both sides see `IAnalytics`. Frontend team writes the same `analytics.track(...)` call site as the use case, gated by the same type contract.
|
||||
- **Template-tier surface stays clean.** Optional core package; consumers who don't want analytics don't install it.
|
||||
|
||||
### Negative
|
||||
|
||||
- **Manifest schema grows by one field.** Every existing feature's manifest gains an optional `analyticsEvents: []` field. The migration is cosmetic (empty arrays everywhere) but it's still a schema touch.
|
||||
- **Six conformance ESLint rules become seven.** `no-undeclared-analytics-event` joins the set. CLAUDE.md and conformance-quickref.md need updates.
|
||||
- **`assertFeatureConformance` symbol map grows.** Each feature's `bind-production.ts` adds the `Analyzed` brand check when the manifest declares analytics events.
|
||||
- **Brand composition is now four-deep.** `withSpan(... withCapture(... withAudit(... withAnalytics(... factory(deps)))))`. Documented in `docs/glossary.md` but still a real complexity step.
|
||||
- **One more decision the consumer must make.** Choosing a vendor is non-trivial; the template's only mitigation is to provide the contract + point at ADR-022 for the gate.
|
||||
|
||||
### Neutral
|
||||
|
||||
- **`flush()` on the interface.** Means the consumer's graceful-shutdown wiring must call it. Documented in `analytics.md`. Same shape as `bus.flush()` would have if we ever added one.
|
||||
- **React provider scaffold not auto-wiring route events.** Consumers wire their framework's router events to `pageView()` themselves. Acceptable; the same pattern exists for Sentry route tracking.
|
||||
- **No CI guardrail beyond what already exists.** ADR-022 + ADR-023 cover the meaningful cross-cutting surface; analytics has no single boolean to grep for.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-006 — vertical feature packages (boundary tags this fits within)
|
||||
- ADR-014 — Sentry observability (the channel this is structurally distinct from)
|
||||
- ADR-015 — events + jobs (the channel `analyticsEvents` is orthogonal to)
|
||||
- ADR-017 — OpenTelemetry + PII boundary (the policy this explicitly diverges from)
|
||||
- ADR-018 — audit + compliance (the channel this most closely mirrors)
|
||||
- ADR-022 — library evaluation policy (the gate that backend choice goes through)
|
||||
345
docs/decisions/adr-025-eu-compliance-baseline.md
Normal file
345
docs/decisions/adr-025-eu-compliance-baseline.md
Normal file
@@ -0,0 +1,345 @@
|
||||
## ADR-025 — EU compliance baseline (DPA/GDPR template scope)
|
||||
|
||||
**Status:** Accepted
|
||||
**Date:** 2026-05-18
|
||||
**Related:** ADR-006 (vertical feature packages), ADR-017 (OTel + observability PII boundary), ADR-018 (audit + compliance), ADR-022 (library evaluation policy — extended here for sub-processors), ADR-023 (CI security + supply chain — SBOM amended here), ADR-024 (product analytics channel)
|
||||
**Companion PRDs** (one per epic, sequenced):
|
||||
|
||||
- `docs/work/prds/compliance-manifests-pii-retention-subprocessors.prd.md` (Epic A)
|
||||
- `docs/work/prds/dsr-consent-and-cookie-banner.prd.md` (Epic B)
|
||||
- `docs/work/prds/security-headers-rate-limit-sbom.prd.md` (Epic C)
|
||||
- `docs/work/prds/compliance-docs-scaffolds.prd.md` (Epic D)
|
||||
|
||||
## Context
|
||||
|
||||
A DPA/GDPR compliance playbook (22 sections, stack-agnostic) was reviewed against the template's current state on **2026-05-18**. The audit produced a clean three-way split:
|
||||
|
||||
1. **Already covered** by prior ADRs:
|
||||
- PII boundary on observability (ADR-017 §7 — `sendDefaultPii: false` CI gate, server-side scrubbing, replay masking)
|
||||
- Audit logging baseline (ADR-018 — `@repo/core-audit`, DPA-aligned schema, `eraseSubject` pseudonymization)
|
||||
- EU library residency (ADR-022 — hard filter in `/evaluate-library`)
|
||||
- Supply-chain + CI security (ADR-023 — Renovate SHA pinning, Socket.dev, audit signatures, CodeQL, gitleaks, trace revalidation)
|
||||
- Analytics PII deferral (ADR-024 — explicit consumer-policy boundary)
|
||||
|
||||
2. **Template-shaped gap** — 10 items the playbook flagged that the template can codify _before_ any consumer adopts it. These are conformance-pattern shaped (manifest fields, brands, ESLint rules, generators) or scaffolding (interfaces, default middleware, fill-in docs).
|
||||
|
||||
3. **Product-shaped (deferred)** — 3 items that need product shape before being meaningful. Each has a documented trigger condition for revisit.
|
||||
|
||||
The motivating pressure: a consumer adopting this template today gets ~50% of the playbook's surface for free. The 10-item template-shaped gap is what this ADR plans (raising coverage to ~80%); the remaining ~20% product/process/legal scope stays consumer-owned and is documented as such.
|
||||
|
||||
Audit + DSR is the canonical confusion to flag upfront: **audit _records_ personal-data access (immutable journal); DSR _acts_ on the underlying data in response to user requests (mutator/exporter).** They are sibling concerns, not duplicates.
|
||||
|
||||
## Decision
|
||||
|
||||
Ship the 10 template-shaped items across **four epics**, sequenced A → B → D with C interleaved opportunistically. Defer 3 items explicitly. Add 3 new manifest fields, 2 new optional cores, 3 new ESLint rules.
|
||||
|
||||
### The 4 epics
|
||||
|
||||
#### Epic A — Declarative compliance manifests
|
||||
|
||||
**Items:** PII inventory + data retention + sub-processor inventory.
|
||||
|
||||
Three declarative artifacts, each driven by a different surface and a generator that emits the audit-evidence YAML:
|
||||
|
||||
| Artifact | Declaration site | Generator output |
|
||||
| ----------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------- |
|
||||
| PII inventory | `custom.pii: { category, purpose, retention, exportable, restrictable }` per Payload **field** | `compliance/data-map.yml` |
|
||||
| Retention policy | `custom.retention: { activeRetention, postDeletion, purgeSchedule, hardDeleteAfter }` per Payload **collection** | `compliance/retention-policy.yml` |
|
||||
| Sub-processor inventory | Extended ADR-022 library traces — frontmatter fields `is-sub-processor`, `processes-pii`, `data-sent`, `region`, `dpa-signed`, `sccs-required`, `contact` | `compliance/sub-processors.yml` |
|
||||
|
||||
**Key design decisions:**
|
||||
|
||||
- **PII at the field level, not the manifest level.** PII is a storage question ("what personal data does this system hold?"), not an action question. Existing manifest fields (`audits`, `publishes`, `analyticsEvents`) are all event-shaped — emissions from use cases. PII fields don't fit that shape. Tagging at the field level puts the metadata where DSR consumes it at runtime.
|
||||
|
||||
- **Retention at the collection level, not per use case.** Same rationale — retention is a storage property. Per-field PII retention overrides apply where the PII tag specifies a stricter retention than the collection default (more-specific wins). Background purge job in `core-shared/jobs` reads collection config at boot.
|
||||
|
||||
- **Sub-processors via ADR-022 traces, not standalone file.** Every direct-dep library trace already records EU residency, license, CVE acceptance. Extending it with the sub-processor fields (DPA signed date, SCCs, contact, PII processed, data sent, region) unifies two related obligations in one record. Pure-HTTP sub-processors (REST calls without an SDK) get hand-authored entries with no backing trace as an exception, CI-flagged.
|
||||
|
||||
The three generators run via `pnpm compliance:emit-all`. CI gate verifies generator output matches the source declarations (drift detection).
|
||||
|
||||
**Background purge job:** `core-shared/jobs/retention-purge.job.ts`. Reads `custom.retention` from each collection at boot; schedules per-collection purge cadence; emits an `IAuditLog.record({ action: "DELETE", reason: "retention-policy" })` audit entry per row purged.
|
||||
|
||||
#### Epic B — DSR + consent + cookie banner
|
||||
|
||||
**Items:** DSR scaffold + consent abstraction + cookie consent UI.
|
||||
|
||||
Builds the user-rights surface end-to-end:
|
||||
|
||||
**`@repo/core-dsr`** — new optional core. Four interfaces:
|
||||
|
||||
```ts
|
||||
interface IDataExport {
|
||||
exportSubjectData(
|
||||
subjectId: string,
|
||||
format: "json" | "json-ld",
|
||||
): Promise<UserDataBundle>;
|
||||
}
|
||||
interface IDataDelete {
|
||||
deleteSubjectData(
|
||||
subjectId: string,
|
||||
mode: "soft" | "cascade-hard",
|
||||
): Promise<DeletionCertificate>;
|
||||
}
|
||||
interface IDataRectify {
|
||||
updateSubjectField(
|
||||
subjectId: string,
|
||||
collection: string,
|
||||
field: string,
|
||||
value: unknown,
|
||||
): Promise<void>;
|
||||
}
|
||||
interface IProcessingRestriction {
|
||||
setRestriction(subjectId: string, granted: boolean): Promise<void>;
|
||||
isRestricted(subjectId: string): Promise<boolean>;
|
||||
}
|
||||
```
|
||||
|
||||
DSR ops walk Payload collections at runtime, using the field-level `custom.pii` tags from Epic A. `IDataExport` walks fields tagged `exportable: true`; `IDataDelete` walks all PII fields and cascades; `IProcessingRestriction` writes a flag on the user record that every read path checks.
|
||||
|
||||
**Scope cuts on DSR:**
|
||||
|
||||
- Art. 20 (portability) folded into Art. 15 (access) — same `IDataExport` with format option
|
||||
- Art. 21 (objection) → consent epic via `IConsent.withdraw`
|
||||
- Art. 22 (automated decision-making) → deferred (no ML in template; future ADR when a consumer adds automated decisions)
|
||||
|
||||
DSR ops are themselves PII access events — every `IDataExport`/`IDataDelete`/`IDataRectify` call writes an audit entry. After `IDataDelete`, `core-audit.IAuditLog.eraseSubject(actorId, "pseudonymize")` scrubs the audit trail (preserves the events, removes the identifier).
|
||||
|
||||
**`@repo/core-consent`** — new optional core. Sibling channel parallel to audit/analytics:
|
||||
|
||||
```ts
|
||||
interface IConsent {
|
||||
isGranted(subjectId: string, category: ConsentCategory): Promise<boolean>;
|
||||
grant(
|
||||
subjectId: string,
|
||||
categories: ConsentCategory[],
|
||||
record: ConsentRecord,
|
||||
): Promise<void>;
|
||||
withdraw(subjectId: string, categories: ConsentCategory[]): Promise<void>;
|
||||
getCategories(subjectId: string): Promise<ConsentCategory[]>;
|
||||
}
|
||||
```
|
||||
|
||||
`ConsentCategory` is a consumer-typed string-literal-union (default: `"essential" | "functional" | "analytics" | "marketing"`, extensible). Conformance treatment:
|
||||
|
||||
- Brand: `ConsentChecked` attached by `withConsent` wrapper at bind time
|
||||
- Manifest field: `requiresConsent: ["analytics"]` per use case
|
||||
- ESLint rule: `no-undeclared-consent-check` cross-checks `consent.requires("X")` literal calls
|
||||
- Boot assertion: `assertFeatureConformance` requires `ConsentChecked` brand when `requiresConsent.length > 0`
|
||||
|
||||
Consent grant/withdraw events are themselves audited (`auditLog.record({ action: "CONSENT_GRANT", category: "marketing" })`).
|
||||
|
||||
**`<CookieConsentBanner>` in `@repo/core-ui`** — atomic component. Default visual treatment baked for EU prominence requirements (Reject + Accept side-by-side, same size, equal prominence per EU regulator guidance). Granular (essential/functional/analytics/marketing). Consumer wires `IConsent` via React context. Storybook story doubles as human reading-room for compliant UX.
|
||||
|
||||
**Endpoint scaffolds:** `/api/gdpr/{export,delete,rectify,restrict}` — consumer-wireable routes. Live in `apps/web-next/app/api/gdpr/` and `apps/web-tanstack/src/routes/api/gdpr/`.
|
||||
|
||||
#### Epic C — Security hardening
|
||||
|
||||
**Items:** Security headers middleware + rate-limit primitive + SBOM in CI.
|
||||
|
||||
Small individual scope; grouped for dispatch efficiency. Each item is independent.
|
||||
|
||||
- **Security headers middleware** — default Next.js + TanStack middleware shipping HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy. CSP customizable per consumer (separate config; default is restrictive). Lives in `core-shared/security` plus per-framework re-export.
|
||||
|
||||
- **Rate-limit primitive** — fourth conformance channel after audit/analytics/consent. `IRateLimit` interface in `core-shared/rate-limit`:
|
||||
|
||||
```ts
|
||||
interface IRateLimit {
|
||||
consume(
|
||||
key: string,
|
||||
weight?: number,
|
||||
): Promise<{ allowed: boolean; remaining: number; resetAt: Date }>;
|
||||
reset(key: string): Promise<void>;
|
||||
}
|
||||
```
|
||||
|
||||
- Brand: `RateLimited` attached by `withRateLimit` wrapper
|
||||
- Manifest field: `rateLimit: { window: "1m", budget: 60 }` per use case (defaults; runtime overrides via `ctx.rateLimit` config)
|
||||
- ESLint rule: `no-undeclared-rate-limit` warns when a use case in an auth/write/export category lacks a `rateLimit` declaration
|
||||
- Boot assertion: `assertFeatureConformance` requires `RateLimited` brand when `rateLimit` is set
|
||||
- Consumer wires Redis/Upstash impl; `NoopRateLimit` always-allows for tests + dev
|
||||
|
||||
Rate-limit budgets at the manifest level are _defaults_ — overridable at runtime via `ctx.rateLimit` config (mirrors how analytics backend is consumer-wired). Manifest declaration is for the binding gate; runtime values are deployment-environment-specific.
|
||||
|
||||
- **SBOM in CI** — `cyclonedx-npm` step in `ci.yml`, artifact uploaded per release. Amendment to ADR-023.
|
||||
|
||||
#### Epic D — Compliance docs scaffolds
|
||||
|
||||
**Items:** Fill-in templates for runbooks, policies, and the pre-launch checklist.
|
||||
|
||||
Pure docs work. Lands last so it references the manifest fields, interfaces, and middleware shipped by A/B/C.
|
||||
|
||||
**Template-shipped (under `docs/compliance/`):**
|
||||
|
||||
- `data-map.example.yml` — generator-output reference
|
||||
- `retention-policy.example.yml` — generator-output reference
|
||||
- `sub-processors.example.yml` — generator-output reference
|
||||
- `incident-runbook.template.md` (fill-in)
|
||||
- `dsr-procedure.template.md`
|
||||
- `backup-policy.template.md`
|
||||
- `password-policy.template.md`
|
||||
- `device-policy.template.md`
|
||||
- `onboarding.template.md`
|
||||
- `offboarding.template.md`
|
||||
- `README.md` (explains the `docs/compliance/` vs `compliance/` split)
|
||||
|
||||
**Consumer-created (under `compliance/` at repo root):**
|
||||
|
||||
- `data-map.yml` (generator output)
|
||||
- `retention-policy.yml` (generator output)
|
||||
- `sub-processors.yml` (generator output from extended ADR-022 traces)
|
||||
- `*.md` (filled-in copies of templates)
|
||||
|
||||
**Plus:** `docs/guides/pre-launch-compliance-checklist.md` — playbook §19 verbatim with template-specific wiring noted.
|
||||
|
||||
### Sequencing
|
||||
|
||||
**Order: A → B → D, with C interleaved opportunistically.**
|
||||
|
||||
Hard dependencies:
|
||||
|
||||
- B's `IDataExport`/`IDataDelete` walk Epic A's PII tags at runtime → A must finish before B story 1
|
||||
- D's `data-map.example.yml` documents Epic A's PII schema → A's design must be settled before D
|
||||
- D's `dsr-procedure.template.md` references Epic B's endpoints → B should be design-settled before D's PRD is decomposed
|
||||
|
||||
C is dependency-free; sandcastle picks C stories during gaps in A/B/D.
|
||||
|
||||
### Deferrals (explicit, with revisit triggers)
|
||||
|
||||
| Deferred | Why deferred | Trigger to revisit |
|
||||
| ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ |
|
||||
| **RBAC primitive** (roles + permissions + tenant scoping) | Needs product-side decisions: which roles exist, multi-tenant or not, permission granularity | First downstream consumer ships with a stable role model |
|
||||
| **MFA + password policy + lockout** (`auth` feature extension) | Needs identity-infrastructure choices (TOTP/WebAuthn), threat-model-specific policy values, OTP delivery vendor (ADR-022 territory) | First downstream consumer establishes auth threat model |
|
||||
| **Breach detection patterns** (failed-login burst, bulk-access anomaly, off-hours admin) | Needs real auth flows, analytics backend, on-call infrastructure, product-specific anomaly definitions | First downstream consumer has live traffic + observability backend |
|
||||
| **GDPR Art. 22** (automated decision-making) — sub-deferral within Epic B | Template has no ML/automated decisions | First downstream consumer adds automated decisions |
|
||||
|
||||
Each deferral has a documented trigger so the decision-when can be answered by the consumer, not the template authors.
|
||||
|
||||
### Consumer-scope items (explicitly out of template)
|
||||
|
||||
These appear in the playbook but are NOT template-shaped:
|
||||
|
||||
- **Infrastructure (§1, §12)** — EU region pinning of compute/storage/backups, TLS at deploy edge, encryption-at-rest config, VPN/bastion network boundaries, backup strategy + restore testing
|
||||
- **Legal (§17)** — DPA, Privacy Policy, ToS, SCCs for non-EU sub-processors, DPIA artifacts, RoPA documents
|
||||
- **Organizational (§14, §15)** — MDM enrollment, HR onboarding/offboarding scripts (the _script_ is template; the _policy_ is consumer), NDAs, security training, background checks, quarterly access reviews, pentest scheduling
|
||||
|
||||
Epic D ships fill-in templates for some documentation artifacts above; the values stay consumer-filled.
|
||||
|
||||
### Manifest schema impact
|
||||
|
||||
Per-use-case fields grow from 5 to 7:
|
||||
|
||||
- Existing: `mutates`, `audits`, `publishes`, `consumes`, `analyticsEvents`
|
||||
- Added by ADR-025: `requiresConsent`, `rateLimit`
|
||||
|
||||
Per-Payload-collection `custom` config grows:
|
||||
|
||||
- Added by ADR-025: `pii` (per field), `retention` (per collection)
|
||||
|
||||
Per-library-trace frontmatter grows (extends ADR-022):
|
||||
|
||||
- Added by ADR-025: `is-sub-processor`, `processes-pii`, `data-sent`, `region`, `dpa-signed`, `sccs-required`, `contact`
|
||||
|
||||
### Conformance ESLint rule impact
|
||||
|
||||
Rule count: 7 → 10. New rules at warn severity (matching the audit/analytics-event convention):
|
||||
|
||||
- `no-undeclared-consent-check` — `consent.requires(...)` literal calls must match manifest's `requiresConsent`
|
||||
- `no-undeclared-rate-limit` — auth/write/export categorized use cases without `rateLimit` field
|
||||
- `pii-declaration-must-be-complete` — Payload `pii: true` fields missing required sub-keys (category, purpose, retention)
|
||||
|
||||
### Generator + CI gate inventory
|
||||
|
||||
| Generator | Source | Output | CI gate |
|
||||
| ---------------------------------- | ----------------------------------------------------------- | --------------------------------- | -------------------------------------------- |
|
||||
| `pnpm compliance:data-map` | Payload field `custom.pii` | `compliance/data-map.yml` | output matches collections (drift detection) |
|
||||
| `pnpm compliance:retention-policy` | Payload collection `custom.retention` | `compliance/retention-policy.yml` | output matches collections |
|
||||
| `pnpm compliance:sub-processors` | `docs/library-decisions/*.md` with `is-sub-processor: true` | `compliance/sub-processors.yml` | output matches traces |
|
||||
| `pnpm compliance:emit-all` | runs all three | three files | runs all three CI gates |
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
### A. One mega-epic covering all 10 items
|
||||
|
||||
Single PRD, single epic, ~30-40 stories. Pros: tight coupling across the manifest-schema changes; one review surface. Cons: enormous PR backlog with no natural checkpoint; reviewer fatigue; if half-shipped, the partial state leaves an ambiguous compliance surface.
|
||||
|
||||
**Rejected.** The four-epic split keeps each PRD focused enough for a useful single-document review.
|
||||
|
||||
### B. Per-item ADRs (10 ADRs)
|
||||
|
||||
One ADR per item — finer granularity. Pros: each architectural decision recorded in isolation. Cons: 10 ADRs to maintain, much repetition (each restates the playbook context), no unifying strategy doc, harder to answer cross-cutting questions like "why these 3 deferrals" without re-reading 4+ ADRs.
|
||||
|
||||
**Rejected.** ADR-025 is the unifying strategy; per-epic PRDs handle implementation specifics. Future deepening decisions on individual items can spawn their own ADRs as needed (e.g., when DSR cascade semantics need a specific architectural call, that becomes ADR-NNN).
|
||||
|
||||
### C. Wait until the first downstream consumer asks
|
||||
|
||||
Don't build any of this until a consumer arrives with a real compliance requirement.
|
||||
|
||||
**Rejected.** The whole value proposition of this template is that DPA/GDPR-shaped consumers don't have to invent these surfaces. The "named-consumer-now" rule from ADR-022 applies to _library adoption_, not to template surface — the consumer of this surface is "every downstream EU-bound product," which is real and immediate.
|
||||
|
||||
### D. PII declared at use-case manifest level instead of Payload field level
|
||||
|
||||
`pii: [{ category, purpose, retention }]` per use case, mirroring audit/publishes.
|
||||
|
||||
**Rejected.** Duplicates metadata across every use case touching the same field. Wrong semantic layer — PII is a storage question, not an action question. DSR (Epic B) needs runtime access to PII tags at the field level to walk Payload collections; manifest-level tags would require synthesis at runtime.
|
||||
|
||||
### E. DSR split across multiple cores (`@repo/core-data-export`, `@repo/core-data-delete`, etc.)
|
||||
|
||||
Maximum granularity. Pros: consumers adopt only what they need. Cons: 4+ packages to scaffold, tight coupling in practice (delete cascade needs to know export's PII tags), overkill — each "core" would have one interface.
|
||||
|
||||
**Rejected.** One `@repo/core-dsr` with 4 interfaces, mirroring `core-shared`'s tracer/logger/metrics packaging pattern (multiple interfaces in one package). Opt-in is at the package level, not the interface level.
|
||||
|
||||
### F. Consent folded into `core-audit` or `core-analytics`
|
||||
|
||||
`IConsent` added to an existing core.
|
||||
|
||||
**Rejected.** Audit _records_, consent _gates_ — different abstractions. Conflates package purpose. Analytics is only one of many consent-gated channels (marketing, profiling, cookies, third parties); putting consent inside analytics is too narrow.
|
||||
|
||||
### G. Rate-limit as interface-only (no brand)
|
||||
|
||||
Just an `IRateLimit` contract. Consumers call `rateLimit.consume(...)` where they need.
|
||||
|
||||
**Rejected.** Rate-limit drift would only surface when traffic hits — way too late. The five-latency drift detection is the template's signature pattern. Skipping it for rate-limit when it's universally applicable to auth/write/export endpoints (per playbook §5) leaves a real hole. Manifest declaration at the binding gate + runtime budget override gives both static enforcement and deployment flexibility.
|
||||
|
||||
### H. Compliance docs all under `docs/compliance/` (no root `compliance/` directory)
|
||||
|
||||
Templates and live artifacts co-located.
|
||||
|
||||
**Rejected.** The template ships only the _shape_; the consumer fills in the _evidence_. Auditors expect `compliance/` at the repo root (matches playbook §16 + standard SOC 2/ISO 27001 audit prep). Splitting locations matches the template-vs-consumer mental model used throughout this session.
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
|
||||
- **Compliance surface ~80% template-shipped.** From ~50% pre-ADR (audit + PII boundary + library residency + supply chain) to ~80% post-epics. Remaining 20% is consumer-scope by design.
|
||||
- **Conformance pattern extended consistently.** Two new manifest fields (`requiresConsent`, `rateLimit`) plus two new collection-level `custom.*` extensions all follow the established pattern. Three new ESLint rules at warn level. Three new manifest-driven generators. New consumers learn the pattern once.
|
||||
- **DPA audit posture improves materially.** Sub-processor inventory, retention policy, data map, DSR endpoints, cookie consent are concrete artifacts a regulator or customer audit expects.
|
||||
- **Explicit deferrals prevent premature design.** RBAC, MFA, breach detection, Art. 22 won't be re-suggested by future agents — ADR-025 records the trigger conditions.
|
||||
- **Two new optional cores** (`core-dsr`, `core-consent`) match the established pattern. Template-tiers grows by two; scaffold path is `pnpm turbo gen core-package <name>`.
|
||||
- **Rate-limit gets first-class treatment.** Fourth conformance channel; auth/write/export endpoints can't ship without a declared budget.
|
||||
- **The audit ↔ DSR distinction is documented.** Future agents won't conflate the two — glossary entries plus this ADR's "Context" section make the split explicit.
|
||||
|
||||
### Negative
|
||||
|
||||
- **Manifest schema grows substantially.** Per-use-case fields go from 5 to 7. Per-collection `custom` config gains two extensions. Doc burden in glossary + `conformance-quickref.md` increases proportionally.
|
||||
- **Conformance ESLint rule count: 7 → 10.** CLAUDE.md and quickref need rule-table updates each epic.
|
||||
- **Two new optional cores to maintain.** Each needs versioning + CHANGELOG (per ADR-021).
|
||||
- **`compliance/` directory becomes a new root location.** Adds a top-level directory alongside `docs/`, `packages/`, `apps/`. Consumers will see it; it's intentional but it's a new convention.
|
||||
- **DSR cascade is non-trivial.** `IDataDelete` walking every Payload collection's PII fields requires Epic A's PII tags to be complete and correct. Epic B will surface gaps in Epic A's coverage during integration.
|
||||
- **ADR-022 amendment.** Adding sub-processor fields to library traces is an extension to ADR-022's frontmatter spec. Existing traces need backfill (the weekly revalidation cron from ADR-023 will surface incomplete traces).
|
||||
- **ADR-023 amendment.** SBOM generation step adds one workflow line; minor but counted.
|
||||
|
||||
### Neutral
|
||||
|
||||
- **No new CI gates beyond what generators introduce.** The three drift-detection gates (data-map, retention-policy, sub-processors) all reuse the existing CI workflow shape.
|
||||
- **No vendor lock-in.** All interfaces (DSR cascade target, consent backend, rate-limit backend, security-header CSP values) remain consumer-decisions. Template ships interfaces + Noop/reference impls only.
|
||||
- **Deferred items remain deferred.** ADR-025 doesn't preclude building RBAC/MFA/breach-detection/Art. 22 later — it just establishes that those decisions wait for product shape.
|
||||
- **Cookie banner ships with opinionated EU-prominence defaults.** Consumers can override visual treatment but the default is the legally-defensible shape.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-006 — vertical feature packages (boundary tags new optional cores fit within)
|
||||
- ADR-017 — OTel + observability PII boundary (the boundary `IConsent` does NOT cross — observability stays id-only)
|
||||
- ADR-018 — audit + compliance (the sibling channel `core-dsr` complements without overlapping)
|
||||
- ADR-022 — library evaluation policy (extended here with sub-processor frontmatter fields)
|
||||
- ADR-023 — CI security + supply chain (SBOM amended here; rate-limit complements the supply-chain stack)
|
||||
- ADR-024 — product analytics channel (sibling capture channel; `IConsent` from Epic B will gate `IAnalytics.track` calls in consumer products)
|
||||
208
docs/decisions/adr-026-cross-feature-readers.md
Normal file
208
docs/decisions/adr-026-cross-feature-readers.md
Normal file
@@ -0,0 +1,208 @@
|
||||
# ADR-026 — Cross-feature synchronous readers
|
||||
|
||||
**Status:** Accepted
|
||||
|
||||
**Date:** 2026-05-28
|
||||
|
||||
## Context
|
||||
|
||||
The monorepo's vertical-slice architecture (ADR-006) enforces strict feature isolation: each vertical owns its data end-to-end, and cross-feature communication flows through the event bus (ADR-015, rule E0). This works well for **reactions** ("user signed up → send welcome email"), but the architecture has no mechanism for **synchronous domain queries** across features.
|
||||
|
||||
Three concrete scenarios expose the gap:
|
||||
|
||||
1. **Permission checks.** Blog's `createArticle` needs to verify the author has the "editor" role. The raw user record is available via Payload's `relationTo`, but evaluating "does role X grant permission Y in context Z?" is domain logic that belongs to the auth vertical.
|
||||
2. **Computed state.** A billing feature needs to know whether a subscription is active after applying trial logic, grace periods, and plan rules. That evaluation belongs to the subscriptions vertical.
|
||||
3. **Validated existence.** A comments feature needs to verify a referenced article exists and is in "published" status — a check that includes blog-domain invariants, not just a row lookup.
|
||||
|
||||
Payload's `relationTo` handles raw data joins at the database level (and should continue to be used for that), but it cannot evaluate business rules owned by another vertical. Events cannot answer synchronous questions. The architecture needs a third cross-feature mechanism.
|
||||
|
||||
## Decision
|
||||
|
||||
**1. Introduce readers: synchronous, read-only cross-feature query contracts.**
|
||||
|
||||
A **reader** is a minimal interface exported by a feature that exposes domain queries to other verticals. It complements events (async reactions) and `relationTo` (raw data joins) without replacing either.
|
||||
|
||||
| Cross-feature need | Mechanism | Sync/Async | Example |
|
||||
| ---------------------- | -------------------- | ----------- | ------------------------------ |
|
||||
| Raw data join | Payload `relationTo` | Sync (DB) | Article card shows author name |
|
||||
| Domain query | Reader | Sync (code) | "Does user have editor role?" |
|
||||
| Reaction / side effect | Event bus (ADR-015) | Async | "User signed up → send email" |
|
||||
| Deferred work | Job queue (ADR-015) | Async | "Resize uploaded image" |
|
||||
| State delivery / push | Realtime (ADR-016) | Async | "New comment appeared" |
|
||||
|
||||
**2. Four rules, parallel to events (E0/E1) and jobs (J0).**
|
||||
|
||||
- **Q0 — Readers are for cross-feature synchronous domain queries only.** In-feature reads are direct use-case calls. If the caller and the data owner are in the same vertical, use the use case directly — don't route through a reader.
|
||||
- **Q1 — Reader contracts (interfaces) are public; implementations are private.** The owning feature exports `I<Feature>Reader` from a `./reader` subpath. The implementation class (`<Feature>Reader`) is internal, constructed by the feature's binder. Consumers import the type only. Parallel to rule E1 for event handlers.
|
||||
- **Q2 — Readers are strictly read-only. Cross-feature writes go through events.** A reader may only delegate to use cases declared `mutates: false` in the feature manifest. Enforced by `ReadOnly<F>` TypeScript brand at compile time and `assertReaderPurity` at boot time. If you need to tell another vertical that something happened, publish an event.
|
||||
- **Q3 — Reader cycles are a design error.** If Feature A reads from Feature B and Feature B reads from Feature A, the boundaries are wrong. Resolution strategies: (a) one direction is a UI composition concern — compose at the app layer instead; (b) one direction can be async — use an event; (c) the two features should be one vertical.
|
||||
|
||||
**3. One reader per feature, grown on demand.**
|
||||
|
||||
Each feature that exposes cross-feature queries ships a single `I<Feature>Reader` interface (e.g., `IAuthReader`, `ITenantReader`). The interface starts minimal and grows as consumers need more methods. If the interface becomes bloated, that's a signal the vertical is too fat.
|
||||
|
||||
**4. Readers wrap existing use cases — they don't add domain logic.**
|
||||
|
||||
The reader is a thin facade over the owning feature's use cases. It does not contain business rules itself. If a reader needs logic that doesn't exist as a use case, the correct response is to create the use case first (manifest-first ordering), then have the reader delegate to it.
|
||||
|
||||
```typescript
|
||||
// packages/auth/src/infrastructure/readers/auth.reader.ts (INTERNAL)
|
||||
export class AuthReader implements IAuthReader {
|
||||
constructor(
|
||||
private checkRole: ReadOnly<ICheckUserRoleUseCase>,
|
||||
private getUser: ReadOnly<IGetUserUseCase>,
|
||||
) {}
|
||||
|
||||
async hasRole(userId: string, role: string): Promise<boolean> {
|
||||
return this.checkRole({ userId, role });
|
||||
}
|
||||
|
||||
async exists(userId: string): Promise<boolean> {
|
||||
const user = await this.getUser({ id: userId });
|
||||
return user !== null;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Because the reader wraps use cases, no `MockReader` class is needed. In dev-seed mode the same `AuthReader` class works — the use cases beneath it are backed by mock repositories populated with seed data. In consumer tests, an inline vitest mock of `IAuthReader` suffices.
|
||||
|
||||
**5. Readers live under `integrations/readers/`, exported via `./reader` subpath.**
|
||||
|
||||
The reader is an outward-facing integration boundary, parallel to `integrations/api/` (HTTP consumers) and `integrations/cms/` (Payload admin). File layout:
|
||||
|
||||
```
|
||||
packages/<feature>/src/
|
||||
integrations/
|
||||
api/ # outward: HTTP consumers
|
||||
cms/ # outward: Payload admin
|
||||
readers/ # outward: other verticals
|
||||
<feature>.reader.interface.ts # IFeatureReader (PUBLIC)
|
||||
<feature>.reader.ts # FeatureReader (INTERNAL)
|
||||
<feature>.reader.test.ts
|
||||
index.ts # exports type { IFeatureReader } only
|
||||
```
|
||||
|
||||
The `package.json` exports map gains a `./reader` entry:
|
||||
|
||||
```json
|
||||
{ "./reader": "./src/integrations/readers/index.ts" }
|
||||
```
|
||||
|
||||
**6. Wiring: binder returns reader, `bindAll()` threads it to consumers.**
|
||||
|
||||
Feature binders that expose a reader return it:
|
||||
|
||||
```typescript
|
||||
// bindProductionAuth(ctx) returns { reader: IAuthReader }
|
||||
const authResult = bindProductionAuth(ctx);
|
||||
bindProductionBlog(ctx, { authReader: authResult.reader });
|
||||
```
|
||||
|
||||
Consuming binders accept readers as a second parameter alongside `ctx`:
|
||||
|
||||
```typescript
|
||||
export function bindProductionBlog(
|
||||
ctx: BindProductionContext,
|
||||
readers: { authReader: IAuthReader },
|
||||
): void;
|
||||
```
|
||||
|
||||
Ordering in `bindAll()` is explicit — the owning feature binds first, then consumers. A cycle in `bindAll()` is a compile-time error (TypeScript cannot type the return before the call), which enforces rule Q3 structurally.
|
||||
|
||||
**7. Manifest field: `reads: ["<feature>"]` per use case.**
|
||||
|
||||
The feature manifest declares cross-feature read dependencies:
|
||||
|
||||
```typescript
|
||||
useCases: {
|
||||
createArticle: {
|
||||
mutates: true, // this use case mutates its OWN feature's data
|
||||
reads: ["auth"], // this use case queries ANOTHER feature's reader (read-only on auth side)
|
||||
publishes: [],
|
||||
consumes: [],
|
||||
audits: [],
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
Note: `mutates` and `reads` are orthogonal. `mutates` describes whether this use case writes to its own feature's repositories. `reads` describes which other features' readers it queries. A mutating use case can read from another feature's reader — the read-only constraint (Q2) is enforced on the **provider** side (the reader can only wrap non-mutating use cases), not on the consumer side.
|
||||
|
||||
Conformance gates verify:
|
||||
|
||||
- **ESLint rule `no-undeclared-reader`:** Code calls a reader method but manifest doesn't declare `reads`. (Parallel to `no-undeclared-event-publish`.)
|
||||
- **Boot assertion `assertReaderPurity`:** Every use case wired into a reader is declared `mutates: false` in the manifest.
|
||||
- **Boot assertion `assertFeatureConformance`:** Every `reads` entry has a corresponding reader injected into the binder.
|
||||
- **`pnpm conformance`:** Cross-feature reader closure — every `reads: ["auth"]` resolves to a feature that exports `./reader`.
|
||||
|
||||
**8. Read-only enforcement via `ReadOnly<F>` brand.**
|
||||
|
||||
A new branded type prevents mutating use cases from being wired into readers at compile time:
|
||||
|
||||
```typescript
|
||||
type ReadOnly<F> = F & { readonly __readonly: unique symbol };
|
||||
```
|
||||
|
||||
Use cases declared `mutates: false` receive the `ReadOnly` brand at bind time. The reader constructor only accepts `ReadOnly`-branded use cases. Passing a mutating use case produces a TypeScript error.
|
||||
|
||||
The brand is verified at boot time by `assertReaderPurity`, which cross-references the reader's wired use cases against the manifest's `mutates` field. If a `mutates: true` use case is wired into a reader, the app refuses to boot.
|
||||
|
||||
**9. No reader-level instrumentation.**
|
||||
|
||||
Readers delegate to use cases that are already wrapped with `withSpan` and `withCapture` at bind time. Adding reader-level spans would create redundant parent spans for every cross-feature query. Use case spans are sufficient for tracing.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Events for everything (status quo).** Rejected for domain queries — events are async and fire-and-forget. You cannot `await bus.publish("auth.check-role")` and get an answer back. Forcing queries through the event bus would require request-scoped correlation IDs, reply channels, and timeouts — essentially rebuilding synchronous RPC over an async bus.
|
||||
|
||||
- **Direct use-case imports across features.** Rejected — violates vertical isolation. If blog imports `checkUserRoleUseCase` from auth, it takes a transitive dependency on auth's repository interfaces, DI symbols, and internal structure. A change inside auth's use case can break blog's compilation.
|
||||
|
||||
- **Shared query interfaces in `core-shared`.** Rejected — `core-shared` is infrastructure. Putting `IAuthReader` there means core-shared accumulates feature-specific domain types, which inverts the dependency direction (core depends on feature concepts).
|
||||
|
||||
- **A standalone `core-protocols` package.** Rejected as premature — adds a new package for what is currently a type-only export. If the number of readers grows beyond 5-6, this can be reconsidered. For now, the owning feature is the natural home.
|
||||
|
||||
- **Gateways (reader + writer in one interface).** Rejected — synchronous cross-feature writes are dangerous. A failure in the target feature's write path would fail the caller's request. Writes should be fire-and-forget (events) so the caller's request path is not coupled to the target's write availability. See rule Q2.
|
||||
|
||||
- **Bidirectional readers (allowing cycles).** Rejected — cycles indicate wrong feature boundaries. Three resolution strategies exist (UI composition, event for one direction, merge features), making a runtime cycle-breaking mechanism unnecessary. See rule Q3.
|
||||
|
||||
- **Rely solely on Payload `relationTo`.** Rejected as the sole mechanism — `relationTo` gives raw data, not domain-evaluated answers. It also doesn't work in dev-seed/test mode with mock repositories. However, `relationTo` remains the correct choice for raw data joins where no domain logic is needed.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
|
||||
- Verticals can answer synchronous domain queries for other verticals without violating isolation.
|
||||
- The manifest's `reads` field makes cross-feature coupling visible, greppable, and agent-readable — same as `publishes`/`consumes` for events.
|
||||
- Read-only enforcement (`ReadOnly<F>` brand + `assertReaderPurity`) prevents accidental cross-feature mutations.
|
||||
- Cycle detection is structural (compile-time in `bindAll()`) — no runtime checks needed.
|
||||
- No new mock infrastructure — existing use case mocks power the reader in dev-seed; inline vitest mocks suffice for consumer tests.
|
||||
- The pattern is consistent with existing conventions: integration boundary (`integrations/readers/`), public contract + private implementation (rule Q1 parallels E1), manifest declaration + conformance check.
|
||||
|
||||
**Negative:**
|
||||
|
||||
- Adds a fourth cross-feature coupling mechanism (alongside events, jobs, and realtime). Developers and agents must choose correctly. The decision matrix in section 1 mitigates this.
|
||||
- Feature binders that expose readers change their return type (from `void` to `{ reader: I<Feature>Reader }`). `bindAll()` ordering becomes explicit. This is intentional — it makes the dependency graph visible — but it's a change to existing binder signatures.
|
||||
- The `reads` manifest field and `no-undeclared-reader` ESLint rule are new conformance machinery. Implementation cost is bounded (follows the exact pattern of `publishes`/`consumes` + `no-undeclared-event-publish`).
|
||||
- Reader interfaces can grow organically in ways that are hard to audit. Mitigated by the "one reader per feature, grown on demand" rule and the principle that a bloated reader signals a fat vertical.
|
||||
|
||||
## Implementation notes
|
||||
|
||||
- **Generator:** A `pnpm turbo gen reader` generator should be added to scaffold the `integrations/readers/` structure, add the `./reader` export to `package.json`, and create the interface + implementation + test files. Not required for day one — hand-authoring the first reader is acceptable while the pattern stabilizes.
|
||||
- **Existing features:** None of the five template features (auth, blog, media, marketing-pages, navigation) currently need readers. The first reader will be created when a product vertical requires a cross-feature domain query. Auth is the most likely candidate (`IAuthReader` for permission checks).
|
||||
- **`BindContext` is unchanged.** Readers flow as binder-to-binder parameters (via `bindAll()`), not through `ctx`. This keeps `BindContext` focused on infrastructure concerns.
|
||||
- **Payload `relationTo` continues unchanged.** Readers supplement it, they don't replace it. Use `relationTo` for raw data joins; use readers for domain-evaluated queries.
|
||||
|
||||
## Out of scope (deferred)
|
||||
|
||||
1. **ESLint rule `no-undeclared-reader`.** Follows the `no-undeclared-event-publish` pattern. Deferred until the first reader is exercised.
|
||||
2. **Contract evolution / versioning for readers.** Same as event contracts (ADR-015 §deferred-3) — no migration story for breaking reader interface changes yet.
|
||||
|
||||
## Planned
|
||||
|
||||
1. **`pnpm turbo gen reader` generator.** Will scaffold `integrations/readers/` + `./reader` export subpath + interface + implementation + test. Follows the `gen event` Plop pattern with anchor protocol.
|
||||
|
||||
## Related
|
||||
|
||||
- ADR-006 — Vertical feature packages (the isolation model readers operate within)
|
||||
- ADR-008 — Per-feature DI containers (reader wiring uses the same container model)
|
||||
- ADR-010 — Turborepo boundaries (feature → feature type imports are allowed; reader contracts are type-only)
|
||||
- ADR-015 — Cross-feature events and background jobs (readers complement events; rules Q0–Q3 parallel E0/E1/J0)
|
||||
Reference in New Issue
Block a user