LocalDb Phase 2 deleted the bespoke replicators, so three config keys changed
meaning or died outright:
- ScadaBridge:StoreAndForward:ReplicationEnabled is fully dead. Deleted the
property, its 10 config entries, and the 5 test references.
- SqliteDbPath / SiteDbPath are now migration-only: they name the legacy files
SiteLocalDbLegacyMigrator drains at boot, not live databases. Both mandatory
rules are relaxed accordingly (StartupValidator's Site-only Require, and the
S&F validator's non-empty rule) — an absent value now means "nothing to
migrate", so an already-migrated node can drop the key. DatabaseOptions-
Validator still rejects a present-but-blank value.
- SiteRuntime:ConfigFetchRetryCount's only reader was SiteReplicationActor.
Deleted with its validator rule.
The two path keys stay present in every config, now with a comment explaining
why: removing them would strand un-migrated data on a node that has not yet
started once.
Both relaxations are pinned by the inverse of the test they replace
(Site_MissingSiteDbPath_IsAccepted..., EmptySqliteDbPath_IsAccepted...), each
verified to fail with the old rule restored.
Note: deploy/wonder-app-vd03/appsettings.Site.json is under a gitignored
deploy/ tree, so its edit is local-only and must be repeated on the box.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Tasks 14, 15 and 16, landed as ONE commit.
PLAN DEFECT: these three tasks cannot compile separately. SiteReplicationActor
takes a ReplicationService and calls ReplaceAllAsync (Task 14 deletes both);
DeploymentManagerActor Tells message types declared in ReplicationMessages.cs
(Task 15 deletes it); AkkaHostedService constructs the actor (Task 16). Any
ordering leaves a broken intermediate. Combining them also strengthens the
invariant Task 14 already stated for itself — the two mechanisms never both
run, and never neither.
Registered 8 tables in SiteLocalDbSetup.OnReady: sf_messages plus the 7 site
config tables. notification_lists and smtp_configurations are deliberately NOT
registered — permanently empty by design, so registering them would open a
standing replication channel whose only historical payload was plaintext SMTP
passwords. Migrate stays the LAST call in OnReady, after all registrations, so
migrated rows enter the oplog through live capture triggers.
Deleted: SiteReplicationActor, ReplicationMessages.cs, ReplicationService,
StoreAndForwardStorage.ReplaceAllAsync, and 6 test files. ReplaceAllAsync is
not merely unused but unsafe to keep: a mass DELETE on a now-replicated table
would be captured and shipped to the peer.
Kept ActiveNodeEvaluator (delivery gate + heartbeat still need it) with its doc
corrected, and activeNodeCheck in AkkaHostedService (SiteCommunicationActor).
The positional-argument hazard the plan flagged was real: removing
DeploymentManagerActor's optional IActorRef? replicationActor shifted 6
trailing optionals, and 4 test call sites bound the wrong arguments with no
compile error at some positions. Converted them to named arguments where
possible — Props.Create builds an expression tree, which rejects out-of-position
named args, so the rest are padded positionally with a comment saying why.
The Task 7 'not yet registered' test was INVERTED rather than deleted, and is
exact in both directions: too few means a table silently stops replicating, too
many means the SMTP tables leak. Added a separate security-named test for those
two, and a composite-PK test (LWW keys on the full PK, so a truncated key set
would collapse distinct rows). The convergence suites now get their
registrations from the real OnReady — their temporary harness registration is
deleted, so they prove the cutover rather than agreeing with themselves.
Verified: build 0 warnings; SiteRuntime 512, StoreAndForward 130, Host 330,
AuditLog 355, ExternalSystemGateway 142, HealthMonitoring 97, LocalDb
integration 16 — all pass, 0 failures.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Task 12 + Task 13. No production behaviour change in either.
Task 12: DeploymentManagerActor.HandleDeployArtifacts already purges
notification_lists and smtp_configurations on every artifact apply, but nothing
pinned the actor's CALL to it — ArtifactStorageTests covers the storage method
only. Task 15 deletes SiteReplicationActor's copy, making this the sole
remaining call site, and Task 16 edits this actor's wiring; dropping the call
would leave plaintext SMTP passwords on disk with every suite still green.
Verified red-first by commenting the call out: the pin fails with that message.
Task 13: StoreDeployedConfigIfNewerAsync STAYS. Re-verified both callers —
SiteReplicationActor:375 (dies at Task 15) and SiteReconciliationActor:166
(survives). Reconciliation is a per-node startup self-heal against central
whose fetch races real deploys, so the deployed_at guard still does real work
there. Doc comment rewritten to say so, and to warn against porting the guard
onto the replication path where it would fight the HLC rather than help it.
Also corrected a stale 'guarded standby write' section header in the tests.
Re-ran the Task 13 step 2 scope check: ConfigFetchRetryCount's only production
reader remains SiteReplicationActor:157, so its option + validator rule stay
until Task 17, after Task 15 deletes the actor.
Verified: build 0 warnings; SiteRuntime 533, Host 329, StoreAndForward 153,
LocalDb integration 16 — all pass.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Tasks 5 and 6 of the Phase 2 plan, committed together because their test
fallout is entangled — several fixtures construct both stores.
StoreAndForwardStorage and SiteStorageService now take ILocalDb. Connections
come from ILocalDb.CreateConnection(), which hands out an already-open,
pragma-configured connection carrying the zb_hlc_next() UDF the capture triggers
call; a raw connection would lack the UDF and every write to a replicated table
would fail closed. Deleted with the connection strings: S&F's
EnsureDatabaseDirectoryExists and its per-open busy_timeout pragma, and the site
service's BusyTimeoutFloorSeconds normalization — LocalDb owns all of it now.
DI: AddSiteRuntime's string overload is gone (nothing left to supply), so the
Host calls the no-arg form. ScadaBridge:Database:SiteDbPath and
StoreAndForwardOptions.SqliteDbPath survive only as the migrator's source
locations in Tasks 8/9.
Two things the plan did not anticipate, both worth reading:
1. FOUND A REAL LATENT DEFECT, from Phase 1, now fixed. The plan assumed
directory creation simply moved to LocalDb along with file ownership. It did
not: the LocalDb library never creates the parent directory, and
SqliteLocalDb opens the file eagerly in its constructor — so a missing
directory is a hard boot failure ("SQLite Error 14: unable to open database
file"), not a degraded start. The default site config points at the RELATIVE
path ./data/site-localdb.db, so any site node without a pre-existing data/
directory fails to boot. The docker rig escapes only because its volume mount
happens to create /app/data — a coincidence that would have hidden this until
a bare-metal or fresh deployment. This has been latent since Phase 1 made
LocalDb:Path required; deleting S&F's EnsureDatabaseDirectoryExists here
would have widened it. Re-established the guarantee at the layer that now
owns the path (SiteLocalDbDirectory.Ensure, called before AddZbLocalDb) and
pinned it with SiteLocalDbDirectoryTests. Non-vacuity is not assumed: two
tests written against the wrong assumption failed with exactly this
SQLite Error 14 before the fix existed.
2. Test fallout was ~7x the plan's estimate. The plan named "fixtures" in one
project; the constructor change actually reaches 40 files across 7 test
projects, and most used Mode=Memory;Cache=Shared — which LocalDb has no
equivalent for, so every one had to move to a real temp file. Rather than
copy the Phase 1 TestLocalDb fixture into 7 projects, added a shared
tests/ZB.MOM.WW.ScadaBridge.TestSupport library (not a test project) so the
WAL-sidecar cleanup and the "real, not stubbed" rationale live in one place.
Retargeted rather than deleted, in both directions: the S&F WAL test now asserts
against the LocalDb-backed store (WAL genuinely is LocalDb's job), while the
directory-creation test moved to Host.Tests (that guarantee is NOT LocalDb's).
SiteStorageServiceTests.Initialize_EnablesWalJournalMode got the same treatment.
DeploymentManagerMediumFindingsTests induced a persistence failure via an
unopenable path, which no longer reaches the assertion since the fixture now
throws first; it induces the same failure shape via an uninitialized store.
Verified: full solution build 0 warnings; SiteRuntime 532, Host 318,
AuditLog 355, ExternalSystemGateway 142, HealthMonitoring 97,
StoreAndForward 153 — 1597 passed, 0 failed.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Task 4 of the Phase 2 plan. All nine table definitions plus MigrateSchemaAsync
and TryAddColumnAsync move into a static sync Apply(SqliteConnection) depending
only on Microsoft.Data.Sqlite, so the Host can apply the DDL to a LocalDb-managed
connection before RegisterReplicated installs the capture triggers.
Per the plan, PRAGMA journal_mode=WAL stays in InitializeAsync — LocalDb owns
the connection's pragmas, and the service still opens its own connections until
Task 6.
One deviation: TryAddColumnAsync's `catch (SqliteException) when
(ex.Message.Contains("duplicate column"))` becomes a PRAGMA table_info probe,
matching OperationTrackingSchema. The message-matching form depends on an error
string that is not part of SQLite's contract, and it swallowed every
SqliteException whose message happened to contain that substring. The probe also
drops the ILogger dependency, which is what let the class become static. Cost:
the per-column "Migrated: added column" info log is gone — it fired once per
column per legacy database and nothing consumes it.
Also added a test beyond the plan's specified one, for the same reason as Task 3:
the specified test asserts against a freshly-created database, where CREATE TABLE
already lists the migration columns, so it would pass with every ALTER deleted.
The added test starts from the pre-migration shapes with a row present and proves
the migration path runs and preserves data across a NOT NULL DEFAULT add.
SiteRuntime suite 532 passed / 0 failed; full solution build 0 warnings.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Task 13.
SiteEventLogger's XML doc said 'Not replicated to standby. On failover, the new
active node starts a fresh log.' That is now false and was the exact behaviour
Phase 1 set out to fix.
OperationTrackingOptions.ConnectionString and SiteEventLogOptions.DatabasePath
marked MIGRATION-ONLY: nothing reads them but SiteLocalDbLegacyMigrator, and
they are safe to delete from a config once a node has migrated.
ScadaBridge CLAUDE.md gains a consolidated-site-database entry covering the
GUID/opaque-token contract change, the migration-only keys, the incidental
CWD-outside-the-volume data-loss fix, the fail-closed default-OFF replication
posture, and the fact that ZbTelemetryOptions.Meters is a silent allowlist.
scadaproj CLAUDE.md's LocalDb row goes from 'no app adoption yet' to the honest
current state, per the component-status-honesty convention: ScadaBridge Phase 1
only, on an UNMERGED branch, replication live-proven on the rig's site-a pair
ONLY, default-OFF everywhere else, Phase 2 not started, no other app adopted, no
production deployment replicating.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Task 4 of the LocalDb Phase 1 adoption plan.
OperationTrackingStore took IOptions<OperationTrackingOptions> and opened its own
SqliteConnection from ConnectionString. It now takes ILocalDb and gets every
connection - writer and the two ad-hoc reader paths - from CreateConnection().
This is not cosmetic. OperationTracking is a RegisterReplicated table, so its
capture triggers call zb_hlc_next(). That UDF is registered per connection by
ILocalDb and by nothing else, so a raw SqliteConnection would fail closed on
every write. Connections from CreateConnection() also arrive already open -
calling Open()/OpenAsync() on one throws - hence the removed OpenAsync calls on
the reader paths.
OperationTrackingOptions.ConnectionString is now vestigial for this store; the
database location is LocalDb:Path. The options class stays (retention settings)
and the config key stays bound for the site config DB.
InitializeSchema is kept but is now always a no-op in the host: onReady runs
while ILocalDb is being constructed, strictly before this constructor can
receive it. It remains so a directly-constructed store (tests, tooling) is
self-sufficient.
Tests: the store fixture moves off mode=memory&cache=shared onto a real ILocalDb
over a temp file. There is no in-memory mode - LocalDbOptions.Path is a
filesystem path - and testing through a raw in-memory connection would no longer
resemble the host. Verifier connections stay raw on purpose: this fixture never
registers the table, so no trigger fires and the UDF is never reached.
Three DI fixtures needed LocalDb:Path added, because resolving
IOperationTrackingStore now forces ILocalDb construction:
SiteCompositionRootTests, SiteAuditWiringTests, and the AuditLog
CombinedTelemetryHarness.
Verified: build 0 warnings; Host 294/294, SiteRuntime 530/530, AuditLog 354/354.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
Tasks 2 + 5a-writer + 5b of the LocalDb Phase 1 plan, landed together because
they are one indivisible change: the schema, the writer that fills it, and every
consumer that assumed the old id semantics.
WHY the id changes. Site pairs will replicate site_events with last-writer-wins on
the primary key. With INTEGER PRIMARY KEY AUTOINCREMENT both nodes independently
mint 1, 2, 3... for unrelated events, so sync would treat them as the same row and
silently overwrite. A GUID makes the event log a pure union across the pair.
Task 2 - schema extracted so the Host's AddZbLocalDb onReady can create the tables
before RegisterReplicated installs capture triggers (pre-registration rows are
never captured):
- new OperationTrackingSchema.Apply / SiteEventLogSchema.Apply, plain
Microsoft.Data.Sqlite, no LocalDb dependency
- both stores delegate their InitializeSchema to them (idempotent, so a
directly-constructed store still works)
- OperationTracking is unchanged - it already had a TEXT PK and replicates as-is
Task 5a - SiteEventLogger mints Guid.NewGuid("N") per event and inserts it.
Task 5b - three consumers assumed a monotonic integer id. All three move to
timestamp ordering; leaving any one behind would be a live bug:
- EventLogQueryService: "id > $afterId" would return an ARBITRARY subset of a
GUID-keyed table and SILENTLY DROP ROWS from page-through. Now a composite
(timestamp, id) keyset cursor with an opaque string token; timestamps are not
unique, so id is the tie-break that guarantees exactly-once paging.
- EventLogPurgeService: "ORDER BY id ASC LIMIT 1000" would delete a RANDOM batch
instead of the oldest. Now orders by timestamp.
- EventLogEntry.Id and both ContinuationTokens: long -> string.
WIRE COMPATIBILITY. Those DTOs cross the site<->central Akka boundary
(SiteCommunicationActor -> CommunicationService -> ManagementActor / CentralUI).
No rolling-upgrade shim is needed because both sides ship in the same deployable
and the rig redeploys as a unit. Checked: no Akka serializer binding pins these
types by name. A stale numeric token degrades to "start from the beginning"
(a visible repeat) rather than throwing or losing rows.
Tests: the two that encoded the old semantics were rewritten to guard the new
invariant rather than deleted - uniqueness instead of monotonicity, and
oldest-purged-first keyed on timestamp. That second test also exposed a latent
weakness: the bulk seed stamped every row with the same UtcNow, so "oldest" was
never actually well-defined; rows now get distinct increasing timestamps, kept
inside the retention window so the retention purge does not eat them first.
Verified:
dotnet build ZB.MOM.WW.ScadaBridge.slnx -> 0 Error(s), 0 Warning(s)
SiteEventLogging.Tests -> 70 passed (12 new schema tests, red-first)
CentralUI.Tests -> 925 passed
Commons.Tests -> 684 passed
SiteRuntime.Tests -> 529 passed, 1 pre-existing flaky failure
(InstanceActorChildAttributeRaceTests - passes 3/3 in
isolation with and without this change)
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
The NuGetAuditSuppress in Directory.Packages.props was masking a LIVE high-severity
vulnerability, not documenting an accepted one. Only the Host project resolved a
patched SQLitePCLRaw.lib.e_sqlite3 2.1.12 (transitively, via ZB.MOM.WW.Auth.ApiKeys).
Every other SQLite consumer - AuditLog, SiteRuntime, StoreAndForward, SiteEventLogging
and 11 test projects - still resolved the vulnerable 2.1.11.
The suppression's rationale was factually wrong: it claimed 'the only patched native
lib is the SQLitePCLRaw 3.x line'. 2.1.12 patches this advisory within the 2.1.x line,
so the feared risky force-override of the whole family to 3.x is unnecessary.
Fix: explicit PackageReference to the patched 2.1.12 in each SQLite-consuming project,
plus the central PackageVersion row. Suppression removed, so the advisory is audited
again rather than silenced.
Rejected alternative: CentralPackageTransitivePinningEnabled. It clears the advisory in
one line but makes every central version a ceiling for transitive resolution, demanding
bumps to Google.Protobuf, Grpc.Net.Client, Microsoft.Data.SqlClient and Newtonsoft.Json.
That is a gRPC/data-access change to a production SCADA platform and deserves its own
reviewed commit.
Verified: forced full-solution restore reports no NU1903 (only the pre-existing,
unrelated AngleSharp NU1902 in CentralUI.Tests); all four previously-vulnerable src
projects now resolve 2.1.12; 55 projects build 0 warnings / 0 errors; 1108 tests pass
across the SQLite layer (AuditLog 354, ConfigurationDatabase 357, Security 181,
StoreAndForward 152, SiteEventLogging 64).
Gitea #18: the process-wide ScriptExecutionScheduler was reached via a static
singleton (Shared) directly inside ScriptActor, ScriptExecutionActor, AlarmActor,
AlarmExecutionActor, and ScriptSchedulerStatsReporter, so it could not be
substituted — a shared mutable global for anything running more than one logical
site in a process, most visibly the test assembly. Add an optional scheduler
injection seam to all five: the Host injects nothing and gets the shared pool
(byte-for-byte unchanged), while a test (or a future multi-site host) hands an
actor its own instance. Spawning actors thread their scheduler down to the
execution children they create. Shared() now also recreates a disposed cached
instance rather than returning it (new IsDisposed guard), so a disposed scheduler
can never silently poison every later script/alarm execution.
Gitea #16: ScriptActorTests now runs on its OWN scheduler (disposed at class
teardown), so the faulted-task test can no longer strand a worker on the
process-wide pool and starve unrelated test classes (one prior run failed 22
tests). EvalGate's wait is bounded to 30 s (was unbounded — the actual leak
mechanism) and reset per test; the teardown releases one permit per worker
instead of Release(Entries), which under-released in exactly the failure case.
Full SiteRuntime suite: 6/6 runs green (524/524); was 3/6 failing on clean main.
Fixes: #18Fixes: #16
VERIFY-THEN-FIX: the plan-authoring discovery is CONFIRMED. No code anywhere in
src/ registered IOperationTrackingStore in DI, yet AkkaHostedService and
AuditLog SCE both comment that AddSiteRuntime provides it. Every consumer
resolved it via GetService (null-tolerant) and silently ran degraded:
cached-drain scheduler never armed, PullSiteCalls reconciliation seam never
wired, Tracking.Status degraded to audit-only. This is a FUNCTIONAL FIX riding
the hygiene plan: AddSiteRuntime now registers the store (site-only), so
site-local cached-call tracking runs in its intended mode. OperationTrackingOptions
is now bound + eagerly validated in the Host site-options block.
Adds SiteStreamManager.SubscribeSiteAlarms(IActorRef) — same broadcast-hub
wiring as Subscribe(...) but with no per-instance filter and forwarding only
AlarmStateChanged events (AttributeValueChanged dropped). Returns a subscription
id torn down via the existing Unsubscribe, symmetric with the per-instance
subscribe. Backs the Task 2 SubscribeSite gRPC server handler.
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
Closes the operator parity gap the deferred-work register tracked as #12: native
alarm source overrides could only be set one-at-a-time, while attribute overrides
had a CSV bulk path. Adds an all-or-nothing CSV import for native sources.
- Commons: extract the RFC-4180 line splitter into a shared CsvLineSplitter
(refactor OverrideCsvParser onto it — no behavior change, pinned by its tests);
new NativeAlarmSourceOverrideCsvParser (header SourceName,Connection,
SourceReference,Filter; blank = inherited).
- Commons: NativeAlarmSourceOverrideEntry + bulk SetInstanceNativeAlarmSource-
OverridesCommand (auto-registered via the reflection command registry).
- ManagementService: Deployer-gated handler — flattens once, validates every
source resolves + is unlocked + no duplicates up front, then upserts the whole
batch under a single SaveChanges (true all-or-nothing txn). Added to the frozen
authorization matrix; dispatch-coverage guard passes.
- CLI: `instance native-alarm-source import --instance-id --file` (parity with
`instance import-overrides`) + README.
- Tests: native parser (Commons), CLI parse/entry mapping, 3 bulk-handler tests
(happy path single-commit, locked-source reject, unresolved-source reject).
Also corrects a stale XML-doc line in ScriptRuntimeContext (WaitForAttribute
quality-gated mode is shipped, not "planned") and updates the deferred-work
register: marks #7/#13/#15/#16/#20 verified-resolved, #12 as CLI/API-shipped with
only the Central UI upload affordance still pending.
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
Site side: additive proto after_id field, ReadChangedSinceAsync gains an optional
afterId cursor and deterministic (UpdatedAtUtc, TrackedOperationId) ordering — a
batch fully inside one UpdatedAtUtc instant no longer re-reads the same page
forever. Absent/empty afterId preserves the exact legacy inclusive >= contract, so
an older central is unaffected. Regenerated the checked-in gRPC C#.
Success now requires BOTH persistence commit AND an InstanceActorInitialized
readiness signal from the actor's PreStart — persistence can commit before the
actor's async init has run or failed, so persistence alone must not report
Success. An actor that dies during init never signals readiness; the Terminated
fallback fails that deployment and rolls back the optimistic state. The
persisted-row rollback is deferred until the store commits so it cannot race the
optimistic write.
Deviation from plan Task 15: the plan's swallow-only guard did not handle the
persistence-first ordering (empirically the store commits before the Terminated
signal, so Success was reported for a dead actor). Added the readiness handshake
to make the join deterministic.
Joseph Doherty