docs(register): close rows 85 (premise corrected — ensure moved into the CreateInstanceActor funnel; not replication-specific, enable path was a second live instance) and 83 (shared DeploymentManagerStartupProbe gate); add rows 89-91 (connection-only deploy not applied on peer, no peer-deletion removal path, SiteScriptCompileCacheTests absolute-count fragility)

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

@@ -75,12 +75,15 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
 | 80 | ~~**Unreplicated sites report NON-NULL `LocalDbReplicationConnected: false` / `LocalDbOplogBacklog: 0` — contradicting the documented nullable semantics** *(Low-Medium, observability correctness)*~~ **RESOLVED 2026-08-16 (`a2f0086f`, with row 73).** Root cause was type-level: `ISiteHealthCollector.SetLocalDbReplicationStatus(bool …)` made null INEXPRESSIBLE for the connected flag, and the Host wired the bridge unconditionally — the replication engine is registered on every site node and idles answering `false`/`0` with no peer. Fix mirrors the `SiteEventLogReplicationCheck` precedent exactly: the Host evaluates `SiteLocalDbSetup.ReplicationIsConfigured` once and passes it as a third provider to `AddLocalDbReplicationHealthBridge` (the SAME captured value now also feeds `SiteEventLogReplicationCheck`, so the two surfaces can't disagree); unconfigured short-circuits to `(null, null)`; the collector widens to `bool?`. The rest of the chain needed NO change — proto already `BoolValue`/`Int64Value` wrappers, `NextBacklogAboveSince` already leaves the anchor unchanged on null, KPI source already conditional: row 33's machinery was correct and had simply never received a null (its null branch was dead code in production until now). `false` now means exactly one thing: configured, peer session down = real fault. Trap that produced the bug, now pinned in tests: configured-ness is `PeerAddress` **OR** `ApiKey` — "no PeerAddress" is NOT "unreplicated" (a passive-half node carries the key alone); the pre-existing Host fixture asserting `false`/`0` was exactly such a node and its assertion is CORRECT — kept with an explaining comment, plus a new genuinely-unreplicated fixture. Fail-before: gate behaviourally reverted → `UnreplicatedSite_HealthReport_ReportsNullForBothLocalDbFields` fails `Expected: null / Actual: False`. | Found at the previous wave's close-out verifying row 33 | Closed 2026-08-16. | — |
 | 81 | **The site-event-log storage cap cannot be enforced on a replicated node with an undrained oplog** *(Medium-Low, measured 2026-08-16 during the row-34 work)* | Row-34 measurement harness: with a 64 MB cap and 400 k rows, an unreplicated node reached the cap (66.98 MB); the replicated node stopped **2.5x OVER cap** (167.7 MB, 226 k rows still present), tripped by the existing "size did not decrease" guard — which is doing its job (prevents wiping the table) but leaves the cap unmet. Documented in §5 of `docs/plans/2026-08-15-site-events-purge-burst-measurement.md`. | Root cause: `GetDatabaseSizeBytes()` measures `page_count − freelist_count` over the WHOLE consolidated LocalDb file, which since Phase 1/2 includes `__localdb_oplog` + `__localdb_row_version` — so each 1000-row trim delete frees ~390 KB of `site_events` pages while ADDING ~1000 oplog rows + ~1000 tombstones to the same file; the trim fights itself, and a node whose peer is down cannot enforce its cap at all. The "1 GB event log" budget is in practice shared with replication bookkeeping the event log doesn't own. A correct fix measures `site_events`' OWN pages (e.g. SQLite `dbstat`) — a change to the cap's semantics deserving its own decision. | Next SiteEventLogging session, or first field report of a site LocalDb file materially over its event-log cap |
 | 82 | **`AuditLog.Tests` flakes under parallel full-solution runs — `MsSqlMigrationFixture` contention** *(Low, test-infra)* | Found INDEPENDENTLY by two agents this wave: one saw 4 failures (green in isolation, 383/383 standalone), the other saw a run with 29 failures all originating in `MsSqlMigrationFixture..ctor` → `Migrator.MigrateAsync`, then two clean sequential runs | The fixture does not tolerate a concurrently-running suite contending for the same MS SQL instance — a row-32-class shared-fixture issue, but cross-PROCESS (xunit collections can't fix it; needs a DB-level mutex, per-run database name, or serialized suite ordering). Produces false reds for anyone running suites in parallel; the previous wave's "first fully-clean 8,250-test run" is partly luck on this axis. | First CI adoption of parallel suite runs, or next test-infra session |
-| 83 | **Post-boot start-path tests must gate on the DM's first instance-count report — `ReconcileStartsInstanceTests` still uses a bare `Task.Delay(500)`** *(Low, test-infra)* | Row-78 fix session: the agent's first draft wrote the config row before the DM's boot load read storage, and three tests silently proved the BOOT LOAD works, not the sweep — passing for the wrong reason. Fixed by gating every test on the first `ISiteHealthCollector.SetInstanceCounts` report. `ReconcileStartsInstanceTests` (row 67's suite) has the same trap behind a bare `Task.Delay(500)` — currently passes, but on a loaded machine could start proving the boot load instead (row 40 replaced this pattern in `DeploymentWarmThenGateTests` but missed this file) | Convention worth pinning: any test asserting a post-boot start path gates on the first instance-count report first. One file to convert; the pattern is already written next door in `InstanceSweepSelfHealTests`. | Next SiteRuntime test touch, or first flake in `ReconcileStartsInstanceTests` |
+| 83 | ~~**Post-boot start-path tests must gate on the DM's first instance-count report — `ReconcileStartsInstanceTests` still uses a bare `Task.Delay(500)`** *(Low, test-infra)*~~ **RESOLVED 2026-08-17 (`524bdbbe`).** `ReconcileStartsInstanceTests` converted to the first-`SetInstanceCounts`-report gate; the pattern was extracted into a shared `TestSupport/DeploymentManagerStartupProbe` now used by both this suite and `InstanceSweepSelfHealTests`, so the convention has one implementation to copy instead of two to keep in sync. SiteRuntime 633/633 green. | Row-78 fix session | Closed 2026-08-17. | — |
 | 84 | **`LogReplicationBacklogNote` can fire twice in one purge tick** *(Low, cosmetic)* | Row-34 session: once per phase when both the retention purge AND the cap trim delete rows in the same tick; pre-existing, unchanged by the pacing work | Two Information notes where one would do; harmless. | Next SiteEventLogging touch |
-| 85 | **DCL connection actors are not spawned for connection configs that arrive via LocalDb replication — row 78's gap, one layer down** *(Medium-Low, availability; proven live 2026-08-16 by the row-78 close-out probe)* | The row-78 live probe (fresh node-a file, pair restart, replication-seeded store): the instance sweep started `rig-probe-1`/`soak-notify-1` correctly, but every tag subscribe then failed for 6+ min with `Unknown connection: OPC PLC Simulator — will retry` / dcl-manager `No connection actor for OPC PLC Simulator`, because connection actors are spawned only by the boot-time load and deploy/apply commands — the replication-seeded connection row sat in the store with no actor. One more pair restart healed it (boot load saw the seeded rows; all tags Good). | The instance-side subscribe retry loop (5 s) means the moment the connection actor exists everything converges — so the fix is exactly the row-78 shape one layer down: teach the DM sweep (or the DCL manager) to ensure a connection actor exists for every stored site data connection, reusing the same guards. Until then, the sweep half-heals a replication-seeded node: instances run, but data-sourced attributes sit Uncertain until a restart. | Next SiteRuntime/DCL session — natural companion to the row-78 sweep, likely a small extension of it |
+| 85 | ~~**DCL connection actors are not spawned for connection configs that arrive via LocalDb replication — row 78's gap, one layer down** *(Medium-Low, availability; proven live 2026-08-16 by the row-78 close-out probe)*~~ **RESOLVED 2026-08-17 (`8f2c2813`) — with a premise correction AND a widened blast radius.** The row's mechanism was wrong: `data_connection_definitions` has NO production reader (its getter is test-only) — connection actors are created solely from the `Connections` map in an instance's own flattened config via `EnsureDclConnections`, so there was never a store→actor path to repair. The real defect: that ensure was the CALLER'S job and only 2 of 4 instance-start paths did it (boot batch ✅, `ApplyDeployment` ✅, `StartStoredInstance` ❌ — both the row-67 ensure-started and the row-78 sweep — and `HandleEnableResult` ❌; the two misses are exactly the two paths added later). So the bug was NOT replication-specific: an ordinary unreplicated site that boots with an instance disabled and enables it later hit the same dead end, because the boot load reads only enabled configs. Fix is structural: `EnsureDclConnections` moved INTO `CreateInstanceActor` (the documented single creation funnel), before the already-exists guard so the two working paths keep exact semantics; reuses the per-name hash guard so steady state is silent; a fifth path cannot forget. Store-level sweep pass explicitly REJECTED (memo §7.4): it wouldn't have fixed this (the table is empty on deploy-only sites) and the unguarded form ping-pongs the hash slot against the deploy path, dropping live subscriptions every tick. Fail-before by behavioral revert: 4 of 7 new tests fail (sweep-started, reconcile-ensure-started, enable-after-disabled-boot, exactly-once-across-sweeps). SiteRuntime 633/633, DCL 320/320. Sibling gaps found during the fix recorded as rows 89/90. | Proven live 2026-08-16 by the row-78 close-out probe (sweep started `rig-probe-1`/`soak-notify-1` but every tag subscribe failed for 6+ min with `Unknown connection: OPC PLC Simulator — will retry`; a second pair restart healed it) | Closed 2026-08-17. Live rig re-probe at wave close-out: replay quarantine-and-restart, expect tags Good with NO second restart. | — |
 | 86 | **CLI and UI disagree on deployment site-scoping for Administrators** *(Medium, authorization)* | Found by the 2026-08-17 Low-tail sweep while re-verifying row 37 | `ManagementActor.cs:2586` grants an **Administrator role bypass** of site scoping on `deploy status`, while the UI's `SiteScopeService` is role-independent (system-wide iff the user carries zero `SiteId` claims). An Administrator carrying site claims therefore sees the whole fleet via the CLI but only their sites on the Deployments page — same user, same data, two answers, and one of them is wrong. This is row 37's duplication having drifted into behavior; decide which semantic is intended (role bypass vs claims-only) and enforce it on BOTH paths, ideally as part of the row-37 consolidation. | Next ManagementService/CLI session, or the row-37 consolidation — whichever comes first |
 | 87 | **CLI deployment scoping is non-transactional and unbounded** *(Low)* | Found by the 2026-08-17 Low-tail sweep | The CLI path resolves site grants to an instance-id list in a separate, non-transactional read (`ManagementActor.cs:2590-2596`), reloading EVERY instance per scoped query; and `PageSize: 0` on the wire maps to `take: null` = "no limit" (`DeploymentManagerRepository.cs:434`), materializing the entire retention window in one query. The UI path is structurally incapable of both. Bounded today by the purge window and low CLI traffic. | Row-37 consolidation, or first slow/OOM `deploy status` report |
 | 88 | **Unparseable CLI `--status` silently means "no filter"** *(Low, UX)* | Found by the 2026-08-17 Low-tail sweep | `ManagementActor.ParseDeploymentStatus` returns null on an unparseable status value, so a typo (`--status Sucess`) silently returns ALL statuses instead of an error — the caller cannot distinguish "filter applied, nothing matched" from "filter ignored". Fix: fail loudly on an unrecognized value (mirror the row-46 principle: loud failure over silent empty/full results). | Next CLI touch to the deploy command group |
+| 89 | **Connection-only artifact deploy does not reach the peer node's DCL** *(Medium-Low, availability)* | Found by the 2026-08-17 row-85 fix session | `data_connection_definitions` IS one of the ten replicated tables, but only the node that received the deploy runs `HandleApplyArtifactDataConnectionsToDcl` — the peer gets the row via CDC and keeps serving the OLD endpoint until a restart or an instance redeploy touches it. Not fixable by another sweep: applying a definition change to a live connection needs an arbitration decision first (does the instance's flattened-config `Connections` hash or the standalone definition hash win when they disagree?), because the row-85 fix deliberately made the flattened config the sole creation source. | Next artifact-deploy or DCL session — decide the arbitration, then apply-on-CDC becomes safe |
+| 90 | **No removal path for a connection deleted on the peer** *(Low, mirror image of row 89)* | Found by the 2026-08-17 row-85 fix session | `RemoveConnectionCommand` is sent only from the artifact-deploy reconcile on the node that processed the deploy; a connection deleted on the peer leaves a live connection actor here until restart. Bounded: the orphan actor serves a connection no instance references, wasting a device session but corrupting nothing. Same arbitration prerequisite as row 89 — fix them together. | With row 89 |
+| 91 | **`SiteScriptCompileCacheTests.GetOrAdd_SameCodeAndGlobals_ComputesOnce` is parallel-load fragile** *(Low, test-infra; row-82 family)* | Found by the 2026-08-17 row-85 fix session: failed once in 4 full-suite runs, green in isolation AND green on base — pre-existing, probability merely raised by the suite growing | `SiteScriptCompileCache` is a process-wide static and the test asserts an absolute `Hits == 1` after a global `Clear()`; xUnit runs other classes in parallel in the same process, so any concurrently-compiling test breaks the absolute count. Recommended fix: row-74's established delta-over-scoped-action assertion pattern (a collection fixture would serialize more than needed). | Next SiteRuntime test touch, or first flake in a run that matters |
 | 68 | ~~**`deploy.sh` run from a git worktree silently rehomes every node's bind-mount data dir**~~ **RESOLVED 2026-08-15 (guard added in the same close-out).** The compose files' bind mounts are relative (`./site-a-node-a/data`), so a deploy from a worktree checkout mounts the worktree's copies — the rig keeps running and looks healthy, but its SQLite stores split from the main checkout's and everything written there is orphaned by the next deploy from main. Bit the rows-46–53 close-out live: the seeding agent's `rig-probe-1` deploy persisted into worktree-homed dirs and "vanished" after the orchestrator's redeploy (no tombstone, no product bug — LWW/tombstone forensics ruled out deletion; the row sat in `.claude/worktrees/…/docker/site-a-node-a/data/site-localdb.db` the whole time). Both `docker/deploy.sh` and `docker-env2/deploy.sh` now refuse to run from a `.claude/worktrees/` path (override: `SCADABRIDGE_ALLOW_WORKTREE_DEPLOY=1`). | Found during the rows-46–53 close-out | Closed 2026-08-15. | — |
 
 ## Resolved (verified against the code 2026-07-10)