docs: close WP-72..75, regenerate behaviour spec
CI / changes (push) Successful in 8s
CI / lint (push) Successful in 1m9s
CI / frontend (push) Successful in 2m27s
CI / backend (push) Successful in 1m56s
CI / e2e (push) Successful in 3m16s
CI / semgrep (push) Successful in 1m7s
CI / api-client-drift (push) Successful in 1m50s
CI / storybook-a11y (push) Successful in 11m4s

Four close-outs and their README rows. The behaviour spec is regenerated
once here rather than per-track — it derives from every test name in the
repo, so any track running it would have conflicted with the other three.

Records two findings the arc surfaced but did not cause: the /brief/preview
staleness for non-DemoOwner identities (blocking per-spec identity isolation
in brief-v2.spec.ts), and that WP-72/73 had to share a commit because both
edit Program.cs — separate execution waves prevented build collisions but
did not produce separable diffs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
eho
2026-08-19 16:34:54 +02:00
co-authored by Claude Sonnet 5
parent 6fa27d1c53
commit ae7781efef
6 changed files with 317 additions and 10 deletions
+4
View File
@@ -122,6 +122,10 @@ for its existing violations, so every WP ends green.
| [WP-69](WP-69-intake-scholing-threshold-enforcement.md) | Enforce the scholing threshold server-side | 12 · DDD hardening | done |
| [WP-70](WP-70-test-data-builders.md) | Test-data builders: illegal fixtures unrepresentable (ADR-0006) | 12 · DDD hardening | done |
| [WP-71](WP-71-test-framework-coherence.md) | Test framework coherence: BDD/DDD alignment, close the escape hatches | 12 · DDD hardening | done |
| [WP-72](WP-72-delete-legacy-submit-endpoints.md) | Delete the dead legacy submit endpoints | 12 · DDD hardening | done |
| [WP-73](WP-73-domain-unions.md) | `RegistrationStatus` and `Aanvraag` as closed unions | 12 · DDD hardening | done |
| [WP-74](WP-74-e2e-isolation.md) | E2E isolation without a new backend endpoint | 12 · DDD hardening | done |
| [WP-75](WP-75-fe-be-seam-closure.md) | Close the remaining FE/BE seams | 12 · DDD hardening | done |
Sequencing dependencies (stated in the WPs too): 01 before 1015 (axe covers story churn);
03/04 before 0509 (boundaries stop new violations during refactors); 06 before 07 (typed
@@ -0,0 +1,56 @@
# WP-72 — Delete the dead legacy submit endpoints
Status: done (6bc00a9)
Phase: 12 — DDD hardening
## Why
`POST /api/v1/intakes` and `POST /api/v1/herregistraties` were dead from the UI — the wizard
submits through `POST /applications/{id}/submit`, and nothing in `apps/` or `libs/` called the
generated `intakes()`/`herregistraties()` client methods. They were also strictly **less
capable** than the endpoint that replaced them: they minted a bare reference and wrote no
`Aanvraag`, made no ZGW/OpenZaak call, and performed no document-ownership check.
WP-69 hardened `/intakes` with a 400 last session. Deleting the surface is the stronger fix;
WP-69's `/applications/{id}/submit` enforcement — the path the wizard actually uses — is
untouched.
## Decisions (pre-made)
1. Delete both routes together. Their two `EndpointTests` are `[Theory]`s parameterised across
_both_ routes, so deleting one would leave an `InlineData` row 404-ing.
2. **Keep** the shared `Submit(...)` helper, `ReferentieResponse`, `SubmissionRules.NewReference`
and the whole `IdempotencyStore` path — `/registrations` and `/change-requests` still use
them, and `IdempotencyTests` covers the latter.
3. This WP owns the wire artifacts; no other track runs `gen:api`.
## Acceptance criteria
- [x] Both routes return **404** against a live backend (verified by curl, not by inference).
- [x] Zero references remain in `libs/shared/src/infrastructure/api-client.ts`.
- [x] `gen:api` diff is **pure deletion** — 124 lines out of `swagger.json`, 109 out of the API
client, zero additions.
- [x] Backend tests 245 → 240, exactly the 5 deleted cases (2 `[Theory]`s × 2 rows + 1 `[Fact]`).
- [x] `Submit(...)` and the idempotency path survive with their live callers intact.
## Verification
```bash
cd backend && dotnet test BigRegister.slnx --filter "Category!=Integration"
npm run ci
curl -X POST http://localhost:5000/api/v1/intakes -d '{"uren":500}' # 404
```
## Notes
Committed together with WP-73 (`6bc00a9`): both edit `Program.cs`, and splitting them would
have produced a commit that does not build. The two were run in separate execution waves to
avoid a concurrent `dotnet build` collision — but since neither committed independently, the
file-level entanglement remained at integration time. Worth remembering when planning future
parallel backend tracks: **separate waves do not produce separate commits.**
## Follow-ups
- `docs/reference/fp-tea-atomic-design.md:587` / `ARCHITECTURE.md:464` still teach a
`visibleSteps`-with-a-`'scholing'`-step intake the fixed-3-step wizard no longer matches
(inherited from WP-69).
@@ -0,0 +1,90 @@
# WP-73 — `RegistrationStatus` and `Aanvraag` as closed unions
Status: done (6bc00a9)
Phase: 12 — DDD hardening
## Why
Two backend domain types still allowed illegal states, against `CLAUDE.md`'s non-negotiable #3.
`RegistrationStatus` was a flat record whose **own doc-comment** admitted only `Geregistreerd`
should carry a herregistratie deadline — and noted the frontend modelled it correctly as a
discriminated union while the backend did not. It also made `reden` nullable on all three
variants where the FE requires it on two.
`Aanvraag` was a mutable EF class with 14 public setters. Its `StatusAt` carried **five
`Referentie!` null-forgiving derefs** plus a `SubmittedAt!.Value` — the compiler saying out loud
that "Submitted ⇒ Referentie != null" was convention, not type. WP-68 left it mutable
deliberately; WP-70/71 bought most of the safety with a test-only builder, which was itself a
hand-rolled prototype of the union this WP builds for real.
## Decisions (pre-made)
1. **Full union, not private setters.** The cheaper option (flip 14 setters to `private set`,
3 files, no migration) was rejected in favour of the honest modelling.
2. `RegistrationStatus` → abstract record + three sealed variants behind a private base ctor.
Chosen over WP-68's static-factory shape (`AanvraagStatus`) because with only 4 read sites the
abstract record is affordable and makes **reading** safe too, not just construction.
3. `Aanvraag``Concept | Submitted | Decided` (with `Decided` further split into
`Goedgekeurd | Afgewezen | MeerInfoGevraagd`), the EF row demoted to `AanvraagEntity` behind
a two-way mapper.
4. The `(Owner, Type)` "at most one unsubmitted aanvraag" rule is an **aggregate-set** invariant —
it cannot live on the entity and stays procedural in `CreateConcept` under the lock. Stated in
code so nobody tries to move it.
5. No migration, no schema change, no wire change.
## Acceptance criteria
- [x] **Illegal construction is a compile error, proven not assumed.** Each was attempted, the
compiler error recorded, then reverted:
| Attempted illegal state | Compiler error |
| -------------------------------------- | ------------------------------------------------------------------------------------------------- |
| `Decided` with no referentie | `CS9035: Required member 'Aanvraag.Decided.Referentie' must be set` |
| `Geschorst` with a HerregistratieDatum | `CS1739: The best overload for 'Geschorst' does not have a parameter named 'HerregistratieDatum'` |
| `Afwijzen` with no toelichting | `CS9035: Required member 'Aanvraag.Decided.Afgewezen.Toelichting' must be set` |
- [x] All five `Referentie!` derefs and the `SubmittedAt!.Value` are **gone**, not suppressed.
`IZaakSource.CreateZaak` narrows to `Aanvraag.Submitted`, removing the same class of deref
in both `LocalZaakSource` and `OpenZaakZaakSource`.
- [x] `reden` is now required on `Geschorst`/`Doorgehaald`, matching the FE union.
- [x] `HerregistratieRule.IsStatusConsistent` deleted as dead code — the type now guarantees what
it checked, and its test **could no longer construct the illegal state it existed to
catch**. That failure to compile is the proof the refactor worked.
- [x] Backend 242 → 241, exactly that one deleted test. No other count change.
- [x] `RegistrationStatusDto` and the application DTOs byte-identical — confirmed by diffing a
live backend's `/swagger/v1/swagger.json` against the checked-in copy. No `gen:api`.
## The `Draft` decision (made explicitly)
`ApplicationStore`'s doc-comment claimed `Draft` was "Concept only" (`Draft != null ⇒ !Submitted`),
but `Submit` never cleared it — so the invariant was **violated in production**. Resolved in
favour of the code matching the comment: `Submitted`/`Decided` simply have no `Draft` property,
so submitting drops it. Verified nothing reads a submitted aanvraag's draft — `draft-sync.ts`'s
`applyResume` is the only consumer of `ApplicationDetailDto.Draft` and only ever resumes an
unsubmitted wizard.
## Deviations
- **`Aanvraag` (EF row) renamed to `AanvraagEntity`.** The domain union needed the bare name to
match `RegistrationStatus`/`AanvraagStatus` conventions; keeping both would make every file
importing both namespaces ambiguous (`CS0104`). The table name is unaffected — EF derives it
from the `Applications` `DbSet` property, not the CLR type.
- **Step invariant loosened** from `0 <= StepIndex < StepCount` to `<=`: `CreateConcept` produces
`(0, 0)` before the wizard's first draft sync, which the strict form would reject at creation.
- `AanvraagBuilder.Decided(...)` now delegates to the real union constructors, dropping its own
hand-rolled toelichting guard; a one-line wrapper keeps the `.Decided(...).Build()` chain
source-compatible for existing call sites.
## Verification
```bash
cd backend && dotnet format --verify-no-changes && dotnet test BigRegister.slnx --filter "Category!=Integration"
npm run ci
```
## Follow-ups
- Making `Besluit` flow through the generated client as an enum rather than a `string` would
remove that FE/BE seam entirely rather than guarding it (WP-75 added the guard) — but it is a
wire change.
@@ -0,0 +1,86 @@
# WP-74 — E2E isolation without a new backend endpoint
Status: done (42f7bd6)
Phase: 12 — DDD hardening
## Why
The three Playwright specs shared one mutable backend and admitted it in their own comments
("Restart the backend between CI runs — a second run would see a leftover Concept"). A crashed
mid-wizard run poisoned every subsequent run via `CreateConcept`'s 409, and both mutating specs
acted as the same identity (`DocumentStore.DemoOwner`), so any new state-touching spec would
collide immediately.
## Decisions (pre-made)
**WP-70 recorded the fix as a dev-only seed endpoint. That premise was wrong**, and exploration
established why:
- The DB path already routes through `IConfiguration` (`Program.cs`,
`Db.ConnectionString = GetConnectionString("AppDb") ?? …`), so `ConnectionStrings__AppDb` as an
env var gives a throwaway DB with **zero backend change** — the same trick
`TestWebApplicationFactory` already uses per-test.
- `StubIdentityProvider` **already honours** an `X-Subject` header; the only gap was that no FE
interceptor sent one.
- The backend has **no `IsDevelopment()` gate anywhere** (grep: zero hits), so a seed endpoint
would have had to invent the codebase's first environment gate — a new security posture for no
gain.
So: throwaway DB + a dev-only `X-Subject` interceptor. No new endpoint, no environment gate.
## Acceptance criteria
- [x] **`npm run e2e` passes twice back-to-back with no backend restart** — the actual acceptance
test, and the thing that failed before this WP.
- [x] **`X-Subject` observed on a real request** reaching the backend (`X-Subject: 111222333` on
`GET /api/v1/uploads/categories`), not merely wired.
- [x] Each new BSN elfproef-verified by script against the weights `[9,8,7,6,5,4,3,2,-1]`.
- [x] No backend change, no new endpoint, no `IsDevelopment()` gate.
- [x] Committed port config still defaults to 4200 (verification used an override).
## Notes on the two caveats
- **`reuseExistingServer` stays on.** Flipping it to `false` would hard-fail `npm run e2e` for
anyone already running the docker stack on 4200/5000 — a real local-workflow regression. The
consequence (the throwaway DB only applies when Playwright itself spawns the backend; always
true in CI) is documented in a comment on the `webServer` entry.
- **Unique DB filename per invocation**, with `global-setup.ts` sweeping only _prior_ runs'
leftovers. A fixed name unlinked mid-run is only safe if SQLite's pool never reopens by path
afterwards; under `fullyParallel` that risks silently recreating an empty, unmigrated DB.
## Deviation: interceptors alone were not enough
Two hand-written call sites bypass Angular's interceptor chain (as `CLAUDE.md` documents) and
needed `X-Subject` stamped explicitly:
- `libs/shared/src/upload/upload.adapter.ts`'s raw XHR upload — without this every uploaded
document landed under `DemoOwner`, breaking submit for any other identity.
- `apps/ssp/.../letter-preview.adapter.ts`'s preview fetch (plus `cache: 'no-store'`, correct
regardless since the endpoint sends no `Cache-Control`).
## Known gap (a real backend bug, not caused by this WP)
Under any BSN other than `DemoOwner`, `GET /brief/preview` returns a **sent** letter still
carrying the draft watermark — while `curl` against the same backend at the same instant returns
the correct frozen archive. Client caching was ruled out (`no-store`, then cache-busting query
strings), the dev proxy was ruled out, and it reproduced across two BSNs and never for
`DemoOwner`. This points at a staleness/race in `BriefStore`'s SQLite read path.
`brief-v2.spec.ts` therefore keeps the shared `zorgverlener` identity — it still gains
throwaway-DB repeatability, just not per-spec identity isolation. `actors.ts` reserves a
`briefOpsteller` actor for whoever fixes the backend. **Tracked as a follow-up below.**
## Verification
```bash
npm run e2e # twice consecutively, no backend restart
npm run lint && npm run typecheck && npm test && npm run build
```
Note: port 4200 was held by an unrelated container on the dev machine, so verification ran with
`E2E_BASE_URL` pointed at an alternate port. The committed default is unchanged.
## Follow-ups
- **`/brief/preview` staleness for non-`DemoOwner` identities** (above) — the blocker for giving
`brief-v2.spec.ts` its own identity.
@@ -0,0 +1,65 @@
# WP-75 — Close the remaining FE/BE seams
Status: done (6fa27d1)
Phase: 12 — DDD hardening
## Why
WP-71 added `scripts/check-seam.sh` guarding one literal pair (the scholing threshold) and
documented three further FE/BE duplications that nothing tested across the seam. This closes
them — two by deletion, one by a guard, one by an actual fix.
## Decisions (pre-made)
1. **Dead reference impls get deleted, and `CLAUDE.md` is amended.** This overturns the
documented policy that server-owned rules "stay in `domain/*.policy.ts` as reference impl +
unit test". That policy is precisely what kept dead code alive. Blast radius is small:
`registration.policy.ts` is the only `*.policy.ts` in the repo.
2. Guard the `Besluit` tag list by **extending** `check-seam.sh`, not adding a second script.
3. The phone seam gets a **contract test**, not a grep check — see below.
## Acceptance criteria
- [x] `isHerregistratieEligible` deleted (uncalled; dead by its own doc-comment) along with
`isStatusConsistent` (also uncalled — WP-71 had added a spec for it the session before).
The three live exports (`statusLabel`, `statusColor`, `herregistratieDeadline`) stay, and
`herregistratieDeadline` gained direct coverage it previously only had transitively.
- [x] `CLAUDE.md` amended: server-owned rules live **only** on the server; the FE may mirror a
server-supplied _value_ (a threshold, a bound) for instant feedback, but never
reimplements the _algorithm_. ADR-0001's matching claim aligned.
- [x] `check-seam.sh` guards the `Besluit` tag list, **proven to fail** when a fourth member is
added to the C# enum only, naming both files and both lists. Anchored on the full
declaration so it avoids the "greps all matches" trap WP-69 documented.
- [x] Phone contract test added and green; backend stripping fixed.
## The phone divergence was real, not latent
WP-71 recorded this as latent because the Angular app normalises before sending — true of _that_
path. The contract test proved the two sides genuinely disagreed: the backend returned **422**
for `+31612345678` and `(06) 12345678`, both of which the FE's own `parseTelefoonnummer`
accepts. Any non-Angular client, crafted POST, or future FE change would have hit it.
`SubmissionRules.RejectPhoneChange` now strips exactly what the FE strips (`[\s\-()]`, then a
leading `+31``0`) before applying the shared `^0\d{9}$`. The FE value object was not touched —
it is the more permissive and correct side.
**Why a contract test rather than a grep check:** both sides carry the identical `^0\d{9}$`
literal, so a drift check would have compared them, found them equal, and reported all clear.
The divergence was in the _normalisation before_ the regex — invisible to text comparison. Worth
remembering when choosing between the two guard styles: grep checks catch drifting **constants**,
contract tests catch drifting **behaviour**.
## Verification
```bash
npm run check:seam # both checks OK
npm run ci
cd backend && dotnet test BigRegister.slnx --filter "Category!=Integration"
```
## Follow-ups
- Making `Besluit` flow through the generated client as an enum rather than a `string` would
remove that seam entirely rather than guarding it — a wire change, so not done here.
- The herregistratie-eligibility seam is closed by deletion; if a FE mirror is ever reintroduced,
the disjoint-fixture problem returns and would need a contract test, not a grep check.