ScadaBridge/archreview/00-OVERALL.md

ScadaBridge Deep Architecture Review — Overall Report

Date: 2026-07-08 Scope: Full system — all 27 components, docker/deploy topology, tests, and design docs Method: 8 parallel domain reviews, each reading the component design docs first and then the actual source (findings are grounded in file:line citations in the domain reports). Dimensions: stability, performance, conventions, underdeveloped areas.

Report Index

#	Report	Domain	Verdict (one line)
01	Cluster, Host & Failover	ClusterInfrastructure, Host, HealthMonitoring, docker/Traefik/deploy	High micro-level polish undone by one critical foundation crack: the split-brain resolver is configured but never enabled — automatic failover does not work for hard crashes.
02	Communication & Store-and-Forward	ClusterClient, gRPC streaming, S&F engine	High code quality in-the-small; mid maturity as a distributed system — serious lifecycle/topology defects the test suite is structurally blind to.
03	Site Runtime & DCL	SiteRuntime, DataConnectionLayer, SiteEventLogging	Mature and unusually carefully engineered; residual risk in adapter reconnect lifecycle, script containment, and deployment honesty.
04	Data & Audit Backbone	ConfigurationDatabase, AuditLog, NotificationOutbox, SiteCallAudit, KpiHistory	Genuinely mature data layer with scale-dependent operational time bombs that will surface within months of production volume.
05	Templates, Deployment & Transport	TemplateEngine, DeploymentManager, Transport, ScriptAnalysis	Failure-path engineering is production-grade, but the Transport import apply path and newest template features contain multiple silent-data-loss and staleness defects.
06	Edge Integrations	InboundAPI, ExternalSystemGateway, NotificationService, DelmiaNotifier	Mature, heavily-hardened edge layer; risk concentrated in one silent cache-coherence bug and acknowledged ESG spec gaps.
07	UI, Management & Security	CentralUI, ManagementService, CLI, Security	Mature, carefully engineered surface undermined by sharp asymmetries: an auth-disabled deploy artifact, ignored deploy scoping, inconsistent secret elision.
08	Conventions, Tests & Underdeveloped Areas	Cross-cutting	Unusually disciplined codebase — the documented conventions are actually followed; gaps are narrow, specific, and mostly consciously logged.

Severity Tally (tag occurrences per report)

Report	Critical	High	Medium	Low
01 Cluster/Host/Failover	1	5	13	9
02 Communication/S&F	2	6	11	11
03 Site Runtime/DCL	0	4	6	11
04 Data/Audit Backbone	0	6	11	6
05 Templates/Deploy/Transport	2	13	15	5
06 Edge Integrations	0	4	11	11
07 UI/Management/Security	2	8	12	5
08 Conventions/Tests	0	4	6	10
Total (approx.)	7	50	85	68

(Counts are raw tag occurrences; a few findings are cited in more than one report — e.g. the DisableLogin deploy artifact appears in both 01 and 07.)

Overall Verdict

ScadaBridge is an unusually disciplined codebase at the micro level. The conventions documented in CLAUDE.md are actually followed — verified, not assumed: zero DateTime.Now in src/, persistence-ignorant Commons entities, a clean project-reference graph with no site→central contamination, correlation IDs on request/response messages, additive message contracts, idempotent ingest keyed on EventId/NotificationId, timing-oracle-free API-key auth, and only two real TODOs in ~190k LOC. Test volume is real (~5,050 behavioral tests across 30 projects — Akka TestKit, bUnit, Playwright, MSSQL-backed integration tests).

The system's risk is not diffuse sloppiness. It concentrates in exactly the four places a single-process test suite cannot see:

1. Real multi-node failover — the paths that only execute when a node hard-crashes, a partition forms, or the standby takes over.
2. Cache coherence across process/node boundaries — compiled-script and staleness caches that are only invalidated on the node that made the change.
3. Cross-environment data movement — the Transport bundle import apply path, where DTOs, spec, and apply code drifted apart.
4. Production-scale data volume — purge/retention/KPI paths tested only at small scale.

Bottom line: production-grade engineering, not yet production-proven HA. The single most important sentence in this review: automatic failover for hard crashes does not currently work (finding C1 below), and no test in the repo would have caught it.

The Critical Findings (fix before anything else)

C1. The split-brain resolver is configured but never enabled. BuildHocon never sets akka.cluster.downing-provider-class, so the keep-oldest SBR settings are inert: unreachable nodes are never downed, and cluster singletons (site DeploymentManager = all data collection; all six central singletons) never migrate on a hard crash or partition. The documented ~25s failover only works for graceful stops. The repo's own Akka reference notes (AkkaDotNet/03-Cluster.md) show the required line. (Report 01; Host/Actors/AkkaHostedService.cs:225-270)

C2. Editing a site's addresses crash-loops central routing to all sites. CentralCommunicationActor stops and immediately recreates a named top-level ClusterClient in the same message handling; Context.Stop is async, so the name collides and the actor crash-restarts into a permanent loop. Tests mock the client factory, so this is never exercised. (Report 02; CentralCommunicationActor.cs:514-521)

