From 25a5d415a5d21c3b141f766a2257c4d443930500 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 18:29:05 +0200 Subject: [PATCH] docs(adr): land ADR-C-001, ADR-C-003, ADR-C-007 and ADR-C-009 The architect approved the four ADR-fix tickets. All four change what the architecture documents claim. No code changes. ADR-0001, ADR-C-001: the worked example claimed the POC has no real backend. It rewrites against `backend/src/BigRegister.Api`. Every path it named is repointed. The out-of-scope list drops two discharged bullets: 33 `parse*` boundaries exist, and `npm run gen:api` is real. ADR-0001, ADR-C-003: a new section states that the generated client is the wire contract. A hand-written `contracts/*.dto.ts` is the exception for two cases only. The four survivors stay, because NSwag emits every property as optional and flattens `RegistrationStatusDto` into five optional strings. The `parse*` trust boundary stays mandatory, because a generated type is a compile-time claim about the wire and not a runtime guarantee. ADR-0003, ADR-C-007: four paths moved in WP-67 and are repointed. Point 4 kept the principle and changed its example to `skeleton` and `spinner`. Two of its claims were false and the amendment says so: `app-alert` wraps the vendored `.feedback` classes, and `site-header` composes the vendored `.titlebar`. ADR-0004, ADR-C-009: the exception section states a four-part test instead of one named exception. `OrgTemplateStore` and `FeatureFlagStore` both pass it. RB-07 gated this ticket, because clause 4 needs an audited allow path. RB-07 landed that, so the ADR does not ratify a control that the code lacks. Three tickets need a matching CLAUDE.md correction in the same diff. CLAUDE.md section 2 loses the false `alert` example. Section 4 gets the generated-client rule and the four-part test. Two findings were wrong. ADR-C-001 asked to keep an out-of-scope bullet that reads "SessionStore is in-memory". The session persists to `localStorage` now, so the bullet covers multi-tab sync only. ADR-C-007 flagged one half of point 4 and missed that the other half is equally false. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 24 +++- .../refactor-backlog/99-backlog.md | 8 +- .../refactor-backlog/_status.md | 17 +-- .../implementation/adr-c-001.md | 69 +++++++++++ .../implementation/adr-c-003.md | 64 ++++++++++ .../implementation/adr-c-007.md | 65 ++++++++++ .../implementation/adr-c-009.md | 71 +++++++++++ .../0001-bff-lite-decision-dtos.md | 111 +++++++++++++----- .../architecture/0003-cibg-huisstijl.md | 47 +++++--- .../architecture/0004-stamdata-as-code.md | 50 ++++++-- 10 files changed, 458 insertions(+), 68 deletions(-) create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-001.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-003.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-007.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-009.md diff --git a/CLAUDE.md b/CLAUDE.md index 8bc7e7f..3c7401a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -124,8 +124,10 @@ than hardcoding one app's content — the two apps' primary nav genuinely differ should be **composition of existing blocks** — adding building blocks is the exception, not the default. Atoms are thin wrappers over CIBG Huisstijl (Bootstrap 5.2) CSS classes (`btn`, `form-control`, `card`, …); we own only a small typed `input()` API, -the design system does the visuals. (Where CIBG lacks a class — e.g. `alert` — the atom is a -small hand-rolled surface built from the token bridge; see ADR-0003.) +the design system does the visuals. (Where CIBG lacks a class — e.g. `skeleton`, +`spinner` — the atom is a small hand-rolled surface built from the token bridge and carries a +`// CIBG-GAP EXTENSION:` marker; see ADR-0003. `alert` is **not** such a case: it wraps the +vendored `.feedback feedback-*` classes.) ### 3. State: make illegal states unrepresentable @@ -175,8 +177,14 @@ herregistratie eligibility) or _config value_ (server sends threshold, FE applie for instant feedback, server re-validates as authority — e.g. scholing threshold). FE keeps only **format** validation, never as authority. -DTO lives in `contracts/`; a hand-written `parse*`/`toDomain` in `infrastructure/` -validates the untrusted shape and maps DTO → domain. Wiring a real .NET backend +The generated client +(`libs/shared/src/infrastructure/api-client.ts`, `npm run gen:api`, drift-checked in CI) **is** +the wire contract — consume its types directly, as 19 of the 20 adapters do. A hand-written +`contracts/*.dto.ts` is the exception, only where codegen does not reach the endpoint or types +it too loosely (the four survivors are all the latter — the generator emits every property as +optional and flattens unions); such a file must still import nothing. Either way a hand-written +`parse*`/`toDomain` in `infrastructure/` validates the untrusted shape and maps DTO → domain — +**a generated type is a compile-time claim about the wire, not a runtime guarantee.** Wiring a real .NET backend touches only `infrastructure/` + `contracts/` (see ARCHITECTURE §6). Server-owned rules live **only** on the server, with no FE mirror to drift from it — the FE may mirror a server-supplied _value_ (a threshold, a bound) for instant feedback, but @@ -185,8 +193,12 @@ never reimplements the _algorithm_. **Business-tunable reference data ("stamdata") is config-as-code, not a DB.** Tables the business controls (profession↔diploma map, thresholds, policy-question text) live as typed C# in `backend/.../Stamdata/`, validated at build by `StamdataValidationTests` (a bad edit -fails CI, never prod) — never runtime-editable. Org-templates are the deliberate exception -(operational per-org config in SQLite). UI copy is `$localize`. See ADR-0004. +fails CI, never prod) — never runtime-editable. Operational configuration is the deliberate +exception, and ADR-0004 states it as a four-part test rather than a list: the catalog lives in +code, an unknown key fails closed, the value is operational rather than a shared business rule, +and writes are admin-capability-gated **and** audited. Two surfaces pass it today — +`OrgTemplateStore` (per-org letterhead) and `FeatureFlagStore` (rollout switches), both in +SQLite. A third surface must pass the same test, not argue by analogy. UI copy is `$localize`. See ADR-0004. ### 5. Testing diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md index fcd230b..e8ba3e0 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md @@ -251,10 +251,10 @@ diff** (CLAUDE.md's own precedence rule: "the docs win — update this file"). | ID | ADR | What the amendment does | Gates / blocks | CLAUDE.md edit? | Effort | Compliance | Status | | ------------- | ---- | -------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------- | ------ | ------------ | -------- | -| **ADR-C-001** | 0001 | Rewrite the worked example against the real backend; drop the 2 discharged out-of-scope bullets (every path it names no longer exists) | nothing | no | S | — | pending | -| **ADR-C-003** | 0001 | State that the generated client **is** the wire contract post-codegen; scope `contracts/` to codegen gaps | any ticket deleting the 4 surviving `contracts/*.dto.ts`, or adding a hand-written DTO for a generated endpoint. **No open ticket below is blocked today** — recorded so a future one is. | **yes (§4)** | S | — | pending | -| **ADR-C-007** | 0003 | Repoint 5 WP-67-stale paths; replace the **factually false** `app-alert` hand-rolled example (it wraps vendored `.feedback` classes) | nothing | **yes (§2)** | S | — | pending | -| **ADR-C-009** | 0004 | Generalise "the org-templates exception" into a stated four-part test; list both passing surfaces | **RB-07.** Clause (4) is "writes are admin-capability-gated **and** audited". Today they are gated and _not_ audited — sign this before RB-07 and the ADR ratifies a control the code does not implement. | **yes (§4)** | S | **SIGN-OFF** | pending | +| **ADR-C-001** | 0001 | Rewrite the worked example against the real backend; drop the 2 discharged out-of-scope bullets (every path it names no longer exists) | nothing | no | S | — | **done** | +| **ADR-C-003** | 0001 | State that the generated client **is** the wire contract post-codegen; scope `contracts/` to codegen gaps | any ticket deleting the 4 surviving `contracts/*.dto.ts`, or adding a hand-written DTO for a generated endpoint. **No open ticket below is blocked today** — recorded so a future one is. | **yes (§4)** | S | — | **done** | +| **ADR-C-007** | 0003 | Repoint 5 WP-67-stale paths; replace the **factually false** `app-alert` hand-rolled example (it wraps vendored `.feedback` classes) | nothing | **yes (§2)** | S | — | **done** | +| **ADR-C-009** | 0004 | Generalise "the org-templates exception" into a stated four-part test; list both passing surfaces | **RB-07.** Clause (4) is "writes are admin-capability-gated **and** audited". Today they are gated and _not_ audited — sign this before RB-07 and the ADR ratifies a control the code does not implement. | **yes (§4)** | S | **SIGN-OFF** | **done** | | **ADR-C-005** | 0002 | _(already landed — see "Already done")_ | was the gate on RB-13; now cleared | — | — | — | **done** | **No ADR-fix is proposed against ADR-0002 §3's non-sharing rule.** Agent 06 considered it diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md index 4bc5e83..e7cb9f0 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md @@ -14,14 +14,15 @@ ## Phase 3 — implementation -| CD batch | Tickets | Status | Notes | -| -------- | ---------------------------------------- | ------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | RB-01, RB-02, RB-03, RB-04, RB-05, RB-06 | **complete** | Six commits on `refactor/adr-c-006-shared-route-guards`, one per ticket, each with `implementation/rb-0N.md`. `npm run ci` green. Every ticket left a test that was verified red without its fix. Carryover: RB-01's residual belongs to **RB-09** (the content endpoint is reached by a plain browser navigation with no identity header — BIO-002); `Pii.MaskTail` now lives in `Domain/People/Pii.cs`, **use it in RB-11** rather than hand-rolling a second masker; RB-06 additionally deleted `SubmissionRules.RejectRegistratie` (judgement call, recorded). | -| 2 | RB-07, RB-08, RB-09, RB-10, RB-11 | **complete** | All five merged; `npm run ci` green **on the fixed gate** (see below). **RB-07** unblocks signing **ADR-C-009** and closes CQ-004's outstanding half. **RB-09** makes Production throw at startup when no real `IIdentityProvider` exists — note an environment that is neither Development nor Production (e.g. `Staging`) still fails fast, but at `GetRequiredService`, with a worse message. **RB-10** landed `parseStoredSession` twice, once per app, deliberately — TE-001/BL-002 say extract-to-shared contradicts ADR-0002; **RB-13** differentiates them. **RB-11** corrected a factual error in BIO-012 (the proefbrief error mapping was inlined, not already a separate function) and left the step-up as a literal moved one layer up to its only caller — BIO-006(c) stays a production gate. **RB-01's residual is still open** and is not solved by RB-09: the upload-content link is a plain browser navigation with no credential. | -| 3 | RB-12, RB-13, RB-14, RB-15, RB-16, RB-17 | **complete** | All six merged; `npm run ci` green (14 steps — RB-14 added one — backend 260/260). **RB-12 rejected the ticket's binary framing:** of 47 routes only 16 use one of the five admin wrappers; of the remaining 31 only 10 are genuinely public, the other 21 are ownership-scoped inline (`ctx.Zorgverlener()`/`ctx.Caller()`) or use another mechanism. The allow-list therefore carries **a reason per route**, not a blanket "public" label. Known limitation: detection is `.Gate("XAdmin")` metadata declared at mapping time — **a declaration, not a derivation**, so it cannot catch a route that declares a gate it does not have. **This is RB-19's safety net; read `rb-12.md` before starting RB-19.** **RB-13** measured `ssp/auth` ↔ `bhp/auth` duplication at **32 lines each side, down from 168** (backlog expected <40); each app holds only its own `Principal` variant, which is ADR-C-004's own proposed resolution, and ADR-0002's "Known debt" section became an amendment. **RB-14** could not be built as written — `dotnet list package --vulnerable` exits 0 on a High advisory (verified), so a bare `- run:` would have been a gate that enforces nothing; `scripts/dotnet-audit.sh` matches the output instead and is shared by `ci.yml` and `ci-local.sh`. **RB-15** used a third environment name (`Staging`) in its test, since RB-09 makes Production fail to boot at all. | | -| 4 | RB-18..RB-23 | not started | RB-19 is the only **High**-risk ticket; it needs RB-12's route-table test first. | -| 5 | RB-24..RB-30 | not started | RB-25/26/27 all depend on RB-24. | -| 6 | RB-31, RB-32, RB-33 | not started | | +| CD batch | Tickets | Status | Notes | +| -------- | ------------------------------------------ | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | RB-01, RB-02, RB-03, RB-04, RB-05, RB-06 | **complete** | Six commits on `refactor/adr-c-006-shared-route-guards`, one per ticket, each with `implementation/rb-0N.md`. `npm run ci` green. Every ticket left a test that was verified red without its fix. Carryover: RB-01's residual belongs to **RB-09** (the content endpoint is reached by a plain browser navigation with no identity header — BIO-002); `Pii.MaskTail` now lives in `Domain/People/Pii.cs`, **use it in RB-11** rather than hand-rolling a second masker; RB-06 additionally deleted `SubmissionRules.RejectRegistratie` (judgement call, recorded). | +| 2 | RB-07, RB-08, RB-09, RB-10, RB-11 | **complete** | All five merged; `npm run ci` green **on the fixed gate** (see below). **RB-07** unblocks signing **ADR-C-009** and closes CQ-004's outstanding half. **RB-09** makes Production throw at startup when no real `IIdentityProvider` exists — note an environment that is neither Development nor Production (e.g. `Staging`) still fails fast, but at `GetRequiredService`, with a worse message. **RB-10** landed `parseStoredSession` twice, once per app, deliberately — TE-001/BL-002 say extract-to-shared contradicts ADR-0002; **RB-13** differentiates them. **RB-11** corrected a factual error in BIO-012 (the proefbrief error mapping was inlined, not already a separate function) and left the step-up as a literal moved one layer up to its only caller — BIO-006(c) stays a production gate. **RB-01's residual is still open** and is not solved by RB-09: the upload-content link is a plain browser navigation with no credential. | +| 3 | RB-12, RB-13, RB-14, RB-15, RB-16, RB-17 | **complete** | All six merged; `npm run ci` green (14 steps — RB-14 added one — backend 260/260). **RB-12 rejected the ticket's binary framing:** of 47 routes only 16 use one of the five admin wrappers; of the remaining 31 only 10 are genuinely public, the other 21 are ownership-scoped inline (`ctx.Zorgverlener()`/`ctx.Caller()`) or use another mechanism. The allow-list therefore carries **a reason per route**, not a blanket "public" label. Known limitation: detection is `.Gate("XAdmin")` metadata declared at mapping time — **a declaration, not a derivation**, so it cannot catch a route that declares a gate it does not have. **This is RB-19's safety net; read `rb-12.md` before starting RB-19.** **RB-13** measured `ssp/auth` ↔ `bhp/auth` duplication at **32 lines each side, down from 168** (backlog expected <40); each app holds only its own `Principal` variant, which is ADR-C-004's own proposed resolution, and ADR-0002's "Known debt" section became an amendment. **RB-14** could not be built as written — `dotnet list package --vulnerable` exits 0 on a High advisory (verified), so a bare `- run:` would have been a gate that enforces nothing; `scripts/dotnet-audit.sh` matches the output instead and is shared by `ci.yml` and `ci-local.sh`. **RB-15** used a third environment name (`Staging`) in its test, since RB-09 makes Production fail to boot at all. | | +| 4 | RB-18..RB-23 | in progress | Split into three waves to keep the merge order honest, because three of the six tickets touch `Program.cs`. **Wave A (dispatched, parallel):** RB-18, RB-20, RB-21, RB-22 — no file overlap between them. **Wave B:** RB-23, which must merge after RB-22 (expand/contract pair: the FE must tolerate the 404 before the BE returns it). **Wave C:** RB-19 alone and last — it is the only **High**-risk ticket, it reorders all 48 endpoints in `Program.cs`, and landing it last means it reorders the final content instead of conflicting with RB-18's and RB-23's edits to the same file. RB-19 also needs RB-12's route-table test as its safety net; read `rb-12.md` first, including its stated limitation that detection is a declaration, not a derivation. | +| 5 | RB-24..RB-30 | not started | RB-25/26/27 all depend on RB-24. | +| 6 | RB-31, RB-32, RB-33 | not started | | +| ADR-fix | ADR-C-001, ADR-C-003, ADR-C-007, ADR-C-009 | **complete** | All four signed and landed by the architect on 2026-08-27, in one commit; doc-only, no code touched. Three carried the mandatory matching `CLAUDE.md` edit in the same diff (§4 twice, §2 once). **ADR-C-009's RB-07 gate was satisfied first** — all four clauses of its new test were verified against both `OrgTemplateStore` and `FeatureFlagStore` before signing, so the ADR does not ratify a control the code lacks. **Two findings were wrong and are corrected in the notes:** ADR-C-001 told us to keep an out-of-scope bullet reading "`SessionStore` is in-memory", which RB-10/RB-13 made false (the session now persists to `localStorage`; only multi-tab sync is still open), and ADR-C-007 flagged only the `.alert` half of ADR-0003's point 4 — its "header/side-nav use `.nav` + a local blue bar" clause is equally false (`site-header` composes the vendored `.titlebar`/`.logo__*`). ADR-C-007 also over-listed one path: `public/cibg-huisstijl/` never moved. ADR-C-003's open question was decided explicitly — **the 4 hand-written `contracts/*.dto.ts` stay**, because NSwag emits every property optional and flattens `RegistrationStatusDto` into five optional strings, which would make an illegal state representable (CLAUDE.md §3). Gates released: ADR-C-003 (contracts cleanup) and ADR-C-009 (a third runtime-editable surface). Still pending, untouched: **ADR-C-008 → RB-32** — 9 `CIBG-GAP` markers vs 8 register rows, missing row is `language-switcher`. | **Standing caveat for every batch:** `dotnet test` reports one failure, `OpenZaakIntegrationTests.Admin_cases_returns_the_seeded_zaak_mapped_through_real_HTTP_and_JWT`, diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-001.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-001.md new file mode 100644 index 0000000..ef3c39a --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-001.md @@ -0,0 +1,69 @@ +# ADR-C-001 — rewrite ADR-0001's worked example against the shipped system + +Status: **implemented** · 2026-08-27 · Source finding: `06-adr-conformance.md` §ADR-C-001 + +## What was wrong + +ADR-0001's §"Worked example in this POC" opened with _"This POC has no real backend (static +mock JSON + fake submit timers), so the 'BFF output' is a static file"_. That premise is +false and every path the section cited was gone. The decision itself was intact; only the +description had drifted. + +## What changed + +| File | Change | +| -------------------------------------------------------- | --------------------------------------------------------------------------- | +| `docs/reference/architecture/0001-...md` §Worked example | rewritten against `backend/src/BigRegister.Api`; all six paths repointed | +| same file, §Out of scope here | 4 bullets → 2, plus a paragraph recording which two were discharged and why | + +No code changed. No CLAUDE.md edit was required for this finding. + +## Paths corrected, each verified + +| Claimed | Actual | +| -------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | +| "no real backend … static file" | `backend/src/BigRegister.Api`, `var api = app.MapGroup("/api/v1")` at `Program.cs:168` | +| `public/mock/dashboard-view.json` | `GET /api/v1/dashboard-view` (`Program.cs:172`) | +| `public/mock/intake-policy.json` | `GET /api/v1/intake/policy` (`Program.cs:193`) | +| `src/app/registratie/contracts/dashboard-view.dto.ts` | `apps/ssp/src/app/registratie/contracts/dashboard-view.dto.ts` | +| `src/app/registratie/infrastructure/dashboard-view.adapter.ts` | `apps/ssp/.../infrastructure/dashboard-view.adapter.ts`, `parseDashboardView` at `:50` | +| `src/app/herregistratie/contracts/intake-policy.dto.ts` | **deleted** — the DTO is now the generated `IntakePolicyDto`; the adapter is `apps/ssp/src/app/herregistratie/infrastructure/intake-policy.adapter.ts` | + +`apps/ssp/public/mock/` does not exist (`ls`: no such directory). + +## The finding was wrong about one out-of-scope bullet + +ADR-C-001 said to _"reduce §Out of scope to the two items still genuinely open (the +`BigProfileStore` optimistic-update race, and session persistence / multi-tab sync)"_, +carrying the original bullet's parenthetical **"`SessionStore` is in-memory"**. That +parenthetical is no longer true, so the bullet could not be kept verbatim. + +- `apps/ssp/src/app/auth/application/session.store.ts:13` reads + `parseStoredPrincipal(localStorage.getItem(STORAGE_KEY))`, and `:41` writes it back. + Session persistence **has landed** (RB-10 extracted the parser, RB-13 renamed it + `parseStoredPrincipal`). The file even carries a `ponytail:` note explaining the choice of + `localStorage` over `sessionStorage`. +- Multi-tab sync has **not** landed: `grep` for a `storage` event listener across `apps` and + `libs` returns nothing. + +The bullet was therefore narrowed to multi-tab sync only, and states that the session itself +now persists. Recording this because the finding, taken literally, would have re-asserted a +false claim in the same edit that removed two others. + +The other two survivors were verified rather than assumed: `BigProfileStore` still holds +`pending` as a bare `signal(false)` with `begin`/`confirm`/`rollback` mutating it +(`big-profile.store.ts:61-74`), so the concurrent-submit race is real. + +## Discharged bullets, both verified + +- _"Runtime DTO validation on **every** endpoint (only the dashboard view has it)"_ — 33 + distinct `export function parse*` boundary functions exist across `apps` and `libs`. +- _"Real OpenAPI/TypeSpec codegen toolchain"_ — `npm run gen:api` (`package.json:12`) runs + `dotnet swagger tofile` then `nswag run`, emitting + `libs/shared/src/infrastructure/api-client.ts` (2329 lines). CI's `api-client-drift` job + regenerates and runs `git diff --exit-code` (`.github/workflows/ci.yml:319-321`). + +## Scope discipline + +Descriptive drift only, as the finding states. The decision, the options table, the two +policy shapes and the migration sequence are untouched. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-003.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-003.md new file mode 100644 index 0000000..c1731df --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-003.md @@ -0,0 +1,64 @@ +# ADR-C-003 — state that the generated client is the wire contract + +Status: **implemented** · 2026-08-27 · Source finding: `06-adr-conformance.md` §ADR-C-003 + +## What was wrong + +ADR-0001 set "one source of truth that generates types for both sides" as the target state. +The code reached it. CLAUDE.md §4 still stated the pre-codegen rule — _"DTO lives in +`contracts/`"_ — as standing law, so §4 could be cited to justify both deleting the four +survivors and adding new hand-written DTOs for already-generated endpoints. + +## What changed + +| File | Change | +| ---------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| `docs/reference/architecture/0001-...md` | **new** §"Where the contract lives, after codegen" | +| `CLAUDE.md` §4 | the flat "DTO lives in `contracts/`" rule replaced with the generated-client rule + the two exceptions | + +No code changed. Per CLAUDE.md's own precedence rule, the ADR was amended first and +CLAUDE.md corrected to match, in one diff. + +## The decision the finding asked for: the four survivors stay + +ADR-C-003 required an explicit, recorded decision on the four remaining hand-written DTOs. +**They stay**, all four under exception case 2 ("the generator types the shape too loosely"). +This is not a preference — adopting the generated shapes would violate CLAUDE.md §3. + +Evidence. NSwag emits every property as optional, and flattens a discriminated union into a +bag of optional fields: + +| | generated (`api-client.ts`) | hand-written (`dashboard-view.dto.ts`) | +| ----------------------- | ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------- | +| `DashboardViewDto` | `registration?`, `person?`, `decisions?` — all optional (`:2017`) | all three required | +| `RegistrationDto` | six optional fields (`:2202`) | six required fields | +| `RegistrationStatusDto` | **one flat record of five optional strings**, `tag?: string` (`:2211`) | a real union of three variants, `tag: 'Geregistreerd' \| 'Geschorst' \| 'Doorgehaald'`, per-variant fields required | + +The generated `RegistrationStatusDto` makes `{ tag: 'Geregistreerd', doorgehaaldOp: '…' }` +representable. That is precisely the illegal state CLAUDE.md §3 exists to forbid, and the +`parse*` boundary would have to reconstruct the union by hand anyway. + +The ADR therefore records that retiring these four is **not** a cleanup to schedule. It +becomes correct only if the backend annotates its DTOs so the generator emits required +properties and real unions — which names the actual prerequisite instead of leaving the +question open. + +## Verified counts, not carried over from the finding + +- Hand-written `contracts/*.dto.ts`: **4** — + `apps/ssp/src/app/registratie/contracts/{brp-address,dashboard-view,duo-diplomas}.dto.ts` + and `libs/beheer/src/contracts/stamdata.dto.ts`. +- All four duplicate generated types **by the same names**: `BrpAddressDto` (`:1997`), + `DashboardViewDto` (`:2017`), `DuoLookupDto` (`:2056`), `DuoDiplomaDto` (`:2047`), + `PolicyQuestionDto` (`:2168`), `ManualDiplomaPolicyDto` (`:2106`), `StamdataColumnDto` + (`:2246`), `StamdataTableDto` (`:2253`), `StamdataTableSummaryDto` (`:2261`). None is a + codegen gap — the finding's "case 1" has no occupant today, which is worth knowing. +- The `parse*` boundary is restated as mandatory regardless of type provenance. The amendment + says why in one line: a generated type is a compile-time claim about the wire, not a + runtime guarantee. + +## Gate released + +ADR-C-003 blocked any ticket that would delete the four `contracts/*.dto.ts` files or add a +hand-written DTO for a generated endpoint. No open ticket needed it. The rule is now written +down, so a future one can be judged against it rather than against a stale §4. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-007.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-007.md new file mode 100644 index 0000000..a3f2a3c --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-007.md @@ -0,0 +1,65 @@ +# ADR-C-007 — repoint ADR-0003's WP-67 paths and fix its point 4 + +Status: **implemented** · 2026-08-27 · Source finding: `06-adr-conformance.md` §ADR-C-007 + +## What was wrong + +Two separate defects in one ADR. Every file path in ADR-0003 predated WP-67's monorepo move, +and decision point 4 made a claim about `app-alert` that the code contradicts. + +## What changed + +| File | Change | +| ---------------------------------------- | ---------------------------------------------------------------------------------- | +| `docs/reference/architecture/0003-...md` | points 1, 2, 4 and both §Consequences bullets rewritten | +| `CLAUDE.md` §2 | the `alert` parenthetical corrected to `skeleton`/`spinner` + a denial for `alert` | + +No code changed. ADR first, CLAUDE.md to match, one diff. + +## Paths, each verified + +| Claimed | Actual | +| ------------------------ | ---------------------------------------------------------------------- | +| `src/styles.scss` | `libs/shared/styles.scss` — one copy, both apps' `angular.json:41,169` | +| `src/index.html` | `apps/ssp/src/index.html` **and** `apps/behandelportal/src/index.html` | +| `.storybook/` | `.storybook-ssp/` and `.storybook-behandelportal/` | +| `src/docs/cibg-gaps.mdx` | `libs/shared/docs/cibg-gaps.mdx` | + +**One path in the finding's list needed no change.** ADR-C-007 implied point 1's +`public/cibg-huisstijl/` had moved with the rest. It has not: `public/` is still at the repo +root, and both apps' `angular.json` asset entries read `"input": "public"` (`:38`, `:166`). +Both Storybook configs serve it as `staticDirs: ['../public']`. Point 1's vendoring path is +left as written; only its `index.html` clause changed. + +## Point 4: the finding was right, and understated + +ADR-C-007 flagged the `.alert` half of point 4. Verified: `libs/shared/src/ui/alert/alert.component.ts` +documents itself as a _"Thin wrapper over the vendored `.feedback feedback-*` classes"_, its +template binds `.feedback-info/-success/-warning/-error`, its only local CSS is a 3-line flex +fix, and it carries **no** `CIBG-GAP EXTENSION` marker. `grep` confirms `feedback-error` is +present in `public/cibg-huisstijl/css/huisstijl.css` — the class is vendored, so `alert` is not +a gap. + +**The finding missed that the same sentence's second claim is also false.** Point 4 said "the +header/side-nav use `.nav` + a local blue bar". They do not: + +- `site-header.component.ts` composes the vendored `.titlebar` and `.logo__*` classes + (`grep` confirms `titlebar` in the vendored CSS) and its own comment says the titlebar + _"keeps its own robijn fill — `--ro-layout` — untouched"_. +- `shell.component.ts` emits only `.layout`, `.main`, `.content`, `.skip` — page scaffolding. +- No `.nav` class appears in either, and neither carries a gap marker. + +Both corrections are stated in the amended point 4 rather than silently dropped, so a reader +comparing the old text against the code can see which claim was retired and why. + +## Replacement example chosen + +`skeleton` and `spinner`, as the finding proposed. Both are in the gap register, both carry +markers reading "No loading-skeleton/spinner class in the vendored build", and both are +genuinely absent — the cleanest live illustration of the principle point 4 exists to state. + +## Noted, not fixed: the gap register is still one row short + +`grep` finds **9** `CIBG-GAP EXTENSION` markers; `libs/shared/docs/cibg-gaps.mdx` has **8** +rows. The missing one is `language-switcher`. That is **ADR-C-008 → RB-32** (batch 6), not +this ticket, and it was left alone. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-009.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-009.md new file mode 100644 index 0000000..9ed51ca --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-009.md @@ -0,0 +1,71 @@ +# ADR-C-009 — state the runtime-editable-config exception as a test, not a list + +Status: **implemented** · 2026-08-27 · Source finding: `06-adr-conformance.md` §ADR-C-009 +· Gated on: **RB-07** (satisfied — batch 2) + +## What was wrong + +ADR-0004 said "never runtime-editable" and then named **one** exception in the singular, +justified narrowly ("specific to one sub-organization's identity"). WP-47 added a second +runtime-editable SQLite surface, `FeatureFlagStore`, whose own doc-comment states the +equivalence the ADR did not: _"SQLite-backed like `OrgTemplateStore`, same single-gate +idiom."_ + +The code is right; the ADR's text was wrong. A closed list of one leaves the next +operational-config surface with no principle to test itself against. + +## What changed + +| File | Change | +| ---------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `docs/reference/architecture/0004-...md` | §"The deliberate exception: org-templates" → §"The deliberate exception: operational configuration" — a four-part test plus a table of the two passing surfaces | +| same file, §Context + the table | `src/locale/*.xlf` → `apps//src/locale/*.xlf` (two apps since WP-67) | +| `CLAUDE.md` §4 | the singular "Org-templates are the deliberate exception" replaced with the four-part test | + +No code changed — the finding says so outright, and verification confirmed it. + +## Why the RB-07 gate was real, verified clause by clause + +Clause (4) of the test is "writes are admin-capability-gated **and** audited". Signing this +ADR before RB-07 would have ratified a control the code did not implement. RB-07 has landed, +so the clause is now true. Read at `backend/src/BigRegister.Api/Program.cs:863-923`: each of +the five gates now computes `var ok = …`, calls `AuditAuthz(ctx, capability, resource, ok, +principal)` with the **real** boolean, and only then branches. `FlagsAdmin`'s own comment +names this ticket: _"this is the surface CQ-004/ADR-C-009 hinge on."_ + +All four clauses were checked against both surfaces rather than assumed: + +| Clause | `OrgTemplateStore` | `FeatureFlagStore` | +| ------------------- | ----------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------- | +| (1) catalog in code | the `OrgTemplateDto` shape + `OrgTemplateRules` validate before save (`OrgTemplateStore.cs:49`) | `FeatureFlags.Catalog` (`Domain/Features/FeatureFlags.cs:15`) | +| (2) fails closed | unknown `subOrgId` → `null` → endpoint 404s (`:44-45,:55-56,:72-73,:94-96`) | `Set` returns false for an unlisted key (`:54`); `IsEnabled` returns false (`:42-43`) | +| (3) operational | one sub-organisation's letterhead | an on/off rollout switch | +| (4) gated + audited | `OrgAdmin` → `orgtemplate:edit` (`Program.cs:863`) | `FlagsAdmin` → `flags:manage` (`Program.cs:914`) | + +`FeatureFlagStore`'s own comment states clause (1) and (2) explicitly: _"The CATALOG … is +code … this store only holds the admin's on/off overrides. An unknown key is never +writable/enabled — the code catalog is the authority."_ + +## Judgement calls + +- **Clause (2) is about the write/enable path, not every read.** `OrgTemplateStore` has a + deliberate read-path fallback for briefs from before WP-23 (`:110-114`, its own `ponytail:` + comment): an empty `SubOrgId` falls back to the first seeded sub-org rather than failing a + whole screen. That is a preview convenience on a read; the four write entry points all + return `null` for an unknown sub-org. The clause is worded "cannot invent a setting, + enable a feature, or be written" so this read fallback is not caught by it. Recorded + because a reader checking clause (2) against `OrgTemplateStore.cs` will meet that + fallback first. +- **Org-templates' publish/rollback versioning is mentioned but excluded from the test.** It + is stronger than the test requires, and making it a fifth clause would block a legitimate + flag-style surface that has nothing to version. +- **The stale `src/locale/*.xlf` paths were fixed in the same diff**, though ADR-C-009 did + not flag them. They are two occurrences of the same WP-67 drift ADR-C-001 and ADR-C-007 + exist to correct, in the section being edited, and leaving a known-false path in a document + while amending it is the exact failure mode those two findings describe. Scope creep is + two words wide here; the alternative is filing a third ticket for it. + +## Gate released + +ADR-C-009 blocked "any ticket proposing a third runtime-editable config surface". Such a +ticket can now be judged against a written test rather than by analogy to org-templates. diff --git a/docs/reference/architecture/0001-bff-lite-decision-dtos.md b/docs/reference/architecture/0001-bff-lite-decision-dtos.md index 7e18872..7d68c69 100644 --- a/docs/reference/architecture/0001-bff-lite-decision-dtos.md +++ b/docs/reference/architecture/0001-bff-lite-decision-dtos.md @@ -69,44 +69,96 @@ the governance/transparency artifact. The frontend keeps only **format** validation (postcode shape, integer parsing) for instant feedback — never as the authority. +### Where the contract lives, after codegen + +The paragraph above says "manage it with one source of truth that generates types for +both sides". That target state has arrived, so this section states which artifact is now +the contract. + +**The generated client is the wire contract.** `libs/shared/src/infrastructure/api-client.ts` +is regenerated from the backend's OpenAPI document by `npm run gen:api`, and CI fails on +drift (the `api-client-drift` job regenerates it and runs `git diff --exit-code`). It is the +single source of truth for the shape of every endpoint. An adapter consumes its types +directly; 19 of the 20 infrastructure adapters do. + +**A hand-written `contracts/*.dto.ts` is the exception, for two cases only:** + +1. **Codegen does not reach the endpoint** — a hand-rolled `fetch`/XHR path that the + generator never sees. +2. **The generator types the shape too loosely** — the generated type compiles but is + weaker than the wire really is. + +In either case the hand-written file must still import nothing. It describes the wire, not +the domain. + +**The `parse*` trust boundary is unchanged and stays mandatory**, whichever way the type +arrived. A generated type is a compile-time claim about the wire, not a runtime guarantee: +the server can send anything. `infrastructure/` validates the untrusted shape and maps it +onto the domain, exactly as before. + +**The four surviving hand-written contracts stay.** They are +`apps/ssp/src/app/registratie/contracts/{brp-address,dashboard-view,duo-diplomas}.dto.ts` +and `libs/beheer/src/contracts/stamdata.dto.ts`. All four fall under case 2, and the +dashboard view shows why: the generator emits every property as optional, and it flattens +a discriminated union into a bag of optional fields. + +```ts +// generated — every field optional, `tag` a bare string, all variants merged +interface RegistrationStatusDto { + tag?: string | undefined; + herregistratieDatum?: string | undefined; + geschorstTot?: string | undefined; + reden?: string | undefined; + doorgehaaldOp?: string | undefined; +} + +// hand-written — a real discriminated union, per-variant fields required +type RegistrationStatusDto = + | { tag: 'Geregistreerd'; herregistratieDatum: string } + | { tag: 'Geschorst'; geschorstTot: string; reden: string } + | { tag: 'Doorgehaald'; doorgehaaldOp: string; reden: string }; +``` + +Adopting the generated shape here would push `undefined` handling into every consumer and +make an illegal state representable, which CLAUDE.md §3 forbids. Retiring these four is +therefore **not** a cleanup to schedule; it becomes correct only if the backend annotates +its DTOs so the generator emits required properties and real unions. + ## Worked example in this POC -This POC has no real backend (static mock JSON + fake submit timers), so the -"BFF output" is a static file; the `decisions` block stands in for what the backend -would compute. Two slices were implemented to demonstrate **both** policy shapes: +Implemented against the real backend, `backend/src/BigRegister.Api`. Two slices demonstrate +**both** policy shapes. **A. Dashboard profile → one aggregated, decision-enriched call (decision-flag).** -- Contract: `src/app/registratie/contracts/dashboard-view.dto.ts` +- Endpoint: `GET /api/v1/dashboard-view` (`Program.cs`), one call replacing three. +- Contract: `apps/ssp/src/app/registratie/contracts/dashboard-view.dto.ts` (`DashboardViewDto` = registration + person + `decisions`). -- Endpoint: `public/mock/dashboard-view.json` (one call replaces three). - Boundary parse: `parseDashboardView()` in - `src/app/registratie/infrastructure/dashboard-view.adapter.ts` validates the - untrusted shape and maps DTO → domain (hand-written; no schema lib for one - contract). -- `BigProfileStore` now derives `profile` and `decisions` from the single - validated view (was a 3-resource `map2`). One request → one consistent snapshot. -- `herregistratie.page.ts` reads `decisions.eligibleForHerregistratie` instead of - computing it client-side. That rule is server-owned: it lives only in - `HerregistratieRule.cs`, with no FE mirror to drift from it (WP-75). -- The unused upstream adapters/mocks (`brp.adapter.ts`, `registration.json`, - `brp.json`) were deleted — those calls live behind the BFF now. + `apps/ssp/src/app/registratie/infrastructure/dashboard-view.adapter.ts` validates the + untrusted shape and maps DTO → domain (hand-written; no schema lib). +- `BigProfileStore` derives `profile` and `decisions` from the single validated view (was a + 3-resource `map2`). One request → one consistent snapshot. +- `herregistratie.page.ts` reads `decisions.eligibleForHerregistratie` instead of computing + it client-side. That rule is server-owned: it lives only in `HerregistratieRule.cs`, with + no FE mirror to drift from it (WP-75). **B. Intake scholing threshold → config value.** -- Contract: `src/app/herregistratie/contracts/intake-policy.dto.ts`. -- Endpoint: `public/mock/intake-policy.json` (`{ "scholingThreshold": 1000 }`). +- Endpoint: `GET /api/v1/intake/policy` (`Program.cs`), serving + `IntakePolicy.ScholingThreshold`. +- Contract: the generated `IntakePolicyDto`; the adapter is + `apps/ssp/src/app/herregistratie/infrastructure/intake-policy.adapter.ts`. - `intake.machine.ts`: the hardcoded `LAGE_UREN_DREMPEL` constant is gone; - `lageUren(a, scholingThreshold)` and validation take the value, which lives in - machine state and is set via a `SetPolicy` message. A `SCHOLING_THRESHOLD_DEFAULT` - remains only as the offline fallback. + `lageUren(a, scholingThreshold)` and validation take the value, which lives in machine + state and is set via a `SetPolicy` message. A `SCHOLING_THRESHOLD_DEFAULT` remains only + as the offline fallback. - `intake-wizard.component.ts` fetches the policy and dispatches `SetPolicy`. - WP-69: the backend re-validates the threshold as the authority on submit — `IntakePolicy.RejectIncompleteScholing` runs before `POST /applications/{id}/submit` - (intake-typed) writes anything, 400ing an incomplete scholing answer instead of - silently accepting a crafted POST that skips it. (WP-72 deleted the legacy - `POST /intakes` endpoint this once also covered — deleting the surface is a stronger - fix than 400ing on it.) + (intake-typed) writes anything, 400ing an incomplete scholing answer instead of silently + accepting a crafted POST that skips it. (WP-72 deleted the legacy `POST /intakes` endpoint + this once also covered — deleting the surface is a stronger fix than 400ing on it.) ## Migration sequence (for the real app) @@ -119,12 +171,17 @@ would compute. Two slices were implemented to demonstrate **both** policy shapes ## Out of scope here (next steps, not built in the worked example) -- Runtime DTO validation on **every** endpoint (only the dashboard view has it). - Optimistic-update race fix in `BigProfileStore` (`beginHerregistratie`/`rollbackHerregistratie` can leave `pending` wrong under concurrent submits). -- Session persistence / multi-tab sync (`SessionStore` is in-memory). -- Real OpenAPI/TypeSpec codegen toolchain. +- Multi-tab session sync. The session itself now persists (`localStorage`, read back + through `parseStoredPrincipal`), but a change in one tab does not reach another — no + `storage` listener exists. + +Two bullets were discharged and removed. Runtime DTO validation is no longer "only the +dashboard view": 33 `parse*` boundary functions exist. The OpenAPI codegen toolchain is +real: `npm run gen:api` generates `libs/shared/src/infrastructure/api-client.ts` and CI +drift-checks it. ponytail: build the pattern once on one slice; copy it across screens when the real backend lands, rather than scaffolding all of it up front. diff --git a/docs/reference/architecture/0003-cibg-huisstijl.md b/docs/reference/architecture/0003-cibg-huisstijl.md index 78edb54..8ddc4d5 100644 --- a/docs/reference/architecture/0003-cibg-huisstijl.md +++ b/docs/reference/architecture/0003-cibg-huisstijl.md @@ -19,28 +19,47 @@ layer — not a palette swap. ## Decision 1. **Vendor the package** under `public/cibg-huisstijl/` (not an npm dep — it was delivered as files), - loaded via a `` in `src/index.html` so the CSS's relative `url(../fonts|icons|images)` - references resolve at runtime. Storybook serves the same via `staticDirs`. -2. **Token bridge over token rewrite.** `src/styles.scss` redefines the app's ~54 `--rhc-*` tokens + loaded via a `` in each app's `index.html` (`apps/ssp/src/index.html` and + `apps/behandelportal/src/index.html` — two since WP-67) so the CSS's relative + `url(../fonts|icons|images)` references resolve at runtime. `public/` stays at the repo + root and both apps' `angular.json` targets copy it. Both Storybook instances serve the + same via `staticDirs: ['../public']`. +2. **Token bridge over token rewrite.** `libs/shared/styles.scss` — one copy, both apps' + `angular.json` point at it (WP-67) — redefines the app's ~54 `--rhc-*` tokens onto CIBG values (`--bs-*` where one exists, CIBG palette hex otherwise). The `--rhc-*` names are now an internal alias set; the _values_ are CIBG. This avoided rewriting 300+ token references and - keeps the "components reference tokens" convention intact. (`styles.scss` is exempt from - `check:tokens`, so palette hex lives in that one file only.) + keeps the "components reference tokens" convention intact. (`libs/shared/styles.scss` is + exempt from `check:tokens`, so palette hex lives in that one file only.) 3. **Re-skin atoms, keep their `input()` APIs.** Each `shared/ui` atom now emits Bootstrap/CIBG classes (`app-button` → `btn btn-primary`, `text-input` → `form-control`, radio/checkbox → `form-check-*`); domain pages compose the same atoms and barely changed. -4. **Hand-roll what CIBG's build drops.** CIBG omits Bootstrap's `.alert` and `.navbar`, so `app-alert` - is a small token-styled surface and the header/side-nav use `.nav` + a local blue bar. Local class - names that collide with Bootstrap components were renamed (`.card` → `.app-card`, badge → `.status-badge`). +4. **Hand-roll what CIBG's build drops, and mark it.** Where the vendored build has no class for a + concept, the component is a small token-styled surface carrying a `// CIBG-GAP EXTENSION:` marker. + The clearest live examples are `skeleton` and `spinner`: CIBG documents "Laadindicatie" but the + vendored build ships no loading-skeleton or loading-spinner class, so both are built from the token + bridge. Local class names that collide with Bootstrap components were renamed (`.card` → `.app-card`, + badge → `.status-badge`). + + Two claims this point used to make were wrong and are corrected here. **`.alert` is not a gap:** + `app-alert` is a thin wrapper over the vendored `.feedback feedback-*` classes — the design system + owns surface and icon, and the component adds only the icon's a11y label and a flex fix. It carries + no gap marker, correctly. **The header is not hand-rolled either:** `site-header` composes the + vendored `.titlebar` and `.logo__*` classes and leaves the robijn fill (`--ro-layout`) untouched. + The `shell` template's `.layout`/`.main`/`.content` classes are page scaffolding, not a substitute + for a missing design-system component, so they carry no marker either. + 5. **System-font stack; no licensed fonts.** `--bs-font-sans-serif` is overridden to `system-ui`; the licensed RO/Rijks **text** woffs are removed from the vendored copy (CIBG icon font kept). Logo stays a text wordmark. Interactivity stays Angular-driven (no Bootstrap JS). ## Consequences -- Wiring the design system touches `styles.scss` (token bridge), `index.html`, `angular.json` - (`public/` already copied), and `.storybook/` — plus the class strings in ~40 `shared/ui` + - `shared/layout` + a few domain components. The `@rijkshuisstijl-community/*` deps are dropped. +- Wiring the design system touches `libs/shared/styles.scss` (token bridge), both apps' + `index.html`, both `angular.json` targets (`public/` already copied), and both Storybook config + dirs (`.storybook-ssp/` and `.storybook-behandelportal/` — separate since WP-67, because a single + merged tsconfig cannot resolve both apps' `@auth/*` at once) — plus the class strings in ~40 + `libs/shared/ui` + `libs/shared/layout` + a few domain components. The + `@rijkshuisstijl-community/*` deps are dropped. - `check:tokens` still guards raw hex in components; the token bridge + hand-rolled surfaces comply. - Known benign build warning: _"Unable to locate stylesheet: /cibg-huisstijl/css/huisstijl.min.css"_ — Angular's index optimizer doesn't process a `public/` stylesheet at build time. The asset is copied @@ -49,6 +68,6 @@ layer — not a palette swap. intentionally dropped, so we accept the warning. - Renaming the internal token names from `--rhc-*` to `--app-*` is possible later but out of scope. - Hand-rolled components (point 4) are tracked in the **CIBG gap register** - (`src/docs/cibg-gaps.mdx`, Storybook "Foundations/CIBG Gap Register"): every deviation from the - design system carries a `// CIBG-GAP EXTENSION:` marker so it's auditable rather than silently - drifting. + (`libs/shared/docs/cibg-gaps.mdx`, Storybook "Foundations/CIBG Gap Register"): every deviation + from the design system carries a `// CIBG-GAP EXTENSION:` marker so it's auditable rather than + silently drifting. diff --git a/docs/reference/architecture/0004-stamdata-as-code.md b/docs/reference/architecture/0004-stamdata-as-code.md index 1e7d362..1a05976 100644 --- a/docs/reference/architecture/0004-stamdata-as-code.md +++ b/docs/reference/architecture/0004-stamdata-as-code.md @@ -20,7 +20,7 @@ was neither isolated nor validated: - All reference data and thresholds are **compiled-in C# constants**, served through screen-shaped BFF-lite endpoints; the frontend renders decisions and holds no reference data (ADR-0001). -- User-facing UI copy is already **`$localize`** (`src/locale/*.xlf`) — git-tracked, and a +- User-facing UI copy is already **`$localize`** (`apps//src/locale/*.xlf`) — git-tracked, and a second locale is a translation file, not a code change. That is already the compile-time model for text. - The profession↔diploma map lived as a _private_ `Dictionary` inside `DiplomaRules`, mixed @@ -62,17 +62,49 @@ production database, never runtime-editable. | Kind | Home | Gate | | ------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | | Reference tables + tunable numbers (professions↔diplomas, thresholds, policy questions, document categories) | `Stamdata/` typed C# **or** typed JSON data-file (`professions.json`), optionally valid-timed | compiler (shape; + values when C#) + `StamdataValidationTests` (values, references, validity windows) | -| User-facing UI copy | `$localize` → `src/locale/*.xlf` | build (`i18nMissingTranslation: error`) | +| User-facing UI copy | `$localize` → `apps//src/locale/*.xlf` | build (`i18nMissingTranslation: error`) | | Letter / brief passage content | config-as-code in the backend (seed content), **not** the DB | compiler + endpoint tests | -### The deliberate exception: org-templates +### The deliberate exception: operational configuration -Per-organization letterhead (return address, footer, signature, margins) **is** -runtime-editable in SQLite, via the org-template admin editor (WP-23/26). That is -intentional and does not contradict this ADR: it is _operational configuration_ owned by an -admin persona, versioned with publish/rollback inside the app, and specific to one -sub-organization's identity — not the shared business rules a wrong value would break for -everyone. Stamdata (the rules and reference tables the whole register runs on) stays code. +"Never runtime-editable" above is the rule for **stamdata** — the shared reference tables +and business rules the whole register runs on. It is not a ban on all persisted +configuration. Some configuration is operational rather than business-rule, and belongs to +an admin persona at runtime. + +This section states the **test** rather than a list, so the next surface can check itself +instead of arguing by analogy. Runtime-editable persistence is permitted only when all four +hold: + +1. **The catalog lives in code.** What may be set — the keys, the schema, the defaults, + the descriptions — is compiled in and reviewed through git. The store holds values, never + the definition of what a value means. +2. **An unknown or unlisted key fails closed.** A row the code catalog does not know cannot + invent a setting, enable a feature, or be written. A bad row is inert, not authoritative. +3. **The value is operational.** Per-organisation identity, or an on/off rollout switch — + not a shared business rule whose wrong value breaks the register for everyone. This is the + clause that keeps stamdata out. +4. **Writes are admin-capability-gated and audited.** The write path goes through an `Authz` + capability gate, and the gate records the decision — allow as well as deny — in + `AuthzAuditStore`. + +**Two surfaces pass this test today.** + +| Surface | (1) catalog in code | (2) fails closed | (3) operational | (4) gated + audited | +| ----------------------------- | ----------------------------------------------- | ------------------------------------------------------------ | --------------------------------- | ------------------------------- | +| `OrgTemplateStore` (WP-23/26) | the `OrgTemplateDto` shape + `OrgTemplateRules` | unknown `subOrgId` → `null` → the endpoint 404s | one sub-organisation's letterhead | `OrgAdmin` → `orgtemplate:edit` | +| `FeatureFlagStore` (WP-47) | `Domain/Features/FeatureFlags.Catalog` | unknown key → `Set` returns false (404); `IsEnabled` → false | an on/off rollout switch | `FlagsAdmin` → `flags:manage` | + +Clause (4) became true for both only with RB-07, which moved `AuditAuthz` from each gate's +deny branch into the gate itself so the allow path is recorded too. Before that, both +surfaces were gated and **not** audited, and this ADR would have ratified a control the code +did not implement. + +Org-templates also carry publish/rollback versioning inside the app, which is stronger than +the test requires but not part of it. + +Stamdata itself — the rules and reference tables — fails clause (3) by construction and +stays code. ## Consequences