docs(register): row-85 live-probe evidence (Created DCL connection fired alongside the sweep self-heal in the original failure sequence — no second restart, all tags Good); row-96 evidence from the close-out run (Playwright drill-in nav timeout under parallel load, green isolated)
This commit is contained in:
@@ -77,7 +77,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 82 | ~~**`AuditLog.Tests` flakes under parallel full-solution runs — `MsSqlMigrationFixture` contention** *(Low, test-infra)*~~ **RESOLVED 2026-08-17 (`da23f2dc`) — three mechanisms, and the row's "per-run database name" prescription was already in place (the fixture has minted `ScadaBridgeAuditMigTest_<guid>` per class since it was written).** (1) The headline fingerprint was create+migrate SATURATION: nothing bounded how many fixtures ran `CREATE DATABASE` + the ~100-migration chain at once (measured peak: 45 concurrent test DBs / 19 blocked requests / 98 sessions on one container), pushing DDL past EF's default 30 s command timeout — every observed failure was `..ctor → MigrateAsync → SqlException 258 Execution Timeout Expired`, which xunit reports against every test in the class (hence "29 failures"). Fix: a machine-wide counting semaphore on `sp_getapplock` in `master` (`ScadaBridgeTestMigrate/0..N-1`, 4 slots, `SCADABRIDGE_MSSQL_TEST_MIGRATE_SLOTS` override) held across create+migrate — the arbiter must live on the shared SERVER because the contention is cross-process; gate connection is deliberately UNPOOLED (a pooled one defers the session-lock release until reuse, stranding a slot); crashed process ⇒ SQL releases the lock; a gate deadline miss proceeds UNGATED so the throttle can never itself be a red. Timeouts raised to 300 s migrate / 120 s create-drop (bounded queue = busy but progressing); once-per-process sweep of orphaned `ScadaBridgeAuditMigTest_*` >2 h; connectivity probe retries 3× (a busy server misread as absent flips every MSSQL test to a green-looking Skip — worse than a red). (2)+(3) Two more flakes behind it: `PartitionPurgeTests` and `NotifyDispatcherAuditTrailTests` LEAKED 100 ms-tick actors (purge / outbox dispatcher) whose only stop was timeout-bounded `TestKit.Dispose` — a leaked dispatcher even delivered the NEXT test's notification through its own exhausted adapter, stranding it `Retrying`. Fixed with `Watch`/`Stop`/`ExpectTerminated` in the creating test. Evidence: 10× concurrent ConfigurationDatabase.Tests went from 10/10 processes failing (17–77 failures each) to 3 consecutive 10/10-green runs; the AuditLog 3×+1× mix from failing to 3 clean runs; standalone AuditLog 383/383, ConfigurationDatabase 395/395 (no runtime regression), SiteCallAudit 66/66 (inherits the fixture); zero orphan DBs or stuck app locks left. Host.Tests/PlaywrightTests hit the PERSISTENT DB on an already-serialized axis — untouched. Systemic residue recorded as rows 95/96. Doc: `docs/test_infra/test_infra_db.md`. | Found independently by two agents in the 2026-08-16 wave | Closed 2026-08-17. | — |
|
||||
| 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)*~~ **RESOLVED 2026-08-17 (`524bdbbe`).** `ReconcileStartsInstanceTests` converted to the first-`SetInstanceCounts`-report gate; the pattern was extracted into a shared `TestSupport/DeploymentManagerStartupProbe` now used by both this suite and `InstanceSweepSelfHealTests`, so the convention has one implementation to copy instead of two to keep in sync. SiteRuntime 633/633 green. | Row-78 fix session | Closed 2026-08-17. | — |
|
||||
| 84 | ~~**`LogReplicationBacklogNote` can fire twice in one purge tick** *(Low, cosmetic)*~~ **RESOLVED 2026-08-17 (`ddb3c32e`).** The note is emitted at most once per tick. Fail-before: `RunPurge_WhenBothPhasesDeleteRows_LogsTheBacklogNoteOnceForTheWholeTick` fails pre-fix with `Assert.Single(): collection contained 2 matching items`. | Row-34 session | Closed 2026-08-17. | — |
|
||||
| 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)*~~ **RESOLVED 2026-08-17 (`8f2c2813`) — with a premise correction AND a widened blast radius.** The row's mechanism was wrong: `data_connection_definitions` has NO production reader (its getter is test-only) — connection actors are created solely from the `Connections` map in an instance's own flattened config via `EnsureDclConnections`, so there was never a store→actor path to repair. The real defect: that ensure was the CALLER'S job and only 2 of 4 instance-start paths did it (boot batch ✅, `ApplyDeployment` ✅, `StartStoredInstance` ❌ — both the row-67 ensure-started and the row-78 sweep — and `HandleEnableResult` ❌; the two misses are exactly the two paths added later). So the bug was NOT replication-specific: an ordinary unreplicated site that boots with an instance disabled and enables it later hit the same dead end, because the boot load reads only enabled configs. Fix is structural: `EnsureDclConnections` moved INTO `CreateInstanceActor` (the documented single creation funnel), before the already-exists guard so the two working paths keep exact semantics; reuses the per-name hash guard so steady state is silent; a fifth path cannot forget. Store-level sweep pass explicitly REJECTED (memo §7.4): it wouldn't have fixed this (the table is empty on deploy-only sites) and the unguarded form ping-pongs the hash slot against the deploy path, dropping live subscriptions every tick. Fail-before by behavioral revert: 4 of 7 new tests fail (sweep-started, reconcile-ensure-started, enable-after-disabled-boot, exactly-once-across-sweeps). SiteRuntime 633/633, DCL 320/320. Sibling gaps found during the fix recorded as rows 89/90. | Proven live 2026-08-16 by the row-78 close-out probe (sweep started `rig-probe-1`/`soak-notify-1` but every tag subscribe failed for 6+ min with `Unknown connection: OPC PLC Simulator — will retry`; a second pair restart healed it) | Closed 2026-08-17. Live rig re-probe at wave close-out: replay quarantine-and-restart, expect tags Good with NO second restart. | — |
|
||||
| 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)*~~ **RESOLVED 2026-08-17 (`8f2c2813`) — with a premise correction AND a widened blast radius.** The row's mechanism was wrong: `data_connection_definitions` has NO production reader (its getter is test-only) — connection actors are created solely from the `Connections` map in an instance's own flattened config via `EnsureDclConnections`, so there was never a store→actor path to repair. The real defect: that ensure was the CALLER'S job and only 2 of 4 instance-start paths did it (boot batch ✅, `ApplyDeployment` ✅, `StartStoredInstance` ❌ — both the row-67 ensure-started and the row-78 sweep — and `HandleEnableResult` ❌; the two misses are exactly the two paths added later). So the bug was NOT replication-specific: an ordinary unreplicated site that boots with an instance disabled and enables it later hit the same dead end, because the boot load reads only enabled configs. Fix is structural: `EnsureDclConnections` moved INTO `CreateInstanceActor` (the documented single creation funnel), before the already-exists guard so the two working paths keep exact semantics; reuses the per-name hash guard so steady state is silent; a fifth path cannot forget. Store-level sweep pass explicitly REJECTED (memo §7.4): it wouldn't have fixed this (the table is empty on deploy-only sites) and the unguarded form ping-pongs the hash slot against the deploy path, dropping live subscriptions every tick. Fail-before by behavioral revert: 4 of 7 new tests fail (sweep-started, reconcile-ensure-started, enable-after-disabled-boot, exactly-once-across-sweeps). SiteRuntime 633/633, DCL 320/320. Sibling gaps found during the fix recorded as rows 89/90. | Proven live 2026-08-16 by the row-78 close-out probe (sweep started `rig-probe-1`/`soak-notify-1` but every tag subscribe failed for 6+ min with `Unknown connection: OPC PLC Simulator — will retry`; a second pair restart healed it) | Closed 2026-08-17. **Live-proven on the rig same day** in the exact original failure sequence (pair stopped, node-a `site-localdb.db` quarantined as `.probe85-quarantine-2026-08-17`, pair restarted): reconcile "0 fetched" at 08:29:59, then at the 08:30:54 sweep tick `Created DCL connection OPC PLC Simulator` fired ALONGSIDE the two self-heal Warnings (pre-fix they appeared alone), zero `No connection actor`/`Unknown connection` lines on either node, all three `rig-probe-1` tags Good with live values ~2 min after restart — NO second restart; steady-state silence confirmed (0 Created/Updated lines across subsequent ticks); replication reseeded (`localDbReplicationConnected: true`, backlog 0), site-b/c still correctly omit the fields. | — |
|
||||
| 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 |
|
||||
@@ -88,7 +88,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
|
||||
| 93 | **The cap model's page-layout constants are empirical for the CURRENT `site_events` schema** *(Low, maintenance trap)* | Row-81 fix session (2026-08-17) | Fill factors (0.78/0.75), cell overheads (16/8) and especially `IndexedColumnCount = 4` are calibrated to today's table + four indexes. A schema change — new column, new index — silently invalidates them; a fifth index in particular would be silently under-counted. Any `site_events` schema migration must re-run the memo §4 calibration and update the constants together. | Any schema change to `site_events` or its indexes |
|
||||
| 94 | **`SqliteCapabilityTests` is a deliberate tripwire — do not "fix" it by weakening the assertion** *(Low, standing note)* | Row-81 fix session (2026-08-17) | The test FAILS BY DESIGN if a future SQLite bump ships `dbstat`/`sqlite_dbpage` support. The correct response at that point is to REPLACE the sampled page-layout model with exact `dbstat` measurement (retiring rows 92/93 with it), not to relax the test. The test's own comment says the same. | `SqliteCapabilityTests` fails after a SQLite/provider bump |
|
||||
| 95 | **The leaked 100 ms-tick test-actor pattern is systemic — three more classes carry it** *(Low, test-infra; row-82 residue)* | Row-82 fix session (2026-08-17): two classes fixed (`PartitionPurgeTests`, `NotifyDispatcherAuditTrailTests`), three more found carrying the same shape | `AuditLogPurgeActorTests` (benign today only because exactly ONE test there touches the fixture DB — adding a second DB-backed test reintroduces row 82), `OutageReconciliationTests`, and `SiteAuditReconciliationActorTests` all create 100 ms-tick actors against a shared fixture DB with no deterministic stop (only timeout-bounded `TestKit.Dispose`, which does not fail on expiry). None failed across ~15 parallel runs, so left untouched for proportionate blast radius. Fix pattern is already written: `Watch`/`Stop`/`ExpectTerminated` in the creating test. | Next touch to any of the three classes, or first flake matching the leaked-actor fingerprint (a prior test's rows mutated mid-test) |
|
||||
| 96 | **xunit parallelism is uncapped almost everywhere — `MaxParallelThreads` unset, only 3 of ~20 test projects have an `xunit.runner.json`** *(Low, test-infra policy)* | Row-82 fix session (2026-08-17) | Every suite runs at processor-count collection parallelism, and a solution-level `dotnet test` multiplies that across concurrent processes — the general multiplier behind the row-82/91 flake class. The row-82 fix bounds the DB-migration axis specifically; a project-wide parallelism default (or per-suite runner configs for the resource-heavy suites) is a policy decision worth making once, deliberately, rather than per-flake. | Next test-infra session, or CI adoption of parallel suite runs |
|
||||
| 96 | **xunit parallelism is uncapped almost everywhere — `MaxParallelThreads` unset, only 3 of ~20 test projects have an `xunit.runner.json`** *(Low, test-infra policy)* | Row-82 fix session (2026-08-17) | Every suite runs at processor-count collection parallelism, and a solution-level `dotnet test` multiplies that across concurrent processes — the general multiplier behind the row-82/91 flake class. The row-82 fix bounds the DB-migration axis specifically; a project-wide parallelism default (or per-suite runner configs for the resource-heavy suites) is a policy decision worth making once, deliberately, rather than per-flake. Supporting evidence from the 2026-08-17 close-out run: `AuditLogPageTests.DrillInFromParentExecution_FiltersGridToSpawnerExecution` (Playwright, untouched by the wave) hit a 30 s navigation timeout under full-suite parallel load and passed isolated in 1 s — one more member of this class. | Next test-infra session, or CI adoption of parallel suite runs |
|
||||
| 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