ehoandClaude Sonnet 5
ae7781efef
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>
87 lines
4.3 KiB
Markdown
87 lines
4.3 KiB
Markdown
# 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.
|