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 13a945f..fcd230b 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md @@ -113,12 +113,12 @@ Every ticket tracing to a `BIO-` finding, plus every row on agent 07's authorita | **RB-09** | backend/Domain + Program.cs | security | `IIdentityProvider` can express "no identity"; stub Development-only; fail fast in Production | §7 "Single-impl interface `IIdentityProvider`"; BL-006 | S | Med | **P1** | 2 | — | **SIGN-OFF** | **done** | | **RB-10** | ssp/auth + bhp/auth + ssp/shell | testability | Extract `parseStoredSession` (×2 apps) + spec `redactProfile`; assert a stored BSN yields `''` | §3a auth 42.9%/46.2% (worst FE line, §8); file LH 2/LF 20, BRH 3/BRF 13 | S | Low | **P1** | 2 | — | **SIGN-OFF** | **done** | | **RB-11** | ssp/brief + libs/shared/infra | security | Dev hatches out of prod on the 3 hand-written `fetch` paths; export their parse boundaries; fix the doc | §3b ssp/brief 42% reach (11/26, none `ui/`); §3a 68.8% branch | M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** | -| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | open | -| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | open | -| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | open | -| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | open | -| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | open | -| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | open | +| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | **done** | +| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | **done** | +| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** | +| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** | +| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** | +| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** | | **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | open | | **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | open | | **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | open | diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md index 10535bc..4bc5e83 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md @@ -14,14 +14,14 @@ ## 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 | not started | RB-13 depends on RB-09. | -| 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 | 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 | | **Standing caveat for every batch:** `dotnet test` reports one failure, `OpenZaakIntegrationTests.Admin_cases_returns_the_seeded_zaak_mapped_through_real_HTTP_and_JWT`, @@ -47,3 +47,35 @@ Fixed in `build: stop ci-local.sh swallowing the first half of every paired step weaker than it reads; the batch-2 completion run above is the first one made on the honest gate (13/13 steps, exit 0). Nothing has since been found wrong with batch 1, but it has not been re-verified under the fixed gate either. + +## Dispatching implementation agents — what actually goes wrong + +Batches 2 and 3 ran tickets as parallel agents in git worktrees. Six of seven agent-runs hit at +least one of these. Put all of it in the prompt. + +1. **The worktree base is not reliable.** **Four of the six** agents were handed a worktree + branched from a stale ancestor — batch 3 was **three for three**, all landing on `ae7781e`, + an unrelated lineage missing every RB ticket _and_ this backlog directory. Make step zero: + `git log --oneline -8`, confirm a **named expected commit**, `git merge` the target branch if + absent, and report which it was. The one agent that was not told to do this found out by luck. +2. **Agents park on background tasks.** Two agents in batch 2 ran `npm run ci` in the background, + then ended their turn waiting for a notification that never usefully arrived; one finished its + work twice and never committed it. Ban `run_in_background` and Monitor, **and say explicitly to + pass the Bash tool's own `timeout: 600000`** — a batch-3 agent still auto-backgrounded because + "run it in the foreground" alone does not defeat the 120s default. +3. **`git checkout ` destroys the work.** Agents verify a test is red by undoing the fix; + restoring it with `git checkout` reverts the whole file. Tell them to undo and redo with edits. +4. **`behaviour-spec.mdx` conflicts on nearly every merge.** It is generated and every agent + regenerates it. Resolve centrally with `npm run gen:behaviour-spec && git add`; tell agents to + expect it and never hand-edit. +5. **Concurrent `dotnet test` runs flake** — a burst of `SQLite Error 1: 'no such table: Documents'`. + Partly host pressure, but RB-12 found a real cause: a bare `new WebApplicationFactory()` + races on the static `Db.ConnectionString`. Use the house `TestWebApplicationFactory` + + `IClassFixture` idiom. Do not trust a backend green while another agent is testing. +6. **Agent worktrees live inside the repo**, so `prettier --check .` walks into them — fixed by + ignoring `.claude/worktrees/` in both `.prettierignore` and `.gitignore`. + +**Telling agents to report a ticket as wrong pays off.** Three did: BIO-012 was factually wrong +about the proefbrief error mapping (RB-11), RB-12's wrapper/public binary did not fit the code, and +RB-14 as worded would have shipped a non-gate. None of the three would have been caught by a review +of the diff alone.