lmxopcua/archreview/00-OVERALL.md

OtOpcUa — Deep Architecture Review: Overall Report

Date: 2026-08-19 Commit: 12b0035a (master, clean tree) Method: Seven parallel domain re-reviews, same scopes and four dimensions (stability, performance, conventions, underdeveloped areas) as the original. This is the second update (round 3): round 1 ran 2026-07-08 at 9cad9ed0, round 2 on 2026-07-12 at f6eaa267. Every round-2 finding was re-verified against code at 12b0035a — not against plan records or remediation claims — and each domain got a fresh-eyes pass with extra scrutiny on the code that landed in the window. Per-domain reports carry a Prior-finding status (f6eaa267 → 12b0035a) table; remediation history is in plans/STATUS.md.

Delta reviewed: ~1,274 commits since f6eaa267 — by far the largest window this review has absorbed. In it: all twelve Round-2 remediation plans (R2-01…R2-12) merged; the v3 dual-namespace address space (Batch 4) and the 2026-08-15 surgical-deploys rework; the per-cluster mesh program (Phases 1–7 + bootstrap guard), the ClusterClient→gRPC migration, ConfigSource/ConfigServe fetch-and-cache, the gRPC telemetry stream, DB-less driver nodes; the LocalDb consolidation (config cache + alarm S&F + condition state); the #520 API-key/ACL workstream (8 stages); the comms-loss watchdog (#527); the continuous-historization live-gate arc (#491/#537/#538); the Galaxy write-confirmation arc; four new drivers (MQTT/Sparkplug B, MTConnect, SQL poll, Calculation); the /raw project tree + 9 modals; and the 65-module code-review system. Test projects grew 47 → 57.

Report index

#	Report	Scope
01	Core & composition pipeline	Core, Core.Abstractions, Commons, Configuration, Cluster; AddressSpace Composer/Planner/Applier
02	Scripting, virtual tags & alarm engines	Core.Scripting(+Abstractions), Core.VirtualTags, Core.ScriptedAlarms, Core.AlarmHistorian
03	Server host, runtime & OPC UA	OpcUaServer, Host, Runtime, ControlPlane, Security; mesh, ACL, node manager
04	AdminUI (Blazor)	AdminUI pages, ScriptAnalysis, tag editors, Raw tree, ACL authoring, authorization
05	Protocol drivers	Modbus, S7, AbCip, AbLegacy, TwinCAT, FOCAS, OpcUaClient, MQTT, MTConnect, Sql, Calculation (+Contracts, +Cli)
06	Gateway integrations	Driver.Galaxy(+Browser,+Contracts), Driver.Historian.Gateway, continuous-historization recorder
07	Client, tooling & engineering system	Client.CLI/Shared/UI, Analyzers, build/CPM, CI, test architecture, repo hygiene

Consolidated maturity matrix (1–5, round-2 score in parentheses)

Subsystem	Stability	Performance	Conventions	Underdeveloped
01 Core & composition	4.5 (4)	4 (3)	4 (4)	4 (3)
02 Scripting & alarms	4 (3.5)	3.5 (3)	4 (4)	3.5 (3)
03 Server & runtime	3 (3)	4 (3)	4 (4)	3.5 (4) ▼
04 AdminUI	4 (4)	4 (3)	4 (3)	3.5 (3)
05 Protocol drivers	3.5 (3)	3 (2)	4 (3)	3.5 (3)
06 Gateway integrations	4.5 (4)	4.5 (4)	4.5 (4)	4 (3)
07 Client & engineering	4 (3.5)	3.5 (3.5)	4 (4)	4 (3)

Read of the matrix: 19 of 28 cells rose, 8 held, and one fell — 03 Underdeveloped (4 → 3.5). The rises are earned: Round 2's plans targeted the carried backlog directly with code and tests, and the per-report prior-finding tables show 73 of ~201 round-2 findings closed outright and 31 partially closed (97 carried). Performance moved most (every domain but 07 rose — surgical deploys, batched value pipeline, S7 block reads, dirty-set coalescing, AdminUI projections, group-commit outbox). The two cells that did not move are the ones that matter most: 03 Stability holds at 3 because the new mesh/ACL/watchdog code contributed this round's only Critical and two of its five Highs, and 03 Underdeveloped fell because the "built-but-never-wired" class — retired in round 2 for sinks and DI — re-emerged in the new subsystems as comment-enforced invariants and write-only fields.

---

Status of round 2's top findings

