From 4cf71e92a5ed41241eedbcde28781143e0b21180 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:44:08 -0400 Subject: [PATCH] =?UTF-8?q?docs(register):=20close=20row=2039=20=E2=80=94?= =?UTF-8?q?=20OtOpcUa=20conditional=20LocalDb=20registration=20+=20baselin?= =?UTF-8?q?e/deregister=20self-heal=20ported=20cross-session=20(3e62f161,?= =?UTF-8?q?=20df533916)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/2026-07-08-deferred-work-register.md | 2 +- 1 file changed, 1 insertion(+), 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 a1efe74f..75eda580 100644 --- a/docs/plans/2026-07-08-deferred-work-register.md +++ b/docs/plans/2026-07-08-deferred-work-register.md @@ -32,7 +32,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round | 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**~~ **RESOLVED 2026-08-17 (merge `ad0100f2`, agent commit `9f67147e`).** `QueryDeploymentSummariesAsync` and `DeploymentRecordSummary` are DELETED; the CLI/HTTP `QueryDeployments` handler now rides `QueryDeploymentListPageAsync` + `DeploymentListFilter` — one row shape (`DeploymentListRow`), one scoping mechanism (site-id via the query join), total count available to both surfaces (`DeploymentListPage` now also owns `MaxPageSize` + `DefaultPageSize = 50`). Rows 86–88 were resolved with the consolidation, as this row anticipated. Row 36's 4-case substring-search Theory untouched and still passing. Remaining UI-vs-CLI asymmetry (`GetDeploymentStatusCountsAsync` status tiles are UI-only) is deliberate, not drift. **Live-probed on the rig:** CLI `deploy status --status Success --page-size 5` returns the consolidated page — rows now carry `siteId` + `instanceUniqueName`, and the response carries `totalCount` (11) the CLI never had. | Found by R3; consolidation shipped with rows 86–88 | Closed 2026-08-17. | — | | 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 | +| 39 | ~~**OtOpcUa: late-enabled replication cannot baseline pre-existing rows**~~ **RESOLVED 2026-08-18 in OtOpcUa (`3e62f161` + review round `df533916`, branch perf/followups-r3).** ScadaBridge's WP1.3/WP3.3 pattern ported: registration follows replication config — configured ⇒ `RegisterReplicated(..., baselineExistingRows: true)` (probed live on 0.2.1: seeds `__localdb_row_version` at HLC 0), unconfigured ⇒ boot-time `DeregisterReplicated` self-heal, pair-symmetric; review round added deregister-preserves-data + partial-flip-fails-loudly tests. Green: `LocalDbSetupTests` 15/15, Host.IntegrationTests 226 pass/0 fail. **Stayed pinned to LocalDb 0.2.1 by deliberate choice** — the 0.2.1 API already carries both needed methods, and the v2→v3 one-way bookkeeping upgrade makes a 0.3.x bump a both-nodes-together operation; recorded caveat in OtOpcUa's CLAUDE.md: on 0.2.1 the snapshot-resync flag is per-DATABASE, so a late enable snapshots the whole file rather than per-table (0.3.1 available; bump logged as a deliberate future change in OtOpcUa's own follow-up register). Commit stats cross-checked from this repo. | Found by R4; fixed cross-session by the OtOpcUa agent 2026-08-18 | Closed 2026-08-18. | — | | 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. | — | | 41 | ~~**LocalDb (scadaproj) sync-session tests flake under parallel load**~~ **RESOLVED 2026-08-18 in scadaproj (`3e5a5fd`, LocalDb 0.3.1) — hypothesis CONFIRMED as a product bug, fixed on the product path.** The enqueue→receive→validate→fault path in `HandshakeAsync` has no yields while the writer's wakeup sits in the thread-pool queue, so the catch's `linked.Cancel()` beat the writer 39/40 times in a new pinning test (`Handshake_FaultTeardown_StillTransmitsTheLocalHandshake`) — mismatch detection is symmetric (each side computes its verdict FROM the other's Handshake), so the healthy peer learned nothing: 15 s stall in tests; in production a permanent schema mismatch would be misclassified as a generic disconnect and reconnect-loop at fast backoff instead of the FailedPrecondition/max-backoff path. Fix: `RunAsync`'s fault teardown completes both lanes so the writer drains what's enqueued and exits on its own, with a 2 s deferred cancel as the wedged-transport backstop; caller-initiated cancellation unchanged; **no wire change** (a 0.3.0 peer just finally receives the Handshake it was owed). A/B under doubled load: pre-fix 2/6 full runs failed (exact predicted stack), post-fix 6/6 green (200/200) + 5/5 filtered. The OTHER named test (`BatchBudgetTests.Pump_RowLargerThanTheBudget…`) was a harness defect, not this root cause (scripted peer, no handshake fault): it awaited the peer channel without observing the session task, so any session fault surfaced as an opaque 15 s timeout — hardened with a run-observing `NextAsync` that races the session task at every read site. *Residual (scadaproj-side, pre-existing class):* two non-recurring one-offs in 10 post-fix runs (`Handshake_Exchanged_BothProceed` @53 ms, `Metrics_NullSafe…` @208 ms; detail lost, never recurred in 12 further loaded runs either side) — both share the `SwallowAsync`-catches-only-OCE teardown shape, best guess a cancelled SQLite op surfacing `SqliteException` at teardown; not attributable to this change, not proven either. **0.3.1 published to the feed 2026-08-18** (all three packages, restore-verified from a scratch consumer with a positive control confirming the restored Replication assembly carries the fix; scadaproj main pushed — `3e5a5fd`, `b62981f`, index `dbcbc5b`). ScadaBridge's pin bumped 0.3.0 → 0.3.1 the same day: build clean, all six LocalDb-consuming test suites green (SiteRuntime 652, IntegrationTests 115, HealthMonitoring/ExternalSystemGateway/AuditLog/StoreAndForward all pass); a 0.3.0/0.3.1 mixed pair is wire-safe (teardown-ordering change only). | Found by R1; confirmed pre-existing at base `f627cee`; fixed cross-session by the scadaproj agent 2026-08-18 | Closed 2026-08-18. | — | | 42 | ~~**`build/push.sh`-class publish bug across the family repos**~~ **RESOLVED 2026-08-18 in scadaproj (`b62981f`).** The bug shape: `dotnet pack -c Release -o ./artifacts` with no project argument and no `cd`, so the script packs the CALLER'S CWD. Fixed in LocalDb's script (cd to own repo root, name the solution, absolute output path). **AUDITED 2026-08-15 (read-only, this wave): SEVEN scadaproj scripts still carry the exact shape** — `ZB.MOM.WW.Secrets/build/push.sh`, `ZB.MOM.WW.Theme/build/{pack,push}.sh`, `ZB.MOM.WW.Audit/build/{pack,push}.sh`, `ZB.MOM.WW.Auth/build/{pack,push}.sh` (all `dotnet pack -c Release -o ./artifacts` with no project argument and no `cd`, plus a CWD-relative `nuget push "./artifacts/*.nupkg"` that would publish whatever sits in the caller's artifacts dir). MxAccessGateway's `scripts/pack-clients.ps1` is SAFE (`$PSScriptRoot`-anchored, explicit csproj paths); HistorianGateway and OtOpcUa have no pack scripts. Fix = copy LocalDb's `0038d46` pattern into the seven scripts; lives in the scadaproj repo, outside this wave's edit scope. Fixed: all seven scripts now carry the `0038d46` pattern (cd to own repo root, named `.slnx`, absolute artifacts output, anchored push glob); verified by `bash -n` on all seven plus a positive control on the pack half only (Audit's `pack.sh` run from a foreign CWD packed only `ZB.MOM.WW.Audit` into its own artifacts dir, caller's directory untouched); no push script executed, no credentials touched. Commit stat cross-checked from this repo: exactly the seven listed files. | Found by R1 the hard way (incident: 25 unrelated packages briefly published to the feed, deleted + verified same session); fixed cross-session by the scadaproj agent 2026-08-18 | Closed 2026-08-18. | — |