docs(register): close rows 46, 48-53 with evidence; add rows 54-66 from the fix wave's discoveries

docs/plans/2026-07-08-deferred-work-register.md

@@ -39,14 +39,27 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
 | 43 | ~~**Central UI import wizard leaks a `BundleSession` slot on every abandoned import — imports lock out for 30 min**~~ **RESOLVED 2026-08-15 (`70838982`).** New `IBundleImporter.Abandon(sessionId)` does the same zero-then-remove release `ApplyAsync` already did (T-007 discipline), idempotent (double/unknown/expired ids are no-ops) and only ever given the caller's own random-GUID id, so it cannot evict a concurrent circuit's session. Wired into EVERY abandonment path: wizard new-upload, `TryLoadAsync` pre-overwrite, Back-to-Upload/`ResetSessionState`, 3-strike lockout reset, `Dispose` — plus two CLI leaks the original row didn't know about: `transport preview` released nothing at all (every preview burned a slot for 30 min) and `transport import` stranded the session on pre-apply aborts; both now release in a `finally`. `BundleSessionEvictionService` is documented as the TTL backstop, not an abandonment path. Fail-before-fix stash-verified (3 of 4 new bUnit tests fail pre-fix). 10 new tests. | Found live by R6 | Closed 2026-08-15. | — |
 | 44 | ~~**`BundleUnlockRateLimiter` is dead code — the documented per-IP unlock throttle is not enforced**~~ **RESOLVED 2026-08-15 (`5ab26a42`).** `BundleImporter.LoadAsync` now registers every unlock attempt against `MaxUnlockAttemptsPerIpPerHour` (10). Ordering is load-bearing: AFTER the per-bundle 3-strike check (hammering a locked bundle spends no hourly budget) and BEFORE the decrypt (a throttle denial burns no per-bundle strike, so the throttle can't be abused to lock out a legitimate import); successful unlocks count too. Denial = `BundleUnlockThrottledException` (placed in Commons so the management surface returns a curated message). Client key: Central UI via a scoped `CircuitClientAddressProvider` snapshotting the IP at circuit establishment; CLI via additive `ManagementEnvelope.ClientIp`. No attributable IP falls into one shared `<unattributed>` bucket — stricter, never a bypass; per-client isolation is only real behind a ForwardedHeaders-trusted proxy (same caveat as the login throttle, documented). Fail-before-fix verified (4 of 6 new tests fail with the limiter neutralised; 2 negative controls). | Found by R6 | Closed 2026-08-15. | — |
 | 45 | ~~**Notification Outbox dispatcher shares one `DbContext` across up to 4 parallel deliveries**~~ **RESOLVED 2026-08-15 (`9e769735`).** `_adaptersCache`/`_adaptersScope`/`ResolveAdapters()` deleted; the typed adapter is resolved inside the per-delivery DI scope `DeliverGatedAsync` already opens (static `ResolveAdapter` preserving DI last-wins + the null-adapter park path), so each concurrent delivery gets its own adapter and `DbContext`. Nothing shared was lost: the Email adapter builds a fresh SMTP client per attempt from an injected factory, the SMS adapter draws from the singleton `IHttpClientFactory`, and CentralUI's adapter-gated Type selector (`NotificationChannelCatalog`) is an independent scoped consumer, untouched. Fail-before-fix stash-verified (simulated DbContext throwing EF's exact second-operation exception under a rendezvous-guaranteed overlap: 1 of 2 rows persisted pre-fix; 1-vs-4 distinct adapter instances). Root cause of note: the bug was INTRODUCED by code-review finding NotificationOutbox-006's caching "fix" (2026-05-28), whose own regression test pinned the broken behavior and predates bounded parallel delivery — that finding is marked SUPERSEDED. NotificationOutbox 153, AuditLog 383 green. | Found by R6; observed live on rig central-b | Closed 2026-08-15. | — |