#	Round-2 finding	Verdict at 12b0035a
1	05/STAB-14 S7 connect-timeout OCE kills subscription polls (High regression)	CLOSED (R2-01 + PR #453) — connect timeout surfaces as TimeoutException at both connect sites; STAB-15 fatal-classification gap also closed.
2	01/S-6 operator-authorable ResilienceConfig driver-brick	CLOSED (R2-02) — range-validated; bulkhead knob (01/U-6) deleted rather than wired; WriteIdempotentAttribute (01/S-8 = 03/S12) now read.
3	02/S13 VT failure leaves stale-Good	CLOSED (R2-03) — with S12 cache-clear race and P7. But see the new stale-Good family below: the same shape survives at three other seams.
4	06/S-11 / U-7 ServerHistorian crash-loop; 0.2.0 DataType delta	CLOSED (R2-06 + #441) — fail-fast validator; Boolean→Int2 mapping; retype policy answered against the real historian.
5	04/C-7 + 04/U-5 + 03/U10 resilience authoring/observability half-built	CLOSED (R2-10) — lossless form round-trip; DriverResilienceStatusTracker has a /hosts reader; InitializeAsync routed through the invoker.
—	Carried Highs from round 1: failure-visibility trio (01/S-1, 03/S4, 06/S-1); AdminUI authz (04/C-1); availability (03/S2/S3/P1); driver fleet; repo hygiene	ALL CLOSED — 01/S-1 FailedNodes surface; 03/S4 default-deny; 06/S-1 fail-closed + correlated Galaxy writes (live-gated); 04/C-1 named policies on all 30 routable pages held under 177 commits by the reflection guard; 03/S2 superseded by #520, 03/S3 async fan-out, 03/P1 surgical deploys; R2-09/R2-11/#457 fleet batch; R2-12 hygiene.

Nothing from round 2's top list is still open. The round-1 "four Criticals" remain fixed (STAB-1/STAB-2 re-verified surviving the S7 fork retirement and TwinCAT replay hardening).

---

Critical/High findings — round-3 ranking

New this round (all in or activated by the window's code):

1. 03/S14 (Critical) — The AdminUI cookie FallbackPolicy (RequireAuthenticatedUser) applies to every mesh gRPC map — artifact fetch, ReportApply acks, telemetry stream, LocalDb sync — because none carries .AllowAnonymous(). On a login-enabled admin node every node-to-central call 401s before the PSK interceptor runs: a fleet-wide deploy-plane outage. Every rig live gate was blind to it because docker-dev runs DisableLogin=true, which registers a pass-everything handler under the cookie scheme name, and MeshGrpcBoundaryTests builds pipelines without the auth stack (Security/ServiceCollectionExtensions.cs:143-148, Host/Program.cs:520-523, 732-783, MeshGrpcBoundaryTests.cs:334-346). One-line-per-map fix; the load-bearing verification is one gate pass with DisableLogin=false + a CI variant of the boundary tests with the real auth stack.
2. 03/S15 (High) — Driver comms loss never marks raw/UNS value variables Bad. The #527 watchdog and the fan-out write BadNoCommunication to Part 9 conditions and the dependency mux, but external OPC UA clients hold stale-Good for the entire outage (DriverHostActor.cs:1248-1283, 1334-1359). Fix via the batched sink, with a force-republish on recovery (the value-dedup trap from the v3 gate).
3. 05/STAB-19 (High) — The Calculation driver launders dependency quality to Good: statusCode is never read and DependencyConsumerMuxAdapter hardcodes 0u, so a comms-lost input yields fresh-Good outputs (or NRE storms on null) (CalculationDriver.cs:330-352, DependencyConsumerMuxAdapter.cs:71-72).
4. 05/STAB-18 (High) — MQTT has no push-model analogue of the poll engine's forceRaise: a ref latched Bad by reconnect/rebuild recovers only when the plant value next changes, because last-value suppression swallows the identical re-publish (MqttSubscriptionManager.cs:325-365, 762-767, SparkplugIngestor.cs:470-506).
5. 03/S16 (High) — Operator "Restart driver" respawns the child without re-sending SetDesiredSubscriptions; _lastComposition is write-only, so the restarted driver is dead until the next deploy (DriverHostActor.cs:2916-2932, 2660-2672). No actor-restart test exists anywhere (03/U6 carried). One PushDesiredSubscriptions call after SpawnChild.

Carried Highs (all three rounds old, all in 02, all anchored on the dormant engine):

• 02/U1 — the ~1,100-line Core.VirtualTags engine stack is dormant in production with no retire/sanction decision; it anchors 02/S1, S10, half of S3, C1 and U7-(b). A decision, not a fix, is what is needed.
• 02/P1 — ScriptSandbox.Build rebuilds the full BCL reference set on every compile — the single reason 02 Performance is not 4.

Mediums worth naming because two reviewers found them independently or because they are one-liners with outsized blast radius:

• 01/S-12 = 03/C5 — Program.cs derives hasAdmin/hasDriver from OTOPCUA_ROLES only while Akka uses ResolveEffectiveRoles (Cluster:Roles); a Cluster:Roles-only node gossips driver with no driver stack and times out every deploy. Sibling 01/C-9: only one of four role-scoped validators lowercases, so "Driver" skips every driver-scoped fail-fast rule. One authority (ResolveEffectiveRoles normalizing + validating) closes both.
• 01/U-8 = 03/P6 — AnnounceAddedNodes hardcodes the Uns realm; surgically-added raw nodes are materialised but never announced — promoted from cosmetic to functional by surgical deploys (the rebuild used to force a re-browse).
• 03/S17 — the comms-loss watchdog uses ?? where it needs max(LastSuccessfulRead, _connectedSinceUtc), so it re-trips on the first tick after a reconnect to a half-broken device. One line.
• 03/S18 — on admin/fused nodes the API-key lookup is a per-activation EF query under the SessionManager lock with CancellationToken.None and no timeout — #520 deleted the LDAP timeout/circuit guards and the ConfigDb store re-introduced network I/O into the same callback.
• 03/S19 — key expiry never reaches live monitored items between deploys (RevalidateMonitoredItems is deploy-triggered only).
• 01/S-9 — the deploy ack is sent independently of the address-space apply outcome, so a degraded apply still seals Applied fleet-wide; 01/S-10 — a malformed-but-nonempty artifact on the boot/restore paths is swallowed to an empty spec list (stops every child), and gzip added a new malformed shape (#485's surviving leg).
• 02/S14 — the VT path stamps every dependency Good (the fourth member of the stale-Good family); 02/S15 — the alarm drain-gate replicated predicate (PeerAddress OR SyncListenPort) disagrees with LocalDbSetup.ReplicationIsConfigured (PeerAddress OR ApiKey): a port-without-key node maroons its alarm queue undrained.
• 04/S-8 — the superseded-open staleness guard was ported to 3 modals and not to 5 siblings; worst case is CSV import committing to the wrong device. 04/S-9 — three EditForms with no ValidationSummary = silent no-op Save (the #505 class).
• 05/STAB-24 — Sql binds key/where values as text; Npgsql won't cast text↔integer, so numeric-key groups fail every poll with wrong guidance — and the fixture cannot show it. 05/STAB-21 — S7's poll-failure backoff is unreachable (engine backs off only on throw; S7's reader never throws for device faults). 05/STAB-26 — MQTT writes are QoS-0 fire-and-forget yet return Good, so write-outcome self-correction never reverts.
• 06/S-12 — a thrown (vs non-Ok-reply) AdviseSupervisory leaves the handle marked supervised; later writes skip the advise forever (1008 against a correlated gateway, phantom-Good against a pre-correlation one) — the one-fault-variant trap inside the S-1 fix.
• 07/P-6 — the whole-solution unit leg's wall-clock now grows linearly with the driver fleet (~45 s of real-time socket windows in one MQTT suite alone).

---

Cross-cutting themes — updated

1. The stale-Good family: quality is dropped at the last seam, four times over

The round's most coherent finding is not a single bug but a shape repeated at four independent seams: driver comms loss never reaches value variables (03/S15), Calculation hardcodes Good (05/STAB-19), MQTT swallows the recovery re-publish (05/STAB-18), and the VT path stamps inputs Good (02/S14). In each case the signal exists end-to-end — the watchdog fires, the mux carries a status, the fan-out writes Bad to conditions — and is discarded exactly one seam before an external client would see it. Round 2 closed the same shape once (02/S13) and called it done. The contract should be stated once — "a comms-lost source propagates Bad to every node that derives from it" — and pinned with one test per seam that holds a Bad input and asserts on the derived node.

2. Verification that cannot see its own blind spot — the rig's auth bypass

Round 2's sharpest lesson was a live test blind to the fault variant it was validating (refused vs timeout). Round 3's is larger: every mesh live gate of the entire per-cluster-mesh program ran with DisableLogin=true, which neutralizes the exact middleware that breaks production (03/S14). Same genre this round: the Sql Postgres fixture's integer-keyed leg doesn't exist, so STAB-24 is invisible; MTConnect DTD hardening is untested; TwinCAT replay remains fake-client-only; AbLegacy has zero reconnect tests; and 07/S-10 notes that three rounds in, no green CI run is evidenced anywhere in-repo. A rig convenience feature is load-bearing for a Critical.

3. "Built-but-never-wired" was retired for sinks and DI — and re-grew in the new subsystems

The round-2 guards (reflection sink-forwarding, registration guards, the analyzer) held and caught real new members. But the window's new code re-created the shape where no guard reaches: _lastComposition is write-only (03/S16), Backing*-field parity between the classifier whitelist and UnsReferenceVariable is comment-enforced (03/U11), AnnounceAddedNodes covers one realm (01/U-8), ProvisionHistorizedTags iterates adds only so a surgical historize-flip never fires EnsureTags (03/U12), the historian health snapshot is still a write-only ledger on its third review (06/S-6), and regen-readme.py --check exists but is not CI-gated (07/U-7). Each admits the same cheap structural guard that already proved itself.

4. One rule stated twice diverges — won for TagConfig, re-lost for roles and predicates

R2-11 + TagConfigIntent + shared NodeId authorities ended the domain's worst duplication (01/C-1/C-3, with a corpus parity test). The window then produced fresh instances of the same disease: role normalization in one validator of four (01/C-9) and two role sources (01/S-12 = 03/C5); the alarm drain-gate predicate vs LocalDbSetup.ReplicationIsConfigured (02/S15); the effective-name rule spelled twice with divergent empty-string semantics (02/C9); EquipmentReferenceMap's two spellings; S7 Inspect vs its init-time guards (05/UNDER-10); MTConnect's probe parsing a stricter dialect than its factory (05/CONV-9). The authoring-vs-deploy-gate arc (#535→#559) showed the method — one authority, enforced in the service, mirrored by the gate — and it applies verbatim.

5. Fixes applied N−1 wide

The AdminUI report names it directly (04/S-8: three modals guarded, five siblings not; 04/S-1/S-2 stragglers; 04/S-6 one-way mirror), and the drivers report shows the cross-driver version: AbCip's retained ProbeTask never reached FOCAS or AbLegacy (05/STAB-6/23), FOCAS's parsed-address cache never reached TwinCAT (PERF-6), TwinCAT's ConcurrentDictionary never reached FOCAS's other two maps (STAB-32). The correct pattern now coexists in-tree beside the wrong one, which makes these internal inconsistencies rather than missing knowledge — and makes a reflection census (as PageAuthorizationGuardTests does for policies) the right tool.

6. Doc drift is now a regression vector

Stale XML summaries that describe exactly the behavior a past fix reverted appear in 03 (default-allow gate summaries C6, EnsureVariable silent-ignore U9, the audit mapper's dormancy claim U3, BuildSecurityPolicies U8), 01 (the applier doc contradicting itself on PureRemove/AddRemoveMix, DriverHost's named-pipe promise, #522 tombstones in the present tense), 05 (S7's ctor backoff claim, the mux adapter's "quality stripped" comment, TransferSubscriptionsOnReconnect's default), and 07 (deferment.md contradicting its own header). Each invites a confident restoration of the bug. The habit fix is updating the summary in the same commit as the body; the mechanical fix is the doc-sweep over the named sites.

7. The live gate is the most productive tool in the repo — keep it honest and keep running it

Every closure that mattered in 06 was live-gate-driven, and each gate found defects that 100+ green unit tests coexisted with (StorageRateMs=0, Int1 degeneracy, the ConfigureAwait(false)/Self actor kill, dual-node double-capture, the 1007→Good truncation, poison-tag pinning). The two new R2 guards with negative controls (falsification tests for the drain gate, the comms-loss conjunction, the quarantine discriminator) are the house pattern at its best. Theme 2 is the counterweight: a gate is only as good as the rig's fidelity to production.

---

Prioritized action list (round 3)

#	Action	Source	Effort	Impact
1	.AllowAnonymous() on every mesh gRPC map (the PSK interceptors are the gate); one rig/gate pass with DisableLogin=false; a standing CI variant of MeshGrpcBoundaryTests with UseAuthentication/UseAuthorization + the FallbackPolicy	03 S14	Small	Critical — fleet-wide deploy-plane outage on any login-enabled central
2	Stale-Good family: comms-loss fan-out writes BadNoCommunication to raw + UNS value variables with force-republish on recovery; Calculation threads the real statusCode; MQTT re-subscribe initial push (or suppression reset); VT path honours input quality. One contract test per seam	03 S15, 05 STAB-19/18, 02 S14	Medium	High — external clients read stale data through every outage
3	HandleRestartDriver re-pushes desired subscriptions after SpawnChild (consume _lastComposition); add the first actor-restart test; fix the watchdog ?? → max(...)	03 S16/S17, 03 U6	Small	High — operator restart yields a dead driver
4	One role authority: ResolveEffectiveRoles normalizes + validates; Program.cs and all four validators consume it; boot guard asserting Akka and Host role views agree	01 S-12/C-9, 03 C5	Small	Medium-High — misconfigured node times out every deploy silently
5	ACL hot path on admin/fused nodes: per-call timeout or small TTL cache in ConfigDbApiKeyStore (revocation stays immediate via the authorizer's per-op check); low-frequency RevalidateMonitoredItems timer for clock-driven key expiry	03 S18/S19	Small	Medium-High
6	Surgical-deploy residuals: realm-aware announce for raw nodes; NotSubsumed filter on the UNS-reference remove loop + inverse-Organizes severing; EnsureTags for changed historized refs; reflection guard for Backing* parity	01 U-8, 03 P5/P6/S20/U11/U12	Medium	Medium-High — rebuild ratchet over-fires; raw adds undiscoverable
7	Deploy honesty: post a degraded-ack amendment after apply completes (S-9); treat malformed-nonempty artifact bytes as "no answer" on boot/restore (S-10)	01 S-9/S-10	Small-Medium	Medium — #485's surviving leg
8	Driver fleet round-3 batch: Sql typed-parameter binding + integer-key fixture leg (STAB-24); wire ConnectionBackoff into S7 EnsureConnectedAsync (STAB-21); S7 write-pass outcome accumulator (STAB-22); MTConnect pump generation stamp (STAB-20); FOCAS/AbLegacy loop joins (STAB-23/6); MQTT write honesty + claim backoff (STAB-25/26/27); ILoggerFactory on the four silent factories (CONV-3)	05	Medium	Medium-High
9	Decide the dormant Core.VirtualTags engine (retire closes S1/S10/half-S3/C1/U7-b in one commit); statically cache the ScriptSandbox reference set; unify the drain-gate predicate with LocalDbSetup.ReplicationIsConfigured	02 U1/P1/S15	Small-Medium	Medium-High — oldest open Highs in the repo
10	AdminUI N−1 sweep: port the _openGeneration guard to the five unguarded modals (CSV import first); ValidationSummary on the three silent EditForms + a reflection guard; confirm-dialog the API-key Delete; ErrorBoundary around @Body	04 S-8/S-9/S-5/S-3	Small	Medium
11	Gateway seam: try/catch the advise invoke and TryRemove on any failure (S-12); wire-or-delete the historian health snapshot with a registration guard (S-6); log + ledger the losing RawPath on a duplicate attributeRef (S-13); internal deadline on ReadViaSubscribeOnceAsync (S-3)	06	Small	Medium
12	Engineering system: evidence one green Gitea CI run in-repo; raise MIN_EXECUTED; CI-gate regen-readme.py --check; Category=SlowUnit split before the unit leg outgrows its budget; TWE on the four remaining src projects; global.json; doc-sweep of the stale summaries named in theme 6	07 S-10/S-9/U-7/P-6/C-2/S-7; 01 C-7; 03 C6/U3/U8/U9	Small	Medium — trust in the verification system and the docs

Overall assessment

Round 2's plans were executed, and executed well: every one of round 2's top-nine findings and every carried round-1 High is verifiably closed in code, most of them live-gated, and the matrix rose in 19 of 28 cells with Performance — the weakest dimension for two rounds — moving in six of seven domains. The window also absorbed an enormous amount of new architecture (the mesh split, the ACL data plane, the dual namespace, surgical deploys, four drivers, LocalDb) without a single regression to a previously-closed finding.

The honest counterweight is that the new code carries this round's sharpest findings, and they cluster into two families the house has met before. The Critical (03/S14) is a verification blind spot: a rig convenience flag neutralized the one middleware that breaks production, across every gate of a seven-phase program. The four stale-Good findings (03/S15, 05/STAB-19, 05/STAB-18, 02/S14) are the same optimistic-success shape round 2 closed once at 02/S13 — the quality signal exists end-to-end and is dropped at the last seam before a client. Items 1–3 above are small, close everything Critical/High that is new, and should land before the next feature window; item 2 deserves a stated contract and a per-seam test so the family stops re-appearing one seam at a time. Items 4–7 are the mesh/deploy hardening that the new architecture now needs, and item 9 is the decision that has waited three rounds.