docs(register): record Low-tail sweep dispositions — close rows 58 (mutation-verified retention pins), 60 (premise corrected: DTOs not EF entities; lazy-loading pin), 65 (trigger fired — prune rides BundleSessionEvictionService); refresh rows 36/37/59/64 with re-verification evidence; add rows 86-88 (CLI/UI deployment scope divergence, CLI unbounded non-transactional scoping, silent --status parse fallback)
This commit is contained in:
@@ -29,8 +29,8 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 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**~~ **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 |
|
||||
| 36 | **Deployments free-text search is unindexable by construction** | Found by R3; re-verified 2026-08-17 (Low-tail sweep) | 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 (`DeploymentManagerRepository.cs:100-104`; only searched column with any index is `DeploymentId`, unseekable under a leading wildcard). Acceptable at the purge-bounded row count. **2026-08-17 refresh: the row undercounted the cost by half** — `BuildListQuery` is shared with `GetDeploymentStatusCountsAsync`, so one search = TWO scans of the join (list + status tiles), mitigated by the UI's trailing-edge debounce (one pair per pause, not per keystroke). Anchoring the id/hash legs was evaluated and REJECTED as a micro-fix: `DeploymentListFilter.Search`'s doc comment promises substring matching and a 4-case `[Theory]` deliberately pins mid-string matches — changing it is a contract renegotiation, not an optimization. Fix if hot: renegotiate to prefix on id/hash legs, or full-text. *(Low)* | Search latency complaints on the Deployments page |
|
||||
| 37 | **Two paged reads of `DeploymentRecords` with different contracts** | Found by R3; re-verified 2026-08-17 (Low-tail sweep: both shapes intact, `DeploymentManagerRepository.cs:110-149` and `:399-449`) | `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. **2026-08-17: the duplication has now DRIFTED into real behavioral divergences, recorded as rows 86–88 (site-scope authorization divergence, CLI unbounded read + non-transactional scope resolution, silent `--status` parse fallback) — a consolidation should resolve those rows with it.** *(Low structurally; the drift itself is row 86, Medium)* | Rows 86–88, or next time either query's shape changes |
|
||||
| 38 | **OtOpcUa: `RoslynVirtualTagEvaluatorTests` racing-clear test is load-fragile** (OtOpcUa repo, not this one) | Found by R4 during the full-suite bump verification | `Evaluate_racing_ClearCompiledScripts_never_fails_with_disposed` fails under full-solution parallel test load, passes 3/3 isolated: its design assumes at most one `ClearCompiledScripts` lands inside one evaluation (so the product's one-shot retry suffices); under contention two can. Same timing-assumption family as the R5 sweep, but lives in OtOpcUa. Test-side fix: bound clear cadence relative to observed evaluation latency, or assert on retry-exhaustion instead of zero failures. | Next OtOpcUa test session, or when it fails a run that matters |
|
||||
| 39 | **OtOpcUa: late-enabled replication cannot baseline pre-existing rows** (OtOpcUa repo, not this one) | Found by R4; documented in OtOpcUa's CLAUDE.md | `LocalDbSetup.OnReady` registers all four tables unconditionally on every driver node, so it can never pass `baselineExistingRows: true` (only correct when gated on replication being configured). Turning replication ON for a long-running default-OFF node converges on subsequent writes only — existing rows are never baselined. ScadaBridge closed this class with conditional registration + boot-time `DeregisterReplicated` self-heal (WP1.3/WP3.3); porting it is a behavior change beyond a pin bump (registered-set pinned by tests, deregistration must be pair-symmetric). Low current impact: site-a was enabled from fresh. | Before enabling replication on an established OtOpcUa node |
|
||||
| 40 | ~~**Wall-clock sleeps used as synchronization in tests (absence-assertion class)**~~ **RESOLVED 2026-08-15 (`7a7d345b`).** All six listed sites replaced with observable edges + positive controls, strengthening proven by stash-based negative controls: `ScriptDeadlineAtEnqueueTests:128` now pins the watchdog via a genuinely-wedged positive control (armed strictly later, self-releasing) + per-script absence assertion + detach counter pinned at 1 — **disabling the entire stuck-script watchdog passed the OLD test 2/2 and fails the new one**; `:105` now proves the run was still queued when its budget expired via `QueueDepth` edges (the actual design claim — the old sleep asserted nothing about it); `DeploymentWarmThenGateTests:117` (+2 sibling `Task.Delay(500)`s) — the empty-site zero is now the manager's own observation (phantom-startup control: old passed 2/2, new fails); `SiteAlarmLiveCacheServiceTests:195,216` poll a new subscription-cancellation observable across 3 linger windows, each closing with a positive control (control: linger cancellation disabled + viewer guard dropped — `Resubscribe_Within_Linger` old passed 2/2, new fails; honest caveat: the other case's old later assertion did already catch that break, so its gain is the observable + control, not a false-pass fix); `DataConnectionActorTests:435` waits for the Connected health publication so the subscribe provably runs against Connected (the sleep guarded no assertion — determinism gain only). Bonus: the de-padded schedule exposed a LATENT race in `InstanceActorChildAttributeRaceTests` (~1-in-6 under load, A/B'd 14/14 green on the unmodified tree so pre-existing) — fixed with a bounded-deadline resolution retry; pattern recorded as row 77. | Found by the R5 sweep (`9fb52153`) | Closed 2026-08-15. | — |
|
||||
@@ -51,14 +51,14 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 55 | ~~**`tagResolutionCounts` reads `{totalSubscribed:0, successfullyResolved:0}` for a connection with 2 live Good-quality subscribed tags** *(Low)*~~ **RESOLVED 2026-08-15 (`5da27e05`).** All three suspected reporting-path hypotheses were WRONG — live debugging showed the zeros and the `good:2` on the SAME report from the SAME collector on the active node, ruling out standby shadowing, keying races, and the central merge in one stroke. The real bug is a DCL omission, not a regression: `DataConnectionActor.HandleSubscribeCompleted` — the first-time, everything-resolves-cleanly subscribe path — was the missing FOURTH `UpdateTagResolution` push site. `BecomeConnected`'s legitimate 0/0 push fires before the stashed subscribe unstashes (proven in live logs: "Entering Connected state" follows the subscribe line), `ScheduleTagResolutionRetry` cancels outright at zero unresolved, and any reconnect masks the bug via the batch path — so the diagnostic was blindest on a perfectly HEALTHY connection, exactly when it should read clean rather than empty. One mirrored push + regression test (stash-verified fail-before: exactly one `0/0` call received pre-fix while the actor's own `GetHealthReport` already said 2/2, pinning the actor-correct/collector-stale asymmetry). Also corrected `Component-DataConnectionLayer.md`, which claimed the counts were "derived at report time" — they are pushed, and a cached push is only as fresh as its last push site. | Row-48 live probe; root-caused live 2026-08-15 | Closed 2026-08-15. | — |
|
||||
| 56 | ~~**The rig's central nodes expose no Prometheus endpoint** *(Low, rig config)*~~ **RESOLVED 2026-08-15 (`103f2f19`).** Premise refined on inspection: central already served `/metrics` on its web port (anonymous, no fallback policy) — what was missing was the dedicated 8084 listener the documented docker-exec scrape recipe targets. Both rigs' central appsettings now set `MetricsPort`, and the Host binds it as a PRESENCE check, not a defaulted read — `NodeOptions` defaults to 8084, so a defaulted read would make every existing production central surprise-bind a new port on upgrade, and port-in-use is a hard boot failure. | Row-48 live probe | Closed 2026-08-15. | — |
|
||||
| 57 | ~~**`audit-config query` has no CLI table projection** *(Low, cosmetic)*~~ **RESOLVED 2026-08-15 (`b2cab59c`).** `AuditConfigTableProjection` mirrors `TemplateTableProjection`: unwraps the `{entries, totalCount}` envelope to rows (`id, timestamp, user, action, entityType, entityName, entityId`), drops the fat `afterStateJson`, adds `--detail` to skip projection (matching `template list`), and surfaces `totalCount` as a trailing note row (`showing N of M — use --page/--page-size`) emitted ONLY when the page is a strict subset, with no `id` cell so it can't be mistaken for an entry. The deprecated `audit-log` alias shares the command tree and gets the projector too. Tests serialize the REAL `AuditLogQueryResult`/`AuditLogEntry` types, so a server-side property rename fails the test instead of silently blanking a column. Stale-README follow-up recorded as row 76. | Row-46 fix (2026-08-15) | Closed 2026-08-15. | — |
|
||||
| 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 |
|
||||
| 58 | ~~**`IDeploymentStatusNotifier` singleton retains delegates targeting circuit-scoped Blazor components** *(Low)*~~ **RESOLVED 2026-08-17 (`2807b5a3`).** Still exactly one subscriber (`Deployments.razor`), but the cited pin was BLIND: `Deployments_Dispose_UnsubscribesFromNotifier` was satisfied by the `_disposed` guard alone — mutation-verified (with the `-=` deleted, the old test stayed green while the leak was wide open). Closed by two real pins: `DeploymentsPushUpdateTests.Deployments_Dispose_ActuallyDetachesTheDelegate_NotJustGuardsIt` (structural — the invocation list must shrink to 0) and `EveryCentralUiSubscriberToTheNotifier_UnsubscribesAndIsDisposable` (a source scan pinning this row's own trigger: any new subscriber without a matching Dispose unsubscribe fails the suite). Both fail on the mutation. | Row-49 sweep (CentralUI-006 re-verification) | Closed 2026-08-17. | — |
|
||||
| 59 | **`SiteCallAuditActor` per-tick scope spans all sites × ≤50 upsert pages** *(Low)* | Row-49 sweep (SiteCallAudit-009 re-verification); re-verified 2026-08-17 (Low-tail sweep — loop still serial, line numbers drifted to `SiteCallAuditActor.cs:528-545`) | Safe only because site reconciliation is a serial `foreach … await`; one scope across a parallel fan-out would be row 45 again. 2026-08-17: the missing in-code constraint remark was added at the loop (`2807b5a3` — "serial by design … do not fan out"); a source-scan pin was judged too brittle, so the comment + this row remain the tripwire. | `ReconcileSiteAsync` is ever fanned out with `Task.WhenAll` |
|
||||
| 60 | ~~**`ArtifactDeploymentService.GlobalArtifactSnapshot` holds EF-materialised entities across the parallel per-site send** *(Low)*~~ **RESOLVED 2026-08-17 (`2807b5a3`) — with a premise correction so the row is never reopened on the wrong reasoning.** The row's premise was factually wrong: `GlobalArtifactSnapshot` holds pure Commons DTOs, not EF entities (`ArtifactDeploymentService.cs:170-212`); the EF state that actually crosses the fan-out is the tracked `Site` entities the per-site lambda closes over. The safety conclusion still holds — commands are built serially before the fan-out and lazy loading is confirmed absent — and the load-bearing half of the trigger is now permanently pinned by `ConfigurationDatabase.Tests.AddConfigurationDatabase_DoesNotEnableLazyLoading`. | Row-49 sweep (DeploymentManager-023 re-verification) | Closed 2026-08-17. | — |
|
||||
| 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)*~~ **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 |
|
||||
| 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 (re-verified 2026-08-17: `Component-Transport.md:341`, `docs/components/Transport.md:140`, plus the code XML doc) | 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)*~~ **RESOLVED 2026-08-17 (`2807b5a3`) — the revisit trigger had FIRED.** Transport gained exactly the central scheduler the trigger named: `BundleSessionEvictionService` (central-only `BackgroundService`; `AddTransport` is called only from the central branch of `Program.cs`). The prune tick now rides it, so an idle cluster's rows age out without an unlock ever occurring; the opportunistic unlock-path prune remains. Operational note: the service now has two maintenance duties, deliberately in separate `try` blocks so neither can mask the other. Tests: `FleetWideUnlockThrottleTests` additions + `BundleSessionObservabilityTests` extension, both green. | Row-52 implementation trade | Closed 2026-08-17. | — |
|
||||
| 66 | ~~**No counter for import-session cap rejections** *(Low)*~~ **RESOLVED 2026-08-15 (`90ce0999`).** `scadabridge.central.transport.import_sessions.rejected` on the existing allowlisted meter, emitted ONLY at the post-eviction recheck — the cap is tested twice and a store full of expired sessions reclaims and succeeds, so counting the first test would fire routinely on any node whose sweep hadn't run; duplicate-session-id re-`Open` is also excluded. New tests use delta-over-scoped-action with unique per-test keys (the pre-existing gauge tests' absolute asserts are row 74). | Row-53 implementation note | Closed 2026-08-15. | — |
|
||||
| 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. | — |
|
||||
@@ -78,6 +78,9 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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