Reject read the row as Pending, then landed a blind full-row UpdateAsync. A
concurrent approve could win its own Pending->Approved CAS in that gap and enter
the multi-second WriteTagAsync relay; the reject's update then stamped
Status/ExecutedAtUtc/ExecutionError to Rejected mid-relay, so the two-person-control
record said Rejected for a write that physically executed at the plant. Works
cross-node (both centrals share one MS SQL).
- ISecuredWriteRepository/SecuredWriteRepository: new TryMarkRejectedAsync,
mirroring TryMarkApprovedAsync exactly (conditional UPDATE ... WHERE
Status='Pending', writes only the columns the reject transition owns). No schema
change, no migration, no rowversion column.
- HandleRejectSecuredWrite: uses the CAS; on loss re-reads the row and returns the
status that actually won (same UX shape as approve's lost-race error), and emits
the SecuredWriteReject audit row ONLY on a won CAS.
- Audited the other lifecycle writes: approve/reject/expiry (inline + the
opportunistic list sweep) now ALL transition only out of Pending, so winning the
approve CAS gives that leg exclusive ownership of the row and its post-relay
Executed/Failed update stays safe as an unconditional write. Documented in place.
- Tests: repository-level approve-vs-reject races in both orders (exactly one wins,
loser false, row reflects the winner only) plus reject-vs-expiry; handler-level
lost-race reject (conflict error, no write, no audit row) and approve on an
already-rejected row (no CAS, no relay, no audit).
ConnectAsync reset the event loop and client on reconnect but left
_alarmCts/_alarmSubCount/_alarmSubSources/_alarmStreamPrefix untouched.
The orphaned old RunAlarmStreamAsync loop kept retrying the disposed
client every 5s forever, and SubscribeAlarmsAsync's coverage check saw
the stale state and short-circuited the actor's post-reconnect
transparent re-subscribe (ReSubscribeAllAlarms) without ever opening a
stream on the new client — freezing the native-alarm mirror until
failover/redeploy. Reset the alarm state in ConnectAsync, mirroring the
existing lock-guarded block in DisconnectAsync/DisposeAsync.
An in-flight subscribe launched against adapter generation N can take up to
SeedOverallTimeout (120s) to answer. If the adapter is swapped in that window
(failover, or the row-97 in-place reconfigure), the completion was applied
unconditionally: it stored a dead session's handle under the tag key, so the
LIVE completion for the same tag read as a duplicate and had its working
monitored item released (tag resolved forever, no data); it re-stamped the old
endpoint's seed values with the CURRENT generation, bypassing the stale-data
guard; and a connection-level failure row from the dead adapter knocked the
healthy new connection back into Reconnecting. The alarm twin released the new
live alarm feed the same way.
Both completions now carry the generation AND the adapter instance they ran
against, and both handlers drop a completion whose generation no longer
matches - no handle store, no seed application, no per-tag or connection-level
failure processing, no duplicate-release logic. Handles the dropped completion
did create are released best-effort against the CAPTURED adapter (handles are
adapter-scoped; MxGateway's are plain ints that collide across sessions), and
the existing release sites in both handlers were switched to the message's
adapter for the same reason. The in-flight markers are deliberately left alone
(ReSubscribeAll/ReSubscribeAllAlarms clear them wholesale and may already have
re-registered the tag/source), and the requester gets a failure reply so its
retry timer re-sends.
Tests: DataConnectionStaleCompletionTests - stale handle not stored / live item
not released, release lands on the old adapter only, seed values not delivered,
stale connection-level failure does not re-enter Reconnecting, stale alarm
completion does not release the live feed. All five fail against the unfixed
handlers.
The WP-4 runs measured the stream path only up to the bounded send channel;
the probe deliberately substituted the socket hop with a controllable reader,
which is exactly where the R2 SiteStreamEventBatcher lives — so batching had
never been in any benchmark's hot path. The harness now also attaches per-site
wire probes: a real Kestrel h2c listener hosting the production
SiteStreamGrpcServer, dialed by the production SiteStreamGrpcClient with
batching negotiated, batch sizes read from the production batch_size histogram
via a MeterListener (--wire-probes-per-site, default 10; 0 restores the old
shape). Full-scale run (10x500x75, 20-min window, results doc §8 + raw JSON):
899,807 events in 12,462 frames (mean 72.2 ev/frame, p50=p99=75), wire P99
43.4 ms vs the 250 ms budget — every batch pays the full 25 ms linger since a
75-event burst never hits the 100 cap — zero drops, zero faults, offered load
100.0% of nominal with fan-out doubled. CI smoke Fact now asserts the wire
path carries traffic and observes frames.
The gauge test polls baseline+1, but the process-global gauge's baseline is
captured once: a parallel class's streams closing after capture lowers the
gauge below baseline+1 forever. Survey confirmed only SiteStreamGrpcServerTests
and CleanupVerificationTests drive the real SubscribeInstance handler that
moves the counter; both now share the SiteStreamSubscriberGauge collection
(row-96 pattern). 4/4 full-suite runs green, durations unchanged.
Row 79 executed its own prescribed remedy without waiting for a second
corruption: all ten rig data dirs (8 docker/ nodes + 2 docker-env2/ site
nodes) move from virtiofs bind mounts to named docker volumes
(scadabridge-<node>-data), seeded in place with both rigs stopped and
live-verified (site-a localDbReplicationConnected=true / backlog 0 over
the migrated files; env2 stable, 0 restarts). The feared inspectability
loss is moot — OrbStack exposes volumes at ~/OrbStack/docker/volumes/.
The rebuild surfaced latent env2 drift (pre-Secrets-0.6.2 image): env2
now carries the x-secrets-store-env posture + central data volumes.
Row 93's silent maintenance trap is now loud: SiteEventsSchemaCalibrationTests
pins the site_events columns/indexes the EventLogPurgeService size model is
calibrated against, failing with a pointer to the memo §4 recalibration on
any schema drift. Rows 92 (accepted by design) and 94 (standing tripwire)
annotated accordingly.
ReadStoreSnapshotAsync's snapshot is read off the actor thread and consumed only
when its piped message lands. Row 101 guarded the DEFINITIONS half of that race;
the CONFIG half was left to machinery whose reach is time-limited, and both legs
of the instance reconcile sweep were reachable through it:
- Row-72 leg: a read dispatched before a deploy's persist, landing after the
deploy completed AND after the pipeline released ownership, carries the
revision the deploy replaced. TryRestartStaleInstance then "repairs" the
instance BACKWARD, re-deploying the snapshot's own superseded ConfigJson.
InstanceIsOwnedByADeployPipeline covers only in-flight deploys, and the
per-revision claim has never seen the old revision, so nothing stopped it.
- Row-78 leg: a read dispatched before a delete/disable commits still shows an
enabled row while the actor is already stopped, so StartStoredInstance
resurrects it — and delete clears _sweepAttemptedRevisions, freeing the claim.
Both verified real by neutralising the freshness check and re-running the new
tests (the fail-before site event literally reads "running revision rev-2 is
stale, applying stored revision rev-1"; the deleted instance came back).
_definitionsWriteGeneration becomes _storeWriteGeneration, covering both tables
(the snapshot is one consistent read and is discarded whole, so one counter is
the right shape); DefinitionsGenerationAtRead -> StoreGenerationAtRead on both
internal records. The generation is now bumped at BOTH ends of every mutation —
when the off-thread write is dispatched and when its completion message lands —
because the config paths advance actor state before the write, so an
initiation-only bump would let an in-window read look fresh. Disable, delete and
the init-failure rollback have no actor-thread completion message of their own,
so a minimal StoreWriteCompleted self-message carries the bump for them.
CDC-replicated writes, snapshot resyncs, restored files and
SiteReconciliationActor's fetch deliberately do NOT bump: discovering exactly
those is the sweep's reason to exist.
A stale sweep result is now dropped WHOLE (no overlay apply, no start, no
restart). A stale boot load cannot be dropped — startup must finish — so it
re-dispatches LoadDeployedConfigs(); a fifth consecutive superseded read is
applied anyway with a Warning, and a late one arriving after the boot completed
is dropped rather than looping against the S5 retry timer.
Tests: three fail-before regressions (backward restart, resurrection, boot
re-read) plus two guard twins pinning that the legitimate row-72 restart and
row-78 start still fire on a current-generation snapshot. The row-101
definitions tests keep passing; FreshEmptySweepSnapshot's hardcoded generation
moves 1 -> 2 now that one artifact apply bumps at both ends.
Docs: Component-SiteRuntime.md's stale-snapshot paragraph now describes the
whole snapshot (both halves, both consumers, the boot re-dispatch, the
CDC-no-bump rule), the components mirror follows, and the arbitration memo gains
§11 extending §10 to the config half.
An off-thread snapshot read racing an artifact apply could land after the
apply populated the overlay; its stale absences then tombstoned and stopped
just-applied LIVE connections, which nothing re-creates until an instance
redeploy (the removal evicts the hash slot the overlay push is gated on).
Each read now captures a definitions-write generation at dispatch; a stale
result is discarded whole (additions too — a stale row would reconfigure a
live connection backward). Deterministic repro fails before the fix; a
fresh-generation guard test pins that row-90 disappearance still fires.
`DataConnectionManagerActor.HandleCreateConnection` is create-only (warns and
returns for a name it already holds), so `EnsureDclConnection`'s documented
"a changed config re-issues the create so the DCL adopts it" was never true on
ANY node: a corrected endpoint/credential/backup/failover-count took effect only
when the connection was next built from scratch (node restart, singleton
failover, remove-then-re-add), while the DM's hash cache recorded it as applied.
Rows 89/90 made that honest with a Warning; this closes it.
New `ReconfigureConnectionCommand` (Commons, same shape as the create):
- `DeploymentManagerActor.EnsureDclConnection` now has three outcomes off its one
per-name hash slot — unchanged -> nothing at all, new -> create, CHANGED ->
reconfigure — and logs "Reconfigured DCL connection {Connection}
(protocol={Protocol}) in place" at Information in place of the old Warning. The
winner-resolution / arbitration / tombstone logic is untouched; only what
happens after a change is detected changed.
- `DataConnectionManagerActor` FORWARDS the command to the owning connection
actor, falling back to a create for a name it does not hold, so the command is
safe whatever the manager's state.
- `DataConnectionActor` handles it in ALL THREE states and never stashes it — a
connection pointed at a wrong or dead endpoint lives in Connecting/Reconnecting,
which is exactly when a correction arrives; stashed it would wait on the broken
configuration forever. Handling replaces protocol/primary/backup/failover-count,
re-arms endpoint selection to Primary (the new config's primary is the intent,
whatever the old config had failed over to), and calls the new shared
`SwapAdapter` helper — now used by both failover sites too, so the swap exists
once: unhook + fire-and-forget dispose -> _factory.Create -> rewire -> bump
_adapterGeneration LAST. `BecomeReconnecting(afterReconfigure: true)` then
reconnects immediately (StartSingleTimer replaces any armed reconnect, so no
racing attempts) with the unstable-disconnect accounting and failover check
suppressed — the link was dropped on purpose, and counting it as an unstable
cycle could trip a failover onto the NEW backup before the new primary had been
tried once. ReSubscribeAll/ReSubscribeAllAlarms then re-issue everything from
the untouched subscription registries. Idempotent: a reconfigure carrying the
configuration already in force is a no-op.
Stop-and-recreate was rejected as destructive: subscriptions live inside
DataConnectionActor and InstanceActor cancels its subscribe-retry timer on first
success, so a rebuilt actor would have no subscribers and nothing would ever
re-subscribe.
Tests: new DataConnectionReconfigureTests (7; 5 fail / 2 pass against a
behavioural revert). DclConnectionArbitrationTests gains two row-97 tests and its
CDC-arrival test now asserts the second push is a reconfigure; its ping-pong
guard also asserts zero reconfigures, since a flip would now show up there.
SiteRuntime-010 renamed to ..._ReissuesReconfigureCommand. 3 fail against a
reverted EnsureDclConnection. DataConnectionLayer.Tests 327/327,
SiteRuntime.Tests 643/643, solution build 0 warnings.
Docs: Component-DataConnectionLayer.md gains an "In-place reconfiguration"
section under Endpoint Redundancy; Component-SiteRuntime.md, and the
docs/components/ mirrors, updated; the arbitration decision memo gains an
appended §9 postscript resolving its residual 1 (residuals 2 and 3 stay open).
Register row 98 — the site-scope authorization gate was FAIL-OPEN on an
unresolvable target. The resolve-then-check shape was written as
`if (target != null) EnforceSiteScope(...)`, so a null resolve skipped the
check entirely and a site-scoped caller passed the gate for any id in the
system. Authorization now fails CLOSED: a target that cannot be PROVEN to sit
inside the caller's scope is treated as outside it.
The defect turned out to have THREE more instances than the register recorded.
Besides `EnforceSiteScopeForInstance` and `EnforceSiteScopeForIdentifier`, the
same inline shape sat in `HandleGetInstance`, `HandleGetSite` and
`HandleGetDataConnection` — read handlers that resolve their own target and so
never call the helpers. Those were the sharper half of the bug: they answered
success-with-null for a nonexistent id and Unauthorized for an out-of-scope
one, making the pair an id-enumeration oracle over the whole fleet. All five
now route through one shared `DenyUnresolvedTargetIfScoped` arm.
Two properties are load-bearing and pinned by tests:
- System-wide callers (Administrator, or a Deployer with no site grants) are
untouched — they return at the guards above the lookup and keep the
handler's ordinary "not found" / null answer.
- The denial is indistinguishable from an out-of-scope denial: same
ManagementUnauthorized shape, and a message that never states whether the
target exists.
Register row 99 — CLI `audit query --status delivered` failed client-side while
the server would have accepted it: `AcceptOnlyFromAmong` matches ordinally
against the capitalised literals. Same treatment row 88 gave `deploy status` —
the client-side gate is removed, the valid set moves into the option help text,
and the server becomes the single authoritative check.
That required ADDING the server check, not just relying on one: the shared
audit filter parser was LAX, silently dropping an unrecognised value and
leaving the dimension unconstrained — so removing the CLI gate alone would have
turned a loud client-side failure into a plausible, complete-looking page that
did not answer the question asked. New `ParseEnumListStrict` (case-insensitive,
rejects ordinals — `Enum.TryParse` accepts "3", and a filter is a name-based
contract) backs the ManagementService /api/audit/query + /api/audit/export
endpoints, which now return 400 BAD_REQUEST naming the valid set.
Other AcceptOnlyFromAmong sites surveyed in the audit group: `--channel`,
`--kind`, `--status` on BOTH `query` and `export` (6 uses) plus `export
--format` (rejected `--format CSV` though the server lowercases it) — all seven
fixed here. Deliberately left alone: `template folder reorder --direction`
(pure client-side literal, no server-side parse to defer to) and the global
`--format json|table` (CLI-local output selection, never sent to a server).
The CentralUI AuditLogPage drill-in parser and the CentralUI export endpoint it
feeds deliberately KEEP the lax parse — they read a query string a human may
have bookmarked, where a stale fragment must still render the page. Both
contracts now live side by side in AuditQueryParamParsers and are documented as
non-reconcilable.
Docs: Component-ManagementService.md gains a "Site-scope gate: fail-closed on
an unresolvable target" section and a strict-filter note on the audit
endpoints; Component-Security.md records the fail-closed shape under Deployer
site scoping; the CLI README marks the audit filter values case-insensitive and
states the no-silent-drop / no-ordinals contract.
Tests: 5 new ManagementActorTests fail before the row-98 fix (verified by
reverting ManagementActor.cs) and cover scoped+nonexistent -> denied,
system-wide+nonexistent -> unchanged (both Administrator and no-grant Deployer),
scoped+in-scope -> allowed, and the indistinguishability property. 21 CLI tests
fail before the row-99 fix. Four pre-existing tests that pinned the removed
client-side gate were rewritten to pin the new contract.
The compile cache was a `static` class, so its entry table and its Hits/Count
counters were one process-global blob every test class in the SiteRuntime
assembly wrote into. Row 91 last wave neutered the resulting flakes (per-test
GUID keys + one-sided counter bounds), but the shared mutable state itself
remained: any test calling Clear() evicted entries other parallel classes had
just inserted, and a Clear() from a class outside the serialized collection
would degrade the others to recompiles and re-open the row-91 flake class.
PREMISE CORRECTION: the register speculated production clears "per-instance or
per-deployment script entries" and offered ClearKeys(prefix) as the natural fit.
There are NO production callers of Clear() at all — grep of src/ finds only
GetOrAdd (ScriptCompilationService.CompileCore). Clear() existed solely so
tests could reset the global. A key-scoped API would therefore have been new
production surface built for nobody, so the instance-scoped seam is the fix and
Clear() is deleted outright rather than migrated.
- SiteScriptCompileCache: `internal static class` -> `internal sealed class`;
Cache/EvictionLock/_hits/_accessSequence become instance fields. LRU batch
eviction, the double-checked sweep and the ConcurrentDictionary.ToArray()
snapshot are byte-for-byte the same policy. Clear() removed.
- ScriptCompilationService owns one cache. Public ctor (the DI path) builds its
own; a new internal ctor takes one for tests. Production N4 sharing is
unchanged and now rests on the DI SINGLETON registration — the one service
instance is threaded to DeployCompileValidator and to every Instance Actor —
so that lifetime is load-bearing and is pinned by a new registration test.
- Tests: all three Clear() callers migrated off it. Each test now owns its cache,
which lets the assertions get STRONGER — the row-91 workarounds are retired:
Hits/Count are exact again (no >= baseline bounds), the per-test GUID key
token is gone, and ForeignEntrySlack (512 entries of allowance on the bound)
drops to the racing-thread count in the concurrency test. New test
SeparateCaches_DoNotShareEntriesOrCounters pins the scoping directly: filling
one cache past MaxEntries leaves another's entry, hit count and identity
intact. DeploymentWarmThenGateTests additionally asserts Count == 1 now that
its counter is private.
- The [Collection("SiteScriptCompileCache")] serialization from row 91 is
RELAXED (removed from all three classes) — they no longer touch any shared
state, so there is nothing left to serialize.
- Docs: Component-SiteRuntime.md gains the ownership/singleton note and drops
the Clear() reference; DeploymentManagerActor / DeployCompileValidator /
ServiceCollectionExtensions comments corrected to say where the cache lives.
Build: 0 warnings. SiteRuntime.Tests 643/643, three consecutive full-parallel
runs. AuditLog.Tests correlation classes (other ScriptCompilationService
consumers) green.
maxParallelThreads was unset everywhere and only 3 of ~20 test projects carried
an xunit.runner.json at all, so xunit ran Environment.ProcessorCount collections
at once (14 on the dev box) in every suite. That is the general load multiplier
behind the row-82/91/95 flake family.
Policy: per-suite caps, NOT a global one. Pure unit suites stay uncapped for
speed; a suite gets a config only when a test CLASS costs a real external or
OS-level resource, so ProcessorCount of them at once is a resource question
rather than a CPU question. Four suites qualify, all capped at
maxParallelThreads: 4.
- ConfigurationDatabase.Tests, AuditLog.Tests, SiteCallAudit.Tests — verified as
the three that actually take MsSqlMigrationFixture (the latter two by project
reference). Every MSSQL-backed test class means a CREATE DATABASE plus the
~100-migration chain against the one shared dev SQL Server. 4 is chosen to
MATCH the default slot count of row 82's cross-process sp_getapplock migrate
semaphore, so the in-process and cross-process limits agree rather than
fighting.
- Host.Tests — classes boot real ASP.NET Core hosts and real ActorSystems;
Akka.Remote binds real sockets, Kestrel binds real ports, gRPC dials them.
Threads are capped, collections are NOT serialized. IntegrationTests
(parallelizeAssembly/parallelizeTestCollections false) and
CentralUI.PlaywrightTests (maxParallelThreads 1) already carry stricter
pre-existing settings and are left alone — the Playwright suite drives a browser
against a live rig, where shared UI state, not CPU, is the constraint.
Each capped csproj gains
<Content Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />;
the SDK does not copy it automatically and without it the file is silently
ignored. Verified by moving the copied file out of bin/ to take an uncapped
baseline: ConfigurationDatabase.Tests ran at 216 % CPU uncapped vs 145 % capped,
so the cap demonstrably takes effect.
Cost is close to nil — these suites are latency-bound on an external resource,
not CPU-bound (14-25 % utilisation for the two slowest). Standalone, uncapped ->
capped: AuditLog 383 tests 1m52s -> 1m51s; Host 520 tests 4m05s -> 4m08s;
ConfigurationDatabase 396 tests 10s -> 12s; SiteCallAudit 66 tests 8s -> 8s. All
green.
docs/test_infra/test_infra.md gains a Test Parallelism Policy section: the two
axes (in-process collection threads = this cap; cross-process solution-level
dotnet test = unbounded in general, bounded on the DB-migration path by the
sp_getapplock semaphore), the per-suite table with reasons, the rules (cap
threads rather than serialize; a cap is not a substitute for test isolation; the
Content copy is required), and the default for new projects (no config until the
project acquires a per-class external resource). test_infra_db.md cross-links it
from the semaphore bullet.
Row 82 (da23f2dc) fixed this leak shape in PartitionPurgeTests and
NotifyDispatcherAuditTrailTests; three classes in the same assembly still had
it. Each creates an actor with a fast periodic tick (100 ms purge override,
~0 s reconciliation override) against a shared MsSqlMigrationFixture database
and never stops it deterministically, relying on TestKit's Dispose -> Shutdown
— which is timeout-bounded and does NOT fail when it expires. A survivor keeps
mutating the fixture DB while LATER test classes run.
All three now use the same Watch(actor) / Sys.Stop(actor) /
ExpectTerminated(actor) helper da23f2dc established, applied at EVERY
actor-creation site — 11 in AuditLogPurgeActorTests, 10 in
SiteAuditReconciliationActorTests, 2 in OutageReconciliationTests.
The two real-DB cases are the ones that could corrupt another class:
AuditLogPurgeActorTests' EndToEnd_RealPartition_… keeps switching partitions
out of the shared DB, and SiteAuditReconciliationActorTests'
Tick_Duplicates_NotDoubleInserted holds a live AuditLogRepository over a
DbContext the test is about to dispose. The stub-repo cases are cheaper but not
free: each burns a DI scope and a thread every 100 ms for the rest of the
assembly run.
OutageReconciliationTests needed more than the helper. It already called
Sys.Stop, but (a) fire-and-forget, so the call returns before the in-flight tick
has finished writing to MSSQL, and (b) from the LAST line of the enclosing
`await using` block, so any earlier assertion failure skipped it entirely. Both
stops move to immediately after the last step that needs the actor alive —
before the read-back that counts rows — and the AuditLogIngestActor that
BuildCentralPipeline creates is now stopped too (test 2 was discarding its ref
outright, leaving it holding a service provider the block then disposed).
Where a test's remaining assertions read a collection the actor appends to
(client.Calls indexing, Assert.Empty on a switched-boundaries list), the stop is
placed BEFORE them so they no longer read a moving target.
AuditLog.Tests: 383/383 green (1 m 51 s), 0 skipped.
SiteScriptCompileCacheTests.GetOrAdd_SameCodeAndGlobals_ComputesOnce asserted
Assert.Equal(1, SiteScriptCompileCache.Hits) after a Clear() on what is a
process-wide static cache. The [Collection("SiteScriptCompileCache")] attribute
serializes this class against ScriptCompilationServiceTests and
DeploymentWarmThenGateTests, but NOT against the rest of the assembly: every
class that compiles a script — Instance/Script actor tests, deployment tests —
goes through the same GetOrAdd in parallel and bumps the same global Hits/Count.
Failed ~1 run in 4 under a full parallel solution run, green in isolation.
Fixed with the register's row-74 pattern (98c94588): assert the DELTA
attributable to this test's OWN keys, never an absolute global value. Two habits
carry it.
1. Every test now keys its entries on a per-test-instance guid token (the new
Code() helper embeds it as a comment), so no other class in the assembly can
collide with or serve them. That makes the factory-call counts and
Assert.Same identity checks exact and fully owned — and those are the real
claims: "computed once" and "the second lookup returned that instance".
2. The process-global counters are only ever asserted as one-sided bounds
(>= baseline + n), since foreign traffic can only add. There is no concurrent
Clear() to make a delta go negative: all three classes that call Clear() are
in the serialized collection.
Four assertions changed shape, and two got stronger rather than weaker.
GetOrAdd_SameCode_DifferentGlobals_AreSeparateEntries asserted Count == 2 and
Hits == 0 — both only true while nothing else was compiling. "No cross-globals
hit" is now asserted where it is observable and ours: the second lookup HAD to
compute, the two results are not the same instance, and re-getting either serves
its own entry rather than the other globals surface's. Overflow_StaysWithinTheBound
gained an eviction probe over its own keys — a bound with foreign slack is not
proof a sweep ran; a recompute of an entry we inserted is.
Overflow_EvictsOldestBatch_… had Assert.Equal(hitsBefore + 1, Hits), the same
absolute-delta shape, now a lower bound alongside the sharp Assert.Same.
DeploymentWarmThenGateTests carried the identical latent shape one line before
its own delta assertion — Assert.Equal(0, Hits) immediately after Clear() —
and is converted to a baseline snapshot too.
SiteRuntime.Tests: 633/633 green, twice, at full parallelism (43 s, 42 s).
Closes deferred-work-register rows 89 and 90, and corrects their premise twice.
THE RULE
The `data_connection_definitions` store wins wherever it has an opinion; an
instance's flattened `Connections` entry is a deploy-time snapshot and is
authoritative only for names the store has never owned (the deploy-only site,
where that table is legitimately empty).
PREMISE CORRECTION 1 — the peer's DCL is EMPTY, not stale
`DeploymentManagerActor` is a `site-{SiteId}`-scoped singleton and the only
sender of Create/RemoveConnectionCommand, to its OWN node's `dcl-manager`
(which exists on every node but is populated only where the singleton runs).
So the standby serves nothing. The real defect is that `_createdConnections`
is in-memory per singleton incarnation: a restart or failover rebuilds every
connection from the flattened snapshots and silently discards a newer
connection-only artifact deploy. Row 89 is therefore create-time, not
message-delivery, and NOT replication-specific — the same loss happens on an
unreplicated single-node restart.
PREMISE CORRECTION 2 — a changed config never reached a LIVE connection either
`DataConnectionManagerActor.HandleCreateConnection` is create-only (warns and
returns on a name it holds), so `EnsureDclConnection`'s documented "re-issues
the create so the DCL adopts the new configuration" has never been true on any
node. Now surfaced with a Warning saying exactly that, once per changed config.
Not fixed here: stop-and-recreate drops every tag subscription and InstanceActor
cancels its subscribe-retry timer on first success, so nothing re-subscribes.
MECHANISM
`data_connection_definitions` gains its first production reader. It is read on
the SAME off-thread task as the deployed configs (`ReadStoreSnapshotAsync`), by
the boot load and every sweep tick, into an actor-thread-confined arbitration
overlay applied BEFORE the boot batch and BEFORE the sweep's repair loop. A
CDC-delivered definition change converges within one tick on whichever node runs
the singleton — no new cross-node message. A failed definitions read yields null
and leaves the overlay untouched (distinct from an empty list, which is a real
answer); clearing it would reintroduce the ping-pong through the error path.
THE PING-PONG, KILLED BY CONSTRUCTION
§7.4 of the sweep memo rejected a store-level pass because two sources would
flip the single per-name hash slot, recreating the actor every tick. That is a
property of WHERE the hash is computed: every ensure path now resolves the
winner BEFORE hashing, so only one configuration can ever occupy the slot. The
artifact-deploy path writes its rows into the overlay in the same pass, closing
the same flip arriving through the other door.
REMOVAL (row 90)
Both removal paths share `RemoveConnectionCentrally` and record an in-memory
tombstone, so a centrally removed connection is not resurrected by the next
instance start — a pre-existing bug. Tombstones are deliberately not persisted;
a definition row reappearing clears one.
TESTS
tests/.../Actors/DclConnectionArbitrationTests.cs — 8 tests. Fail-before
(behavioral revert: overlay inert + tombstone check disabled): 5 fail, 3 pass;
the 3 passers are the two deploy-only regression guards and the ping-pong guard.
DOCS
New decision memo docs/plans/2026-08-17-dcl-connection-arbitration-decision.md
(topology, rule, ping-pong mechanism, tombstones, residuals); Component-SiteRuntime,
Component-DataConnectionLayer + their docs/components mirrors; the §7.4 residual in
the sweep memo marked closed with its premise corrected.
Deferred-work register rows 37, 86, 87, 88 — the structural duplication and the
three behavioral divergences it had accumulated, fixed together.
Row 37 (structural). `IDeploymentManagerRepository.QueryDeploymentSummariesAsync`
is DELETED. It and `QueryDeploymentListPageAsync` were two paged reads of the same
insert-only table with different contracts — raw offset vs page number, no total vs
total, no instance join vs joined, instance-id-list scoping vs site-id scoping. The
CLI/HTTP `QueryDeployments` handler now goes through `QueryDeploymentListPageAsync`
with a `DeploymentListFilter`, exactly like the Central UI page. The
`DeploymentRecordSummary` projection went with it (`DeploymentListRow` is the single
row shape); its `MaxPageSize` const moved to `DeploymentListPage`, which also gains
`DefaultPageSize`.
Row 86 (Medium, authorization — the headline). `SiteScopeService.ResolveAsync` was
claims-only, so an Administrator carrying `SiteId` claims saw the whole fleet through
the CLI (`ManagementActor.EnforceSiteScope` has always exempted Administrator) and
only their granted sites on the Deployments page. One user, one dataset, two answers.
Administrator = system-wide is the intended semantic on both sides; the UI was the
defect. The role claim is read explicitly with an OrdinalIgnoreCase comparison rather
than via `IsInRole`, which compares claim VALUES ordinally and only resolves when the
identity carries the matching roleType — the two sides must not disagree on a
differently-cased role name. An Administrator's site claims are not collected at all,
so the invariant "system-wide => empty permitted-id set" holds for every caller.
Designer is NOT exempt: the class doc claimed "any Admin/Design user" is system-wide,
which was true of neither side, so the doc was corrected rather than the bypass
widened.
Row 87 (Low). The scoped branch used to resolve the site grant into an instance-id
list by loading EVERY instance in the system, non-transactionally, on every scoped
query; the grant now rides as `SiteIdScope` and is resolved by the query's own join.
An unparseable permitted-site id is DROPPED, never ignored — dropping can only narrow
a grant, discarding it would widen it to fleet-wide. And `PageSize <= 0` no longer
means UNLIMITED down at the repository (`--page-size 0` materialized the whole
retention window): it means "unspecified" and takes the CLI's own default of 50,
capped at 1000. Bounding without telling would be silent truncation, so the handler
now returns a `DeploymentQueryResult` envelope carrying `totalCount` plus the page
coordinates actually applied — with a CLI table projection (mirroring row 57's
`AuditConfigTableProjection`) that unwraps it and appends a "showing N of M" note.
Row 88 (Low, UX). An unrecognised `--status` fell back to "no filter", so a typo like
`--status Sucess` returned every status — a complete-looking answer to a question the
caller never asked. It is now a `ManagementError` naming the valid values. Parsing
stays case-insensitive; a numeric string is rejected too, since `Enum.TryParse`
accepts "7" and yields an undefined value (guarded with `Enum.IsDefined`). Loud
failure over silent full results, per row 46.
Tests: 7 new `SiteScopeServiceTests` pinning Administrator = system-wide (verified
failing 6/7 against the pre-fix service, the Designer pin passing both ways); the
ManagementActor deployment suite migrated onto the consolidated query with new pins
for the status error, case-insensitive parse, numeric rejection, page-size default/
cap/floor, empty-scope-on-garbage-grant, and "reads no instances at all"; a repository
test for site-scope+status composing over a bounded page; and 6 CLI tests for the new
table projection.
AuditLog.Tests flaked under parallel full-solution runs. Three distinct
mechanisms, all load-sensitive and all green in isolation.
1. MsSqlMigrationFixture create+migrate saturation (the register's headline).
Database NAMING was already isolated (a guid per fixture), but nothing
bounded how many fixtures ran CREATE DATABASE + the ~100-migration chain at
once. xunit parallelises processor-count collections PER PROCESS, and a
solution-level `dotnet test` starts several MSSQL-backed processes together,
so the dev container saw dozens of simultaneous fresh-database migrations.
That is resource saturation, not object-level locking, and it pushed
individual DDL statements past EF's default 30s command timeout — every
failure had the identical fingerprint the register recorded:
MsSqlMigrationFixture..ctor -> Migrator.MigrateAsync -> SqlException
"Execution Timeout Expired" (error 258), which xunit surfaces as
"Class fixture type ... threw in its constructor" against every test in the
class.
Fix: a machine-wide counting semaphore over the create+migrate window, built
from sp_getapplock on master (ScadaBridgeTestMigrate/0..N-1, default 4 slots,
SCADABRIDGE_MSSQL_TEST_MIGRATE_SLOTS to override). An xunit collection cannot
do this — the contention is cross-PROCESS, so the arbiter has to live on the
shared server. The gate connection is unpooled (a pooled connection defers the
implicit release of a session-scoped app lock until reuse) and releases
explicitly; a crashed process drops its connection and SQL releases the lock,
so the gate cannot wedge the suite, and a deadline miss proceeds ungated
rather than failing. Migration commands also get a 300s timeout and
CREATE/DROP DATABASE 120s: under a bounded queue the server is busy but
progressing, so waiting is the correct response. Plus a once-per-process
best-effort sweep of ScadaBridgeAuditMigTest_* databases older than 2h so a
killed run does not leave strays, and the connectivity probe now retries
(3 attempts) — a busy server misread as an absent one would silently turn
every MSSQL test into a Skip, which reads green and is worse than a red.
2. PartitionPurgeTests leaked AuditLogPurgeActors. Each ticks every 100ms and
switches out the Jan-2026 partition; they were only stopped implicitly by
TestKit Dispose -> Shutdown, which is timeout-bounded and does not fail when
it expires. Under load a previous test's actor was still ticking against the
shared per-class fixture DB, so PerChannelOverride_... found its seeded
Jan-15-2026 row already gone (Expected: 1, Actual: 0).
3. NotifyDispatcherAuditTrailTests leaked NotificationOutboxActors, same shape.
The dispatcher claims EVERY due row, not just the test's siteId, so a leaked
actor delivered the next test's notification through its own exhausted
QueuedOutcomeAdapter — a transient outcome, leaving the row in Retrying
(Expected: Delivered, Actual: Retrying) and audit counts short.
Both fixed by stopping the actor inside the test that created it
(Watch/Stop/ExpectTerminated), removing the dependency on shutdown timing.
Evidence. Reproducer: 10 concurrent ConfigurationDatabase.Tests processes
against the one dev SQL Server — before, 10/10 processes failed (17-77 failures
each, 100% the ctor->MigrateAsync timeout); after, 3 consecutive runs 10/10
green, 395/395 each. Realistic shape (3x AuditLog.Tests + 1x
ConfigurationDatabase.Tests concurrent): first post-fix run still showed the
NotifyDispatcher leak, then 3 consecutive clean runs. Standalone: AuditLog.Tests
383/383, ConfigurationDatabase.Tests 395/395 (12s, unchanged from baseline),
SiteCallAudit.Tests 66/66. No orphan databases or stuck app locks left behind.
docs/test_infra/test_infra_db.md documents the ephemeral per-test-class
databases, the throttle and its env override, the timeouts, and the stray sweep.
Row 81 — the cap could not be enforced on a replicated node. MaxStorageMb was
compared against page_count-freelist_count over the WHOLE consolidated LocalDb
file, which since Phase 1/2 holds ten tables plus __localdb_oplog /
__localdb_row_version / __localdb_snapshot_state. Every 1000-row trim batch freed
~390 KB of site_events pages and added ~1000 oplog rows + ~1000 tombstones to the
same file, so the measured size did not fall, the "size did not decrease" guard
fired, and the trim stopped 2.5x over a 64 MB cap with 226 k rows still present.
A node whose peer was down could not enforce its cap at all, and the event log's
budget was in practice shared with bookkeeping it does not own.
The cap now measures site_events AND ITS FOUR INDEXES only, so replication
bookkeeping can neither consume nor block the budget — and the cap stops being a
function of the peer's health, which was the wrong direction of coupling: an
inflated oplog is a replication fault, and letting it drive event deletion would
destroy operational history in response to one.
dbstat — the exact per-table measurement the register proposed — is NOT usable
here, verified before implementing. It is a compile-time option and the bundled
SQLitePCLRaw.lib.e_sqlite3 (SQLite 3.53.3) does not enable it: PRAGMA
compile_options lists no ENABLE_DBSTAT_VTAB, pragma_module_list returns ten
modules with neither dbstat nor sqlite_dbpage, and selecting from dbstat fails
with "no such table". Switching SQLite builds was rejected — the provider is
shared with ZB.MOM.WW.LocalDb and every other consumer, so it trades cap accuracy
for fleet-wide version skew. The footprint is therefore modelled from sampled row
width (500 rows each end, octet_length so multi-byte UTF-8 is not under-counted)
through a page-layout model; whole pages rather than a flat rate, because page
fill collapses once rows approach half a page and a linear model underestimates
that by ~40%. Clamped by whole-file logical size, a true upper bound that can only
correct an over-estimate and never binds on a replicated node.
Index pages count against the cap deliberately: measured at ~34% of the table's
footprint, and the 1 GB default was sized against a 393 B/row figure that already
included them.
Guard semantics restated — two backstops replace the single on-disk check:
(1) row width unsampleable => refuse to trim (checked before the cap comparison,
since a zero width makes "over cap" unanswerable rather than false; the old loop's
answer to an unanswerable cap was to keep deleting); (2) estimated size not
falling across a batch => stop. (2) is the old guard re-based onto the
table-scoped metric, so it can no longer be tripped by replication bookkeeping and
now means exactly one thing: events are arriving as fast as the trim removes them.
For that to be observable the loop re-reads the row count each batch, while
bytes-per-row is sampled once per tick and held fixed — re-sampling per batch
would jitter the estimate as the trim eats the head of the table and could stall
the loop through guard (2), the same self-defeating measurement as row 81 itself.
Row 84 — LogReplicationBacklogNote moved from per-phase to once per tick, driven
by the shared PurgeBudget. A tick where retention AND the cap trim both deleted
used to emit two identical Information notes; the tick total is also the more
useful number beside an oplog spike.
Measured model error: -0.8% / +8.4% / +12.8% across representative workloads,
biased high on purpose (an early cap forfeits budget; a low one fails at its only
job). Independently cross-checks row 34's harness at 397.2 vs 393 B/row.
Fail-before: reverting the estimator to whole-file semantics fails 5 of the new
tests and reproduces the original signature exactly (remaining=2000
estimate=21725184 cap=11534336 — rows destroyed, cap still unmet); restoring
per-phase notes fails the row-84 test with "collection contained 2 matching items".
No config keys added or renamed, so no rig/deploy changes. Decision record:
docs/plans/2026-08-17-site-event-log-cap-semantics-decision.md
Tests: SiteEventLogging 104/104 (was 93), Host 520/520, solution build clean.
Row 67's suite still opened every test with a bare Task.Delay(500) after
constructing the Deployment Manager. Each test then writes its
deployed_configurations row -- and a row that lands before the boot-time load
reads storage is picked up by that load, which means the test passes while
proving the BOOT LOAD works rather than the startup-reconcile path it names. A
sleep only makes that race unlikely; on a loaded machine the load can still be
in flight when it expires and the test silently changes what it proves instead
of failing.
The row-78 fix session hit exactly this trap and closed it in
InstanceSweepSelfHealTests by gating on the first
ISiteHealthCollector.SetInstanceCounts report -- the edge the manager raises
unconditionally the moment its load completes, including for an empty site.
This applies the same gate here.
The probe is now a shared TestSupport helper (DeploymentManagerStartupProbe,
added with row 85) rather than a private nested class, so both suites use one
implementation.
Also pins InstanceSweepIntervalSeconds = 0 for this suite: with the sweep armed
its periodic self-heal would independently start the same instances and the
suite would stop distinguishing the row-67 mechanism from the row-78 one.
The remaining Task.Delay calls in the file are deliberate and left alone -- they
precede assertions that NOTHING happened (a disabled instance stayed stopped, a
redundant ensure-started changed nothing), which cannot be awaited on. The one
delay that was a readiness wait rather than an absence wait is now an
AwaitAssertAsync on the instance actually being routable.
An Instance Actor started by a self-heal came up with NO DCL connection actors,
so every tag it subscribed was answered "Unknown connection: <name>" by the DCL
manager indefinitely. Proven live 2026-08-16 by the row-78 close-out probe: the
sweep correctly started the replication-seeded instances, then every subscribe
failed for 6+ min until a second pair restart.
Root cause is narrower than the register assumed. A DCL connection actor is
never created from data_connection_definitions -- that table has no production
reader at all (GetAllDataConnectionDefinitionsAsync is called only by a test).
It exists only as a side effect of starting an instance, from the Connections
map inside that instance's own flattened config, via EnsureDclConnections.
That ensure was the CALLER's job and only two of the four callers made it:
CreateInstanceActorBatch (boot) ensured OK
ApplyDeployment (deploy) ensured OK
StartStoredInstance MISSING <- row-67 ensure-started + row-78 sweep
HandleEnableResult MISSING <- enabling a disabled instance
The two misses are the two paths added after the original pair -- the signature
of a convention living at call sites instead of in the funnel.
It was never replication-specific: the boot load reads only ENABLED configs, so
a node booting with an instance disabled never creates its connections either,
and enabling it afterwards hit the identical dead end on an ordinary
unreplicated site.
Fix: EnsureDclConnections moves into CreateInstanceActor, already documented as
the single Instance Actor creation funnel (it is where the running revision is
stamped for the same reason), and the two caller-side calls are removed. Placed
before the already-exists guard so the two paths that already called it keep
their exact previous semantics. No new message, option or DCL change -- it
reuses EnsureDclConnection's existing per-name hash guard, so steady state is
silent. Structural rather than another guard: a fifth start path cannot forget.
Deliberately NOT done, with rationale in the memo: a sweep pass over
data_connection_definitions. It would not have fixed row 85 (that table is empty
on a site that has only ever received instance deploys), and re-hashing from it
would fight the deploy path over the same per-name hash slot, recreating
connection actors and dropping live subscriptions on a loop. Two residual
sibling gaps (connection-only artifact deploy replicating to a peer that never
adopts it, and the mirror-image removal case) are recorded in the memo and the
component docs as open, not silently skipped.
Tests: SelfHealCreatesDclConnectionTests, 7 tests asserting on the
CreateConnectionCommand messages sent to a recording DCL stand-in. Verified
fail-before against a behavioral revert (call removed from the funnel, restored
at the two original sites): 4 fail / 3 pass, the 3 being steady-state silence
plus two refactor guards that must pass on both sides.
Docs: sweep design memo gains section 7; Component-SiteRuntime,
Component-DataConnectionLayer, docs/components/SiteRuntime and
docs/components/DataConnectionLayer updated (the deploy-flow step ordering in
docs/components/SiteRuntime was stale as written).
Row 65 — trigger FIRED, fixed. BundleUnlockAttempt rows were pruned only by
nodes actively serving unlocks, so a cluster that stopped importing entirely
left its last rows in place forever. The row's own trigger was "Transport gains
a central scheduler a prune tick could ride" — it has one now:
BundleSessionEvictionService, the central-only 1-minute session sweep added by
the transport-session fixes. It now also ticks the new
IBundleUnlockAttemptWindow.PruneAsync. Both drivers share the ONE 15-minute CAS
cadence claim, so ticking on a 1-minute sweep costs at most one bounded DELETE
per quarter hour and the two can never prune concurrently. PruneAsync is
contractually non-throwing (unlike TryRegisterAttemptAsync, which throws and
lets the limiter own the fail-open decision) — a background sweep has no caller
to report to and must survive a database blip. Both Transport docs updated;
three tests in FleetWideUnlockThrottleTests, incl. the retention sweep running
with no unlock traffic anywhere in the test, which is the whole point.
Row 58 — closed with a pin, and the OLD pin was found blind. The existing
Deployments_Dispose_UnsubscribesFromNotifier is behavioural: it is satisfied by
the _disposed guard alone, so deleting the `-=` from Dispose leaves it green
while the singleton notifier keeps a delegate pinning the circuit-scoped
component. Verified by mutation — with the `-=` removed the old test still
passed and only the two new pins failed. Added:
* a structural retention pin asserting the notifier's invocation list actually
shrinks to 0 on Dispose;
* a source scan pinning the row's stated TRIGGER ("a second page subscribes,
or any subscriber lands without a matching Dispose unsubscribe") — every
CentralUI file that subscribes must also unsubscribe and be IDisposable,
with a non-empty guard so the scan cannot pass vacuously.
Row 60 — closed with a pin, and the register's premise corrected.
GlobalArtifactSnapshot does NOT hold EF-materialised entities: it is projected
into pure Commons DTOs before the fan-out. The EF state that genuinely crosses
into the parallel send is the tracked Site entities the lambda closes over, and
that is safe only while lazy loading is off. Pinned in ConfigurationDatabase
.Tests two ways — no proxies extension on the built DbContextOptions, and no
EntityFrameworkCore.Proxies package reference anywhere in the repo (the
structurally stronger half: UseLazyLoadingProxies() is not a compilable call
without it). The real invariant is now spelled out at the fan-out itself.
Row 59 — no product change; added the missing tripwire remark stating the
constraint the safety argument rests on (serial by design, one scoped DbContext,
do not Task.WhenAll; if it gets slow the fix is a scope PER SITE, not a shared
scope under fan-out). Line numbers in the register have drifted to :528-545.
Rows 36, 37, 64: verification only, no change — see the sweep report.
Joseph Doherty