From 07220bc65512108302e18429e28179dbfaafe776 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 16 Aug 2026 04:56:15 -0400 Subject: [PATCH] =?UTF-8?q?docs(register):=20row-78=20live-probe=20evidenc?= =?UTF-8?q?e=20(sweep=20self-heal=20fired=20in=20the=20original=20failure?= =?UTF-8?q?=20sequence);=20add=20row=2085=20=E2=80=94=20DCL=20connection?= =?UTF-8?q?=20actors=20not=20spawned=20for=20replication-seeded=20connecti?= =?UTF-8?q?on=20configs=20(row=2078=20one=20layer=20down,=20found=20by=20t?= =?UTF-8?q?he=20probe)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/2026-07-08-deferred-work-register.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-07-08-deferred-work-register.md b/docs/plans/2026-07-08-deferred-work-register.md index df0f3c99..7a9f1d78 100644 --- a/docs/plans/2026-07-08-deferred-work-register.md +++ b/docs/plans/2026-07-08-deferred-work-register.md @@ -70,13 +70,14 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 75 | ~~**SecuredWrites Approve renders as a red destructive button** *(Low, UI)*~~ **RESOLVED 2026-08-16 (`40944a0d`).** The real defect was deeper than "red means delete": Approve and Reject rendered as IDENTICAL red buttons, so at the moment of a two-person-control decision colour carried no discriminating information — because `danger` was a two-state flag for a three-state concept. Fix: `DialogSeverity` (`Normal→btn-primary`, `Caution→btn-warning`, `Destructive→btn-danger`); **Approve → Caution amber** (executes a real write to live plant equipment — consequential, not destroying), **Reject keeps Destructive red** (terminal for the request; and holding it red keeps the two opposite decisions distinct — dropping it to neutral would just invert the bias). Only `Destructive` inherits row-28's "Delete" label fallback, or row 28 returns through the new door. Implemented as an ADDITIVE overload deliberately: a 5th optional parameter would silently un-match every 4-arg NSubstitute stub (the row-28 gotcha, null Task → NRE); the overload's only breakage was 6 hand-written fakes failing to COMPILE — the loud mode. `DialogState.Danger` became `Severity` (one field can't disagree with itself). Playwright `.btn-danger` selectors stay valid (`Destructive` mapping unchanged). Fail-before: call site reverted to `danger: true` → both Approve pins fail; `Caution→btn-danger` remap → amber styling pin fails. Decision recorded in `Component-CentralUI.md`. | Row-28 audit (2026-08-15) | Closed 2026-08-16. | — | | 76 | ~~**CLI README's `audit-config query` JSON sample omits `afterStateJson`/`bundleImportId`** *(Low, docs)*~~ **RESOLVED 2026-08-16 (`4ae57bdb`).** Sample now shows both fields with their nullability semantics, and was grown to two entries mirroring the table-output sample below it row-for-row — which is what makes the "the table drops these two fields" sentence checkable. | Row-57 fix (2026-08-15) | Closed 2026-08-16. | — | | 77 | ~~**Test pattern: reflecting `ActorCell.Actor` right after `ResolveOne` is unordered vs child construction** *(Low, test-infra)*~~ **RESOLVED 2026-08-16 — swept clean, no commit needed.** Full `tests/` sweep: `ActorCell` (1 hit — the already-fixed file's comment), `ActorRefWithCell` (1 — fixed file :135), `.Underlying` (1 — same line), `BindingFlags` near actor/cell (2 — same file), `.Cell` (0), `ResolveOne` (3 files: the fixed one; `SiteAlarmLiveCacheServiceTests:432` resolves-then-`Sys.Stop`s, benign; `ReconcileStartsInstanceTests:363` uses the ref only for `IActorRef` equality, benign), `UnderlyingActor` (6 files — all Akka.TestKit `TestActorRef` via `ActorOfAsTestActorRef`, an ORDERED mechanism, not this pattern). The unordered idiom exists in exactly one place and already carries the bounded-deadline retry. | Row-40 session (2026-08-15) | Closed 2026-08-16. | — | -| 78 | ~~**Configs arriving via LocalDb replication start no InstanceActor — row 67's gap through a side door** *(Medium-Low, availability; proven live 2026-08-15)*~~ **RESOLVED 2026-08-16 (`e606b24a`, with row 72 — the DM instance sweep).** A periodic sweep on the Deployment Manager **singleton** (`ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds`, default 60, validated `>= 0`, `0` disables — default is live everywhere with no rig config change, which is intended): each tick re-reads the durable rows off-thread and, on the actor thread, starts any **enabled** stored config with no running actor. Arrival-path-agnostic by design — covers replication seeding, post-boot snapshot resyncs, and any future path. A LocalDb change hook was considered and rejected on evidence: it covers only the replication path and only the replicated half of the fleet (site-b/c aren't), AND `ILocalDb` 0.3.0 exposes no change-notification API at all, so a hook means a cross-repo upstream capability to close a site-local gap. Warning site event on a real heal names the sweep as the trigger; silent when there is nothing to do. Fail-before (sweep unarmed): 5 of 10 new tests fail including the exact live symptom ("A replication-seeded config for an enabled instance left no running Instance Actor"); 5 negative guards (disabled / in-flight-deploy deferral / matching-revision no-op / silence / interval-0) green in both states. Design note: `docs/plans/2026-08-15-instance-sweep-self-heal-design.md`. | This wave's close-out (2026-08-15) | Closed 2026-08-16. | — | +| 78 | ~~**Configs arriving via LocalDb replication start no InstanceActor — row 67's gap through a side door** *(Medium-Low, availability; proven live 2026-08-15)*~~ **RESOLVED 2026-08-16 (`e606b24a`, with row 72 — the DM instance sweep).** A periodic sweep on the Deployment Manager **singleton** (`ScadaBridge:SiteRuntime:InstanceSweepIntervalSeconds`, default 60, validated `>= 0`, `0` disables — default is live everywhere with no rig config change, which is intended): each tick re-reads the durable rows off-thread and, on the actor thread, starts any **enabled** stored config with no running actor. Arrival-path-agnostic by design — covers replication seeding, post-boot snapshot resyncs, and any future path. A LocalDb change hook was considered and rejected on evidence: it covers only the replication path and only the replicated half of the fleet (site-b/c aren't), AND `ILocalDb` 0.3.0 exposes no change-notification API at all, so a hook means a cross-repo upstream capability to close a site-local gap. Warning site event on a real heal names the sweep as the trigger; silent when there is nothing to do. Fail-before (sweep unarmed): 5 of 10 new tests fail including the exact live symptom ("A replication-seeded config for an enabled instance left no running Instance Actor"); 5 negative guards (disabled / in-flight-deploy deferral / matching-revision no-op / silence / interval-0) green in both states. Design note: `docs/plans/2026-08-15-instance-sweep-self-heal-design.md`. **LIVE-PROVEN at close-out (2026-08-16) in the exact original failure sequence:** site-a pair stopped, node-a's LocalDb file quarantined, pair restarted together — node-a's 08:45:55 reconcile reported "0 fetched" (replication had seeded the fresh store; central rightly saw no gap — the precise side door), and at 08:46:50, one sweep interval later, BOTH enabled instances started with the Warning `Instance rig-probe-1 started by deployment-manager sweep self-heal — its config is present in the deployed-config store but no Instance Actor was running until now`; `debug snapshot` live with no extra restart. The probe also exposed the DCL sibling gap — row 85. | This wave's close-out (2026-08-15) | Closed 2026-08-16. | — | | 79 | **OrbStack virtiofs is a live corruption + availability hazard for the rig's bind-mounted SQLite files** *(Medium, rig infra — host-level, not product)* — **mitigation chosen 2026-08-16: accept + document (`851b528c`).** The full hazard description + the proven quarantine-and-reseed recovery runbook now live in `docs/deployment/topology-guide.md` (new section), including the `orb restart docker` VM-sleep recovery and the named-volumes/idle-pause alternatives with their trade-offs. Row stays OPEN as the hazard tracker: a recurrence should trigger actually moving to named volumes or disabling idle-pause rather than re-running the runbook a third time. Original evidence: the OrbStack VM idle-SLEPT mid-session (`vmgr … msg=sleep`) leaving docker hung ~20 min until `orb restart docker`, with virtiofs `refresh_nodeid: race with new lookup` ERRORs on wake; both site-a LocalDb files (row 69) corrupted through this bind-mount layer — macOS virtiofs does not give SQLite true fsync durability, and abrupt container kills mid-write compound it. | This wave's close-out (2026-08-15); runbook shipped 2026-08-16 | Runbook shipped; hazard accepted for now | **Next corruption occurrence ⇒ stop accepting: move the rig data dirs to named volumes or disable OrbStack idle-pause** | | 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` | | 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 | | 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)