C3. Transport bundle import silently drops template inheritance. TemplateDto.BaseTemplateName is exported and diffed but never consumed at apply — derived templates land as root templates and flatten to amputated configs in the target environment. (Report 05; Transport/Import/BundleImporter.cs:1524)

C4. The on-host production deploy artifact ships DisableLogin: true. Every surface (UI, /management, audit REST, debug hub) auto-authenticates with ALL roles — including Operator+Verifier, nullifying the two-person secured-write control. (Reports 01 & 07; deploy/wonder-app-vd03/appsettings.Central.json:39-42)

Cross-Cutting Themes

Theme 1 — Failover is designed but unverified, and multiple independent defects hide on that path (stability)

Beyond C1/C2, the failover path accumulates: leader-vs-oldest confusion ("active node" = cluster leader by address order, but singletons live on the oldest node — the event-log purge gate runs on the wrong node and Traefik can route to a node hosting no singletons); during a central partition both nodes return 200 on /health/active so Traefik serves both indefinitely; the standby site node runs the full S&F delivery pipeline with no active-node gate, so replicated Pending rows are delivered by both nodes (systematic duplicate external calls and DB writes); singleton drain tasks run GracefulStop(10s) inside a 5s CoordinatedShutdown phase; and inbound-API compiled handlers go stale across central failover (the standby serves startup-time delegates with no revision check — an updated method silently serves the old script with HTTP 200).

Root cause is shared: no test anywhere forms a real two-node cluster and kills a node. "Failover" integration tests are single-process simulations; HOCON tests assert strings, not behavior; the risky collaborators (real ClusterClients, the standby node, shared gRPC channels) are mocked away. (Reports 01, 02, 06)

Theme 2 — A family of cache-coherence/staleness bugs around compiled scripts (stability)

The same defect shape appears at least four times: (a) inbound compiled handlers stale across failover; (b) inbound compiled-handler and known-bad-method caches stale after a bundle import overwrites an ApiMethod (old code serves HTTP 200 until restart); (c) _knownBadMethods never cleared on out-of-band fixes (known, in project memory); (d) RevisionHashService omits NativeAlarmSources, so native-alarm edits never flag instances stale and sites mirror old sources indefinitely. Related: every staleness probe / Deployments-page validation compiles every script via Roslyn into the non-collectible AssemblyLoadContext with no caching — CPU spikes plus an unbounded assembly leak. A single "script artifact changed → invalidate everywhere" invalidation contract would close the whole family. (Reports 05, 06)

Theme 3 — Transport import is the least-finished shipped feature: silent data loss (underdeveloped)

