e0f105c3b31b8787fa7f4ec2c9e2953ed1330e23
1316 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Joseph Doherty
|
8524a7f746 |
test(sms-e2e): valid Twilio SID fixture so the create path passes (Gitea #29)
TestAccountSid was 'ACtest123' (AC + 6 chars). The management create
path validates the Account SID against ^AC[0-9a-fA-F]{32}$
(ManagementActor.cs:2205, added 2026-07-10, commit
|
||
|
Joseph Doherty
|
c4dcd9bc02 |
fix(transport): run bundle import inside the execution strategy (Gitea #28)
BundleImporter.ApplyAsync opened a user-initiated transaction directly,
but the central ConfigurationDb context is configured with
EnableRetryOnFailure. SqlServerRetryingExecutionStrategy rejects
user-initiated transactions, so every bundle import threw on any real
SQL Server ('...does not support user-initiated transactions.'). The
whole Transport suite ran on the in-memory provider (no retrying
strategy, BeginTransaction is a no-op) so it never surfaced.
Extract the transactional apply into ApplyMergeAsync and drive it via
_dbContext.Database.CreateExecutionStrategy().ExecuteAsync, so the
strategy owns the BeginTransaction -> apply -> Commit unit as one
retriable block. The delegate resets per-attempt state (change tracker
+ ApplyMergeAsync rebuilds its own summary/resolution accumulators) so a
retried attempt cannot double-apply; a rollback failure is captured and
surfaced on the BundleImportFailed audit row exactly as before. Post-
commit side effects (ScriptArtifactsChanged publish, session zero/
remove) moved outside the retriable delegate so they run once.
Regression test: BundleImporterRetryingStrategyTests runs the import on
SQLite with a retrying execution strategy (RetriesOnFailure == true,
arming the same guard as production). It fails with the exact production
error on the pre-fix code and passes after. Existing rollback/apply
contracts unchanged (104 integration + 154 unit tests green).
|
||
|
Joseph Doherty
|
7fd5cb2b56 |
feat(comm): Phase 4 — delete Akka ClusterClient site↔central transport, gRPC-only
ClusterClient→gRPC migration Phase 4 (docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Phases 2/3 proved both directions on gRPC; this removes the Akka transport underneath. Deleted: - AkkaCentralTransport, AkkaSiteTransport (+ their dedicated tests) - ISiteClientFactory + DefaultSiteClientFactory; CentralCommunicationActor legacy ctor + SelectTransport (Host now builds GrpcSiteTransport and injects it) - ClusterClient creation + both ClusterClientReceptionist.RegisterService calls in AkkaHostedService; the RegisterCentralClient message + receive block - CommunicationOptions.CentralContactPoints; the CentralTransport/SiteTransport coexistence flags; the CentralTransportMode/SiteTransportKind enums gRPC is now the only site↔central transport (site→central CentralControlService via GrpcCentralTransport; central→site SiteCommandService via GrpcSiteTransport), both built unconditionally by the Host. NoOpCentralTransport is the fail-loud null-default so TestKit command-dispatch suites still construct the site actor without a wired transport; production always injects GrpcCentralTransport. Config: CentralGrpcEndpoints is now unconditional — CommunicationOptionsValidator rejects blank entries (role-agnostic), and StartupValidator requires a Site node to list >=1 endpoint (fail-fast, mirrors GrpcPsk). Rig configs moved CentralContactPoints -> CentralGrpcEndpoints (docker x6, docker-env2 x2, Host default, deploy/wonder-app-vd03). Kept Akka.Cluster.Tools (ClusterSingleton still used). Tests: build 0/0; Communication.Tests 640, Host.Tests 421 green. Removed the ClusterClient.Send per-site-routing tests (covered by the transport suites), swapped the ISiteClientFactory-based ctors to a substitute ISiteCommandTransport, converted the audit-push integration relay to an in-process bridge transport. Docs: Component-Communication/Host/StoreAndForward, components/Communication, topology-guide, grpc_streams (SUPERSEDED note), the frame-size known-issue (retired amendment), and CLAUDE.md transport decisions. Not included: the dead IntegrationCallRequest path (#32) is a separate user-owned behavioral decision — SiteEnvelope routing is transport-agnostic so it still compiles. |
||
|
Joseph Doherty
|
86ad4d5c8e |
feat(comm): T1B.3 — central-side gRPC site-command transport seam (default Akka)
Extract the central→site send path in CentralCommunicationActor behind a new ISiteCommandTransport, selected by ScadaBridge:Communication:SiteTransport (Akka | Grpc, default Akka — rollback = flip the flag). CommunicationService's 27 commands, SiteCallAuditActor's 2 parked relays and DebugStreamBridgeActor's subscribe/unsubscribe are untouched; the seam sits below SiteEnvelope. - AkkaSiteTransport: today's per-site ClusterClient path extracted verbatim (the _siteClients lookup + ClusterClient.Send with the reply-to sender preserved, and the "no client ⇒ warn + drop, caller's Ask times out" path). - GrpcSiteTransport: dials the site SiteCommandService (T1B.1 proto client) via SiteCommandDtoMapper, PSK + x-scadabridge-site on the channel through ControlPlaneCredentials, per-command deadlines set EQUAL to today's CommunicationService Ask timeouts (per-command, not per-group: DeploymentState query and TriggerSiteFailover use QueryTimeout; the two parked relays map to QueryTimeout so SiteCallAudit's inner RelayTimeout 10s < 30s ordering holds; WaitForAttribute keeps its dynamic Timeout + IntegrationTimeout). - SitePairChannelProvider: per-site A/B channel pair with sticky failover (flip only on Unavailable — NEVER on DeadlineExceeded, a write/deploy/failover may have run), background failback probe to the preferred node with 1s→60s doubling backoff, PSK invalidation on site removal. Fed by the SAME DB refresh loop (extended to carry GrpcNodeA/GrpcNodeBAddress) — no second poll. Tests: actor-with-substitute-transport (routing, Ask-reply plumbing, per-site lifecycle across refreshes), ResolveDeadline pinned to each command's current Ask timeout, and GrpcSiteTransport/SitePairChannelProvider over dual in-process TestServers (PSK+header+deadline attached, Unavailable failover + stickiness, failback to preferred, no-retry-on-DeadlineExceeded). Proto csproj untouched (no active <Protobuf> item). Full solution builds 0 warnings; Communication.Tests 607 green with Akka default. |
||
|
Joseph Doherty
|
518c699b90 |
feat(comm): extract SiteCommandDispatcher; site serves commands over gRPC too (T1B.2)
Refactor SiteCommunicationActor's central→site routing table into one SiteCommandDispatcher — the single routing truth for the 28 migrated commands (IntegrationCallRequest, the dead 29th, stays on the actor and out of the dispatcher). The Akka actor and the new SiteCommandGrpcService both route through one dispatcher instance so the two transports can never drift on where a command goes. Server-side only: nothing central flips to gRPC yet (that is T1B.3); ClusterClient remains the live path. Decisions worth recording: - Targets preserved byte-for-byte. Lifecycle/OPC UA/query/route → the Deployment Manager singleton proxy; DeployArtifacts/EventLog/parked → their null-guarded handlers with the exact same "handler not available" replies; the parked handler stays NODE-LOCAL (per-node replicated-store owner), never the singleton proxy — pinned by a dispatcher test that asserts the target is the parked probe and NOT the dm proxy. - Sender preservation intact. The actor's command handlers became thin DispatchCommand delegations that still Forward (central Ask → reply routes straight back); the existing SiteCommunicationActorTests pass unchanged, which is the regression guard for that plumbing. UnsubscribeDebugView keeps its fire-and-forget shape: the actor Forwards, the gRPC service Tells + returns the synthetic UnsubscribeDebugViewAck so a unary RPC still answers. - Ack-before-Leave on failover. The dispatcher's PrepareFailover resolves the standby with a DRY-RUN (no leave) to build the ack, and hands back a deferred CommitLeave; the gRPC service returns the ack, then schedules the real Cluster.Leave — so a caller reaching the very node about to leave still gets its ack instead of a broken stream. The actor path keeps today's coupled resolve-and-leave (over ClusterClient the ack Tell only enqueues, so order is immaterial). Proven at both levels: a dispatcher test asserts the ack is built before CommitLeave runs, and a TestServer test asserts the recorded seam order is resolve-then-leave. - ControlPlaneAuthInterceptor gates SiteCommandService by EXTENDING DefaultGatedPrefixes (descriptor-derived), not by adding a constructor — the one-public-ctor invariant and its test stay green. Tests: SiteCommandDispatcherTests (28-command routing incl. parked node-locality and both failover paths) and SiteCommandGrpcService TestServer tests (auth, readiness→Unavailable, one command per oneof group, failover ordering). Full solution build 0/0; Communication.Tests 574 and Host.Tests 377 green. No active <Protobuf> item. |
||
|
Joseph Doherty
|
59b13d317b |
feat(grpc): T1B.1 — site_command.proto + SiteCommandDtoMapper + round-trip goldens
Phase 1B's contract slice: the wire shape and the canonical translation for the 28 central→site commands that leave ClusterClient. No behaviour changes yet — SiteCommunicationActor and CentralCommunicationActor are untouched; the dispatcher refactor (T1B.2) and the central transport seam (T1B.3) consume this. Protos/site_command.proto (package scadabridge.sitecommand.v1, service SiteCommandService): six domain RPCs, each with a `oneof` request/reply envelope. The grouping is what carries deadline policy — every command inside a group shares a CommunicationOptions timeout class today, so one RPC per group keeps the deadline choice in one place on the client and one dispatch switch on the server, while the oneof keeps each command individually typed: ExecuteLifecycle(6) · ExecuteOpcUa(8) · ExecuteQuery(4) · ExecuteParked(5) · ExecuteRoute(4) · TriggerFailover(1). IntegrationCallRequest — the 29th entry on SiteCommunicationActor's receive table — is deliberately excluded as dead code (2026-07-22-integration-call-routing-is-dead-code.md). Contract decisions worth knowing: - Nullable COLLECTIONS ride in per-collection wrapper messages (DeployArtifactsCommand's six artifact lists, CertTrustResult.Certs, RouteToCallRequest.Parameters). proto3 repeated/map collapses null into empty, and that distinction is live at the site — the same silent-data-loss class the transport round-trip guard exposed in PLAN-05 T8. Goldens cover null, empty and populated for each. - Nullable strings use the empty-string-means-null convention already set by AuditEventDtoMapper, with ONE exception: RouteToWaitForAttributeRequest's TargetValueEncoded, where "wait for the empty string" is a real target, so it carries a StringValue wrapper. Both behaviours are asserted, not assumed. - Nullable enums ride in one-field messages (proto3 enums have no presence and no stock wrapper). Enum translation is an explicit switch in both directions — never by ordinal — so reordering a C# enum cannot re-map the wire; every wire enum reserves 0 for _UNSPECIFIED and decodes to a documented safe default rather than faulting a command from a version-skewed peer. - New LooseValueCodec carries the surviving `object?` members (script params and return values, attribute values, tag read/write values) as a type-tagged union so a boxed value keeps its runtime CLR type, as it does today under Akka's type-preserving JSON serializer. Dates ride as invariant round-trip strings, not Timestamp, which would silently normalise away DateTime.Kind and DateTimeOffset.Offset. Lists/maps recurse; anything outside the tagged set falls back to JSON and is documented as CLR-type-lossy. - DebugViewSnapshot gets its own full-fidelity alarm/attribute messages rather than reusing sitestream's AlarmStateUpdate, which flattens values to display strings — right for a live stream, lossy for a snapshot the UI treats as authoritative. The encoder omits an AlarmStateChanged.Condition that already equals the record's derived default, so computed alarms round-trip exactly (record equality compares the nullable backing field, not the property). Tests are reflection-driven so the coverage cannot drift: the round-trip theory enumerates the mapper's own ToProto overloads, the envelope guards enumerate the generated oneof descriptors, and a missing golden fails the build. 216 new tests green (Communication 532 total, Commons 684 total, solution build 0/0). Codegen is checked in under SiteCommandGrpc/ per the sitestream recipe; the <Protobuf> ItemGroup stays commented out (an active one segfaults protoc in the linux_arm64 Docker image). docker/regen-proto.sh now handles every proto in that ItemGroup instead of just sitestream, and re-comments idempotently. |
||
|
Joseph Doherty
|
0e162cb250 |
fix(grpc): central Kestrel gRPC listener was dropping the :5000 HTTP surface (T1A.2 regression)
Caught on the Phase 1A rig proof: central-a logged only "Now listening on: http://[::]:8083" and nothing else. Central UI, the Management + Inbound API, and the /health/* endpoints Traefik + IActiveNodeGate depend on were all gone, with no startup error — the node booted, joined the cluster and served gRPC fine. Cause: calling options.ListenAnyIP in ConfigureKestrel puts Kestrel into explicit-endpoints mode, which SUPPRESSES the URLs from ASPNETCORE_URLS/--urls entirely — it is not additive, contrary to the comment T1A.2 shipped. Central's whole HTTP/1 surface lives on that URL (http://+:5000 on the rig, a different port in prod), so binding only the gRPC port silently deleted it. The site branch has the same shape but no ASPNETCORE_URLS surface to lose — it binds every port it needs explicitly. Fix: parse the port(s) from the configured URLs and re-declare them (Http1AndHttp2) alongside the gRPC port (Http2) in the one ConfigureKestrel call. New Program.ParseHttpBindPorts + CentralHttpBindPortsTests (14 cases: wildcard/ipv6/ hostname hosts, multi-URL, de-dupe, scheme-default, null/blank, unparseable-skipped). Why no test caught it originally: unit/E2E tests use TestServer, which never binds real Kestrel. Only a live node exposes the missing listener — which is exactly what the rig proof is for. |
||
|
Joseph Doherty
|
33b15f10a4 |
feat(grpc): site-side ICentralTransport seam + gRPC transport (T1A.3)
Introduce ICentralTransport as the site->central choke point inside SiteCommunicationActor. The seven site->central sends (notification submit/ status, audit + cached-telemetry ingest, reconcile, health, heartbeat) now delegate to an injected transport instead of owning ClusterClient.Send inline. - AkkaCentralTransport: verbatim extraction of today's ClusterClient.Send path, including the exact sender-forwarding that routes central's reply straight back to the waiting Ask. Default when no transport is injected -> behaviour unchanged, existing SiteCommunicationActorTests pass as-is. - GrpcCentralTransport + CentralChannelProvider: dial CentralControlService with sticky failover + background failback (1s-doubling-cap-60s), PSK + x-scadabridge-site via ControlPlaneCredentials, per-call deadlines mirroring today's Ask timeouts. Cross-node retry ONLY on provably-unsent connect failures; never on DeadlineExceeded. Heartbeat stays fire-and-forget. - StaticSitePskProvider: site's single own-key provider (fail-closed). - CommunicationOptions: CentralTransport flag (default Akka) + CentralGrpcEndpoints (validator: required when transport=Grpc). Host selects the impl; the ClusterClient is created only on the Akka path. Tests: actor-with-fake-transport (7 delegations + fault routing + heartbeat no-fault), AkkaCentralTransport sender-forwarding, GrpcCentralTransport over in-process TestServer (failover flip, sticky, failback, PSK+header, deadline, no-retry-on-deadline), validator. Communication.Tests 371 green, Host.Tests 391 green; the three above-seam suites pass unmodified. |
||
|
Joseph Doherty
|
780bb9c369 |
feat(grpc): host CentralControlService on the central node (T1A.2)
Central now ALSO listens for the seven site→central control messages over gRPC, alongside the existing ClusterClient path. Nothing flips to gRPC yet — sites keep CentralTransport=Akka (T1A.3's job); central simply starts also accepting. - CentralControlGrpcService (Communication.Grpc): decodes each RPC onto the SAME in-process message the ClusterClient path carries, Asks the existing CentralCommunicationActor (zero handler-logic changes), encodes the reply via the T1A.1 mapper. Readiness-gated like SiteStreamGrpcServer.SetReady — Unavailable until AkkaHostedService hands the actor over. Heartbeat stays fire-and-forget (Tell, always-OK, never gated on readiness). Ingest reuses the shared SiteStreamGrpcServer.AuditIngestAskTimeout constant. Fault→status mapping is retry-aware: Unavailable (never dispatched, safe to cross-node retry) vs DeadlineExceeded/Internal (it ran, do not re-send elsewhere). - CentralControlAuthInterceptor (Host): a SEPARATE interceptor class, not a variant constructor on ControlPlaneAuthInterceptor. Central's model is per-site (verify the Bearer token against the key for the site in the required x-scadabridge-site header, via ISitePskProvider) where a site verifies its one own-key — a genuinely different model. Fail-closed on every branch: missing or blank header, unresolvable key, and mismatched token all → PermissionDenied, never pass-through. One public constructor only (the explicit-prefix ctor is internal), pinned by a reflection test — a second public ctor makes Grpc.AspNetCore's GetFactory() throw per-call and silently disables the gate. - Explicit Kestrel h2c listener on new option ScadaBridge:Node:CentralGrpcPort (default 8083, symmetric with sites), mirroring the Site branch. Additive to central's :5000 HTTP/1 surface, which is untouched — gRPC does NOT go through Traefik (HTTP/1 only). Registered by type on AddGrpc; service mapped with MapGrpcService. Port range-validated by NodeOptionsValidator. - Rig: publish the central gRPC port 9013:8083 / 9014:8083 on both central nodes so a later task can exercise it. Tests: CentralControlEndToEndTests (Host.Tests, TestServer + real interceptor + real service over a stub actor) proves auth positives/negatives are distinguishable and covers unary + the ingest bridge shapes; the interceptor is registered BY TYPE, never in DI. CentralControlAuthInterceptorTests pins the per-site gate + one-public-ctor invariant. CentralControlGrpcServiceTests (Communication.Tests, TestKit) covers the readiness gate, fire-and-forget heartbeat, and the DeadlineExceeded-vs-Unavailable status mapping. No active <Protobuf> item. Communication.Tests (356) + Host.Tests (384) green. |
||
|
Joseph Doherty
|
d7455577a8 |
feat(grpc): central_control.proto + DTO mapper for the 7 site→central control RPCs (T1A.1)
Phase 1A of the ClusterClient→gRPC migration needs a wire contract for the seven messages SiteCommunicationActor forwards to /user/central-communication. This lands the contract and its mapper only — hosting (T1A.2) and the site-side transport seam (T1A.3) follow. `Protos/central_control.proto` (package scadabridge.centralcontrol.v1, service CentralControlService) declares SubmitNotification, QueryNotificationStatus, IngestAuditEvents, IngestCachedTelemetry, ReconcileSite, ReportSiteHealth and Heartbeat. Note the direction is the inverse of SiteStreamService: here the site dials and central serves. Decisions worth recording: - The two ingest RPCs IMPORT sitestream.proto and reuse AuditEventBatch / CachedTelemetryBatch / IngestAck rather than redeclaring them. The site telemetry actor already builds those messages, so a second copy would fork one wire contract into two kept in lockstep by hand. ForwardState / IngestedAtUtc stay off-wire exactly as they are today. - Heartbeat replies google.protobuf.Empty — it is fire-and-forget and must never surface a fault onto the heartbeat timer path. - The three NULLABLE SiteHealthReport collections travel inside single-field wrapper messages (ConnectionEndpointMapDto / TagQualityMapDto / NodeStatusListDto). proto3 cannot express presence on repeated/map fields, but null and empty genuinely differ here — SiteHealthCollector emits `ClusterNodes: _clusterNodes?.ToList()` and the central health surface reads null as "not reported", not as "reported empty". Same reasoning drives the BoolValue/Int64Value/DoubleValue wrappers on LocalDbReplicationConnected, LocalDbOplogBacklog and the two age gauges, whose docs are explicit that null is not zero/false. - ConnectionHealthEnum reserves 0 for UNSPECIFIED instead of mapping Connected onto it, and the decoder resolves anything unknown to ConnectionHealth.Error. An unrecognised connection state must not render as "healthy". - Guid? execution ids travel as "D" strings with empty meaning null; a malformed non-empty value throws rather than being laundered into "no correlation". - DateTimeOffset normalizes to a UTC instant (a protobuf Timestamp has no offset). Lossless in practice — every producer stamps UTC — and documented + asserted rather than left implicit. SiteCallDtoMapper gains a ToDto(SiteCall) overload. Its doc comment previously asserted such a method "would be dead code"; that held only while ClusterClient was the sole path from IngestCachedTelemetryCommand (which carries SiteCall, not SiteCallOperational) to central. Comment corrected alongside. Golden tests round-trip every message through a real protobuf encode/decode — DTO → proto → bytes → proto → DTO — with a fully-populated case and a null/empty/minimal case for every optional member. Verified to have teeth by mutation: dropping a scalar, collapsing an empty nullable collection to absent, and nulling a gauge each fail a test. Codegen stays CHECKED IN under CentralControlGrpc/ (protoc segfaults in the linux_arm64 Docker image); no active <Protobuf> item is committed. docker/regen-proto.sh is generalized to `regen-proto.sh [sitestream| centralcontrol|all]` — it now injects the ItemGroup rather than unwrapping a comment, so it no longer depends on there being exactly one Protobuf line, and it restores the csproj verbatim on every exit path. |
||
|
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 |