deps(localdb)+docs: consume LocalDb 0.3.3 (FK-parent registration guard + atomic per-entry apply, arch-review P4-7); Low-wave integration docs — register rows 103 closed / 104 auto_vacuum legacy files / 105 FK-parent constraint; CLAUDE.md bulk-deploy up-to-date filter + --force, 0.3.3 line, infra/LDAP correction (P8-7); plan doc status

This commit is contained in:
Joseph Doherty
2026-08-20 08:56:50 -04:00
parent bbc1b55cc4
commit 09989fdf32
4 changed files with 20 additions and 6 deletions
@@ -95,7 +95,9 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
| 100 | ~~**`SiteScriptCompileCache.Clear()` is process-global — test classes wipe each other's entries**~~ **RESOLVED 2026-08-17 (merge `8a9a8d29`, agent commit `84b405ff`) — premise corrected: there were ZERO production callers of `Clear()`** (the only production entry point is `ScriptCompilationService.CompileCore` → `GetOrAdd`; `Clear()` existed solely for tests), so neither register option was taken as written — `Clear()` was **deleted outright** and the cache became an instance-scoped `sealed class` owned by `ScriptCompilationService`. The N1 "one compile per body per process" property now rests on `AddSiteRuntime` registering that service as a **DI singleton** — a lifetime that became load-bearing, so new `SiteRuntimeServiceRegistrationTests` pins it (a Scoped/Transient downgrade would reintroduce WP3.1's recompile storms with no behavioral test failing). The 37 test sites doing `new ScriptCompilationService(logger)` each silently get their own cache — isolation for the whole assembly, not just the row-91 classes. Row-91 workarounds RETIRED, not preserved: serialized collection removed from all three classes, exact `Hits`/`Count` assertions restored, guid key token gone, `ForeignEntrySlack` 512→8; new `SeparateCaches_DoNotShareEntriesOrCounters` pins the scoping. SiteRuntime.Tests green three consecutive full-parallel runs. | Found by the 2026-08-17 row-91 fix session | Closed 2026-08-17. | — |
| 101 | ~~**`ApplyingArtifacts_RemovedDataConnection_StopsLiveDclConnection` flaked once — suspected overlay-sweep read racing the artifact-apply write**~~ **RESOLVED 2026-08-17 (`0d3a90a9`) — hypothesis VERIFIED, and the production consequence was WORSE than the row guessed.** The race is real: the snapshot is read off the actor thread but compared against the overlay only when the piped message lands, so a read starting before an artifact apply's store writes can land after `HandleApplyArtifactDataConnectionsToDcl` populated the overlay — its stale (even empty) snapshot then "proves" the just-applied names disappeared. Overlay iteration order explains the exact fingerprint (`PlcA` removed first). Two corrections to the row's guesses: (1) it is NOT "self-healing at the next tick" — the removal evicts the hash slot the overlay's push is gated on, so the next fresh sweep clears the tombstone but re-creates nothing; the stopped connection stays dead **until an instance redeploy**; (2) the mid-write "momentarily not seeing the row" framing was the lesser half — the whole-snapshot staleness (read-before-write, land-after-apply) is the dominant window. Fix: `_definitionsWriteGeneration`, bumped on the actor thread by every artifact apply and captured at every snapshot-read dispatch (boot load + sweep); a result carrying a stale generation is discarded WHOLE — additions included, since a stale row pushed at a live connection would reconfigure it backward and flip-flop with the next fresh read. Deterministic repro fails before the fix (`StaleSweepSnapshot_RacingAnArtifactApply_DoesNotRemoveJustAppliedConnections`); `FreshEmptySweepSnapshot_StillRemovesDisappearedConnections` pins that row-90's legitimate disappearance still fires. SiteRuntime.Tests 647/647; rig redeployed, zero spurious removals/discards at boot, all rig-probe-1 tags Good. Memo §10 records the mechanism. **Postscript 2026-08-18 (merge `505f3569`, agent commit `f02e1166`): extended to the WHOLE snapshot.** The instance-config half had the same TOCTOU, and all three hazards were PROVEN real pre-fix: a stale sweep restarts an instance **backward** (`TryRestartStaleInstance` feeds the snapshot's old `ConfigJson` to `HandleDeploy` — a literal old-config redeploy; pipeline ownership only covers in-flight deploys and the old revision was never claimed), a read-pre-delete/disable **resurrects** a just-stopped instance (delete even clears the sweep claim), and the boot path skews `_deployedInstanceNames`. Fix: one `_storeWriteGeneration` over both tables, bumped at BOTH ends of every actor-thread config mutation (deploy persist, enable, disable, delete, rollback, artifact apply) — both ends because `ApplyDeployment`/`HandleDisable`/`HandleDelete` advance actor state BEFORE the off-thread write, so a completion-only bump (the row-101 original argument, which holds only for the artifact apply) leaves a window. Stale sweep discarded whole; stale boot load re-dispatched, capped at 5 consecutive (then applied with a Warning). CDC/`SiteReconciliationActor` writes deliberately never bump — discovering them is the sweep's purpose. 3 new fail-before tests + 2 fresh-generation guards pinning that row-72/78 legitimate repairs survive; SiteRuntime.Tests 652/652 x3; full suite green (one unrelated Communication.Tests gauge flake → row 102); rig probe: disable/enable/redeploy churn over multiple sweep ticks produced zero self-heal events on either site-a node, tags Good. Memo §11. | Observed by the 2026-08-17 row-100 fix session | Closed 2026-08-17; whole-snapshot extension 2026-08-18. | — |
| 102 | ~~**`SiteStreamSubscribersActiveGauge_GoesToOneOnConnect_AndBackToZeroOnCancel` is delta-hardened but still flakes — the baseline can't survive other contributors CLOSING**~~ **RESOLVED 2026-08-18 via the preferred option (row-96 pattern).** The survey found the gauge moves ONLY inside the real `SiteStreamGrpcServer.SubscribeInstance` handler, and exactly two test classes drive it: `SiteStreamGrpcServerTests` (the gauge test's own class) and `CleanupVerificationTests` — `GrpcOptionsWiringTests` instantiates the server but never opens a stream, and the client/aggregator suites subscribe against fakes, so none of those can perturb the counter. Both drivers now share the `[Collection("SiteStreamSubscriberGauge")]` xunit collection (comments on both classes + at the baseline poll explain why the relative read alone was insufficient), so no other contributor can close a stream between the baseline capture and the assertions. Verified 4/4 consecutive full-suite runs green (717/717), durations unchanged (22–25 s vs the prior 24 s) — the serialization is free. Original evidence: the test polls `baseline + 1`, but the process-global gauge's baseline is captured ONCE; capture while parallel classes' streams are up → they close → the gauge settles at this test's own 1 and `baseline+1` is unreachable forever — a fixed-baseline delta only survives contributors that hold constant or increase. | Observed 2026-08-18 (one full-suite failure, `Expected: 4, Actual: 1`; 3/3 green in isolation); fixed same day | Closed 2026-08-18. | — |
| 103 | **Two lesser TTL-bounded circuit-teardown-vs-async-setup races remain in CentralUI** (arch-review 2026-08-19 P6-6's optional tail, deliberately deferred by the fix). The permanent debug-stream leak was closed at `DebugView.razor` + `DebugStreamHub` (dispose-vs-connect take-once pattern, commit `11e623c2`), but two same-family instances with a *different shape* were left: (1) `TransportImport.razor.cs TryLoadAsync` (~:244) — a circuit torn down mid-load strands 1 of the 8 `MaxConcurrentImportSessions` slots until the 30-min `BundleSession` TTL reaps it (partially defeats row 43); (2) `Topology.razor` (~:201-214) — timer created before a disposal re-check. Both are TTL/lifetime-bounded (no permanent leak), which is why they missed the Medium bar. Fix is the same post-await `_disposed` re-check + take-once teardown, but routed through `BundleSession.ReleaseSession`/the wizard-step lifecycle rather than a session-stop call. | Arch-review 2026-08-19 P6-6 remediation (T10 explicitly scoped them out) | Open. | Next CentralUI hardening pass, or opportunistically when either file is next touched. |
| 103 | ~~**Two lesser TTL-bounded circuit-teardown-vs-async-setup races remain in CentralUI**~~ **RESOLVED 2026-08-20 (Low-wave T12, agent commit `c8452136`).** Both instances closed with the Medium-wave T10 take-once discipline: `TransportImport.razor.cs` re-checks disposal after `TryLoadAsync`'s await and releases the import slot through exactly-once ownership (dispose-during-start frees the `BundleSession` slot immediately instead of stranding it for the 30-min TTL), and `Topology.razor` creates its timer under the same post-await re-check so a disposed page leaves no live timer (asserted event-style, not by sleep). Original note: (arch-review 2026-08-19 P6-6's optional tail, deliberately deferred by the fix). The permanent debug-stream leak was closed at `DebugView.razor` + `DebugStreamHub` (dispose-vs-connect take-once pattern, commit `11e623c2`), but two same-family instances with a *different shape* were left: (1) `TransportImport.razor.cs TryLoadAsync` (~:244) — a circuit torn down mid-load strands 1 of the 8 `MaxConcurrentImportSessions` slots until the 30-min `BundleSession` TTL reaps it (partially defeats row 43); (2) `Topology.razor` (~:201-214) — timer created before a disposal re-check. Both are TTL/lifetime-bounded (no permanent leak), which is why they missed the Medium bar. Fix is the same post-await `_disposed` re-check + take-once teardown, but routed through `BundleSession.ReleaseSession`/the wizard-step lifecycle rather than a session-stop call. | Arch-review 2026-08-19 P6-6 remediation (T10 explicitly scoped them out) | Open. | Next CentralUI hardening pass, or opportunistically when either file is next touched. |
| 104 | **Existing site LocalDb files stay `auto_vacuum=NONE`; `PRAGMA incremental_vacuum` after a cap trim is a no-op on them** | Arch-review 2026-08-19 P4-6 (Low-wave T4, `47f03fe6`) | `auto_vacuum` is only honoured on a table-less database, and `LocalDbSchema.EnsureCreated` had always created LocalDb's bookkeeping tables before `SiteEventLogSchema.Apply` ran — so the pragma had never once taken effect on any real file. The fix (`SiteLocalDbSetup.EnsureAutoVacuumOnFreshFile`, run before `AddZbLocalDb`'s lazy singleton constructs `SqliteLocalDb`) only helps **fresh** files; an unconditional `VACUUM` against a live ~1 GB replicated file was explicitly rejected (long exclusive lock on the active node; the rewrite is CDC-invisible but blocks the sync pump). Every node on the docker rig today predates the fix, so freed `site_events` pages there are reused, not returned to the OS — the storage cap (row 81) still bounds the data volume, only the file size stops shrinking. | Operator-scheduled `VACUUM` in a maintenance window (both nodes of a pair stopped together), or when a site's file is next recreated. |
| 105 | **Design constraint (not a defect): a LocalDb-replicated table must never be the parent of a row-mutating `ON DELETE` foreign key (`CASCADE`/`SET NULL`/`SET DEFAULT`) from another table** | Arch-review 2026-08-19 P4-7 → LocalDb 0.3.3 (scadaproj `122940c` + review follow-up `9663279`, Low-wave T5) | `LwwApplier` upserts with `INSERT OR REPLACE` under `foreign_keys=ON`, so the implicit delete fires the child action while CDC capture is suppressed — the child rows change on the applying node only and the pair diverges silently. 0.3.3 makes `RegisterReplicated` refuse such a parent at registration (clear exception naming the child FK), which is a BOOT failure for the site node, so this is enforced, not advisory. None of the ten `SiteLocalDbSetup.ReplicatedTables` has any FK today (zero `REFERENCES` on the live rig file). Model cross-table cleanup as explicit application deletes on each table (as the 3-table deploy cascade already does), never as SQLite FK actions. | Whenever a schema change adds a foreign key between site tables, or an eleventh table joins `ReplicatedTables`. |
| 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)
@@ -1,5 +1,16 @@
# Low-Findings Remediation Plan — 2026-08-19 Arch Review
> **Status 2026-08-20: EXECUTED — all 15 tasks landed** (14 via dedicated `low/T*` worktrees cherry-picked onto
> `low/integration`, T5 as LocalDb **0.3.3** in scadaproj `122940c` + review follow-up `9663279`, published to the
> Gitea feed and pinned here). Honest outcomes of the two *plausible-needs-verification* items: **P3-4 reproduced**
> (deterministically, by construction; how often the >10s header path occurs in production is NOT demonstrated —
> noted in the test's doc comment) and **P7-5 reproduced** (4-instance cascade measured on a real SQLite-backed
> `ScadaBridgeDbContext`). Plan decisions 1–3 were all taken as written (P7-6 behaviour fix + `--force`; P4-6 real
> fix via `SiteLocalDbSetup.EnsureAutoVacuumOnFreshFile` — fresh files only, register row 104; P4-7 as 0.3.3, with
> the FK-parent constraint recorded as register row 105). Integration caught one cross-task regression (T12's
> Topology fixture vs. T13's new `IStaleInstanceProbe` ctor dependency) and one bundle-review Minor (T11's copied
> pattern re-rendered after Dispose) — both fixed in `57c8973b`. Register row 103 closed.
Scope: the **21 Low** findings left open after the High wave (`c4d3c8b8..d18f344e`) and the
Medium/Med-High wave (`d18f344e..9b67c7cb`, deployed to the rig), **plus register row 103**
(the two TTL-bounded lesser instances of P6-6, deferred by the Medium wave to "next