feat/grpc-phase0-psk
1306 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Joseph Doherty
|
228ff8b428 |
fix(grpc): one public constructor on ControlPlaneAuthInterceptor — two made the gate inert
Caught by the Phase 0 live gate, not by the suite. Grpc.AspNetCore registers the interceptor BY TYPE, and InterceptorRegistration.GetFactory() throws "Multiple constructors accepting all given argument types have been found" when more than one public constructor is applicable. The interceptor had two: the DI one and a prefix-set overload added for later phases. The failure mode is nasty. The throw happens inside the interceptor pipeline on every call, so nothing fails at startup — the site node boots, joins, reports healthy. Every gated call then dies with Unknown / "Exception was thrown by handler", which reads as a handler bug rather than an auth bug. And it fails OPEN in the sense that matters least and closed in the sense that matters most: no call is ever authorized, but no call is ever correctly REFUSED either, so the rig showed identical errors for a correct key, a wrong key and no key at all. Live evidence, site-a: three PullAuditEvents calls, three identical InvalidOperationExceptions in the node log. Fix: the prefix-set constructor is internal (Host.Tests already has InternalsVisibleTo). Later phases extend DefaultGatedPrefixes rather than adding a second public registration shape. Why the tests missed it, and what changed: ControlPlaneAuthEndToEndTests registered the interceptor with AddSingleton alongside AddGrpc, so DI handed back the instance and Grpc.AspNetCore's activation path — the thing that throws — never ran. The harness now registers exactly as Program.cs does, by type and not in DI. Plus a direct reflection assertion that the type has exactly one public constructor, since that is the real invariant and it is cheap to pin. |
||
|
Joseph Doherty
|
2ee84af1c0 |
feat(grpc): PSK-authenticate the site gRPC control plane; drop the vestigial management receptionist registration
Phase 0 of the ClusterClient→gRPC migration
(docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Standalone hardening: it
closes a gap that exists today and is a precondition for moving command/control
onto gRPC in later phases.
T0.1 — delete the ManagementActor ClusterClientReceptionist registration.
It was built for an out-of-cluster CLI that was never written: the shipped CLI
speaks HTTP Basic to /management, which asks the actor in-process through
ManagementActorHolder. Nothing in the repo ever sent to /user/management. The
actor still runs there; only the cross-boundary advertisement is gone. Six
documents claimed the CLI used ClusterClient — including the CLI's own README
"Architecture Notes" — and are corrected here rather than left to rot.
T0.2 — record, do not port, the dead integration-routing path.
IntegrationCallRequest is unwired at BOTH ends: RouteIntegrationCallAsync has
zero callers anywhere, and RegisterLocalHandler(Integration, …) appears only in
a test, so production always answers "Integration handler not available". It is
excluded from the gRPC contract (28 of 29 commands migrate) rather than
enshrined on an additive-only wire format, and deleting it during a
transport migration would mix a behavioural change into a change whose whole
value is that behaviour is identical. See
docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md.
T0.3 — preshared-key authentication on SiteStreamService.
The service shipped with no auth at all: plaintext h2c, no interceptor, so
anything that could reach a site node's :8083 could open a live data stream or
read audit rows back via PullAuditEvents/PullSiteCalls. ControlPlaneAuthInterceptor
now gates /sitestream.SiteStreamService/ — modeled on LocalDbSyncAuthInterceptor
(constant-time compare, fail-closed, PermissionDenied) but gating a SET of
service prefixes so phases 1A/1B add services rather than interceptors. LocalDb
sync keeps its own separate key: it authenticates the pair partner, not central,
and collapsing the two would make a site's central-facing key also admit writes
into its database.
Keys are per site (SB-GRPC-PSK-<siteId>), never fleet-wide, so a compromised
site yields only its own. Central attaches them through ControlPlaneCredentials,
which binds CallCredentials to the channel — covering unary and streaming
uniformly, and letting the key resolve asynchronously, which a client
interceptor could not do without blocking. All three central→site channel
creation sites go through it (SiteStreamGrpcClient and both audit pull invokers);
the pull invokers' channel caches are re-keyed by (site, endpoint) because
credentials are per-site and bound to the channel.
Two decisions beyond the plan:
* StartupValidator now requires GrpcPsk on Site nodes. The plan specified only
the runtime gate, but fail-closed with no boot check produces a node that
joins, answers heartbeats and reports healthy while refusing every stream,
audit pull and telemetry ingest — silent and total. Same reasoning as the
existing inbound API-key pepper rule.
* Added Communication:SitePsks as a central-side key map. The plan assumed
central would read the store, seeded via a dev KEK; the docker rig
deliberately boots with no master key, so store-only resolution would leave
it unable to dial its own sites. The store stays primary — it is the only
source that can serve a site added at runtime — with the map covering
key-less hosts and one-off pins. Neither source falling back to
"unauthenticated" is the invariant.
T0.4 — dev keys on both rigs and tests.
34 tests. The seven that matter most exercise a real in-process gRPC stack over
TestServer: the unit tests on either side of the wire would both stay green if
the halves disagreed, and gRPC refuses call credentials on a plaintext channel
by default — the UnsafeUseInsecureChannelCallCredentials opt-in is only provable
by making a real call. They confirm correct key passes on unary AND streaming,
wrong key and no-credentials both get PermissionDenied, and an unresolvable key
fails the call with nothing reaching the service.
OPERATIONAL: a site node upgraded to this build without a key will not boot.
That includes the gitignored deploy/wonder-app-vd03/ overlay.
|
||
|
Joseph Doherty
|
c8e2f4da02 |
feat(cluster): site-pair manual failover relayed from the central UI (Task 10)
Central and each site are SEPARATE Akka clusters, so central cannot act on a
site's membership -- it asks. New TriggerSiteFailover/SiteFailoverAck contract
travels the existing ClusterClient command/control channel (mirroring the
RetryParkedOperation relay); the site's own SiteCommunicationActor performs the
graceful Leave and acks the outcome.
- ClusterFailoverCoordinator moved out of Host into Communication/ClusterState,
beside ActiveNodeEvaluator. Both paths now share ONE oldest-Up implementation;
SiteCommunicationActor cannot reference Host, and the two definitions must not
drift or the node asked to leave stops being the singleton host.
- Site scope is the SITE-SPECIFIC role (site-{SiteId}), not the base Site role --
site singletons are placed on the former, so the base role would move the wrong
node. Pinned by a unit test asserting the role string and by a real-cluster test.
- Site-side guards: refuses a command addressed to another site (a misroute must
never fail over a site the operator did not select), refuses when there is no
peer, and reports a fault as an ack rather than throwing into supervision --
a restart there would drop central's Ask into a bare timeout and lose the reason.
- Ack is sent before the Leave takes effect so it still reaches central.
- UI: the same control now serves both scopes via a SiteId parameter. The site
confirmation deliberately does NOT claim the admin's page will disconnect --
it won't, and crying wolf there devalues the central warning that is real. A
site refusal and an unreachable site surface distinctly.
- Rolling upgrade: a site on an older binary has no handler, so the message
dead-letters and the Ask times out, reported as "site did not respond". That
is the honest outcome; documented on the contract.
Fallout fixed: HealthPageTests now renders the page inside
CascadingAuthenticationState with the real policy set and IAuthorizationService,
because the cards embed an AuthorizeView. That mirrors production, where the
layout supplies the cascading value.
|
||
|
Joseph Doherty
|
caf14a3e03 |
feat(ui): admin manual-failover control on the health page
CentralFailoverControl lives in Components/Health/ (matching AuditKpiTiles / SiteCallKpiTiles) rather than inline in Health.razor, so it is testable without standing up the whole dashboard's DI graph. - Admin-gated via AuthorizeView + RequireAdmin. The Health page itself is intentionally all-roles, so the gate belongs on the control, not the page. - Disabled with an explanatory title when the pair has no online standby; the authoritative guard remains server-side against live cluster membership. - Confirmation dialog (IDialogService, the page idiom) warns that singletons hand over, in-flight work on the active node is interrupted, and THIS PAGE will disconnect and reconnect against the new active node -- Traefik routes the UI to the node being restarted, so a working failover otherwise reads as a crash the admin caused. - A refused failover (service returns null) surfaces the refusal; the UI never reports a failover that did not happen. 7 bUnit tests. Two harness requirements that bit first: AuthorizeView needs a cascading AuthenticationState (the app supplies it from the layout), and BunitContext pre-registers a placeholder IAuthorizationService that throws on policy evaluation -- both handled the same way NavMenuTests documents. Runbook paragraphs added to Component-ClusterInfrastructure.md (new Manual Failover section) and docker/README.md. |
||
|
Joseph Doherty
|
f679d5c749 |
feat(cluster): manual central failover service — graceful Leave of the oldest Up member
Admin-triggered failover of the central pair. IManualFailoverService is declared in CentralUI (plain strings, so that project stays Akka-free); AkkaManualFailoverService implements it in the Host and is registered only in the Central branch. - Leave, never Down: singletons hand over instead of being killed. - Target = oldest Up member with the Central role, mirroring ActiveNodeEvaluator, so the node acted on is exactly the one hosting the singletons (never the leader, whose address-ordered definition diverges from singleton placement after a restart). - Peer guard: returns null when fewer than 2 Up Central members — failing over a lone node is an outage, not a failover. - Audited BEFORE the Leave is issued via ICentralAuditWriter: the acting node can be the one that goes away, and an audit written after could be lost to the shutdown it describes. Best-effort — audit failure never blocks the failover. New audit taxonomy: AuditChannel.Cluster + AuditKind.ManualFailover (operator-initiated topology actions are not script trust-boundary crossings, but are exactly what an audit log exists to attribute). Lock-in tests updated 5->6 channels, 16->17 kinds. alog.md §4 updated per the lock-in tests' contract. Both tables were already stale -- the Channel row omitted SecuredWrite and the Kind table claimed 10 while the code had 16 -- so they are completed here, not merely appended to. ManualFailoverTests: 3 real-cluster tests (oldest leaves + survivor takes over, peer guard refuses with a positive still-running assert, dry-run probe does not perturb). |
||
|
Joseph Doherty
|
4a6341d871 |
feat(cluster): self-first seed ordering closes the boot-alone outage gap
Every node now lists ITSELF as seed-nodes[0] and its partner second. Akka runs FirstSeedNodeProcess -- the only bootstrap path that can form a NEW cluster when no peer answers InitJoin -- exclusively for seed-nodes[0]; every other node runs JoinSeedNodeProcess and retries InitJoin forever. That is why a lone cold-starting central-b never came Up (the "registered outage gap"), and self-first ordering closes it using Akka's own protocol. - 6 node appsettings swapped (the *-node-b configs; the -a nodes were already self-first). All 14 shipped node configs now satisfy the invariant. - StartupValidator enforces it at boot, comparing host AND port -- the invariant fails silently when broken, so it is enforced loudly. NOTE: the gitignored deploy/wonder-app-vd03/ overlay must be reordered before its next deploy or that node will refuse to boot. - SelfFirstSeedBootstrapTests: real in-process clusters at production failure-detection timings, incl. a falsifiability control proving the OLD peer-first ordering never forms. Rejected alternative (implemented, measured, discarded): an external self-form timer calling Cluster.Join(SelfAddress) after a window. It sits outside Akka's join handshake and so cannot tell "no seed answered" from "a seed answered and the join is in flight". On a routine standby restart the peer is alive but the join stalls behind removal of the node's own stale incarnation; a Join(self) during TryingToJoin abandons the in-flight join and forms a second cluster at the same address -- still split after 90s. Docs that claimed self-first ordering was unsafe for simultaneous cold start are corrected: while mutually reachable the InitJoin handshake converges them to one cluster (measured). |
||
|
Joseph Doherty
|
cf3bd52f93 |
feat(cluster): auto-down downing strategy — either-node crash now fails over (owner decision 2026-07-21: availability over partition-safety)
Two-node keep-oldest could NEVER survive a crash of the oldest/active node:
Akka.NET 1.5.62 KeepOldest.OldestDecision only lets down-if-alone rescue a
side with >= 2 members, so the 1-vs-1 survivor takes DownReachable and downs
ITSELF — proven live on the rig ('SBR took decision ... including myself')
before this change. static-quorum(1) is worse (IsTooManyMembers -> DownAll);
keep-majority just re-keys the fatal crash to the lowest address.
SplitBrainResolverStrategy gains 'auto-down' (new default): BuildHocon emits
Akka's AutoDowning provider with auto-down-unreachable-after = StableAfter.
The leader among the REACHABLE members downs the unreachable peer, so the
survivor takes over singletons and /health/active in ~25s regardless of which
node died. Accepted trade (explicit owner decision): a real network partition
runs dual-active until an operator restarts one side. keep-oldest remains
supported; DownIfAlone validation is now scoped to it.
Live drill on the rebuilt rig: active-crash TAKEOVER in 28s (victim still
down; all 7 singletons Younger->Oldest), standby-crash removal 27s with 0
routing blips; victims rejoin as standby in 2s. New real-cluster tests pin
both directions (SbrFailoverTests.AutoDown_*); TwoNodeClusterFixture gains a
strategy knob. All 16 appsettings flipped (src, docker, docker-env2, and the
gitignored wonder-app-vd03 overlay on disk — owner must sync to the host).
Docs: decision record docs/plans/2026-07-21-auto-down-availability-decision.md,
Component-ClusterInfrastructure downing section rewritten, drill + README
reworked (active mode now asserts takeover), deferred-work SBR row resolved.
|
||
|
Joseph Doherty
|
166f07fa68 |
chore(localdb): adopt LocalDb 0.1.1 and drop the directory shim
LocalDb 0.1.1 creates the parent directory of LocalDb:Path itself, so the SiteLocalDbDirectory shim this repo carried through Phase 2 is deleted along with its call site. The gap was found here but was never ScadaBridge's alone — every LocalDb consumer had it — so the fix moved to the library. SiteLocalDbDirectoryTests is RETAINED and retargeted rather than deleted with the shim. It was already written against the site registration path, not the mechanism, so it needed only its Ensure() call removed: what a site node requires is that resolving ILocalDb not fail on a fresh machine, regardless of who provides that. Verified it still earns its place — pinned back to LocalDb 0.1.0 it fails inside SqliteLocalDb..ctor -> SqliteConnection.Open(), so it genuinely depends on the library behaviour and not on a coincidence. Also corrects the coverage-split note in StoreAndForwardStorageTests, which asserted that directory creation is "NOT LocalDb's" — true when written, wrong as of 0.1.1. Build 0 warnings; Host 330, StoreAndForward 130, LocalDb integration 20 pass. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
15013156bf |
test(localdb): two-node convergence for config tables + sf_messages
Four scenarios over the real site pair, driven through the REAL SiteStorageService rather than hand-written SQL. The sibling suites had to hand-write SQL because they were specifications written BEFORE the cutover; this suite runs after it, so it can drive the shipped writers — which is what makes the cascade scenario meaningful. - DeployedConfigRow_ConvergesToB_ColumnForColumn: the existing suite asserts config_json only, so a capture that dropped or defaulted any other column would still pass. deployed_at matters most — SiteReconciliationActor's guarded write compares it. - RemovingAnInstance_ConvergesAllThreeCascadeTables: the plan's flagged highest-risk case. RemoveDeployedConfigAsync deletes from three tables in one transaction, the schema has no foreign keys, and CDC ships three independent per-table streams. A dropped delete leaves a permanently stale override or alarm row on the standby, invisible until the instance name is redeployed. Carries a never-removed control instance, without which "the cascade converged" is indistinguishable from "node B lost these tables entirely". - ANativeAlarmBurst_Converges_AndTheOplogDrains: convergence alone would pass if entries replicated but were never acked, and an oplog that only grows trips the caps into a snapshot resync. - RowsWrittenOnBWhileItsListenerIsDown_SurviveTheRejoin: the union-survives property is per-table, and an unregistered table is silently local-only rather than an error, so it is re-proved on the tables the N1 scenario does not touch. Non-vacuity verified as the plan requires: with the eight Phase 2 RegisterReplicated calls commented out in SiteLocalDbSetup, all four go red (4 failed / 0 passed); restored, 20/20 pass across the three LocalDb suites. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
605e56829e |
chore(config): retire ReplicationEnabled, make legacy db paths migration-only
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 |
||
|
Joseph Doherty
|
037798b367 |
feat(localdb)!: replicate site config + sf_messages via CDC, delete the bespoke replicators
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 |
||
|
Joseph Doherty
|
79ce51612e |
test(site): pin the active-node notification-config purge; scope the guarded write
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 |
||
|
Joseph Doherty
|
c56bf4ae65 |
test(localdb): port resync + config replication intents as CDC convergence specs
Task 11. Companion to Task 10, covering the intents held by SiteReplicationActorTests and SfBufferResyncPredicateTests. N1 Critical is re-expressed, not ported literally (D2). SfBufferResyncPredicate existed because ReplaceAllAsync was a destructive DELETE-then-INSERT, so a wrong-direction resync wiped a live buffer and the code needed an oldest-Up predicate to decide authority. LocalDb's snapshot resync merges per row under LWW and never deletes, so this asserts the property the guard protected — no node loses rows to a peer's snapshot — with no directional-authority assertion, because there is no active/standby asymmetry left to enforce. The setup is the wipe scenario made concrete: each node writes rows the other never sees while partitioned, plus a contended row, then they resync. DEVIATION on the zero-fetch assertion. The plan asked for a fetcher test double recording zero invocations; this harness has no actor system, no central and no IDeploymentConfigFetcher in the graph, so the double would record zero calls whether or not the fetch path still existed. An assertion that cannot fail is worse than none, so the test proves the positive half — config reaches B over replication alone — and the file records that the negative half is proved by Task 15 deleting the code and the build still passing. It also records the D1 scope note: SiteReconciliationActor survives and legitimately fetches at startup, so 'the standby never fetches, ever' would be false. Non-vacuity verified by unregistering deployed_configurations: all 4 fail. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
2bbe66311d |
test(localdb): port store-and-forward replication intents as CDC convergence specs
Task 10. Specifications first, deletion second: the bespoke ReplicationService (explicit Add/Remove/Park/Requeue over Akka) dies at Task 14, so its behaviour is restated here as outcomes the CDC replacement must still deliver. Written in terms of ROWS, not operations — under CDC there is no Add or Park message to observe, only a row that must end up right on both nodes. Not ported: ReplicationOperations_AreDispatchedInIssueOrder. It asserts the mechanism (inline fire-and-forget dispatch), and CDC capture is asynchronous and batched by construction. Its portable content is the ordering OUTCOME — add-then-remove must never converge to present — which is a test here, with that reasoning recorded in the file so it does not read as an accidental drop. DEVIATION: extracted the Phase 1 fixture into LocalDbSitePairHarness rather than duplicating ~150 lines. Phase 1's tests now derive from it and still pass unchanged. The harness registers the Phase 2 tables itself, since production OnReady does not until Task 14; that method is marked for deletion at the cutover, and the 8-table list is written literally so a cutover registering the wrong set fails these tests instead of agreeing with itself. Non-vacuity verified by unregistering sf_messages: 6 of 7 failed. The 7th — the ordering test — PASSED, because an absent row is also what a pair that replicates nothing looks like. Fixed with a control row that must converge in the same window, so the absence is evidence rather than silence. Also corrected two comments from Task 9 that claimed Task 14 makes notification_lists/smtp_configurations replicated. It explicitly does not register them, for the same reason the migrator skips them. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
5ddc7eed6d |
feat(localdb): migrate legacy scadabridge.db config tables into the consolidated DB
Task 9. Seven of the nine site configuration tables are copied out of the legacy scadabridge.db in a single transaction, with one rename at the end: a partial config migration would leave a site node running against half its old configuration, which is worse than failing startup outright. notification_lists and smtp_configurations are deliberately NOT migrated. Both are purged on every deploy and permanently empty by design since the site write paths were removed (2026-07-10), but a pre-fix legacy file can still hold rows — and smtp_configurations.password is plaintext. Task 14 makes these tables replicated, so migrating them would push plaintext SMTP passwords across a channel whose only historical payload was exactly that. The tables are still created; only their historical contents stay behind. Generalizes Task 8's MigrateTable into MigrateFile(many tables, one transaction, one rename), keeping the legacy/current column intersection. Five tests, including a column-parity test asserting each declared column list equals the live SiteStorageSchema's. That mismatch is otherwise invisible: a typo'd column is silently dropped by the intersection, and a missing one silently leaves data behind. Non-vacuity verified twice — once by removing the Migrate call (3 fail), once by wrongly adding notification_lists and smtp_configurations to the table map (the skip test fails). Verified: solution build 0 warnings; Host 329, SiteRuntime 532, StoreAndForward 153 — all pass. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
bdc0dffea2 |
feat(localdb): migrate legacy store-and-forward.db into the consolidated DB
Task 8. Adds ResolveStoreAndForwardPath + MigrateStoreAndForward, wired as a third call in Migrate alongside the two Phase 1 files. Unlike the Phase 1 paths, the store-and-forward default sits INSIDE the data volume, so a real deployment has a real file here holding undelivered messages. This migration genuinely moves data; losing it would discard exactly the buffered calls store-and-forward exists to protect. No id synthesis: sf_messages.id is already a caller-assigned TEXT primary key, so INSERT OR IGNORE is idempotent across a crash-then-rerun. The copy intersects the legacy column set with the current one rather than naming all 16 columns outright. A file from an older build predates execution_id / parent_execution_id / last_attempt_at_ms, and naming a missing column throws 'no such column' — which the existing reader treats as an unrecognised shape and silently discards every row. A required-column (PK) guard keeps that tolerance from degrading into copying NULL-keyed rows. Four tests: copy+rename, crash-before-rename idempotence, the __localdb_oplog assertion that catches migrate-before-register, and the older-column-set case. All four verified to fail without the Migrate call. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
f8aa02e2a9 |
feat(localdb): create config + sf_messages tables in the consolidated DB
Task 7. SiteLocalDbSetup.OnReady now applies SiteStorageSchema and StoreAndForwardSchema alongside the Phase 1 schemas, so the nine site configuration tables and the store-and-forward buffer live in the consolidated LocalDb file. Deliberately NOT registered for replication. The bespoke SiteReplicationActor and the StoreAndForward ReplicationService still own these tables until the Task 14 cutover deletes both and registers them in one commit; registering early would run two replicators over the same rows and let either one's defects hide behind the other's writes. Pinned by two tests through the real composition root: one asserting all twelve tables exist, one asserting ReplicatedTables is EXACTLY the Phase 1 pair. Non-vacuity verified by removing the DDL and observing the failure. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
f2efeb37b7 |
refactor(sf,site): both stores take ILocalDb instead of a connection string
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
|
||
|
Joseph Doherty
|
ac5eb12cce |
refactor(site): extract SiteStorageSchema.Apply from SiteStorageService
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
|
||
|
Joseph Doherty
|
9e3239c5d9 |
refactor(sf): extract StoreAndForwardSchema.Apply from the storage class
Task 3 of the Phase 2 plan. Mirrors OperationTrackingSchema: the sf_messages
DDL, the four additive ALTERs, the last_attempt_at_ms backfill and the due-index
move verbatim into a static sync Apply(SqliteConnection), depending only on
Microsoft.Data.Sqlite. The Host needs to apply this to a LocalDb-managed
connection before RegisterReplicated installs the capture triggers; the store
still calls it so a directly-constructed store stays self-sufficient.
Two deviations from the plan as written, both deliberate:
1. The PRAGMA journal_mode=WAL in InitializeAsync STAYS. The plan's Step 4
snippet drops it, but Task 4 explicitly says not to move the equivalent
pragma ("LocalDb owns the connection's pragmas") — the two tasks contradict
each other. Keeping it preserves behaviour for the intermediate commits and
for directly-constructed stores; it becomes moot in Task 5 when the store
stops opening its own connections. Dropping it now would quietly regress the
concurrent-writer support the pragma's own comment documents.
2. Added a second test beyond the plan's. The specified test asserts only
against a freshly-created table, where CREATE TABLE already lists all 16
columns — it would pass with every ALTER deleted. The added test starts from
the pre-upgrade 12-column shape with a row in it and proves the upgrade path
runs and preserves data.
That second test also surfaced that the backfill lands 1 ms low: julianday()'s
double day-fraction cannot represent every millisecond. Pre-existing behaviour,
carried over verbatim, asserted with a 1 ms tolerance rather than pinning a
precision the implementation never had.
Suite: 154 passed, 0 failed.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
|
||
|
Joseph Doherty
|
8652eab98e |
fix(localdb): root-cause the soak disk-I/O failure + ship the hardening follow-ups
The Phase 2 soak's "LocalDb fails under load" blocker is NOT a product defect. Root cause: host-side (macOS) sqlite3 reads of the live, bind-mounted WAL databases. POSIX advisory locks do not propagate across the virtiofs boundary, so the host reader believes it is the only connection, checkpoints on close and resets the WAL to 0 bytes under the container. The container's still-mapped WAL index then references frames that no longer exist and every subsequent statement fails SQLITE_IOERR_SHORT_READ (522) -> primary code 10, permanently until the process reopens the database. Reproduced on demand both on the rig (one sqlite3 SELECT reset a 4.6 MiB WAL and produced the first error one second later) and in a minimal python:3.12-alpine repro with no LocalDb or .NET involved. Refuted the converse: a freshly-reopened node sustained the full soak load 10+ minutes with zero errors. The original brief's isolation was confounded - both nodes had been poisoned by the same sampling pass, and a poisoned standby looks healthy only because it issues almost no statements. Corrections annotated in place. Hardening shipped alongside: - SqliteErrorCodes.Describe: log SQLite primary AND extended codes at the LocalDb-adjacent catch sites (the missing extended code is what made the original diagnosis so slow). - SiteAuditTelemetryActor: stop touching ActorContext across an await (NotSupportedException), with regression coverage. - infra/reseed.sh: apply the MSSQL init scripts explicitly. The official mssql/server image does not implement /docker-entrypoint-initdb.d, so a fresh volume hung the reseed forever; compose mounts annotated as informational. Deliberately NOT done: detect-and-reopen self-heal in SqliteLocalDb. It defends only against external interference, which is now prevented at the source, and same-kernel production readers see the locks correctly. Build 0 warnings; SiteAuditTelemetryActorTests 9/9, SiteEventLogging 70/70. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
95c108f1d7 |
fix(localdb): allowlist the replication meter + record the Task 12 live gate
Task 12 (live gate on the docker rig) found one real defect, which this fixes.
THE DEFECT. The plan asserted LocalDb metrics "need no work - the
ZB.MOM.WW.LocalDb.Replication meter flows through the existing AddZbTelemetry
Prometheus export". It does not. ZbTelemetryOptions.Meters is an ALLOWLIST: an
unlisted meter's instruments are never observed, with no error, no warning, and
no missing-metric signal anywhere. The rig's /metrics scrape returned 301 lines
and zero localdb_* series. Only looking for them found it.
Fixed by hoisting the allowlist to SiteServiceRegistration.ObservedMeters and
adding LocalDbMetrics.MeterName. The pin test asserts on that constant rather
than on OTel's observed set, because AddZbTelemetry applies its options to a
local instance and never registers IOptions - there is nothing resolvable to
interrogate. The end-to-end proof is the live scrape below; the test exists to
stop the entry being dropped again. Called out as such in the test.
LIVE GATE EVIDENCE (docker rig, 8 nodes, rebuilt via docker/deploy.sh):
1. Consolidated DB created on every site node at /app/data/site-localdb.db -
inside the mounted volume, unlike the legacy CWD-relative databases.
2. Replication proven REAL, not two independent local writes. Both site-a nodes
hold the identical row and, decisively, the identical originating node_id and
HLC in __localdb_row_version:
site_events|{"id":"8b06b37c..."}|116946936661147648|65b00319-...|0
__localdb_peer_state shows a completed handshake in both directions.
3. Convergence: 5/5 events on both nodes, __localdb_row_version byte-identical
across the pair, oplog drained to 0, dead_letter 0 on both.
4. SITE failover drill. Note the repo's failover-drill.sh targets CENTRAL nodes
and does not exercise this claim, so the site-pair flip was run directly:
stopped site-a-a (the node holding the history); site-a-b survived with the
complete pre-flip history (3/3 events) plus its own takeover event. Restarted
site-a-a; it caught up on what it missed and both nodes reconverged to the
same 5 ids. This is the failure Phase 1 exists to prevent.
5. localdb_* now exported after the fix:
localdb_sync_reconnects_total{...} 1
localdb_oplog_depth{...} 0
6. DEFAULT-OFF pin, live and side-by-side: site-b (no Replication section) boots
identically, creates its consolidated DB, registers the engine - and stays
inert. Zero replication log lines, no reconnect counter at all, so it never
dialled anything.
One transient WRN at startup on site-a-a ("Connection refused", reconnecting) is
node A dialling before node B's listener was up; it reconnected within a second.
Expected for a pair that starts together.
Verified: build 0 warnings; Host wiring tests 11/11.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
|
||
|
Joseph Doherty
|
3c395d794a |
test(localdb): two-node site-pair convergence over a real gRPC transport
Task 10 of the LocalDb Phase 1 adoption plan. Two ScadaBridge site nodes replicating the consolidated site database over loopback Kestrel h2c, through the REAL fail-closed auth interceptor - not a stand-in. This is the test that answers the question Phase 1 exists to answer. Every piece upstream of it - schema helpers, DI wiring, the interceptor - can be individually green while the pair still fails to converge. It initializes both databases through SiteLocalDbSetup.OnReady rather than a hand-written schema, so the tables, primary keys and registration ORDER under test are the ones the host actually runs. A local schema would prove only that the test agrees with itself. Scenarios: A->B, B->A (bidirectional even though only A dials, which is what makes failover safe in either direction), LWW convergence on a contended operation, the event union across both nodes, and a peer-offline-then-rejoin catch-up. The event-union scenario is the one that pins the GUID id change: under the old autoincrement scheme both nodes independently mint id=1,2,3..., and last-writer-wins on the primary key would silently DESTROY one node's events rather than merge them. Asserting the union count is what catches that. The whole suite passes in ~0.6s, which is fast enough to be suspicious of a vacuous pass, so it was verified red-first the hard way: deliberately mismatching the two nodes' ApiKey turns all 5 red with 2m30s of timeouts. The tests really do run through the interceptor and the real transport. Node B's database survives its host teardown (registered as a pre-constructed instance, which MS.DI does not dispose), so the rejoin scenario is a genuine rejoin rather than a fresh node. Offline - no docker, no external services. Verified: build 0 warnings; IntegrationTests 80/80. Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
98b94771ae |
feat(localdb): one-time migrator for the pre-Phase-1 site databases
Task 6 of the LocalDb Phase 1 adoption plan. Copies site-tracking.db and
site_events.db into the consolidated database, then renames each source to
<name>.migrated.
Runs AFTER RegisterReplicated, deliberately: capture is trigger-based, so rows
inserted before registration would never enter the oplog and would never reach
the peer - silently. There is a test asserting migrated rows land in
__localdb_oplog, because every other test in the file would still pass with that
ordering reversed.
Idempotent twice over. Tracking rows keep their existing TEXT primary key, so
INSERT OR IGNORE dedupes naturally. Legacy events had autoincrement integer ids,
which the consolidated schema replaced with GUIDs; they get DETERMINISTIC ids of
the form mig-{NodeName}-{legacyId} rather than fresh GUIDs, so a migration that
crashes between commit and rename cannot duplicate history on the next boot. The
NodeName prefix keeps the two nodes of a pair from colliding on their independent
legacy id sequences. Both properties are covered, including an explicit
crash-window test that undoes the rename and re-runs.
All-or-nothing: the copy commits in one transaction and the rename happens only
afterwards, so a failure throws out of OnReady, fails host startup, and leaves
the legacy files untouched. Deviation from the plan: it specified
BeginTransactionAsync, but OnReady is a synchronous Action<ILocalDb>. A raw
transaction on a CreateConnection() connection gives identical atomicity and
identical trigger capture without blocking on async in a startup callback.
Legacy paths come from the OLD config keys and fall back to the CWD-relative code
defaults, because that is where the old code actually wrote them. Note this means
both legacy databases have always lived outside the mounted data volume and were
already discarded on every container recreate - so on the rig this migrator will
usually find nothing, which is a legitimate no-op rather than a failure. Phase 1
incidentally fixes that data-loss bug.
Non-failure cases are covered too: missing files, a legacy file with no such
table, and in-memory legacy connection strings from test/dev configs.
Verified: build 0 warnings; Host 316/316 (9 migrator tests new).
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
|
||
|
Joseph Doherty
|
59c695191c |
feat(localdb): fail-closed auth on the sync endpoint + replication health signal
Tasks 8 and 9 of the LocalDb Phase 1 adoption plan.
Task 8 - LocalDbSyncAuthInterceptor. The replication library's LocalDbSyncService
verifies nothing; inbound auth is explicitly the host's job. Without this,
anything able to reach a site node's gRPC port could stream arbitrary rows into
the consolidated site database - including OperationTracking, which central
reconciles from.
Scoped strictly to /localdb_sync.v1.LocalDbSync/; SiteStream shares the same
AddGrpc pipeline and passes through untouched. Fail-closed: with no
LocalDb:Replication:ApiKey configured NO sync stream is accepted, authenticated
or not. That is deliberate - "no key" is the default every site node ships with,
so treating it as "no auth required" would expose the endpoint on precisely the
most common configuration. Comparison is FixedTimeEquals over UTF-8 bytes.
All four server handler shapes are gated, not just unary: the sync RPC is a
bidirectional stream, so gating only the unary path would leave the real endpoint
open while every unary test still passed. There is a test for that.
Deviation from the plan: it specified Grpc.Core.Testing for the fake
ServerCallContext. That type ships in the retired native Grpc.Core package and
does not exist on the grpc-dotnet stack this solution uses; a minimal
FakeServerCallContext in the test file was the better trade than adding a dead
dependency.
Task 9 - ISyncStatus onto the site health report as LocalDbReplicationConnected
and LocalDbOplogBacklog, via a delegate-seam hosted service following the
AddSiteEventLogHealthMetricsBridge precedent (HealthMonitoring takes no reference
on the replication library). Both are additive init properties, so the
Akka-remoted SiteHealthReport constructor signature is untouched.
Both fields are nullable and the distinction is load-bearing:
- null = the reporter has not run / replication is not wired ("no data");
- false/0 = a real reading. On a node with no peer that IS the healthy
default-OFF state, not an outage.
OplogBacklog is passed through nullable end-to-end because ISyncStatus returns
null when the poll fails - flattening it to 0 would report a replication pair
that cannot read its own oplog as perfectly healthy. The collector stores both
values as one tuple and CollectReport reads it once, so a torn read cannot pair a
fresh Connected with a stale backlog.
Verified: build 0 warnings; Host 307/307 (8 interceptor + 3 health tests new),
HealthMonitoring 97/97, Commons 684/684.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
|
||
|
Joseph Doherty
|
a560e9eaf9 |
feat(localdb): wire 2-node replication for the site database, default OFF
Task 7 of the LocalDb Phase 1 adoption plan.
AddZbLocalDbReplication registers the engine; MapZbLocalDbSync exposes the
passive endpoint the peer node dials. Both are unconditional but INERT by
default: with no LocalDb:Replication:PeerAddress the initiating
SyncBackgroundService starts and immediately idles, and nothing dials the
endpoint. A site pair replicates only once an operator sets a peer on BOTH
nodes. The endpoint shares the Kestrel h2c listener the site gRPC server already
uses, so there are no listener or port changes.
Deviation from the plan: it put AddZbLocalDbReplication in Program.cs. Doing so
would have left it untested - the composition-root tests build
SiteServiceRegistration's graph, not Program.cs - so a registration that silently
went missing would still show green. It now sits next to AddZbLocalDb in
SiteServiceRegistration, where the DI-graph tests actually cover it.
MapZbLocalDbSync stays in Program.cs because it needs the WebApplication.
Two pins added to SiteLocalDbWiringTests, which configures no peer:
- the engine resolves and is idle (not connected, no peer, never synced) -
default-off means inert, not unregistered;
- OplogBacklog is 0, not null. It is deliberately nullable so a failed poll
reads as "unknown" instead of a healthy zero; a real 0 proves the provider is
wired to the oplog, which Task 9's health signal depends on.
Inbound auth on the sync endpoint is Task 8. Until that lands the endpoint is
unauthenticated - which is precisely why replication ships default-OFF and no
config in this repo sets a peer.
Verified: build 0 warnings; Host 296/296, SiteEventLogging 70/70,
SiteRuntime 530/530, Commons 684/684, Communication 312/312.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
|
||
|
Joseph Doherty
|
0d8a80aa27 |
feat(localdb): rewire SiteEventLogger onto the consolidated site database
Completes Task 5a of the LocalDb Phase 1 adoption plan. The GUID-id half landed
with Task 2 (
|
||
|
Joseph Doherty
|
0b5e9b44f6 |
feat(localdb): rewire OperationTrackingStore onto the consolidated site database
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 |
||
|
Joseph Doherty
|
2977e6c45c |
feat(localdb): register the consolidated site database in the composition root
Task 3. Site nodes now open one ZB.MOM.WW.LocalDb-managed database holding OperationTracking and site_events as replicated tables. SiteLocalDbSetup.OnReady applies both schemas and then RegisterReplicated's them. That order is load-bearing and easy to get silently wrong: capture is trigger-based CDC, so rows written before registration are never captured or snapshotted - they would be invisible to the peer forever, with no error anywhere. Storage ships UNCONDITIONALLY, no feature flag. With no peer configured LocalDb is just a local SQLite file, so this is behaviour-equivalent to what site nodes do today. Replication is opt-in and lands in Program.cs (Task 7), which owns the gRPC host the sync endpoint needs. Config: LocalDb:Path added to all EIGHT site-node appsettings (the plan said six - docker-env2's site-x pair exists too), plus the dev template and the wonder-app-vd03 production sample. All ten are required, not optional: LocalDbOptions.Path is validated with ValidateOnStart, so a site node without it fails fast at startup. That is the desired posture, and it is what caught ActorPathTests - the one Host fixture that actually starts a host - whose config now sets the path too. Note the path choice fixes an existing data-loss bug in passing: site-tracking.db and site_events.db both defaulted to CWD-relative paths (/app/...), outside the mounted volume, so they were silently lost on every container recreate. The consolidated database lives on /app/data. Tests are container-built against the REAL SiteServiceRegistration.Configure, not a hand-assembled ServiceCollection - this family has shipped three wiring defects that only a real graph would catch (inert Secrets replicator 0.2.0, singleton deadlock 0.2.2, Secrets.Ui missing IAuditWriter). They assert the library's own view of what is registered rather than that we called the right method. Verified (all after the change): dotnet build ZB.MOM.WW.ScadaBridge.slnx -> 0 Error(s), 0 Warning(s) Host.Tests -> 294 passed (5 new, red-first) SiteRuntime.Tests -> 530 passed CentralUI.Tests -> 925 passed Commons.Tests -> 684 passed Communication.Tests -> 312 passed SiteEventLogging.Tests-> 70 passed Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
727fa48cba |
feat(localdb): move site_events to application-minted GUID ids
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
|
||
|
Joseph Doherty
|
f056b67e9a |
fix(build): suppress AngleSharp advisory instead of pinning (corrects ecf6b628)
|
||
|
Joseph Doherty
|
ecf6b62850 |
fix(build): pin AngleSharp 1.5.2 to unbreak the solution build
GHSA-pgww-w46g-26qg (NU1902, moderate) on AngleSharp 1.1.1, pulled in transitively by bunit into CentralUI.Tests. Under TreatWarningsAsErrors this made `dotnet build ZB.MOM.WW.ScadaBridge.slnx` fail on a CLEAN main - the whole suite was unbuildable and every "0 warnings / suite green" gate unverifiable. Pre-existing, not introduced here; surfaced while starting LocalDb Phase 1, whose per-task verification depends on a green baseline. Same fix as the identical break in HistorianGateway (historiangw @ 6bc005d): pin the leaf, test-only. AngleSharp is a bunit HTML-parsing dependency and reaches no production project. Bumping bunit does not help - 2.0.33-preview is the current preview line and still resolves the vulnerable AngleSharp. Verified: dotnet build ZB.MOM.WW.ScadaBridge.slnx -> 0 Error(s), 0 Warning(s) (was 1 Error before this commit). Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
fc86e8bfe1 |
fix(secrets): bridge the shared audit seam so /admin/secrets renders (#22)
Secrets.Ui components inject the SHARED ZB.MOM.WW.Audit.IAuditWriter seam, which is deliberately distinct from the repo's own Commons IAuditWriter — the G-4 adoption mounted the UI without registering it, so component activation threw and the page 500'd on every render (hidden behind the login wall; it plausibly never worked). Fix: CentralSharedSeamAuditWriter forwards the shared seam into the central direct-write path (ICentralAuditWriter -> dbo.AuditLog) — NOT the site SQLite chain, which is never resolved on central and has no forwarder there. Registered in the Central composition root next to the Secrets UI authorization wiring. Regression pin: CentralCompositionRootTests resolves the full Secrets.Ui component injection set (ISecretStore / ISecretCipher / shared IAuditWriter) out of the REAL Program.cs via WebApplicationFactory, plus a type check that the seam is the central bridge. Red on the previous commit; the defect class is invisible until the DI graph is actually built. Live-proven on the local docker cluster: /admin/secrets 200 + interactive (Blazor circuit + working @onclick), and secret.add / secret.delete events (both outcomes) landed in central dbo.AuditLog via the bridge. Closes #22 Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts |
||
|
Joseph Doherty
|
8e12f99432 |
feat(secrets): opt-in SQL-Server hub replication for the host secret store
Routes both host-container secret registrations (central role in Program.cs,
site role in SiteServiceRegistration) through a new SecretsRegistration
composition seam that optionally enables ZB.MOM.WW.Secrets.Replicator.SqlServer
hub-replication mode: each node keeps a LOCAL store that syncs bidirectionally
with a shared central SQL hub, so a site cluster keeps resolving secrets
straight through a WAN outage to central.
OPT-IN GATE (the load-bearing part). AddZbSecretsSqlServerReplication validates
its options EAGERLY at registration time, so wiring it unconditionally would
make ScadaBridge fail to START anywhere Secrets:SqlServer:ConnectionString is
unset -- every dev box, every docker node, every existing deployment. The
SQL-Server package is therefore only touched when BOTH Secrets:Replication:
Enabled is true AND a non-blank connection string is present; otherwise the
registration is byte-identical to the previous plain AddZbSecrets call.
Enabled-without-a-connection-string falls back to local-only and logs a warning
rather than failing the node or silently looking healthy.
BOOTSTRAP CYCLE. The hub connection string is itself a secret and can never come
from the hub -- a node cannot read the hub to learn how to reach the hub. It must
arrive from outside the replicated set: an environment variable, or a ${secret:}
reference seeded in that node's own LOCAL store. appsettings.json therefore ships
ConnectionString empty with a _comment saying so (leaf keys starting with '_' are
skipped by the reference expander, verified in SecretReferenceExpander). No real
connection string is committed. The pre-host ${secret:} expander in Program.cs is
deliberately left on a plain local SQLite store for the same reason.
Per-node docker appsettings are intentionally NOT modified -- replication stays
off there for now.
Tests written before the wiring and confirmed red first (2 failed / 4 passed),
green after (6/6). They BUILD a container and RESOLVE from it rather than
asserting over ServiceDescriptors: a decorator can look correct as a descriptor
list and still throw on first resolve because the undecorated concrete store it
depends on is missing -- that exact defect shipped once in this library with all
descriptor-level tests green, so the undecorated SqliteSecretStore gets its own
resolution test.
Verified: Host.Tests 285/285 pass; full build (all projects except the
pre-existing AngleSharp NU1902 CentralUI.Tests restore break) 0 warnings,
0 errors.
Claude-Session: https://claude.ai/code/session_01BL2Vu1ESDQ9SCN4gVKkdts
|
||
|
Joseph Doherty
|
2a0faeab6f |
fix(security): actually fix GHSA-2m69-gcr7-jv3q instead of suppressing it
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). |
||
|
Joseph Doherty
|
b1b4874090 |
fix(siteruntime): injectable ScriptExecutionScheduler + un-leakable expression-fault test
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: #18 Fixes: #16 |
||
|
Joseph Doherty
|
3e84eee195 |
fix(dcl): route native OPC UA alarms by binding identity, not event name
A native OPC UA alarm source's SourceReference has to be two things at once:
it is parsed as a NodeId to open the monitored item, and matched as a plain-name
prefix against the event's SourceName to route the transition to an instance. No
string is both, so a NodeId-form binding subscribed correctly and then silently
dropped every transition — "pymodbus/plc/HR200".StartsWith("nsu=...;s=pymodbus/plc/HR200")
is false. Only the empty (Server-object) binding worked, because StartsWith("")
matches everything, which is why the sole live smoke test never caught it.
Each OPC UA alarm feed is opened for exactly one binding, so every transition on
it belongs to that binding. The adapter now tags each transition's routing
identity (SourceObjectReference) with the binding string verbatim via the pure
OpcUaAlarmMapper.BuildIdentity, making DataConnectionActor's routing key an exact
match regardless of whether the binding is stored as ns=<index> or the durable
nsu=<uri> form. The Server-object aggregate feed keeps an empty routing identity,
so it reaches only "mirror everything" subscribers and never leaks into a
specific-node binding. The per-condition SourceReference key stays the readable
SourceName.ConditionName, so persistence and display are unchanged, and MxGateway
is untouched — its bindings are names and its mapper already emits matching names.
Unblocked by lmxopcua#473 (OtOpcUa now populates SourceNode/SourceName/EventType
on conditions); SourceName is the RawPath, so the per-condition key is unique.
Live end-to-end verification against native alarms still needs a v3 rig.
Fixes: Gitea #17
|
||
|
Joseph Doherty
|
30196d1ab8 |
feat(dcl): bind OPC UA nodes by namespace URI, not baked index
A stored ns=<index> reference is only meaningful against one server's namespace table. A server that adds, removes or reorders a namespace silently re-points every binding: best case BadNodeIdUnknown, worst case the index now names a different namespace holding a colliding identifier and the binding resolves to the wrong node with Good quality. Nothing could detect that, because ScadaBridge stored no namespace URI anywhere. OtOpcUa v3.0 makes this concrete: v2's sole custom namespace and v3's raw namespace both sit at index 2, so v2-era bindings resolve against v3 without error while meaning something else entirely. Adds OpcUaNodeReference as the single translation seam between stored references and the wire. Resolve() accepts both ns=<index>;s=<id> (existing bindings, which keep working unchanged) and the durable nsu=<uri>;s=<id>, mapping the URI to the live index at use time; it is wired into every parse site — subscribe, read, write, alarm-subscribe and browse. An unpublished URI now throws naming the URI rather than binding to whatever occupies that index, and a svr= reference to another server is rejected instead of being resolved against the wrong address space. The browser emits ToDurable(), so what the picker shows is what gets stored and newly-authored bindings are index-proof from the start. That also closes a round-trip gap: browse previously emitted ExpandedNodeId.ToString(), which for a URI- or server-index-carrying reference produced a string NodeId.Parse could not read back — the same method already resolved it correctly 57 lines later. Bindings stored before this change keep their ns= form and keep working; they are only as durable as the server's namespace order. Re-authoring against the picker is what makes them durable, and that re-bind still needs a live v3 rig. Refs: Gitea #14 |
||
|
Joseph Doherty
|
d2a6107cdb |
test(host): serialize Central-boot fixtures to close env-var race
CentralDbTestEnvironment sets five process-wide environment variables, and
Program's AddEnvironmentVariables() reads them at an unpredictable point during
host boot. With xUnit collection parallelization on, one fixture's teardown
could clear a var mid-boot for a sibling. Since the secrets adoption (G-4)
three of those keys are ${secret:...} references that fail closed, turning a
previously benign empty value into a SecretNotFoundException that aborts the
boot — an intermittent CI failure.
Adds a HostBootCollection that serializes every fixture booting a real host
while depending on that shared state, folding in the narrower "ActorSystem"
collection so its members stay serialized with each other as before. Site-role
fixtures stay parallel: they call Configuration.Sources.Clear(), dropping the
env-var provider, so they cannot participate in the race.
CentralDbTestEnvironment now also fails fast if two instances are ever live at
once, making a regression (a fixture added outside the collection) deterministic
rather than intermittent — this is what surfaced the disposed-CTS defect fixed
in the previous commit. Fixture teardown is try/finally so a throwing host
teardown can no longer strand the vars for the rest of the run.
Cost: Host.Tests runs ~2m30s -> ~4m10s; the serialized Central-boot classes are
most of the assembly's parallelism.
Fixes: Gitea #15
|
||
|
Joseph Doherty
|
9110a4eb01 |
fix(auditlog,health): harden hosted-service shutdown against disposed CTS
The host does not guarantee IHostedService.StopAsync is driven before the DI container is disposed — WebApplicationFactory's teardown reaches Dispose first — so cancelling the internal CTS from StopAsync threw ObjectDisposedException and aborted the host's whole shutdown sequence. Four services shared the same copy-pasted lifecycle and the same two races: StopAsync cancelling an already- disposed CTS, and StartAsync reading _cts.Token lazily inside the Task.Run lambda, which faults the loop task the host awaits when Dispose wins that race. Each service now captures the token on the caller's thread, tolerates a disposed CTS, and cancels-before-disposing so the loop is always signalled and its pending Task.Delay sees a cancelled token rather than a dead source. SiteAuditBacklogReporter also gains the outer OperationCanceledException guard its sibling SiteAuditRetentionService already carried (arch-review 04 R2, R7), without which a shutdown landing mid-probe threw TaskCanceledException out of Host.StopAsync. Surfaced while verifying the Gitea #15 test-harness fix: in Host.Tests the aborted teardown skipped the fixture's env-var restore, contaminating every later test in the run. Refs: Gitea #15 |
||
|
Joseph Doherty
|
128f159692 | feat(secrets): resolve MxGateway ApiKey secret: refs at connect time (ScadaBridge G-3 T9) | ||
|
Joseph Doherty
|
41e17d2ff8 | feat(secrets): mount /admin/secrets + register secrets authz in CentralUI (ScadaBridge G-6 T7) | ||
|
Joseph Doherty
|
cf715b813b |
feat(secrets): switch Central config secrets to ${secret:} tokens (ScadaBridge G-4 T4)
Switch the three literal ${SCADABRIDGE_*} placeholders in appsettings.Central.json
to ${secret:} references resolved by the pre-host secrets expander, and update the
_secrets doc note. Fix Host.Tests fixtures that boot the real Program pipeline against
Central config: CentralDbTestEnvironment now also supplies the LDAP service-account
password and JWT signing key as whole-key env overrides (rollback path) so the expander
skips the tokens without a seeded store; the three fixtures that managed env vars
inline now use CentralDbTestEnvironment.
|
||
|
Joseph Doherty
|
1930f19b1a |
fix(test): relax exact compile-cache hit-count assertion to >= 1
SiteScriptCompileCache.Hits is a process-global counter; other test classes in the assembly compile scripts concurrently (not in this serialized collection), so an exact post-Clear count is non-deterministic under a full-assembly parallel run. Assert.Same(r1,r2) remains the definitive proof of one shared Roslyn compile. Integration-surfaced; belongs with plan R2-03 T5/T6. |
||
|
Joseph Doherty
|
53889aec9f |
fix(test): set ScadaBridge:Node:NodeName in IntegrationTests host-boot config
R2-08 T7 added an eager NodeName validator (empty NodeName NULLs the SourceNode audit column) but only patched Host.Tests fixtures; the IntegrationTests WebApplicationFactory never set it, so every host-boot test failed OptionsValidationException. Integration-surfaced; belongs with plan R2-08 T7. |
||
|
Joseph Doherty
|
ccee558ca7 |
merge(r2): r2-plan08
# Conflicts: # src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationOptionsValidator.cs |
||
|
Joseph Doherty
|
ca4b0dd849 | merge(r2): r2-plan04 | ||
|
Joseph Doherty
|
699af8fc16 |
merge(r2): r2-plan06
# Conflicts: # CLAUDE.md |
||
|
Joseph Doherty
|
84fdea60cf | merge(r2): r2-plan05 | ||
|
Joseph Doherty
|
3ad5ad0d7e | merge(r2): r2-plan03 |