docs: refactoring-backlog workspace — baseline + 3 Phase 1 agents
Runs the multi-agent refactoring-backlog pipeline in docs/project/ refactor-backlog-setup/ up to and including three of the seven Phase 1 agents. 00-baseline.md establishes the metrics every later agent must cite, using only tooling already in the repo (vitest lcov, coverlet cobertura, ESLint's core `complexity` rule at threshold 0 for a full distribution, depcruise --metrics). Duplication and C# complexity had no tooling, so tools/baseline-scan.mjs adds a deterministic ~200-line text scan rather than a new dependency; the approximations are labelled as such. Headline: FE 75.1% line coverage but only over the 98 of 220 source files a spec loads; BE 97.6% line / 79.6% branch; 0 layering violations; 7.1% duplication; 25 of 2085 TS functions over CC 10. Then 02-testability, 04-cqrs-light and 06-adr-conformance (27 findings). 01/03/05 were skipped deliberately — the baseline shows little for them to find; 07 (BIO2) and 08 (consolidation) are still open. Each agent corrected a baseline observation of mine, and in every case the error was in something derived rather than measured: - BL-007 counted ~13 adapter "mutations" from the `runSubmit` helper name; 5 of those call sites are reads. It also missed 3 real mutations that reach the raw ApiClient and never return a Result. - BL-002 diagnosed the 100%-duplicated auth folders as ADR-0002's divergence prediction failing. It never had a chance to fail: §3's `Principal` union was never built. - BL-004 named libs/shared/domain and libs/beheer/contracts as coverage gaps; both are pure type declarations where 0% is unimprovable. All three corrections are recorded inline in 00-baseline.md §10, so agent 08 does not inherit the bad numbers. .prettierignore excludes the agent prompt directories — reflowing their markdown would edit the prompt text itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,582 @@
|
||||
## Scope: ADR-0001…0006 (all six), checked against apps/ssp, apps/behandelportal, libs/shared, libs/beheer, backend/src/BigRegister.Api, backend/tests, e2e
|
||||
|
||||
## Status: complete
|
||||
|
||||
## Last updated: 2026-08-26
|
||||
|
||||
## Depends on: 00-baseline.md
|
||||
|
||||
## ---
|
||||
|
||||
# 06 — ADR conformance
|
||||
|
||||
Every claim below was verified by reading the code, not by trusting the ADR text or
|
||||
CLAUDE.md. Where the two disagree with each other **and** with reality, that is flagged
|
||||
as an ADR-fix, per CLAUDE.md's own "the docs win — update this file" rule.
|
||||
|
||||
**Headline.** The architecture is in good shape. All six ADRs are substantially
|
||||
conformed to in their **decisions**; there is no case where the codebase has quietly
|
||||
adopted a different architecture. What has decayed is the ADRs' **descriptive prose** —
|
||||
four of the six describe a repo layout, a set of files, or an out-of-scope list that
|
||||
WP-49…WP-75 has since overtaken. Five of the eleven findings are therefore ADR-fixes.
|
||||
|
||||
**On BL-002 (the auth duplication), my verdict up front, because the brief asked for an
|
||||
argument rather than an assertion:** ADR-0002's non-sharing rule **needs no amendment and
|
||||
should not be relaxed**. The prediction has not _failed_; it is _unexecuted_. See
|
||||
ADR-C-004/ADR-C-006 for the reasoning and for what should actually be filed instead.
|
||||
|
||||
---
|
||||
|
||||
## ADR-0001 — BFF-lite endpoints + decision DTOs (Accepted)
|
||||
|
||||
**Core decision: conformed.** The two policy shapes the ADR prescribes are both live and
|
||||
correctly applied.
|
||||
|
||||
- _Decision flag_ — verified server-computed and rendered, never recomputed:
|
||||
`apps/ssp/src/app/brief/application/brief.store.ts:103-108` derives `canEdit`/
|
||||
`canApprove`/`canReject`/`canSend`/`canRevealBigNummer` purely from
|
||||
`BriefState.loaded.decisions`, with an explicit comment at line 31 ("this store never
|
||||
computes them itself"). Same shape in
|
||||
`apps/behandelportal/src/app/behandeling/infrastructure/beoordeling.adapter.ts:87-97`
|
||||
(`decisions.canBesluiten`, rejected at the parse boundary if absent).
|
||||
- _Config value_ — `apps/ssp/src/app/herregistratie/domain/intake.machine.ts:52` takes
|
||||
`scholingThreshold` as a parameter; `SCHOLING_THRESHOLD_DEFAULT` (line 43) survives only
|
||||
as the offline fallback the ADR sanctions, wired through
|
||||
`application/intake-policy.store.ts:18-19`. No hardcoded `1000` is used as authority.
|
||||
- _Parse boundary_ — 30 `parse*` functions (baseline §7). ADR-0001's own "out of scope"
|
||||
item _"Runtime DTO validation on every endpoint (only the dashboard view has it)"_ is
|
||||
substantially discharged.
|
||||
- `libs/shared/src/application/remote-data.ts` + `<app-async>`, and `dep:check`'s
|
||||
`ui-not-infrastructure` rule at 0 violations (baseline §6), keep the "infrastructure is
|
||||
the only network layer" clause enforced — with one carve-out, ADR-C-002.
|
||||
|
||||
### ADR-C-001 — ADR-0001's worked example describes a POC that no longer exists
|
||||
|
||||
- **Type: ADR-fix**
|
||||
- **ADR cited:** ADR-0001, §"Worked example in this POC" — opening sentence _"This POC has
|
||||
no real backend (static mock JSON + fake submit timers), so the 'BFF output' is a static
|
||||
file"_; §"Out of scope here", bullets 1 and 4.
|
||||
- **Evidence (verified):**
|
||||
- There is a real backend: `backend/src/BigRegister.Api/Program.cs` (940 lines, 48
|
||||
endpoint mappings — baseline §2/BL-003).
|
||||
- Every artifact the worked example names by path is gone. `find` returns nothing for
|
||||
`public/mock/dashboard-view.json`, `public/mock/intake-policy.json`, or
|
||||
`src/app/herregistratie/contracts/intake-policy.dto.ts`. `apps/ssp/public/mock/` does
|
||||
not exist. The surviving contract file moved to
|
||||
`apps/ssp/src/app/registratie/contracts/dashboard-view.dto.ts` (WP-67's `src/` →
|
||||
`apps/ssp/src/` move), so §A's three cited paths are all wrong.
|
||||
- Out-of-scope bullet 1 ("runtime DTO validation on **every** endpoint — only the
|
||||
dashboard view has it") is stale: 30 `parse*` boundaries exist. Out-of-scope bullet 4
|
||||
("Real OpenAPI/TypeSpec codegen toolchain") is stale: `npm run gen:api` generates
|
||||
`libs/shared/src/infrastructure/api-client.ts` (2372 lines, NSwag) and CI drift-checks
|
||||
it.
|
||||
- **Baseline citation:** baseline §2 (`Program.cs` 940 lines), §7 pattern inventory
|
||||
(`parse*` trust boundaries = 30; excluded-as-generated api-client 2372 lines), BL-003.
|
||||
- **Proposed resolution:** rewrite §"Worked example" against the shipped system and prune
|
||||
the discharged out-of-scope bullets. No code changes.
|
||||
- **What the amended ADR should say:** the worked example should read as _"implemented
|
||||
against `backend/src/BigRegister.Api`"_, cite
|
||||
`apps/ssp/src/app/registratie/{contracts/dashboard-view.dto.ts,infrastructure/dashboard-view.adapter.ts}`
|
||||
and `GET /api/v1/dashboard-view` / `GET /api/v1/intake/policy` as the endpoints, and
|
||||
reduce §"Out of scope" to the two items still genuinely open (the `BigProfileStore`
|
||||
optimistic-update race, and session persistence / multi-tab sync).
|
||||
- **Blocked code tickets:** none. This is purely descriptive drift; the decision is intact.
|
||||
- **Effort: S**
|
||||
|
||||
### ADR-C-002 — `libs/shared/src/upload/` does network outside `infrastructure/`
|
||||
|
||||
- **Type: code-violates-ADR**
|
||||
- **ADR cited:** ADR-0001, §Decision — the DTO/adapter seam; operationalised in CLAUDE.md
|
||||
§4 as _"`infrastructure/` is the **only** layer that touches the network — the
|
||||
anti-corruption boundary"_.
|
||||
- **Evidence (verified):** `libs/shared/src/upload/upload.adapter.ts` injects `ApiClient`
|
||||
(line 3 import, line 56 `inject(ApiClient)`) and opens a raw `XMLHttpRequest` at line
|
||||
118 — i.e. it is a genuine network adapter — yet sits in a top-level `upload/` folder,
|
||||
not under `libs/shared/src/infrastructure/`. Its sibling `upload.machine.ts` is an
|
||||
Elm-style reducer sitting outside any `domain/` folder (the only one of 9 machines to do
|
||||
so). The exception is **hard-coded into the enforcement itself**:
|
||||
`.dependency-cruiser.base.js:105` reads
|
||||
`from: { pathNot: '/infrastructure/|^libs/shared/src/upload/' }` — the rule is written
|
||||
around the violation rather than the violation being fixed, which is why baseline §6
|
||||
reports 0 violations despite this.
|
||||
- **Baseline citation:** BL-010 (verbatim: the exception "is already encoded rather than
|
||||
resolved"); baseline §7 FE pattern inventory ("Infrastructure adapters (read side): 20
|
||||
**+1 outside an `infrastructure/` folder**"; "Elm-style machines: 9 — 8 under a `domain/`
|
||||
folder; outlier `libs/shared/src/upload/upload.machine.ts`").
|
||||
- **Proposed resolution:** move `upload.adapter.ts` → `libs/shared/src/infrastructure/`,
|
||||
`upload.machine.ts` (+ its spec) → `libs/shared/src/domain/`, and
|
||||
`upload-controller.ts`/`upload-shell.service.ts` → `libs/shared/src/application/`. Then
|
||||
**delete the `^libs/shared/src/upload/` carve-out** from
|
||||
`.dependency-cruiser.base.js:105` — that deletion is the acceptance criterion, since it
|
||||
is what proves the exception is resolved rather than relocated. Note the side benefit:
|
||||
`libs/shared/src/domain` currently has 0% spec reach across 3 files (baseline §3b), and
|
||||
this moves a well-specced machine into it.
|
||||
- **Effort: M** (mechanical move + import updates across 30 dependents — `libs/shared/upload`
|
||||
has Ca 30, baseline §6 — plus the depcruise rule edit)
|
||||
|
||||
### ADR-C-003 — `contracts/` vs the generated client: ADR-0001 and CLAUDE.md §4 no longer agree with the code
|
||||
|
||||
- **Type: ADR-fix**
|
||||
- **ADR cited:** ADR-0001, §"Why DTOs _decouple_ rather than couple" — _"Manage it with
|
||||
**one source of truth** (OpenAPI or TypeSpec) that **generates types for both sides**"_ —
|
||||
read against CLAUDE.md §4's flat rule _"DTO lives in `contracts/`"_.
|
||||
- **Evidence (verified):** only 4 hand-written `contracts/` DTO files exist
|
||||
(`apps/ssp/src/app/registratie/contracts/{brp-address,dashboard-view,duo-diplomas}.dto.ts`,
|
||||
`libs/beheer/src/contracts/stamdata.dto.ts`), against 20 infrastructure adapters, 19 of
|
||||
which import types from `@shared/infrastructure/api-client` directly. Crucially, the
|
||||
surviving hand-written contract **documents its own obsolescence**:
|
||||
`dashboard-view.dto.ts:10-11` says _"In production these types are GENERATED from the
|
||||
OpenAPI/TypeSpec spec (one source of truth for both sides)"_. So the code has reached
|
||||
ADR-0001's stated target state, and the hand-written `contracts/` files are the
|
||||
pre-codegen scaffolding — but CLAUDE.md §4 still states the pre-codegen rule as the
|
||||
standing law, which will mislead the next feature author.
|
||||
- **Baseline citation:** baseline §7 FE pattern inventory — _"`contracts/` DTO files: 4 —
|
||||
most adapters consume NSwag-generated types directly instead"_; agent-brief note "Only 4
|
||||
`contracts/` DTO files exist".
|
||||
- **Proposed resolution:** amend ADR-0001, then correct CLAUDE.md §4 to match (CLAUDE.md's
|
||||
own precedence rule requires this direction).
|
||||
- **What the amended ADR should say:** add a short §"Where the contract lives, after
|
||||
codegen": the generated client (`libs/shared/src/infrastructure/api-client.ts`,
|
||||
regenerated by `npm run gen:api`, drift-checked in CI) **is** the wire contract and the
|
||||
single source of truth; a hand-written `contracts/*.dto.ts` is warranted only where
|
||||
codegen does not reach — a hand-rolled `fetch`/XHR endpoint, or a shape the generator
|
||||
types too loosely — and in either case the hand-written file must still import nothing.
|
||||
The `parse*` trust boundary in `infrastructure/` is **unchanged and still mandatory**
|
||||
regardless of where the type came from: a generated type is a compile-time claim about
|
||||
the wire, not a runtime guarantee. Then decide explicitly whether the 4 survivors stay
|
||||
(they are more precise than the generated shapes) or are retired — and record which.
|
||||
- **Blocked code tickets:** any ticket that would either (a) delete the 4 remaining
|
||||
`contracts/` files or (b) add new hand-written DTOs for already-generated endpoints must
|
||||
wait for this amendment; today CLAUDE.md §4 can be cited to justify both directions.
|
||||
- **Effort: S** (ADR + CLAUDE.md edit; the follow-on code decision is separately sized)
|
||||
|
||||
---
|
||||
|
||||
## ADR-0002 — User groups as actors, not bounded contexts (**Proposed**, amended WP-67)
|
||||
|
||||
This is the ADR with the most divergence, and the one the baseline pointed me at. Three
|
||||
findings. The core modelling decision — contexts drawn by capability, not by who logs in —
|
||||
**is** conformed to: there is no `zorgverlener/` or `behandelaar/` folder anywhere, the
|
||||
contexts are `registratie`/`herregistratie`/`brief`/`behandeling`/`beheer` (capability
|
||||
names), and both apps integrate through one backend aggregate via decision DTOs.
|
||||
|
||||
### ADR-C-004 — the `Principal` union never landed, although actor #2 did
|
||||
|
||||
- **Type: code-violates-ADR**
|
||||
- **ADR cited:** ADR-0002 §3 ("Separate identity from authorization") — _"Model the
|
||||
principal as a **discriminated union** … This replaces the flat `Session` the day a second
|
||||
actor arrives"_; §Consequences — _"The one concrete FE change when actor #2 lands is
|
||||
`Session → Principal` in the `auth` context"_; §"Out of scope" — deferred _"until a second
|
||||
actor is actually introduced"_. Actor #2 was introduced in WP-61 and consolidated in
|
||||
WP-67. **The deferral condition has been met and the change was not made.**
|
||||
- **Evidence (verified):**
|
||||
- `grep -rn "Principal" apps libs --include=*.ts` returns exactly **one** hit, and it is
|
||||
a comment: `libs/shared/src/infrastructure/role.ts:8`. The type does not exist in the
|
||||
frontend.
|
||||
- `apps/behandelportal/src/app/auth/domain/session.ts` is byte-identical to the SSP's and
|
||||
still reads `interface Session { readonly bsn: string; readonly naam: string }` — a
|
||||
Behandelaar carrying a BSN, which §3 names as the precise thing the union exists to make
|
||||
unrepresentable.
|
||||
- The backoffice login is literally the citizen login. `apps/behandelportal/src/app/auth/ui/login.page.ts:31`
|
||||
is `async login(bsn: string)`, renders `intro="Log in op uw persoonlijke BIG-register
|
||||
omgeving."`, and calls `SessionStore.login(bsn)` →
|
||||
`auth/infrastructure/digid.adapter.ts:14`, which returns
|
||||
`ok({ bsn: r.value, naam: 'Dr. A. (Anna) de Vries' })`. A behandelaar logs into the
|
||||
backoffice as a zorgverlener, by DigiD, with a citizen's name in the header.
|
||||
- The medewerker identity that _does_ exist bypasses the auth model entirely:
|
||||
`apps/behandelportal/src/app/auth/infrastructure/medewerker.interceptor.ts:14-19` stamps
|
||||
`X-Medewerker`/`X-Rollen` onto every `/api/v1/` request. It never touches `Session`.
|
||||
- **Baseline citation:** BL-002 (`ssp/auth` 211/211 significant lines duplicated;
|
||||
`session.store.ts` 39 windows, `login-form.component.ts` 35, `login.page.ts` 23) — the
|
||||
100% figure is the _measurement_ of this finding: the files are identical **because** the
|
||||
modelling change that would differentiate them was skipped.
|
||||
- **Proposed resolution:** land `Session → Principal` as ADR-0002 §3 specifies. In
|
||||
`apps/behandelportal`: replace `Session` with the `medewerker` variant, replace
|
||||
`DigidAdapter` with a `MedewerkerAdapter` that resolves `MEDEWERKER_ID` + `currentRollen()`
|
||||
(the values `medewerker.ts` already holds) into a `Principal`, and make `login.page.ts`
|
||||
an SSO-stand-in entry rather than a BSN form. In `apps/ssp`: the `zorgverlener` variant.
|
||||
This makes the two `auth` contexts genuinely differ — which is what ADR-0002 §3 asserted
|
||||
would happen and is the honest resolution of BL-002.
|
||||
- **Effort: M**
|
||||
|
||||
### ADR-C-005 — ADR-0002 is still `Proposed` after two apps shipped against it
|
||||
|
||||
- **Type: ADR-fix**
|
||||
- **ADR cited:** ADR-0002 header, `Status: Proposed · Date: 2026-07-01`.
|
||||
- **Evidence (verified):** `apps/behandelportal` exists with 29 source files and 3 contexts
|
||||
(baseline §2); the ADR has been amended once in-document (§"Amendment (WP-67,
|
||||
2026-08-01)"); its structural rulings are enforced in CI today —
|
||||
`.dependency-cruiser.<app>.js`'s `<app>-no-other-app` and `shared-no-beheer` rules run at
|
||||
`severity: error` with 0 violations. An architectural decision that CI enforces is not
|
||||
"Proposed". The other five ADRs are all `Accepted`, so this is an inconsistency in the
|
||||
ADR set itself, not a deliberate signal.
|
||||
- **Baseline citation:** baseline §7 "ADRs on record" — _"`0002` … (**Proposed**, amended
|
||||
WP-67)"_; baseline §6 (11 `severity: error` rules, 0 violations, 223 modules cruised);
|
||||
baseline §2 (apps/behandelportal: 29 src files, 1 309 lines).
|
||||
- **Proposed resolution:** promote to `Accepted`, dated to WP-67.
|
||||
- **What the amended ADR should say:** `Status: Accepted · Date: 2026-07-01 · Amended
|
||||
2026-08-01 (WP-67)`. Also update §"Out of scope here", which still lists _"Building the
|
||||
Behandeling backoffice application"_ and _"The backend aanvraag status lifecycle +
|
||||
authorization endpoints/DTOs"_ as unbuilt — both shipped (WP-61…67; `AanvraagStatusTag`,
|
||||
`GET /me` capabilities, `Domain/Authz.cs`). The one bullet that stays is real
|
||||
authentication. Leave the `Session → Principal` bullet in scope but re-word it from
|
||||
"deferred until a second actor is introduced" to a stated debt — it is ADR-C-004.
|
||||
- **Blocked code tickets:** none strictly, but agent 08 should surface this **before**
|
||||
ADR-C-004, because ADR-C-004's justification is "the ADR says to do this" and a `Proposed`
|
||||
ADR is weak grounds for a refactor ticket.
|
||||
- **Effort: S**
|
||||
|
||||
### ADR-C-006 — extract the actor-agnostic route guards to `libs/shared` (the part of BL-002 that will never diverge)
|
||||
|
||||
- **Type: code-violates-ADR** — but note carefully: it violates CLAUDE.md §2's
|
||||
"composition over duplication" and the DRY intent, **not** ADR-0002 §3. ADR-0002 §3 is
|
||||
about _identity and login flow_. A route guard is neither.
|
||||
- **ADR cited:** ADR-0002 §3 — the scope of the non-sharing decision is `Principal` and the
|
||||
login flow ("the two groups **authenticate differently**"); §Consequences names
|
||||
`auth.guard.ts` and `session.store.ts` only as the _seams that localise_ the change, not
|
||||
as things that must be duplicated. Read with CLAUDE.md §1's rule that a genuinely
|
||||
cross-app concern belongs in `libs/shared`.
|
||||
- **Evidence (verified):** `diff -ru apps/ssp/src/app/auth apps/behandelportal/src/app/auth`
|
||||
reports **no content differences at all** — 9 of 11 files are byte-identical; the only
|
||||
delta is two _additional_ files in behandelportal (`medewerker.ts`,
|
||||
`medewerker.interceptor.ts`). Within those 9, `auth.guard.ts` is entirely actor-agnostic:
|
||||
`authGuard` (lines 8-12) reads only `SessionStore.isAuthenticated()` and `Router`;
|
||||
`capabilityGuard` (lines 25-33) adds only `AccessStore.can(capability)` and
|
||||
`whenReady()` — and `AccessStore` already lives in `libs/shared/src/application`.
|
||||
Both apps' routes redirect to the same `/login` and `/dashboard` paths
|
||||
(`apps/behandelportal/src/app/app.routes.ts:10,17`). Both app configs already register
|
||||
`{ provide: SESSION_PORT, useExisting: SessionStore }`
|
||||
(`apps/behandelportal/src/app/app.config.ts:65`), so the seam for a shared guard exists
|
||||
today.
|
||||
- **Baseline citation:** BL-002 top clone pairs — `auth.guard.spec.ts` **36 windows** and
|
||||
`auth.guard.ts` **21 windows**, i.e. 57 of the 211 duplicated lines, the single largest
|
||||
block after `session.store.ts`.
|
||||
- **Proposed resolution:** move `authGuard`/`capabilityGuard` + `auth.guard.spec.ts` to
|
||||
`libs/shared/src/application/` (or a `libs/shared/src/routing/`), injecting `SESSION_PORT`
|
||||
instead of the app-local `SessionStore`. One small widening is needed:
|
||||
`libs/shared/src/application/session.port.ts:9-12` currently exposes only
|
||||
`session: Signal<{naam: string} | null>` and `logout()` — add
|
||||
`readonly isAuthenticated: Signal<boolean>` (or have the guard derive it from
|
||||
`session() !== null`, which both `SessionStore`s already do at
|
||||
`session.store.ts:40`). Each app keeps a two-line re-export at `@auth/auth.guard` so its
|
||||
`app.routes.ts` is untouched.
|
||||
- **Explicitly NOT proposed, and why.** No ticket to merge `session.store.ts`,
|
||||
`session.ts`, `digid.adapter.ts`, `login-form.component.ts` or `login.page.ts`, and **no
|
||||
ADR-fix relaxing ADR-0002 §3.** The brief invited me to treat BL-002 as a prediction the
|
||||
code failed to bear out. It is not. Those five files are identical because ADR-C-004 was
|
||||
never executed — the divergence the ADR predicted has in fact already arrived, it just
|
||||
arrived through an orthogonal side door (`medewerkerInterceptor`, a dev-only HTTP header
|
||||
stamp) instead of through the `Principal` seam the ADR designated. Merging them now would
|
||||
cement a citizen DigiD/BSN login as the backoffice's shared login, which is the one
|
||||
outcome ADR-0002 §3 was written to prevent. The correct sequencing is ADR-C-005 (accept
|
||||
the ADR) → ADR-C-004 (land `Principal`) → **re-measure**. My expectation is that
|
||||
post-ADR-C-004 the residual `ssp/auth` ↔ `bhp/auth` duplication drops from 211 lines to
|
||||
under 40 on its own. If ADR-C-004 is still unstarted at the **next backlog cycle**, that
|
||||
is the point at which the ADR-fix conversation becomes legitimate — not now.
|
||||
- **Effort: S**
|
||||
|
||||
### Observation for agent 07 (BIO2), not a ticket here
|
||||
|
||||
`medewerkerInterceptor` is registered **only** inside `isDevMode()`
|
||||
(`apps/behandelportal/src/app/app.config.ts:57-63`). In a production build the backoffice
|
||||
therefore sends no `X-Medewerker`/`X-Rollen` at all, and `StubIdentityProvider` falls
|
||||
through to its zorgverlener default. ADR-0002 §3's "authorization enforced at the backend
|
||||
boundary" holds structurally, but the behandelportal's _identity_ has no non-dev path. The
|
||||
ADR lists real employee SSO as out of scope, so this is not an ADR conformance defect —
|
||||
flagging it because it is the kind of thing a compliance pass should see stated, and
|
||||
because ADR-C-004 is the natural place to close it.
|
||||
|
||||
---
|
||||
|
||||
## ADR-0003 — CIBG Huisstijl (Bootstrap 5.2) as the design system (Accepted)
|
||||
|
||||
**Core decision: conformed.** All five decision points hold. The package is vendored at
|
||||
`public/cibg-huisstijl/` (`css`, `fonts`, `icons`, `images` present, licensed RO/Rijks text
|
||||
fonts absent per point 5); the token bridge is intact at `libs/shared/styles.scss`; atoms
|
||||
emit Bootstrap classes with their `input()` APIs preserved; `check:tokens`
|
||||
(`scripts/check-tokens.sh`) runs in `npm run ci`; the gap register exists at
|
||||
`libs/shared/docs/cibg-gaps.mdx` with 9 `// CIBG-GAP EXTENSION:` markers in code.
|
||||
|
||||
### ADR-C-007 — ADR-0003's file paths and its `app-alert` example are both stale
|
||||
|
||||
- **Type: ADR-fix**
|
||||
- **ADR cited:** ADR-0003 §Decision point 1 (`src/index.html`), point 2 (`src/styles.scss`),
|
||||
point 4 (_"CIBG omits Bootstrap's `.alert` and `.navbar`, so `app-alert` is a small
|
||||
token-styled surface"_), §Consequences (`.storybook/`, `src/docs/cibg-gaps.mdx`).
|
||||
- **Evidence (verified):**
|
||||
- Every path moved in WP-67: `src/styles.scss` → `libs/shared/styles.scss`;
|
||||
`src/index.html` → `apps/ssp/src/index.html` **and** `apps/behandelportal/src/index.html`
|
||||
(two now, not one); `.storybook/` → `.storybook-ssp/` and `.storybook-behandelportal/`;
|
||||
`src/docs/cibg-gaps.mdx` → `libs/shared/docs/cibg-gaps.mdx`.
|
||||
- Point 4's `app-alert` claim is **factually wrong about the current code**.
|
||||
`libs/shared/src/ui/alert/alert.component.ts:13-16` documents itself as a _"Thin wrapper
|
||||
over the vendored `.feedback feedback-*` classes: the design system owns surface + icon"_,
|
||||
and the template (lines 31-37) binds `.feedback-info`/`.feedback-success`/
|
||||
`.feedback-warning`/`.feedback-error`. Its only local CSS is a 3-line flex fix. It is not
|
||||
hand-rolled and carries no `CIBG-GAP` marker — correctly, since it is not a gap.
|
||||
- **CLAUDE.md §2 repeats the same stale claim** verbatim: _"(Where CIBG lacks a class —
|
||||
e.g. `alert` — the atom is a small hand-rolled surface built from the token bridge; see
|
||||
ADR-0003.)"_ Both documents must be corrected, ADR first.
|
||||
- **Baseline citation:** baseline §2 (the monorepo layout the paths must be rewritten
|
||||
against: apps/ssp, apps/behandelportal, libs/shared, libs/beheer); baseline §7 config-seam
|
||||
tokens (`HEADER_NAV_ITEMS`/`HEADER_ADMIN_LINKS`/`DEBUG_PANEL`) — the WP-67 two-app split
|
||||
these paths belong to.
|
||||
- **Proposed resolution:** repoint all five paths; replace the `.alert` example in point 4.
|
||||
- **What the amended ADR should say:** point 4 should keep the principle ("hand-roll what
|
||||
CIBG's build drops, and mark it") but pick a live example — `skeleton` and `spinner` are
|
||||
the cleanest (both in the register, both genuinely absent from the vendored build) — and
|
||||
note that `.alert` was subsequently resolved onto the vendored `.feedback` classes, so
|
||||
it is no longer a gap. CLAUDE.md §2's parenthetical must be corrected in the same diff.
|
||||
- **Blocked code tickets:** none.
|
||||
- **Effort: S**
|
||||
|
||||
### ADR-C-008 — the CIBG gap register is one row behind the markers in code
|
||||
|
||||
- **Type: code-violates-ADR** (the artifact the ADR mandates is incomplete; the fix is a
|
||||
one-row doc edit, not a code change)
|
||||
- **ADR cited:** ADR-0003 §Consequences, final bullet — _"Hand-rolled components (point 4)
|
||||
are tracked in the **CIBG gap register** … every deviation from the design system carries
|
||||
a `// CIBG-GAP EXTENSION:` marker so it's auditable rather than silently drifting."_
|
||||
- **Evidence (verified):** 9 files carry a `CIBG-GAP` marker
|
||||
(`debug-state`, `language-switcher`, `wizard-shell`, `application-link`,
|
||||
`placeholder-chip`, `rich-text-editor`, `skeleton`, `spinner`, `status-badge`); the
|
||||
register table at `libs/shared/docs/cibg-gaps.mdx:26-33` has **8** rows.
|
||||
`libs/shared/src/layout/language-switcher/language-switcher.component.ts:7-9` carries a
|
||||
full, well-formed marker (`"Taal instellen" … no vendored Huisstijl class ships for it …
|
||||
See cibg-gaps.mdx`) and has no corresponding row. The register's own §"Keeping this
|
||||
register honest" concedes there is no automated diff and instructs _"If markers and this
|
||||
table drift, trust the code and fix the table"_ — so this is exactly the drift it
|
||||
predicted, caught by review as designed.
|
||||
- **Baseline citation:** baseline §2 (libs/shared 86 src files / 5 194 lines — the surface
|
||||
the register must cover); baseline §6 (`libs/shared/src/layout` is a cruised module with
|
||||
Ca 22).
|
||||
- **Proposed resolution:** add the `language-switcher` row (CIBG concept: "Taal instellen";
|
||||
reason: no vendored class in this build). Optionally add the CI script the register
|
||||
declines — a ~10-line `grep -l CIBG-GAP | diff` in `scripts/` folded into `check:tokens`
|
||||
would make the drift impossible to reintroduce. I would file the row as the ticket and
|
||||
the script as an explicitly optional second step, matching the register's own
|
||||
proportionality argument.
|
||||
- **Effort: S**
|
||||
|
||||
---
|
||||
|
||||
## ADR-0004 — Stamdata as code (Accepted)
|
||||
|
||||
**Core decision: conformed.** `backend/src/BigRegister.Api/Stamdata/` holds 15 files —
|
||||
typed C# (`Beroep.cs`, `PolicyQuestions.cs`, `Professions.cs`, `StamdataCatalog.cs`,
|
||||
`StamdataTable.cs`, `StamdataFile.cs`) plus the checked-in JSON data-files
|
||||
(`beroepen.json`, `documentconfidentialiteit.json`, `opleidingen.json`,
|
||||
`professions.json`, `specialismen.json`) the WP-29 follow-on introduced. Coverage is 96.8%
|
||||
line (baseline §3c). The `beheer/stamdata` editor is read-only-plus-download, not a write
|
||||
path, exactly as the ADR's own WP-29 note states. There is **no** runtime write endpoint
|
||||
for any stamdata table.
|
||||
|
||||
### ADR-C-009 — feature flags are a second runtime-editable exception the ADR does not acknowledge
|
||||
|
||||
- **Type: ADR-fix**
|
||||
- **ADR cited:** ADR-0004 §Decision — _"Never a production database, never runtime-editable"_
|
||||
— and §"The deliberate exception: org-templates", which names **one** exception in the
|
||||
singular and justifies it narrowly ("specific to one sub-organization's identity").
|
||||
- **Evidence (verified):** `backend/src/BigRegister.Api/Data/FeatureFlagStore.cs` is a
|
||||
second admin-writable SQLite surface, added by WP-47 — after ADR-0004 (2026-07-20) — and
|
||||
**its own doc-comment states the equivalence the ADR does not**: _"Runtime feature-flag
|
||||
state (WP-47). SQLite-backed like `OrgTemplateStore`, same single-gate idiom."_ It exposes
|
||||
`Set(key, enabled)` writing `db.FeatureFlags`, surfaced through the admin page
|
||||
`/beheer/functies` (`apps/behandelportal/src/app/app.routes.ts:44-50`, gated by
|
||||
`capabilityGuard('flags:manage')`).
|
||||
- **Assessment — and why this is an ADR-fix, not a violation.** The design is genuinely
|
||||
ADR-0004-shaped, not a breach of it: the _catalog_ (which flags exist, their descriptions
|
||||
and defaults) is compiled-in code (`Domain/Features/FeatureFlags.Catalog`), only the
|
||||
boolean override persists, and `IsEnabled` fails closed for an unknown key
|
||||
(`FeatureFlagStore.cs:45-47`), so a bad DB row cannot invent a flag. That is the ADR's
|
||||
actual principle — schema and values gated at compile time — applied correctly. What is
|
||||
wrong is the ADR's _text_: it states the rule as a closed list of one exception, which
|
||||
means the next operational-config surface has no principle to test itself against and
|
||||
will either be waved through or blocked on a technicality.
|
||||
- **Baseline citation:** baseline §7 backend pattern inventory — the 7 static stores listed
|
||||
as "Not behind any port", which includes both `OrgTemplateStore` **and**
|
||||
`FeatureFlagStore`; baseline §3c (`backend/Stamdata` 96.8% line / 71.7% branch;
|
||||
`backend/Data` 99.0% / 75.5%).
|
||||
- **Proposed resolution:** amend ADR-0004. No code change; the code is right.
|
||||
- **What the amended ADR should say:** replace §"The deliberate exception: org-templates"
|
||||
with §"The deliberate exception: operational configuration", stating the **test** rather
|
||||
than a list — runtime-editable persistence is permitted only when (1) the catalog/schema
|
||||
of what may be set lives in code, (2) an unknown or unlisted key is rejected/fails closed,
|
||||
(3) the value is operational (per-organisation identity, an on/off rollout switch) and not
|
||||
a shared business rule whose wrong value breaks the register for everyone, and (4) writes
|
||||
are admin-capability-gated and audited. Then list the two surfaces that pass it today —
|
||||
`OrgTemplateStore` (WP-23/26) and `FeatureFlagStore` (WP-47) — and note that both are
|
||||
admin-gated. Add a matching sentence to CLAUDE.md §4, which currently repeats the
|
||||
singular framing ("Org-templates are the deliberate exception").
|
||||
- **Blocked code tickets:** any ticket proposing a third runtime-editable config surface
|
||||
should wait for this test to be written down, rather than arguing by analogy to
|
||||
org-templates.
|
||||
- **Effort: S**
|
||||
|
||||
---
|
||||
|
||||
## ADR-0005 — OpenZaak (ZGW APIs) behind the BFF (Accepted)
|
||||
|
||||
**Conformed, with no findings.** This is the cleanest ADR in the set and I am recording
|
||||
that plainly rather than manufacturing a ticket.
|
||||
|
||||
Verified point by point:
|
||||
|
||||
- The anti-corruption layer is in the .NET BFF and nowhere else. Every ZGW type lives under
|
||||
`backend/src/BigRegister.Api/Zgw/` (8 files: `OpenZaakZaakSource`,
|
||||
`OpenZaakDocumentSource`, `ZgwHttpClient`, `ZgwTokenProvider`, `ZgwZaakMapper`,
|
||||
`ZgwOptions`, `ZgwDiagnosticHandler`, `NotificatieDto`). `grep` for
|
||||
`OpenZaakZaakSource|OpenZaakDocumentSource` outside `Zgw/` returns only the two DI
|
||||
registrations in `Program.cs:67,69` and eight explanatory comments — no ZGW shape reaches
|
||||
a consumer.
|
||||
- The config-switched port pair is exactly as decided: `Program.cs:59-84` reads
|
||||
`Zgw:Enabled` and binds either `OpenZaakZaakSource`/`OpenZaakDocumentSource` (via
|
||||
`AddHttpClient`, with the WP-60 15s timeouts) or `LocalZaakSource`/`LocalDocumentSource`.
|
||||
Default is local, so the POC still runs fully offline as the ADR's second constraint
|
||||
requires.
|
||||
- The FE is untouched by the switch: consumers inject the interface only
|
||||
(`Program.cs:206,281,340,425,434,446,473`), and the same `ApplicationSummaryDto` is
|
||||
returned either way — zero DTO/api-client drift, matching the ADR's first `+`.
|
||||
- The ADR's stated **minus** is still accurate and honestly scoped: `Program.cs:465`
|
||||
confirms _"The local write runs against ApplicationStore directly (not the IZaakSource
|
||||
seam)"_, which is precisely the "only some endpoints have a source interface; each future
|
||||
slice introduces its own seam" consequence the ADR wrote down. An ADR that predicted its
|
||||
own remaining gap and the gap stayed where predicted is conformance, not drift.
|
||||
- `backend/Zgw` is the best-covered backend module at 98.1% line / 85.5% branch (baseline
|
||||
§3c) and the second-lowest duplication (1.8%, baseline §5) — the seam the ADR claimed
|
||||
would be "unit-testable without a live server" measurably is.
|
||||
|
||||
Note for agent 03: BL-006 (no NetArchTest/ArchUnitNET, one assembly) means nothing
|
||||
_enforces_ the "ZGW shapes never leave `Zgw/`" property that this ADR depends on. It holds
|
||||
today by convention. That is agent 03's ticket to size, not mine — I record only that
|
||||
ADR-0005's conformance is currently review-maintained, not CI-maintained.
|
||||
|
||||
---
|
||||
|
||||
## ADR-0006 — Test data through the production door (Accepted)
|
||||
|
||||
**Core decision: partially conformed.** Three of the five mechanisms are properly in place;
|
||||
the frontend replay idiom — the ADR's own flagship — is adopted in one spec out of nine.
|
||||
|
||||
Conformed:
|
||||
|
||||
- §1 backend type-state builder — `backend/tests/BigRegister.Tests/Builders/AanvraagBuilder.cs`
|
||||
exists and is used in 5 test files (`Acceptance/BesluitLifecycleTests.cs`,
|
||||
`Acceptance/IntakeSubmissionTests.cs`, `ApplicationTests.cs`,
|
||||
`Domain/BeoordelingRuleTests.cs`, `OpenZaakZaakSourceTests.cs`) — the exact two files the
|
||||
ADR's Context named as the problem, plus three more.
|
||||
- §4 `RemoteData` named constructors — `libs/shared/src/testing/remote-data.ts` exists.
|
||||
- §5 e2e actors/seed-refs — `e2e/support/actors.ts` exists; no page-object layer was added.
|
||||
- No `'x' as BrandedType` cast appears in any spec (`grep` for
|
||||
`as Postcode|as Uren|as BigNummer|as Bsn` in `*.spec.ts`: zero hits) — the §3 illegal
|
||||
route is genuinely closed.
|
||||
|
||||
### ADR-C-010 — four machine specs hand-roll the exact state literal ADR-0006 §2 forbids
|
||||
|
||||
- **Type: code-violates-ADR**
|
||||
- **ADR cited:** ADR-0006 §2 (_"No object is built directly. A fixture is the result of
|
||||
running real `Msg`s through the real `reduce`"_) and the Decision table's "Pure reducer /
|
||||
state machine (frontend)" row: **Do not** — _"A literal returning `{ tag: 'Editing', ... }`
|
||||
by hand"_; plus §Consequences (_"a hardcoded `errors: {}` fixture literal can't drift from
|
||||
what validation actually produces"_).
|
||||
- **Evidence (verified) — each of these returns a state literal, and three of the four
|
||||
hardcode the `errors: {}` the ADR calls out by name:**
|
||||
- `apps/ssp/src/app/herregistratie/domain/intake.machine.spec.ts:19-25` —
|
||||
`const answering = (...): IntakeState => ({ tag: 'Answering', answers, cursor, errors: {}, scholingThreshold })`.
|
||||
This is the worst instance: `apps/ssp/src/app/herregistratie/domain/intake.testing.ts`
|
||||
exists **in the same folder**, exports `givenIntake`, and is the ADR's own quoted example
|
||||
(§2's code block is `export const givenIntake = given(reduce, initial)`) — yet
|
||||
`givenIntake` is imported only by `intake.acceptance.spec.ts`, never by the machine spec
|
||||
it was written for.
|
||||
- `apps/ssp/src/app/registratie/domain/registratie-wizard.machine.spec.ts:25-31` —
|
||||
`const invullen = (...): RegistratieState => ({ tag: 'Invullen', draft: {...}, cursor, errors: {}, upload: initialUpload })`.
|
||||
- `apps/behandelportal/src/app/behandeling/domain/besluit.machine.spec.ts:5-9` —
|
||||
`const editingWith = (...): BesluitState => ({ tag: 'Editing', draft: {...}, errors: {} })`.
|
||||
- `apps/ssp/src/app/brief/domain/brief.machine.spec.ts:67-74` —
|
||||
`const loaded = (...): BriefState => ({ tag: 'loaded', brief, availablePassages, decisions })`.
|
||||
- For contrast, two specs **do** conform and should be the pattern to copy:
|
||||
`libs/beheer/src/domain/stamdata-editor.machine.spec.ts:17-23` (`seedLoaded()` =
|
||||
`reduce(initial, {tag:'Loaded', …})`) and
|
||||
`apps/ssp/src/app/brief/domain/org-template.machine.spec.ts:29-30`
|
||||
(`reduce({tag:'loading'}, {tag:'DraftLoaded', …})`). Only 1 of 9 machines has a
|
||||
`*.testing.ts`; `change-request.machine.spec.ts:6` honours the idiom but declares
|
||||
`given(reduce, initial)` inline in the spec rather than in a `*.testing.ts`.
|
||||
- **Baseline citation:** baseline §7 FE pattern inventory (Elm-style machines: 9);
|
||||
baseline §3a — `ssp/herregistratie` 70.9% line / 67.8% branch and `ssp/brief` 75.3% /
|
||||
68.8%, i.e. the two modules whose specs hand-roll states are also two of the three
|
||||
weakest-covered non-auth FE modules, which is consistent with fixtures asserting shapes
|
||||
the reducer may not actually produce.
|
||||
- **Proposed resolution:** add `*.testing.ts` next to each of the four machines
|
||||
(`registratie-wizard`, `brief`, `besluit`, plus adopt the existing `intake.testing.ts`),
|
||||
each a one-liner `export const givenX = given(reduce, initial)` per §2, and rewrite the
|
||||
four literal helpers as message replays. Where a state genuinely is not reachable by
|
||||
replay, that is a finding in its own right and should be recorded in the ticket rather
|
||||
than worked around — it means the reducer cannot produce a state the spec asserts.
|
||||
- **Effort: M** (four specs; mechanical but each needs the right message sequence worked out,
|
||||
and `brief.machine.spec.ts` is the largest)
|
||||
|
||||
### ADR-C-011 — `unwrapOk` has zero adopters; its one call site reimplements it inline
|
||||
|
||||
- **Type: code-violates-ADR**
|
||||
- **ADR cited:** ADR-0006 §3 (_"Value objects → `unwrapOk`, never a cast"_) and the Decision
|
||||
table's "Value object / parser" row.
|
||||
- **Evidence (verified):** `unwrapOk` is defined in
|
||||
`libs/shared/src/testing/value-object.ts` and referenced nowhere else in `apps/` or
|
||||
`libs/` except `libs/shared/docs/testing.mdx` — zero spec consumers. The one place that
|
||||
needs it hand-rolls the same three lines:
|
||||
`apps/ssp/src/app/registratie/application/submit-change-request.spec.ts:8-9` reads
|
||||
`const telefoon = parseTelefoonnummer('0612345678'); if (!telefoon.ok) throw new Error('fixture phone should parse');`
|
||||
— semantically identical to `unwrapOk(parseTelefoonnummer('0612345678'))`, which is
|
||||
exactly the duplication the ADR shipped the helper to remove.
|
||||
- **Baseline citation:** BL-004 (122 of 220 FE source files never loaded by any Vitest run
|
||||
— `libs/shared/src/testing` is one of the few modules at 100% reach, so a helper here that
|
||||
nothing imports is measurably dead weight, not merely unexercised); baseline §3a
|
||||
`libs/shared/testing` 3 files, 100% line.
|
||||
- **Proposed resolution:** one-line change in `submit-change-request.spec.ts` to call
|
||||
`unwrapOk`. Then judge honestly whether one call site justifies keeping the helper — if a
|
||||
future ticket finds it still has one consumer, deleting it and keeping the inline guard is
|
||||
the equally valid answer, and ADR-0006 §3's real requirement (never a cast) is satisfied
|
||||
either way. File it as "adopt or delete", not "adopt".
|
||||
- **Effort: S**
|
||||
|
||||
---
|
||||
|
||||
## ADR-fix tickets (require architect approval)
|
||||
|
||||
Agent 08 must surface these five for human sign-off. None of them are code changes; all
|
||||
five change what the repo's architecture documents _claim_, and two of them gate code
|
||||
tickets.
|
||||
|
||||
| ID | ADR | What the amendment does | Gates | Effort |
|
||||
| ------------- | ---- | ------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- | ------ |
|
||||
| **ADR-C-001** | 0001 | Rewrite the worked example against the real backend; drop the two discharged out-of-scope bullets | nothing | S |
|
||||
| **ADR-C-003** | 0001 | State that the generated client **is** the wire contract post-codegen; scope `contracts/` to codegen gaps; correct CLAUDE.md §4 | any ticket deleting the 4 surviving `contracts/*.dto.ts`, or adding a hand-written DTO for a generated endpoint | S |
|
||||
| **ADR-C-005** | 0002 | `Proposed` → `Accepted`; refresh §"Out of scope" (backoffice + status lifecycle + authz DTOs all shipped) | **ADR-C-004** — a `Proposed` ADR is weak grounds for a refactor ticket, so this must land first | S |
|
||||
| **ADR-C-007** | 0003 | Repoint five WP-67-stale paths; replace the false `app-alert` hand-rolled example; correct CLAUDE.md §2 | nothing | S |
|
||||
| **ADR-C-009** | 0004 | Generalise "the org-templates exception" into a stated four-part test; list both passing surfaces; correct CLAUDE.md §4 | any future third runtime-editable config surface | S |
|
||||
|
||||
**Ordering dependency, called out explicitly as the brief requires:** ADR-C-005 → ADR-C-004
|
||||
→ (re-measure BL-002) → possibly-nothing. ADR-C-006 is deliberately **not** in this chain:
|
||||
extracting the actor-agnostic route guards is compatible with ADR-0002 §3 as written and
|
||||
needs no approval. **No ADR-fix is proposed against ADR-0002 §3's non-sharing rule.** I
|
||||
considered it, as instructed, and rejected it: the rule's prediction has not been falsified,
|
||||
it has not been tested, because the change that would test it (ADR-C-004) was never made.
|
||||
Amending an ADR to match code that never executed its decision would ratify the omission
|
||||
rather than the evidence.
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| ADR | Verdict | Findings |
|
||||
| -------- | --------------------------------------------------- | --------------------------------------- |
|
||||
| **0001** | Decision conformed; prose stale | ADR-C-001 (fix), 002 (code), 003 (fix) |
|
||||
| **0002** | Decision conformed; §3 unexecuted; still `Proposed` | ADR-C-004 (code), 005 (fix), 006 (code) |
|
||||
| **0003** | Conformed; two documentation defects | ADR-C-007 (fix), 008 (code/doc) |
|
||||
| **0004** | Conformed; exception clause under-general | ADR-C-009 (fix) |
|
||||
| **0005** | **Fully conformed — no findings** | — |
|
||||
| **0006** | Partially conformed; §2 adopted 1 of 9 | ADR-C-010 (code), 011 (code) |
|
||||
|
||||
Six code tickets (1×M, 1×M, 1×M, 3×S) and five ADR-fixes (all S). No finding proposes
|
||||
introducing a pattern that does not already exist in the repo, and every one cites a
|
||||
baseline observation or metric row.
|
||||
Reference in New Issue
Block a user