Files
atomic-design-poc/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-22.md
T
ehoandClaude Opus 5 7a29f5facc feat(brief): tolerate a 404 on GET /brief with a one-shot reset (RB-22)
BriefStore.load() now treats a 404 from GET /brief as "no brief exists
yet" and calls the existing reset() command once, instead of showing
the generic load-failed error. BriefAdapter.load() gains a
BriefLoadFailure error channel (notFound | error) so the store can
tell a 404 apart from every other failure; every other adapter method
stays on runSubmit, unchanged.

The once-only bound is a field on the store, not a comment: a second
404 (from a later load() call) always falls through to the ordinary
error path, and the recovery path never calls load() again, so no
loop can form.

This is the expand half of CQ-007's split (04-cqrs-light.md). Today's
backend never 404s GET /brief, so the new branch is dead code until
RB-23 (the backend contract half) ships in a later merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 18:43:27 +02:00

12 KiB

RB-22 — BriefStore.load() tolerates a 404, calling reset() exactly once

Status: implemented · 2026-08-27 · Source findings: 04-cqrs-light.md CQ-007 · 99-backlog.md RB-22, "Tickets that were rejected and split" · implementation/rb-17.md (the runResult/runSubmit seam this store already sits on)

This is the expand half of an expand/contract pair. RB-23 (backend: GET /brief 404s when absent; BriefStore.GetOrCreate splits into Get + ResetAndCreate) ships after this ticket, in a later merge. Today's backend never 404s GET /brief, so this ticket's new branch is dead code in the running app — provably backend-frontend-safe by construction.

What was wrong

CQ-007 flags GET /brief (Program.cs:603 → BriefStore.GetOrCreate, Data/BriefStore.cs:50) as the one endpoint in the backend where a GET performs a persisted write, breaking the read/write split every other endpoint respects. The fix is split across both sides of the seam because the FE must be ready to receive a 404 before the backend can safely start sending one. This ticket is the FE half: BriefStore.load() (apps/ssp/src/app/brief/application/brief.store.ts) had no notion of "no brief exists yet" — every adapter failure, 404 included, dispatched BriefLoadFailed and showed the generic error banner. BriefAdapter.load() (brief.adapter.ts) also had no way to tell the store a failure was specifically an HTTP 404: it folded every failure through the shared runResult helper (RB-17), which keeps only a human-readable string and throws away the HTTP status.

The ticket read as filed against the current code: BriefStore.load() is exactly where CQ-007 says it is, BriefAdapter.load() is exactly the read runResult call RB-17 pointed at it, and nothing about either file was factually wrong. Nothing to flag here.

What changed

File Change
apps/ssp/src/app/brief/infrastructure/brief.adapter.ts load()'s error channel becomes BriefLoadFailure ({tag:'notFound'} | {tag:'error', reason:string}) instead of a plain string. load() no longer routes through the shared runResult — it does its own try/catch so it can read the thrown value's HTTP status before folding it away, via the new local isHttpNotFound predicate. Every other method (save/submit/approve/reject/send/reset) is untouched, still on runSubmit.
apps/ssp/src/app/brief/application/brief.store.ts load() branches on BriefLoadFailure: notFound (and not already recovered) calls the existing reset() command directly and applies its result; every other failure (including a repeated notFound) dispatches BriefLoadFailed as before. Extracted applyLoadedView (the success-path body shared by load() and the new recovery path) and added recoverFromMissingBrief.
apps/ssp/src/app/brief/application/brief.store.spec.ts New describe('BriefStore.load — 404 tolerance (RB-22)') with the two required cases. Six pre-existing load: fakes' explicit Result<string, BriefView> return-type annotations updated to Result<BriefLoadFailure, BriefView> (they only ever produce the ok: true branch, so this is a type-only change); two shared ok(v) test helpers that build fakes for both load and save had their return-type annotation dropped in favour of as const inference, since one helper now serves two different error-channel types.
libs/shared/docs/behaviour-spec.mdx Regenerated (npm run gen:behaviour-spec) — picks up the two new it() titles.

No backend/ file was touched — Program.cs and Data/BriefStore.cs are RB-23's, per the ticket's explicit scope.

How the once-only bound is structural

BriefStore gains one field: private hasRecoveredFromMissingBrief = false. load() takes the recovery branch only when r.error.tag === 'notFound' && !this.hasRecoveredFromMissingBrief, and the branch's first statement sets the flag before doing anything else. A second 404 — whether from a second load() call, or in principle from reset() itself somehow also 404ing — falls through to the plain BriefLoadFailed branch instead, on every subsequent call, for the life of the store instance. This is a field on the singleton store, not a comment: nothing in the reachable call graph can flip it back to false.

The loop CQ-007's proposed change warns about ("the reset's own load must not be able to loop") is not merely bounded, it is structurally absent: recoverFromMissingBrief calls this.adapter.reset() and applies its BriefView result directly (the same applyLoadedView the success path uses) — it never calls this.load() again. There is no recursive edge from the recovery path back into load() for the once-only flag to have to stop; the flag exists only to stop a second, separate load() invocation (e.g. a caller retrying navigation) from reaching reset() again.