-| 46 | **CLI `audit-config query` returns `{}` for every filter combination** *(Low)* | Found by R6 against the rig; underlying `AuditLogEntries` rows verified present | Possibly a CLI output-shape/serialization issue; not investigated (out of R6 scope). | Next CLI session |
+| 46 | ~~**CLI `audit-config query` returns `{}` for every filter combination**~~ **RESOLVED 2026-08-15 (`09108018`).** Not a CLI bug: `ManagementActor.HandleQueryAuditLog` returned the repository's ValueTuple `(Entries, TotalCount)` straight to `SerializeResult`; System.Text.Json writes properties only and a ValueTuple's `Item1`/`Item2` are **fields**, so the whole payload serialized to the literal `{}` — every filter combination looked empty with no error anywhere in the stack. Fixed with a named Commons record `AuditLogQueryResult` (mirroring `SecuredWriteListResult`) plus a class-wide guard: `SerializeResult` now throws on any `ITuple` result, so the next tuple-returning handler fails loudly instead of silently emitting `{}`. Fail-before verified (test asserted exactly `"{}"` pre-fix); live re-verified on the rig (`totalCount 2725` unfiltered, `712` for `--entity-type Template --action Delete`). The only other tuple-returning repo method in Commons (`INotificationOutboxRepository.QueryAsync`) is never serialized. | Found by R6 against the rig; underlying `AuditLogEntries` rows verified present | Closed 2026-08-15. | — |
 | 47 | ~~**`SiteStreamGrpcServer` duplicate-stream replacement disposed another handler's `CancellationTokenSource`**~~ **RESOLVED 2026-08-15 (found and fixed in-flight by R2, `e6842c10`).** `SubscribeInstance`'s duplicate-prevention path called `Cancel()` AND `Dispose()` on the replaced stream's CTS, which belongs to that still-running handler's `using var` and is still read — escaping as an unhandled `ObjectDisposedException` under scheduling pressure (confirmed pre-existing on `2b74851f`; load-only, passed 10/10 isolated). Fixed to cancel-only (the owner's `using` disposes exactly once), `Cancel` wrapped for the converse race, mirroring `CancelAllStreams()`. Deterministic regression test via a gate inside the setup window, fail-before/pass-after verified by reinstating the `Dispose()` as a negative control. Recorded here (though closed) because it is a product-bug instance of the disposal-race class, useful precedent for future stream-lifecycle work. | Closed 2026-08-15. |
-| 48 | **Docker rig has no running site instances — live stream probes have no traffic** | Found during the R2 close-out live probe (2026-08-15) | All 7 seeded instances are `soak-*` rows whose deploy fails pre-deployment validation (34 errors: ScriptCompilation ×4, ConnectionBinding ×30 — their bound data connections no longer exist on the sites), and `debug snapshot` returns `instanceNotFound`; `scadabridge_site_connection_up` is 0 on all six site nodes. Consequence: the R2 batching histogram (`scadabridge.site.stream.batch_size`) could not be observed live — batching verification rests on its integration/wire-skew/latency tests, which are thorough. Fix: re-seed a small working instance + data connection set (OPC UA sim is still up) so future live probes have traffic. | Next rig session, or before any live probe that needs event traffic |
-| 49 | **Sweep code-review "Resolved" entries that cached or lifetime-widened a DI-scoped service** *(Low, process)* | Found while fixing row 45: NotificationOutbox-006, a Low perf nit, introduced the row-45 Medium correctness bug, and its regression test actively pinned the broken behavior — written before `MaxParallelDeliveries` existed and never revisited when parallel delivery landed | The failure shape generalizes: a review resolution that widens a scoped service's lifetime is correct at the time and silently invalidated by a later concurrency feature. Sweep `code-reviews/*/findings.md` resolved entries for caching/lifetime-widening of scoped services and re-check each against features added after its date. | Next code-review or test-infra session |
-| 50 | **No guard against capturing a scoped service on a singleton actor (captive dependency)** *(Low)* | Found while fixing row 45 | The actors take root `IServiceProvider` rather than typed services, which defeats .NET's built-in scope validation — the row-45 class of bug is invisible at build time. Cheap mitigations: `ValidateScopes`/`ValidateOnBuild` on the central host's provider in Development, and/or a test asserting no `ReceiveActor` field holds an `IServiceScope`. | Next DI/host-hardening session |
-| 51 | **`BundleLockedException` reaches the CLI as "An internal error occurred"** *(Low, pre-existing)* | Found while fixing row 44 | `ManagementService` doesn't reference `Transport`, so `MapFault` can't curate the 3-strike lockout — a CLI operator gets a correlation id, not the reason. Row 44 solved this shape for the throttle exception by placing it in Commons; `BundleLockedException` needs the same move (public type relocation, low risk, deliberately kept out of the row-44 diff). | Next Transport/CLI session |
-| 52 | **The unlock throttle's counters are per-node and per-process** *(Low, by design but recorded)* | Row 44 implementation note; documented in `docs/components/Transport.md` | Each central node has its own limiter, so the effective fleet cap is 10/hour/IP per node, and a restart clears the window. Acceptable for the threat model today; a shared store is the hardening if it ever matters. | A hardening pass that wants a fleet-wide cap |
-| 53 | **`MaxConcurrentImportSessions` has no observability** *(Low)* | Found while fixing row 43 — R6 could only infer the leak from rotating test failures | Nothing counts open sessions or logs when `Open` rejects at the cap. A gauge (mind the `ObservedMeters` allowlist) or a Warning log on rejection would make a recurrence diagnosable in seconds. | Next Transport or observability session |
+| 48 | ~~**Docker rig has no running site instances — live stream probes have no traffic**~~ **RESOLVED 2026-08-15 (`b5e6821c`).** Working set re-seeded on site-a entirely via the CLI (`soak-*` rows left alone): template `RigProbe` + instance `rig-probe-1`, bound to the OPC PLC sim's built-in moving tags (`SpikeData`, `RandomSignedInt32`) plus a 5s heartbeat script. Deploy Success; `debug snapshot` returns live Good-quality data; `health site` shows `OPC PLC Simulator: Connected`, `good:2 bad:0 uncertain:0`; a 60s `debug stream` produced 239 events; and **`scadabridge.site.stream.batch_size` recorded live for the first time** (count 70 / sum 238, batches to ~10 events) — the R2 batching histogram is now observable on the rig, which was this row's purpose. Reproducible via `docker/seed-live-probe.sh`. Two diagnostic corrections worth keeping: the custom tags in `infra/opcua/nodes.json` are declared but **not simulated** by opc-plc (a healthy-looking connection with zero traffic — bind to the built-ins: `SpikeData`, `RandomSignedInt32`, `StepUp`, `FastUInt1..5`, `SlowUInt1..5`); and `scadabridge_site_connection_up` counts open central→site gRPC subscriber streams, not data connections, so this row's original "0 on all six site nodes" was healthy steady state, not the fault (row 54). | Found during the R2 close-out live probe (2026-08-15) | Closed 2026-08-15. | — |
+| 49 | ~~**Sweep code-review "Resolved" entries that cached or lifetime-widened a DI-scoped service**~~ **RESOLVED 2026-08-15 — swept, no further bug found (`75520bee`).** All 576 resolved findings across the 25 component findings files were re-read for the row-45 generalization (a resolution that caches, hoists, or widens the lifetime of a DI-resolved service, later invalidated by a concurrency/lifetime feature). Nine matched or nearly matched the shape; **NotificationOutbox-006 is the only one that ever became a bug** and was already fixed as row 45. The other eight verify STILL-SAFE or SUPERSEDED-BENIGN — the two with real concurrency nearby each anticipated it explicitly (`DeploymentService`'s `dbGate` serialises the one repository touch inside the bounded-parallel Send phase; `ArtifactDeploymentService` builds all per-site commands serially before fanning out, commenting "DbContext is not thread-safe"). An independent product-code cross-check confirms the pattern is absent: no `IServiceScope` is stored on a field anywhere in `src/` (every long-lived holder takes `IServiceScopeFactory` and opens a scope per operation), every `Task.WhenAll`/`Parallel.*` site either touches no scoped service or opens a per-operation scope, and no scoped-registered service is captured by a singleton. One stale archive corrected: NotificationOutbox-004 was marked Resolved with an `_Unresolved._` body; the real resolution is now recorded. Near-miss shapes recorded as rows 58–60. | Found while fixing row 45 | Closed 2026-08-15. | — |
+| 50 | ~~**No guard against capturing a scoped service on a singleton actor (captive dependency)**~~ **RESOLVED 2026-08-15 (`00a804e6`).** Both mitigations shipped, and they are complementary. (1) `ValidateScopes`/`ValidateOnBuild` in Development on **both** role branches via `Host/ServiceProviderValidation.cs` — written out rather than left to `CreateBuilder`'s silent default; production stays unvalidated deliberately (fail-closed at boot). Switching it on surfaced **two real Site-role defects**: `ReconcileService` and `AuditLogKpiSampleSource` were registered on sites but need central-only repositories, so neither could ever be constructed there — one unconstructible descriptor fails `ValidateOnBuild` for the entire site graph, which is exactly why `SiteHealthEndpointTests` had quietly pinned `UseEnvironment("Production")` to dodge validation. Both moved to central-only registrations (`AddCentralCommunication()` / `AddAuditLogCentralKpi()`); the workaround is gone and the site graph now boots validated. (2) `CaptiveDependencyGuard` reflects over all 28 shipped actor classes and fails on any instance field (backing fields included) typed `IServiceScope`, `DbContext`, or Scoped-in-the-real-containers — the scoped set is derived from the built Central and Site graphs, so a lifetime change auto-re-scopes the check; `IServiceProvider`/`IServiceScopeFactory` fields are allowed (the correct pattern). Proven non-vacuous via permanent positive-control dummy actors. No live violation; the two test-only `_injectedRepository` ctor fields allowlisted with written safety arguments plus a staleness self-test (row 61). Doc: REQ-HOST-11 in `Component-Host.md`. | Found while fixing row 45 | Closed 2026-08-15. | — |
+| 51 | ~~**`BundleLockedException` reaches the CLI as "An internal error occurred"**~~ **RESOLVED 2026-08-15 (`b449fe20`).** Relocated to `Commons/Types/Transport/` beside `BundleUnlockThrottledException` (row 44's pattern); both consumers already carried the namespace, so no call-site edits. `ManagementActor.HandlePreviewBundle`/`HandleImportBundle` now rethrow it as `ManagementCommandException`, so CLI and UI print the lockout reason instead of a correlation id. Fault-mapping tests mirrored from row 44 on both handlers. Follow-up in the same diff (`73f518d6`): deleted the dead `BundleUnlockRateLimitedException` (superseded by row 44, zero references — three similarly-named unlock exceptions with one inert was a live trap). | Found while fixing row 44 | Closed 2026-08-15. | — |
+| 52 | ~~**The unlock throttle's counters are per-node and per-process**~~ **RESOLVED 2026-08-15 (`677263af`).** The trailing-hour window now lives in the central MS SQL `BundleUnlockAttempt` table — the HA store both central nodes already share, so no new infra, port or consensus. `MaxUnlockAttemptsPerIpPerHour` is a true fleet cap and survives a node restart; config keys, semantics and the rig's 500 override are unchanged, as are all row-44 behaviours (lockout checked first so locked bundles spend no budget; denials burn no strike and are not recorded; `<unattributed>` bucket; attempts-not-failures). Three recorded trades: **fails open** (store error → Warning naming client + cap, attempt permitted — availability of legitimate imports outranks a best-effort cap over an already-authenticated Administrator-gated surface; a process-local fallback was considered and rejected as it would still block an operator's legitimate 11th import); **approximate** (count-then-insert is not atomic across nodes; overshoot bounded by in-flight count, self-corrects within the window — row 64); **opportunistic prune on the unlock path** (CAS-claimed ≤ every 15 min per process, ≤1000 rows, retains 2× the window so a prune can't race a count — row 65) rather than a new background service. EF migration `20260815154803_AddBundleUnlockAttemptTable` + hand-written production script `docs/plans/sql/AddBundleUnlockAttemptTable.sql`. Integration test proves two limiter instances over one shared store enforce one combined window, restart durability, and store-failure fail-open. | Row 44 implementation note; documented in `docs/components/Transport.md` | Closed 2026-08-15. | — |
+| 53 | ~~**`MaxConcurrentImportSessions` has no observability**~~ **RESOLVED 2026-08-15 (`372ea4df`).** Gauge `scadabridge.central.transport.import_sessions.open` on the `ZB.MOM.WW.ScadaBridge` meter — already in `SiteServiceRegistration.ObservedMeters` (the single shared allowlist source for both roles; no appsettings pins one), so no allowlist change was needed and the series is not silently dropped. Provider-backed like the S&F queue-depth gauge: registered by `BundleSessionEvictionService.StartAsync`, identity-checked-cleared on `StopAsync`. `IBundleSessionStore.OpenSessionCount` counts only **unexpired** sessions, so the gauge doesn't read "pinned at the cap" for a sweep interval after a burst drains. `BundleSessionStore.Open` logs Warning with open count, cap and TTL on a genuine cap hit only (a store full of expired sessions is reclaimed silently). No alertable rejection counter yet — row 66. | Found while fixing row 43 — R6 could only infer the leak from rotating test failures | Closed 2026-08-15. | — |
+| 54 | **`scadabridge_site_connection_up` is misleadingly named** *(Low, observability)* | Row-48 close-out (2026-08-15) — the row's original mis-diagnosis traced directly to this | It counts open central→site gRPC subscriber streams (`SiteStreamGrpcServer`), not DCL data connections, and correctly reads 0 whenever no viewer is attached. Rename (e.g. `scadabridge.site.stream.subscribers.active`) and/or add a real data-connection gauge — the data exists on the health report (`dataConnectionEndpoints`/`dataConnectionTagQuality`) but neither is exported as a metric. | Next observability/metrics-naming session, or the next time someone reads this series as data-connection health |
+| 55 | **`tagResolutionCounts` reads `{totalSubscribed:0, successfullyResolved:0}` for a connection with 2 live Good-quality subscribed tags** *(Low)* | Row-48 live probe: site-a's health report shows the zero counts while `dataConnectionTagQuality` on the same report says `good:2` | The "device still booting / tag path unresolved" diagnostic is blind if this stays zero. Code inspection at close-out shows `DataConnectionActor` calls `_healthCollector.UpdateTagResolution` with the derived counts at all the right lifecycle points, so this is likely not a `491df111` regression — suspect a reporting-path issue instead (e.g. a standby-node collector's empty entry shadowing the active node's, or a keying/removal race in `SiteHealthCollector`). Needs live debugging. | Next health-monitoring or DCL session |
+| 56 | **The rig's central nodes expose no Prometheus endpoint** *(Low, rig config)* | Row-48 live probe | `docker/site-*/appsettings.Site.json` set `MetricsPort: 8084`, but neither `docker/central-node-a` nor `-b` does, so central-only series (KPI/outbox counters, the new import-session gauge, alarm-cache gauges) cannot be scraped live and any live probe naming "central metrics" is unsatisfiable. | Next rig/observability session |
+| 57 | **`audit-config query` has no CLI table projection** *(Low, cosmetic)* | Row-46 fix (2026-08-15) | `--format table` renders the whole `entries` array as raw JSON inside one cell because the command passes no `tableProjector` (unlike e.g. `template list`). JSON output is correct and is the primary format. | Next CLI session |
+| 58 | **`IDeploymentStatusNotifier` singleton retains delegates targeting circuit-scoped Blazor components** *(Low)* | Row-49 sweep (CentralUI-006 re-verification) | Safe today because the single subscriber unsubscribes in `Dispose` (pinned by test) and the notifier isolates each handler. | A second page subscribes, or any subscriber lands without a matching `Dispose` unsubscribe |
+| 59 | **`SiteCallAuditActor` per-tick scope spans all sites × ≤50 upsert pages** *(Low)* | Row-49 sweep (SiteCallAudit-009 re-verification) | Safe only because site reconciliation is a serial `foreach … await` (`SiteCallAuditActor.cs:649-653`); one scope across a parallel fan-out would be row 45 again. | `ReconcileSiteAsync` is ever fanned out with `Task.WhenAll` |
+| 60 | **`ArtifactDeploymentService.GlobalArtifactSnapshot` holds EF-materialised entities across the parallel per-site send** *(Low)* | Row-49 sweep (DeploymentManager-023 re-verification) | Safe because lazy loading is off and the parallel phase reads only materialised POCOs (per-site commands are built serially first, commented "DbContext is not thread-safe"). | Lazy-loading proxies enabled, or per-site command building moved inside the fan-out |
+| 61 | **`_injectedRepository` test-mode constructors are one call site from re-introducing row 45** *(Low)* | Row-50 guard allowlist: `AuditLogIngestActor` / `SiteCallAuditActor` each expose a constructor taking a scoped repository and cache it for the actor's lifetime | Safe only because no production call site picks that overload (production wiring uses the `IServiceProvider` overload; `SiteCallAuditActor` additionally wraps in `SerializedRepository`). The `CaptiveDependencyGuard` allowlists both with a staleness self-test. Preferred fix: fold the test fixtures onto the `IServiceProvider` overload with a stub provider and delete the overloads. | Anyone adding a production `Props` for either actor, or the next time those actors are touched |
+| 62 | **Five SiteRuntime actors resolve `ISiteEventLogger` off the root provider without a scope** *(Low)* | Row-50 guard sweep: `AlarmActor` (which also caches it in a field), `ScriptActor`, `InstanceActor`, `NativeAlarmActor`, `DeploymentManagerActor` | Correct today (`ISiteEventLogger` is `AddSingleton`), but it is the one pattern that silently becomes row 45 if that registration ever changes lifetime. The guard catches the `AlarmActor` field case automatically; the per-call root resolves it cannot see. | Any change to `ISiteEventLogger`'s lifetime, or a second service pulled this way |
+| 63 | **Rig site containers run without `ASPNETCORE_ENVIRONMENT`, so the row-50 Development validation never fires there** *(Low, rig config)* | Row-50 implementation: central containers set it, site containers don't | The site half of the scope validation is exercised only in CI/tests. Setting it on the six site containers would give live coverage now that the site graph validates clean. | Next rig config pass |
+| 64 | **The fleet-wide unlock throttle's count-then-insert is not atomic across central nodes** *(Low, accepted by design)* | Row-52 implementation trade, documented in both Transport docs | Simultaneous attempts can overshoot `MaxUnlockAttemptsPerIpPerHour` by the number in flight; bounded and self-correcting within the window. A conditional single-statement insert would close it at the cost of provider-specific raw SQL. | A threat model needing a hard cap rather than a rate damper, or evidence of a real brute-force exploiting the overshoot |
+| 65 | **`BundleUnlockAttempt` rows are pruned only by nodes actively serving unlocks** *(Low)* | Row-52 implementation trade | The prune rides the unlock path, so a cluster that stops importing entirely leaves the last rows in place indefinitely (bounded and tiny: one row per permitted attempt, retained 2× the window). | The table appears in a size review, or Transport gains a central scheduler a prune tick could ride |
+| 66 | **No counter for import-session cap rejections** *(Low)* | Row-53 implementation note | Row 53 shipped the open-sessions gauge and a Warning log, so a rejection is diagnosable from logs, but there is no series to alert on. | Someone wants an alert rule on import-session exhaustion rather than a log search |
 
 ## Resolved (verified against the code 2026-07-10)
 Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability.