eho/atomic-design-poc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(backlog): CD batch 6 complete, close the refactor backlog arc

Browse Source 
All three tickets RB-31 to RB-33 merged, one commit per ticket. RB-31 found a
genuine ADR-0006 violation: two registratie-wizard tests asserted a state the
real reducer cannot produce. RB-32 closed ADR-0003's own predicted failure
mode with a permanent CI drift guard rather than a one-time fix. RB-33 chose
deletion over adoption for an unused test helper, since manufacturing a first
caller would have removed no real duplication.

This closes the CD implementation phase. All 33 code tickets and the four
gated ADR-fixes are merged; npm run ci is green after every merge in this
arc, each verified independently rather than trusting an agent's own report.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
eho
2026-08-28 13:33:09 +02:00
co-authored by Claude Opus 5
parent 3441dd4c4e
commit c30d5ec5a5
1 changed files with 5 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/project/refactor-backlog-setup/refactor-backlog/_status.md
+5 -1
View File
@@ -14,6 +14,10 @@
## Phase 3 — implementation
**All six batches complete, 2026-08-28. All 33 code tickets (RB-01..RB-33) and the four gated ADR-fixes (ADR-C-001, ADR-C-003, ADR-C-007, ADR-C-009) merged to `refactor/adr-c-006-shared-route-guards`, one commit per ticket. `npm run ci` green after every merge, verified independently before trusting any agent's own report.** Every batch carrying a **SIGN-OFF** ticket shipped only after the architect approval recorded 2026-08-27 (HALT lifted). Three of the four ADR-fixes required a matching CLAUDE.md correction (§2 once, §4 twice); all three landed in the same diff as their ADR amendment, per CLAUDE.md's own precedence rule.
Across the six batches, several tickets turned out to be factually wrong, incomplete, or overstated relative to the actual code, and every one was reported rather than silently patched over — among them: two ADR-fixes (ADR-C-001's stale out-of-scope bullet, ADR-C-007 catching only half of its own finding), RB-18 (real scope one endpoint, not the several the stale line numbers implied), RB-23 (an unmentioned second `GetOrCreate` call site forced onto the same contract change), RB-25 (overstated which methods the missing token actually blocked), RB-27 (correctly declined to extract abort-vs-error into a signature that cannot express it, and declined an optional move outside its stated scope), RB-28 (TE-006's "cannot test the success case" claim was already false when written), and RB-31 (found a genuinely unreachable state — cursor 2 with no diploma chosen — that the old hand-rolled fixture had been asserting). None of these weakened a ticket; each was implemented correctly once the discrepancy was named.
| 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). |
@@ -21,7 +25,7 @@
| 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 | **complete** | All six merged, one commit per ticket, each on its own merge. `npm run ci` green on the combined tree after every merge (14 steps, exit 0). Ran as three waves, because three of the six touch `Program.cs`: **A** = RB-18/20/21/22 in parallel (no file overlap), **B** = RB-23 after RB-22 (expand/contract), **C** = RB-19 alone and last, so it reordered final content. **Two tickets were incomplete, both reported rather than worked around.** RB-23 found `BriefStore.GetOrCreate` had a **second, unmentioned call site** — `GET /brief/preview` — so the split forced that endpoint to change too or the file would not compile; it got the same `Get` + 404 treatment. RB-18's real scope is **one** endpoint, not the nine BIO-018's stale line numbers implied: `Submit` has exactly one call site (`POST /change-requests`). **RB-22 deliberately left the `runResult` idiom** for `BriefAdapter.load()`: it hand-rolls try/catch to read the HTTP status, because `runResult` folds the error to a string and structurally cannot carry a 404. It still reuses the shared `problemDetail` mapper and models the outcome as the `BriefLoadFailure` union, not a sentinel string. Accepted — reviewed the diff before merging. Its once-only bound is stronger than the ticket asked: `recoverFromMissingBrief` never re-enters `load()`, so CQ-007's retry loop is absent, not merely capped. **RB-22 mispredicted one thing harmlessly:** it expected the regenerated client to parse a `ProblemDetails` 404, but `Results.NotFound()` declares no body so it throws a plain `SwaggerException` (matching the 17 other bare-404 endpoints). `isHttpNotFound` reads only `.status`, so it tolerated both — the pair held because the FE half was written defensively. **RB-19 verification, recorded because RB-12's test cannot do it:** RB-12 proves a `.Gate(...)` marker is present, not that it matches the wrapper the handler calls (its own stated declaration-vs-derivation limit). Checked centrally instead — the sorted list of all 47 route strings is identical before and after, **and so is every (route, `.Gate` marker, wrapper actually called in the handler) triple**, with zero gate/handler mismatches. `gen:api` produced an ordering-only diff in `swagger.json` + `api-client.ts` (only the two moved _and documented_ endpoints changed position; the other three moves are `.ExcludeFromDescription()`), committed rather than left to fail the drift job. |
| 5 | RB-24..RB-30 | **complete** | All seven merged, one commit per ticket. `npm run ci` green on the combined tree after every merge. Ran as three waves, not the two the backlog implied: RB-24 rewrites imports in `brief.store.ts` and `org-template.store.ts`, which are two of RB-28's three targets — a dependency the backlog's "25/26/27 depend on 24" note never mentioned. **A** = RB-24 alone (the move), then **A2** = RB-29 + RB-30 in parallel (backend, no file overlap with the move or each other), **B** = RB-25 + RB-26 + RB-28 in parallel once RB-24 landed, **C** = RB-27 alone last, since it depends on RB-25's transport token. **RB-24 expanded its own scope, correctly.** Deleting the dependency-cruiser carve-out — the ticket's own acceptance criterion — exposed a second, real `ui-not-infrastructure` violation the old path had hidden: three UI components injected `UploadAdapter` for nothing but a one-line wrapper over its own exported pure function. The dispatch prompt said to report a second violation, not fix it; the agent judged this one was on the critical path (`dep:check` cannot pass with the carve-out gone otherwise) and fixed it minimally, reusing the existing pure function. Reviewed before merging — sound. **Two more findings were shown to be stale or overstated, on top of the two ADR-fixes found wrong and RB-18/RB-23's incompleteness from batch 4 — nine total now.** RB-25 found TE-003 overstated its own blocker: of the four methods named, only `upload()` and `cancel()` were actually unfakeable through the missing token — `delete()`/`pollReturning()` already went through the exported `UploadAdapter`. RB-28 found TE-006 already false at the time it was written: `brief.store.spec.ts` already had a `previewLetter` success test via jsdom's spyable `URL`/`window` stubs, contradicting the finding's "cannot test the success case" claim — the overall three-site diagnosis still held and was shipped as instructed. **RB-26 made one real design call**, reviewed before merging: `planFileSelection` must return `UploadMsg[]` per its literal signature, but an accepted file's real `localId` needs `crypto.randomUUID()`, which the ticket itself keeps in the controller. It ships a placeholder `localId: ''` discriminated by `.type` alone and never dispatched — verified the index alignment holds for both the multiple-rejection short-circuit and the per-file path. **RB-27 left one thing unextracted, correctly**: TE-005 lumped abort-vs-error disambiguation into the same extraction as `uploadOutcome`, but abort fires on a different event with no `status`/`responseText` at all — it structurally cannot fit the proposed signature. Left in place as a one-line ternary. The optional `currentScenario()` move into `KeepaliveTransport` was also correctly declined — it would have crossed into `upload-shell.service.ts`, outside this ticket's stated single-file scope. **End state of `libs/shared/upload`** (now split across proper layers): every layer that can hold pure logic has one and is spec'd — `upload.machine.ts` (domain, `planFileSelection`), `upload-shell.service.ts` (application, the `UPLOAD_TRANSPORT` seam), `upload-controller.ts` (application), `upload.adapter.ts` (infrastructure, `uploadOutcome`). Only the XHR/DOM boundary itself stays untested by design — TE-005 was explicit that abstracting `XMLHttpRequest` away is not wanted, since the file documents why XHR (not `fetch`) is required. |
| 6 | RB-31, RB-32, RB-33 | not started | |
| 6 | RB-31, RB-32, RB-33 | **complete** | All three merged, one commit per ticket. `npm run ci` green on the combined tree after every merge. Dispatched as one wave — no file overlap at all (four machine specs in three apps, one docs file, one testing helper plus its one call site). **RB-31 found a real ADR-0006 violation, not a false alarm.** Two `registratie-wizard` tests asserted a state — cursor 2, no diploma chosen — that the real reducer cannot produce, since advancing past `beroep` (cursor 1→2) requires `KiesDiploma`/`KiesHandmatig` to have already run. This is exactly what forcing fixtures through message replay is for: a hand-rolled literal let an impossible state sit in the suite undetected. Fixed by replaying to cursor 1 and applying the diploma choice there instead; `submit()` validates the whole draft regardless of cursor, so the assertions are byte-identical to before — reviewed the diff before merging to confirm the old and new test bodies check the same thing. `intake.machine.spec.ts` also had an existing, correct `intake.testing.ts` sitting unused in its own folder, imported only by the acceptance spec — now wired to both. **RB-32 added the missing `language-switcher` row and took the ticket's explicitly-optional second step**: a ~14-line drift guard in `check-tokens.sh` that diffs every `CIBG-GAP EXTENSION` marker's component directory against the register's rows and fails naming what's missing. Verified the regex before trusting it — two existing rows carry parenthetical suffixes (`wizard-shell (error summary only)`) and the extraction correctly captures only the backtick-quoted name. This closes ADR-0003's own predicted failure mode ("if markers and this table drift, trust the code and fix the table") permanently rather than fixing it once more. **RB-33 made the real adopt-or-delete call the finding asked for, and chose delete.** `unwrapOk` had zero consumers anywhere in the repo since it shipped; manufacturing a first caller purely to satisfy the ticket would have removed no actual duplication, since there was only one occurrence to begin with. Deleted the helper and its doc mention; left the one candidate call site's inline guard alone, since it already satisfies ADR-0006 §3's real requirement (never a cast). |
| 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,