Judgement calls

  • load() no longer uses runResult, only for this one method. runResult (libs/shared/src/application/submit.ts, RB-17) intentionally keeps only a string — every other read in the app is fine with that. This is the first read that needs one more bit (the HTTP status) than runResult exposes, so load() does its own try/catch instead, matching runResult's shape (problemDetail(e, fallback) on the non-404 path) but adding the 404 branch first. libs/shared/src/application/submit.ts itself is untouched — changing a shared helper used by many call sites for one adapter's need was out of scope and unjustified.
  • 404 detection reads (e as {status?:unknown}).status === 404, not SwaggerException.isSwaggerException. The generated client throws a plain SwaggerException for GET /brief today (no OpenAPI 404 response is declared for it yet), but throws the parsed ProblemDetails object instead for an endpoint whose spec does declare a 404 (both shapes carry a status field). Checking status alone, not the SwaggerException type, means this predicate keeps working unchanged once RB-23 regenerates the client with a documented 404 response for briefGET() — no follow-up FE edit needed for detection to keep working.
  • BriefLoadFailure is a new exported type, not a sentinel string. CLAUDE.md's default reflex is a discriminated union over a second flag; a magic string ('__not_found__') compared by identity would have kept load()'s signature at Result<string, BriefView> and touched fewer test lines, but it is exactly the kind of stringly-typed control flow the union tool exists to avoid. The touched-test cost was six type annotations plus two helper signatures, all in the one already-scoped spec file — judged worth it for the correct shape. BriefLoadFailure is exported.
  • On a repeated 404, the store shows BRIEF_LOAD_FAILED (the same generic banner text load() already used for every other failure), not a distinct "still missing" message. No new user-facing copy was needed or added, so no new $localize id and no messages.en.xlf change — confirmed by diffing for $localize occurrences: both hits in the diff are unchanged context lines, not new additions.
  • resetDemo() (the "start over" button) was left untouched, even though it duplicates part of the same apply-a-fresh-view logic now factored into applyLoadedView. It also manages actionState/saveState/rejectionSnapshot that recoverFromMissingBrief correctly does not touch (an automatic recovery on first load is not a user-initiated "start over" action), and refactoring it was not asked for by this ticket.

Verification

  • Verified red without the fix. Temporarily replaced load()'s body (via Edit, not git checkout) with the pre-fix shape — every failure, notFound included, dispatches BriefLoadFailed straight away, no reset() call — and reran the spec file. Both new tests failed: expected "vi.fn()" to be called 1 times, but got 0 times on reset, for both "a 404 drives exactly one reset()" and "a second 404 does not drive a second reset()"; the other 18 tests in the file stayed green. Restored the real fix with a second Edit and reran: all 20 tests in the file green, 30/30 across both touched spec files.
  • npm run ci (foreground, no background/Monitor): green, exit 0 — lint, typecheck, dep:check (341 + 226 modules, 0 violations), format:check, check:tokens, check:seam, tests (ssp 260/260, behandelportal 37/37, shared 138/138, beheer 23/23 — 458 total), ng build --localize (both apps), npm audit (0 vulnerabilities), backend dotnet test (260/260 — the known OpenZaakIntegrationTests.Admin_cases_… container-dependent failure did not reproduce on this run, matching the standing caveat that it needs a live OpenZaak container and is not this ticket's bug), backend dependency audit clean, gen:snippets drift clean, gen:behaviour-spec drift clean once the regenerated file was staged (the local gate diffs the working tree against the index, so it necessarily shows a diff before the file is staged/committed — the same documented, expected behaviour RB-17 recorded, not a defect).
  • npx prettier --check on every touched file (including the reformatted 99-backlog.md table and the regenerated behaviour-spec.mdx): clean.

What RB-23 must do

Once GET /brief in Program.cs returns a real 404 (no ProblemDetails body is required — BriefAdapter.load()'s isHttpNotFound only reads the HTTP status, not the response body), and BriefStore.GetOrCreate splits into Get (query) + the existing ResetAndCreate (already there, already used by POST /brief/reset), this ticket's notFound branch stops being dead code and starts running on first-ever page load for any owner with no persisted brief. Run npm run gen:api as part of RB-23 so briefGET() regenerates with a documented status === 404 branch throwing the parsed ProblemDetails (matching the shape most other endpoints already use) — the detection predicate here already tolerates that shape and needs no FE follow-up change. Two things worth re-verifying once RB-23 lands, not fixing preemptively here: first, that the resulting double round-trip (404, then reset()) is an acceptable UX cost on a first visit, per CQ-007's own framing of this as its least certain finding; second, that this store's hasRecoveredFromMissingBrief field — private to one store instance, reset only by a full reload — is still the right lifetime for the once-only guard once a real 404 can occur in production traffic, not only in a test's fake adapter.