Beyond C3: imported scripts silently lose MinTimeBetweenRuns/ExecutionTimeoutSeconds (and LockedInDerived isn't in the DTOs at all); template NativeAlarmSources aren't transported while their instance-level overrides are (dangling overrides silently dropped at flatten); the exporter hardcodes AreaName: null while docs and CLAUDE.md claim Area-by-name travels (the importer's resolution machinery is fully built — a half-shipped feature presented as done); the import blocker heuristic hard-blocks valid imports on PascalCase false positives with no operator override. The bundle crypto is solid; it's the apply path where spec, DTOs, and code drifted. (Reports 05, 08)

Theme 4 — Scale-dependent time bombs in the data layer (performance/stability)

Each is invisible at test scale and arrives on a clock: the spec'd site SQLite 7-day audit purge does not exist (site audit DBs grow unbounded); SwitchOutPartitionAsync rebuilds the full-table UX_AuditLog_EventId unique index in one transaction with the default 30s CommandTimeout — once the table is large, the daily purge fails permanently and retention silently stops; SiteCalls KPI predicates full-scan the 365-day table ~15×/minute; GetExecutionTreeAsync does a SELECT DISTINCT over the entire AuditLog per drilldown; the S&F retry sweep is serial, unbounded, and single-laned (central down + 200-message backlog ≈ 100 min/sweep with head-of-line blocking); site SQLite runs without WAL/busy-timeout under many concurrent writers; no EF EnableRetryOnFailure anywhere; the persisted backlogTotal KPI trend is hardcoded zero. (Reports 02, 03, 04)

Theme 5 — Security posture is strong in design, leaky in projection (stability/conventions)

The session/JWT design, secured-write CAS race guards, and timing-oracle-free API-key auth are exemplary. But: C4 (DisableLogin artifact); DataConnection configs (plaintext OPC UA passwords), ExternalSystem AuthConfiguration, and DB connection strings are returned raw to any authenticated user via ungated List/Get commands — while SMTP/SMS/API keys are properly elided; deploy artifacts --site-id is silently ignored and deploys fleet-wide with no site-scope check; pending secured writes never expire server-side (the "Expired" status exists only in the UI — a stale setpoint can be approved and written to a live device days later); every management/auth request does a full LDAP bind with zero throttling (password-spray oracle); role-casing split-brain between UI policies (case-sensitive) and the ManagementActor (case-insensitive). (Report 07)

Theme 6 — Script containment is cooperative, not enforced (stability)

Script execution timeout is cooperative-only: a script blocked in synchronous I/O permanently consumes one of the 8 dedicated ScriptExecutionScheduler threads — 8 hung scripts silently halt all script and alarm on-trigger execution site-wide, with zero observability (no stuck-thread health metric). Relatedly: trigger expressions run Roslyn synchronously on the default dispatcher per attribute change; the inbound method timeout abandons the orphaned task while disposing its DI scope (use-after-dispose); site-side compile failures don't reject the deployment — central is still told Success, contradicting the spec. (Reports 03, 06)

Theme 7 — Adapter/channel lifecycle leaks on reconnect (stability)

OpcUaDataConnection/MxGatewayDataConnection overwrite _client on every ConnectAsync without dispose/detach — session/socket leak per flap, plus a stale ConnectionLost handler that can flap the new healthy connection. One debug session's gRPC failover disposes the site's shared cached channel, cancelling every other session's subscriptions (concurrent sessions ping-pong and terminate each other). S&F replication is Task.Run fire-and-forget with possible Add/Remove reordering. (Reports 02, 03)

What's Genuinely Good (keep doing this)

• Conventions hold under audit — UTC everywhere, POCO/repository split, options pattern (where validated), additive contracts, clean reference graph, correlation IDs. (Report 08)
• Idempotency and atomicity where it matters — EventId-keyed audit ingest, NotificationId insert-if-not-exists, verified-atomic AuditLog+SiteCalls dual-write, CAS on deployment status and secured-write approval. (Reports 04, 07)
• Error hygiene at the edges — consistent transient/permanent classification, poison-message parking, enumeration-safe 403s, IHttpClientFactory throughout, bounded audit capture. (Report 06)
• Hardening passes are visible — Become/Stash lifecycle machines, redeploy termination-watch buffering, adapter-generation guards, HOCON escaping, DebugView disposal/thread-marshalling. (Reports 01, 03, 07)
• Deferred work is honestly logged — 23-item inventory in report 08; only ~4 warrant near-term action.

Prioritized Recommendations

P0 — before the next production deployment

1. Enable the SBR downing provider in BuildHocon and prove it with a real two-node kill test (see P2-10). (C1)
2. Remove DisableLogin: true from deploy/wonder-app-vd03 (or add an environment guard that refuses it outside dev). (C4)
3. Fix the ClusterClient stop/recreate name collision (await termination or use generation-suffixed names). (C2)
4. Gate the S&F delivery pipeline (and event-log purge gate) on singleton-host/active-node status — standby must be passive. (Theme 1)

P1 — correctness of shipped features 5. Fix the Transport import apply path: consume BaseTemplateName, carry the dropped script fields + LockedInDerived, transport template NativeAlarmSources, wire AreaName. Add round-trip export→import equivalence tests per entity type. (C3, Theme 3) 6. Define one script-artifact invalidation contract: bundle import, failover activation, and out-of-band updates must all invalidate compiled-handler/known-bad caches; add NativeAlarmSources to RevisionHashService; cache validation compiles (collectible ALC). (Theme 2) 7. Close the secrets-projection gap: elide DataConnection/ExternalSystem/DB-connection secrets in List/Get like SMTP/SMS already are; honor SiteId in HandleDeployArtifacts with a site-scope check; add server-side secured-write expiry. (Theme 5) 8. Make site-side compile failure reject the deployment (per spec) instead of reporting Success. (Theme 6)

P2 — before production data volume / as hardening 9. Data-layer time bombs: implement the site SQLite audit purge; set an explicit long CommandTimeout on partition switch; add the missing SiteCalls KPI index; bound the execution-tree CTE; turn on WAL + busy_timeout for site SQLite; EnableRetryOnFailure; fix the hardcoded-zero backlog KPI. (Theme 4) 10. Build a real two-node failover test rig (docker already provides the topology): kill -9 a node, assert singleton migration, S&F non-duplication, Traefik single-active, inbound handler freshness. This one rig would have caught C1, C2, and three Theme-1 findings. (Theme 1) 11. Script containment: watchdog metric for stuck ScriptExecutionScheduler threads (health-surface it), move trigger-expression evaluation off the default dispatcher, fix the inbound DI-scope use-after-dispose. (Theme 6) 12. Adapter lifecycle: dispose/detach old OPC UA/MxGateway clients on reconnect; per-session (or ref-counted) gRPC channel handling for debug streams; make S&F replication ordered or reconciled. (Theme 7) 13. Housekeeping: add CLI.Tests to the slnx (one line, long overdue); extend ValidateOnStart options validation to the ~14 unvalidated components; batch/parallelize the S&F retry sweep with a per-sweep LIMIT; fix the docs drift catalogued in reports 04 and 08.

Reading Order

If you read only three reports: 01 (the failover crack), 05 (Transport data loss), 04 (the data-layer clocks). Report 08 is the best single picture of overall codebase health and carries the consolidated 23-item deferred-work inventory.