docs(register): close rows 34, 62, 71-78, 80 with evidence; row 79 mitigation chosen (accept + runbook); add rows 81-84 (cap-vs-oplog self-fight, AuditLog MS SQL fixture parallel flake, boot-load test-gating trap, double backlog note)
This commit is contained in:
@@ -27,7 +27,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 31 | **Unpaced-burst absorption is bounded by the shared publish stage, not per-subscriber buffers (load-test finding F3 caveat)** | `docs/plans/2026-08-15-target-scale-load-test-results.md` F3 | Measurement-backed design note, no defect: slow-subscriber isolation is structural and total (a stalled reader loses events only in its own bounded `DropOldest` channel; healthy subscribers delivered 100.00%), but the publish `Source.ActorRef` **upstream** of the `BroadcastHub` is shared, so an unpaced burst degrades every subscriber equally — that stage, not per-subscriber buffering, is the burst ceiling. | First field report of event loss on *healthy* subscribers during a burst, or any redesign of the site stream publish stage |
|
||||
| 32 | ~~**`QueueDepthGaugeTests` full-suite flake — shared static gauge (load-test finding F4)**~~ **RESOLVED 2026-08-15 (`65b19516`).** Reproduced first (2 failures in 5 parallel runs — the gauge read a sibling class's provider). The register's suggested fix (per-test meter) was rejected on inspection: the slot is process-global by product design and 3 of the 6 facts assert the slot-ownership semantics themselves (compare-and-clear identity, stop/restart re-seed), so per-test instances would change what they prove. Correct isolation is mutual exclusion — the three `StartAsync`-calling classes now share xunit collection `QueueDepthGaugeCollection` (with a `[CallerFilePath]`-anchored source-scan drift guard that names an offending file), the other eight classes stay parallel, and `parallelizeTestCollections` was flipped back ON so the fix is exercised rather than masked. Negative control: dropping the attribute fails the guard AND resurfaces the original flake in the same run. 8/8 green parallel runs. | `tests/…StoreAndForward.Tests/QueueDepthGaugeTests.cs`; surfaced by the row-25 verification runs | Closed 2026-08-15. | — |
|
||||
| 33 | ~~**`LocalDbOplogBacklog` has no threshold, KPI history, or dashboard treatment**~~ **RESOLVED 2026-08-15 (`8da60f57`).** Both halves shipped: sampled into KPI history via `SiteHealthKpiSampleSource` (conditionally — 12 samples unreplicated / 14 replicated, so a series gap truthfully means "not replicated") plus a backlog trend chart on the site health panel, and a SUSTAINED threshold badge (`LocalDbBacklogThreshold` 1000 held for `LocalDbBacklogSustainedFor` 15 min, anchored by `LocalDbBacklogAboveSince` in the aggregator) — a bare instantaneous threshold would fire on every replicated site every day from the documented purge burst. Null backlog is a deliberate third branch: it leaves the anchor unchanged (neither evidence of a drain nor of a backlog), so unreplicated sites can never anchor and can never be badged. NOTE the row's premise was false — the `SiteAuditBacklog` "configurable threshold badge" it told us to mirror was described twice in `Component-HealthMonitoring.md` but NEVER BUILT; both stale doc claims corrected, and the new badge was modeled on the real Metrics-stale badge instead. | Found by R7 while documenting the purge burst | Closed 2026-08-15. | — |
|
||||
| 34 | **`site_events` purge burst is unbounded in the oplog and unmeasured at scale** | Found by R7 | Retention deletes are sliced at 1000 rows/statement, but the whole expired set is deleted in one tick, so a replicated site's oplog gains one row per deleted event with no pacing — a first purge after enabling replication (or after a long low-purge period) queues the entire batch at once. The wire path is bounded (`MaxBatchBytes`), but the oplog write itself is not paced, and no measurement exists for batch size or drain time at target scale. Candidate: measure on the rig or in the load harness; consider a per-tick delete ceiling if drain time is material. | Before enabling replication on a site with a large existing `site_events` backlog, or the next load-harness session |
|
||||
| 34 | ~~**`site_events` purge burst is unbounded in the oplog and unmeasured at scale**~~ **RESOLVED 2026-08-16 (`d9a35846`) — measured, and paced.** Measured with the REAL `EventLogPurgeService` over a real `RegisterReplicated` LocalDb file + a loopback sync pair at the rig's `MaxBatchSize: 64`: 393 B/row incl. indexes ⇒ a 1 GB event log holds **~2.73 M rows**, and expiring them in one tick took 285 s and queued **2 731 947 oplog rows = 2.7x LocalDb's `MaxOplogRows` default (1 M)** — enough to make the maintenance tick prune the oplog and flag a FULL ten-table snapshot resync, bought by a routine retention purge. The decisive nuance: the *resident* backlog is produced−minus−drained, so a healthy peer blunts it (391 k peak on loopback, drained 6.0 min) — the old safety margin was a function of peer throughput and vanished with the peer down. Fix: `MaxPurgedRowsPerTick` (default **250 000**, `0` = unlimited escape hatch), one budget SHARED across retention + cap trim; a ceiling-limited tick reschedules after `PurgeCatchUpInterval` (**5 min**, validated `> 0` and `<= PurgeInterval`) so worst-case retention converges in ~55 min instead of 11 days; purge loop is now a delay loop anchored to the END of the previous purge; rate-limited Warning + counter `scadabridge.site.event_log.purge_ceiling_hits`. No purge-exemption path added. Row-33 interplay verified empirically (500 k drained in 46 s vs the 15-min dwell). Fail-before: `PurgeBudget` neutralised → 6 of the new tests fail, all 19 pre-existing pass. Measurement + decision record: `docs/plans/2026-08-15-site-events-purge-burst-measurement.md`. Side-finding recorded as row 81 (storage cap unenforceable on a replicated node). | Found by R7 | Closed 2026-08-16. | — |
|
||||
| 35 | ~~**`DeploymentRecords` index does not cover the deployment-list sort key**~~ **RESOLVED 2026-08-15 (`be29bf47`).** Migration `20260815165452_AddDeploymentRecordListSortIndex` + hand-written `docs/plans/sql/AddDeploymentRecordListSortIndex.sql`: composite `IX_DeploymentRecords_DeployedAt_Id (DeployedAt DESC, Id DESC)`; the old single-column index DROPPED as strictly redundant (`Id` is the clustered PK and was already in the old index's leaf as the row locator — same leaf width, now a declared ordered key; keeping it would cost a second index on every insert/status transition for no read benefit). Serves the three reads sharing the key (`QueryDeploymentListPageAsync`, `QueryDeploymentSummariesAsync`, `GetCurrentDeploymentStatusAsync`). `Id` in the key is also a CORRECTNESS matter: a bulk site deploy stamps many rows in one `DeployedAt` instant, so without the tie-break keyset pages can repeat or drop rows. Recorded caveat: the optimizer may already have served the ordering via a backward scan of the old index, so the "sorts per page" premise was possibly optimizer-dependent — the composite makes it deterministic and self-documenting; an actual plan capture against the rig would quantify the before/after. The ConfigurationDatabase suite exercises the descending composite as real DDL via SQLite `EnsureCreated()`, proving provider portability. | Found by R3 (`35ce1413`) | Closed 2026-08-15. | — |
|
||||
| 36 | **Deployments free-text search is unindexable by construction** | Found by R3 | Four `LIKE '%term%'` predicates (instance name, deployment id, revision hash, user) — leading wildcards preclude index seeks, so a search is a scan of the join. Acceptable at the purge-bounded row count. Fix if hot: anchor the id/hash legs as prefix matches, or full-text. *(Low)* | Search latency complaints on the Deployments page |
|
||||
| 37 | **Two paged reads of `DeploymentRecords` with different contracts** | Found by R3 | `QueryDeploymentSummariesAsync` (CLI/ManagementActor: offset, no total, no instance join, instance-id-list scoping) and R3's `QueryDeploymentListPageAsync` (UI: total + instance join + site-id scoping) duplicate query logic. Consolidating on one contract — giving the CLI a total, both scoping by site — removes the duplication. Genuinely different contracts today, so not urgent. *(Low)* | Next time either query's shape changes |
|
||||
@@ -55,7 +55,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 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)*~~ **RESOLVED 2026-08-15 (`92fe5acc`).** Preferred fix done: both actors now have exactly ONE constructor (`IServiceProvider`); 12 test call sites folded onto stub providers via two `TestSupport` helpers. Also deleted as newly-dead: `OnCachedTelemetryWithoutDualWriteAsync`, `SiteCallAuditActor`'s `_backgroundTimersEnabled` (a constant once the test ctors went), and the private `SerializedRepository` — moved into the test project, since production resolves a fresh repository per scope and the concurrency hazard existed only for fixtures sharing one instance. `CaptiveDependencyGuard` allowlist is now EMPTY; the staleness self-test replaced by two stronger pins — `Allowlist_IsEmpty` plus `FormerlyAllowlistedActors_NowScanClean`, which scans both actors with the allowlist DISABLED, so emptiness is evidence the fields are gone, not that the scan stopped looking. Control: re-adding a field fails both pins and the real scan. | Row-50 guard allowlist | Closed 2026-08-15. | — |
|
||||
| 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 |
|
||||
| 62 | ~~**Five SiteRuntime actors resolve `ISiteEventLogger` off the root provider without a scope** *(Low)*~~ **RESOLVED 2026-08-16 (`9d668861`) — pinned, and the row undercounted: there are SIX sites.** `SiteEventLoggerLifetimePinTests` (Host tests, registration-only inspection, no provider built) pins TWO facts — the interface registration is a *factory* forwarding to concrete `SiteEventLogger`, so pinning the interface lifetime alone doesn't prove callers share an instance; the concrete registration is pinned too, and flipping only the concrete to Scoped is caught ONLY by the second pin (verified both directions, registration restored). The pin's failure message names all six root-resolve sites: the five in this row plus `SiteRuntime/Scripts/ScriptRunLauncher.cs:224` — the run body WP3.1 lifted verbatim out of the deleted execution actors, which inherited the pattern unnoticed. | Row-50 guard allowlist | Closed 2026-08-16. | — |
|
||||
| 63 | ~~**Rig site containers run without `ASPNETCORE_ENVIRONMENT`, so the row-50 Development validation never fires there** *(Low, rig config)*~~ **RESOLVED 2026-08-15 (`0d2d6de6`).** Set on all six `docker/` site containers and both `docker-env2` site containers, mirroring the centrals; both compose files re-parsed to prove well-formedness; the now-stale `Program.cs` comment corrected. A captive dependency in the site graph now fails the container at boot on the rig, not just in CI. | Row-50 implementation | Closed 2026-08-15. | — |
|
||||
| 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 |
|
||||
@@ -63,16 +63,20 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 67 | ~~**Startup reconcile stores a fetched instance config but does not start the InstanceActor** *(Medium-Low, availability)*~~ **RESOLVED 2026-08-15 (`482165cb`).** Root cause: InstanceActors are created from exactly two paths — the boot-time SQLite load in `DeploymentManagerActor.PreStart` and an explicit deploy/enable command — and the reconcile pass fires ~5s AFTER the boot load with a pure file write (`StoreDeployedConfigIfNewerAsync`) as its only side effect. Fix: reconcile collects the enabled instances it stored and Tells a new `EnsureInstanceStartedCommand` to the DM **singleton proxy**, so the actor is created under the singleton no matter which node's pass found the gap. Handler is advisory and idempotent: no-op if already running; re-reads the durable row (authority on config body AND `is_enabled`, so a disable landing after the fetch wins); defers to `_deployWarms`/`_terminatingActorsByName`/`_inFlightDeployments` so deploy-gate ordering and all-or-nothing-per-instance are untouched (it starts an already-persisted config, never applies one); quiet no-op on an absent row (normal on an unreplicated site whose peer healed its own file). The heal is no longer silent — a Warning site event `Instance <X> started by startup reconcile self-heal` fires. Fail-before by behavioral revert: 3 of 8 new tests fail pre-fix, including the exact live symptom (`debug snapshot` → `instanceNotFound`); the 5 negative guards (disabled / failed-fetch / unknown / idempotence) green in both states. SiteRuntime 613/613, Host 504/504. Sibling gaps found during the fix recorded as rows 72/73. | Proven live during the rows-46–53 close-out (2026-08-15) | Closed 2026-08-15. | — |
|
||||
| 69 | ~~**Both site-a nodes' LocalDb files corrupt — replication down, one node crash-looping**~~ **RESOLVED 2026-08-15 (operational recovery, no code change).** Found live during this wave: site-a-b threw `SQLite Error 11: 'database disk image is malformed'` on every sync call (replication `connected: false`, backlog 892, faulted session retried ~every 60s) and, after the redeploy, site-a-a crash-looped (SIGABRT ×12) on the same error at `SqliteLocalDb` construction — the new boot-time re-registration reads pages the old build never touched, so site-a-a's corruption (present but latent) became fatal only on upgrade. Recovery: pair stopped together, both files quarantined (`*.corrupt-2026-08-15`, kept for forensics; 394 MB / 416 MB), fresh files reseeded — site-a-b via its startup reconcile (which live-proved the row-67 fix, see row 67) and CDC snapshot resync; converged to `connected: true`, backlog 0, with the row-27 schema change proving the digest handshake green. Suspected cause is host-level, not product: see row 79. | Found live by the row-55 debug + this wave's close-out redeploy | Closed 2026-08-15. | — |
|
||||
| 70 | **`auto-down-unreachable-after` is deprecated — Akka warns "keep-majority will be used instead" on every boot** *(Medium info — verified NOT currently true, but a live removal risk)* | Row-55 live session; investigated + drill-verified at this wave's close-out | The warning is real (fires at Cluster start on every node, Akka.NET 1.5.62 — same version as the 2026-07-21 decision, so nothing changed underneath; it fires on the presence of the `auto-down-unreachable-after` key). **Drill-proven that the substitution does NOT happen when `downing-provider-class = "Akka.Cluster.AutoDowning"` is EXPLICIT (as `BuildHocon` sets):** `DRILL_MODE=active` kills central-a (oldest AND lowest address) — under keep-majority a 2-node even split keeps the lowest-address side, so surviving central-b would have downed ITSELF (total outage); instead central-b observed OldestChanged → became Oldest, took over all singletons, and the victim rejoined ready 2s after restart (2026-08-15 drill, post-merge rig). The 2026-07-21 decision therefore still holds exactly as documented. The residual risk is the deprecation itself: a future Akka.NET bump can delete `AutoDowning`, silently or loudly breaking the availability-first strategy. | **Any Akka.NET version bump** — re-run `DRILL_MODE=active bash docker/failover-drill.sh` and re-read the release notes before accepting it |
|
||||
| 71 | **Standby nodes log the active-node health check at ERR every 10s** *(Low, log noise)* | Row-55 live session (2026-08-15): site-a-b logs `Health check active-node with status Unhealthy … 'not the site pair primary (standby)'` at Error level every 10s | Correct-by-design standby behavior emitted at an alarming severity — floods the standby's log and trains operators to ignore ERR. Candidate: filter/downgrade the ASP.NET health-check publisher's severity for the expected-standby case. | Next observability/log-hygiene session |
|
||||
| 72 | **Reconcile refreshes a STALE config but does not restart the running InstanceActor** *(Medium-Low — same silent-divergence class as row 67, arguably wider blast radius)* | Row-67 fix session (2026-08-15): `ReconcileGapItem` covers both *missing* and *stale* (`StaleInstance_IsRefreshed` pins a revision bump); for a stale item the new config is written to SQLite while the running actor keeps serving the OLD compiled config until the next restart or redeploy | Deliberately out of row-67's scope: restarting a LIVE instance from reconcile carries redeploy semantics (stop/start, subscription re-establish, in-flight runs) that need their own design decision. Nothing surfaces "stored revision ≠ running revision". | Next SiteRuntime/reconcile session, or first field report of an instance serving a stale config after a central redeploy raced a site outage |
|
||||
| 73 | **`SiteReconciliationActor` is silently skipped when `IDeploymentConfigFetcher` is null** *(Low)* | Row-67 fix session: `AkkaHostedService` (~line 1046) logs only a Warning and spawns no reconciler | The entire self-heal path — file heal AND the new row-67 runtime heal — can be absent with no health-report surface; a misconfigured site looks healthy while unable to heal. Candidate: surface reconciler presence on the site health report. | Next health-report or reconcile session |
|
||||
| 74 | **`BundleSessionObservabilityTests` gauge tests assert absolute values — latent row-32-class race** *(Low, test-only)* | Row-66 implementation (2026-08-15): the pre-existing gauge tests `Assert.Equal(0, baseline)` against a process-global provider slot, safe only because Transport is that slot's sole registrant; the new rejection-counter tests use delta-over-scoped-action with unique keys instead | Same shape as the row-32 flake — becomes flaky the day another suite registers the slot. The fix pattern already exists in the same file. | First flake, or next test-infra session |
|
||||
| 75 | **SecuredWrites Approve renders as a red destructive button** *(Low, UI)* | Row-28 audit (2026-08-15): the call site passes `danger: true`, so Approve is styled destructive; the label is now correct ("Approve") but the styling still signals delete | Approving a secured write DOES execute a device write, so a caution affordance is arguable — which is why row 28 fixed only the label. A deliberate styling decision belongs to whoever owns the page. | Next Secured Writes / Central UI session |
|
||||
| 76 | **CLI README's `audit-config query` JSON sample omits `afterStateJson`/`bundleImportId`** *(Low, docs)* | Row-57 fix (2026-08-15): the real payload returns both fields; the sample doesn't show them | Misleading as written; one-line doc fix, out of row-57's table-projection scope. | Next CLI docs touch |
|
||||
| 77 | **Test pattern: reflecting `ActorCell.Actor` right after `ResolveOne` is unordered vs child construction** *(Low, test-infra)* | Row-40 session (2026-08-15): `InstanceActorChildAttributeRaceTests` failed ~1-in-6 under full-suite load once the sleeps stopped padding the schedule (latent — A/B'd 14/14 green on the unmodified tree); fixed with a bounded-deadline resolution retry | `ResolveOne` throws until the child's cell publishes on the parent, and `cell.Actor` stays null until the child's ctor runs on its own mailbox thread — neither ordered against the test thread. The one instance is fixed; the pattern may exist elsewhere. | Next time a child-resolution test flakes under load |
|
||||
| 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)* | This wave's close-out: site-a-b's reconcile fetched 4 configs at its solo boot; when the pair was then restarted with a fresh site-a-a file, CDC snapshot replication seeded site-a-a's `deployed_configurations` within ~5s of boot — BEFORE its own reconcile ran, so central rightly reported no gap ("0 fetched"), and nothing started the actors: enabled instances sat dead with their configs present (verified `instanceNotFound`) until one more pair restart | The row-67 fix covers configs the reconcile itself fetches; configs that arrive by replication (fresh standby seeded from a peer, or any post-boot snapshot resync) hit the boot load too late and the reconcile not at all. Fix shape: the row-67 `EnsureInstanceStartedCommand` machinery already exists — the missing piece is a trigger when replication writes a `deployed_configurations` row for an enabled instance with no running actor (e.g. a periodic sweep in the DM singleton, or a LocalDb change hook). | Next SiteRuntime/reconcile session — pairs naturally with row 72 |
|
||||
| 79 | **OrbStack virtiofs is a live corruption + availability hazard for the rig's bind-mounted SQLite files** *(Medium, rig infra — host-level, not product)* | This wave's close-out: while verifying, the OrbStack VM idle-SLEPT mid-session (`vmgr … msg=sleep`) leaving docker hung/empty for ~20 min until a manual `orb restart docker`; on wake the virtiofs layer logged repeated `refresh_nodeid: race with new lookup` ERRORs. Both site-a LocalDb files (row 69) were corrupted through this same bind-mount layer — macOS virtiofs does not give SQLite true fsync durability, and abrupt container kills (`--force-recreate`) mid-write compound it | Every ScadaBridge data file on the rig (`site-localdb.db`, `auditlog.db`, secrets) rides a relative bind mount through virtiofs. Candidates: move rig data dirs to named docker volumes (native ext4 inside the VM — but loses host-side inspectability the sessions rely on), disable OrbStack idle-pause, or accept + document the quarantine-and-reseed recovery (now proven: row 69). | Next corruption occurrence, or next rig-infra session |
|
||||
| 80 | **Unreplicated sites report NON-NULL `LocalDbReplicationConnected: false` / `LocalDbOplogBacklog: 0` — contradicting the documented nullable semantics** *(Low-Medium, observability correctness)* | Found at this wave's close-out verifying row 33: site-b/site-c (deliberately unreplicated) health reports carry `false`/`0`, not null — yet CLAUDE.md and the health docs state "both nullable — null means no data, NOT disconnected with an empty backlog" | Two consequences: a dashboard reader sees `connected: false` on a healthy unreplicated site and reads it as a fault; and row 33's null-branch (conditional KPI emission, badge anchor exclusion) never gets its null on the rig, so unreplicated sites emit misleading `localDbReplicationConnected=0` KPI series (the badge stays safe only because a 0 backlog can't sustain). Root cause is site-side report assembly supplying defaults instead of nulls when replication is unconfigured. Fix: thread `SiteLocalDbSetup.ReplicationIsConfigured` into the health report so unconfigured ⇒ null, and the row-33 machinery then behaves as designed. | Next health-monitoring session |
|
||||
| 71 | ~~**Standby nodes log the active-node health check at ERR every 10s** *(Low, log noise)*~~ **RESOLVED 2026-08-16 (`8b761841`).** One narrowly-keyed Serilog `Filter.ByExcluding` in `LoggerConfigurationFactory`: `Level == Error` AND the `DefaultHealthCheckService` category AND `HealthCheckName == "active-node"` — check-NAME keying is what preserves the constraint (`localdb`/`akka-cluster`/`database`/`required-singletons` keep their Error lines, and active-node has no error path of its own on the active side, so the only suppressible event is a standby reporting it's a standby). Rejected on evidence: `MinimumLevel.Override` can only raise a floor (can't rewrite Error→Information), and reporting `Degraded` would change the HTTP status `/health/active` publishes, which is load-bearing for Traefik. Bonus reach: the CENTRAL standby was logging the same Error 2x as often (Traefik probes `/health/active` every 5 s) — covered by the same filter. Fail-before: filter disabled → `StandbyActiveNodeVerdict_IsNotLogged` fails; the 9 must-still-log constraint tests pass either way, proving narrowness. Side-finding: the observed ~10 s SITE-side cadence comes from an out-of-repo prober (nothing in this repo probes a site's `/health/active`), and CLAUDE.md's "site nodes map no `/health/*`" claim was stale — corrected. | Row-55 live session (2026-08-15) | Closed 2026-08-16. | — |
|
||||
| 72 | ~~**Reconcile refreshes a STALE config but does not restart the running InstanceActor** *(Medium-Low — same silent-divergence class as row 67, arguably wider blast radius)*~~ **RESOLVED 2026-08-16 (`e606b24a`, with row 78 — one fix, the DM instance sweep).** Design decision made and recorded (`docs/plans/2026-08-15-instance-sweep-self-heal-design.md`): **converge, don't merely warn** — the sweep re-applies the stored config through **`HandleDeploy`**, the exact path a central redeploy uses, so the compile gate, warm-then-gate ordering, redeploy-supersede, static-override/native-alarm reset and all-or-nothing-per-instance are the production ones (a stop still does not cancel an in-flight run). Rationale: the stored row IS what central successfully deployed; surface-only leaves the operator to hand-issue the very redeploy the sweep would perform. Ordering hazards checked: the sweep never acts on an instance a deploy pipeline owns; a gate-rejected restart is inert, not partial; the synthetic deploy replies to `Self`, never `NoSender` (dead letters are a health metric). "Nothing surfaces it" is now false in BOTH paths — even a failed restart yields two `deployment` site events naming both revisions. Safety property beyond the brief: **one repair attempt per (instance, revision) pair**, so the self-heal can never become a restart loop for an init-crashing actor or a config the gate can never accept. Fail-before: sweep unarmed → `Sweep_RestartsInstance_ServingAStaleRevision` fails showing rev-1's value served while the store holds rev-2. | Row-67 fix session (2026-08-15) | Closed 2026-08-16. | — |
|
||||
| 73 | ~~**`SiteReconciliationActor` is silently skipped when `IDeploymentConfigFetcher` is null** *(Low)*~~ **RESOLVED 2026-08-16 (`a2f0086f`, with row 80).** New tri-state health-report field **`StartupReconcilerActive`** (`bool?`, matching the `LocalDbReplicationConnected` nullable precedent; additive proto **field 29**, vendored C# regenerated via `docker/regen-proto.sh centralcontrol` with the csproj verified restored): null = not reported (central self-report or pre-row-73 site build), false = booted with no fetcher. Latched once at boot by the Host in both branches. The skip branch was raised **Warning → Error** (no configuration legitimately omits the fetcher on a site node) plus a best-effort `deployment`/`Error` site event so evidence outlives the boot log. Central treatment: a **"No self-heal" Warning badge**, deliberately not a failure — config still arrives via replication while the node is up; what's lost is recovery from a deploy landing while it's down. Only explicit `false` is badged (badging null would light the fleet the moment central upgrades ahead of the sites); offline sites skipped, matching the backlog badge. | Row-67 fix session | Closed 2026-08-16. | — |
|
||||
| 74 | ~~**`BundleSessionObservabilityTests` gauge tests assert absolute values — latent row-32-class race** *(Low, test-only)*~~ **RESOLVED 2026-08-16 (`98c94588`).** Both gauge facts converted to delta-over-scoped-action via a `ReadGaugeDelta` helper — and two assertions got STRONGER: the post-clear check (`Assert.Equal(0, …)` — satisfied by any unregistered slot) now asserts opens on the detached store move the gauge by nothing (the actual claim), and the compare-and-clear fact gained a negative control (the superseded store's opens must NOT reach the instrument), without which it passed by coincidence whenever both stores held equal counts. Fail-before: a temporary foreign-provider probe returning 7 pre-start — old body fails `Expected: 0 / Actual: 7`, new body passes the identical scenario (probe deleted, not committed). Documented residual: a registrant clobbering the slot BETWEEN the two reads still needs row-32's mutual-exclusion pattern; cannot arise today (sole constructor of `BundleSessionEvictionService` in the assembly). | Row-66 implementation (2026-08-15) | Closed 2026-08-16. | — |
|
||||
| 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<T>` 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. | — |
|
||||
| 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 |
|
||||
| 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)
|
||||
|
||||
Reference in New Issue
Block a user