# 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`](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](01-core-composition.md) | Core, Core.Abstractions, Commons, Configuration, Cluster; AddressSpace Composer/Planner/Applier |
| 02 | [Scripting, virtual tags & alarm engines](02-scripting-alarms.md) | Core.Scripting(+Abstractions), Core.VirtualTags, Core.ScriptedAlarms, Core.AlarmHistorian |
| 03 | [Server host, runtime & OPC UA](03-server-runtime.md) | OpcUaServer, Host, Runtime, ControlPlane, Security; mesh, ACL, node manager |
| 04 | [AdminUI (Blazor)](04-adminui.md) | AdminUI pages, ScriptAnalysis, tag editors, Raw tree, ACL authoring, authorization |
| 05 | [Protocol drivers](05-protocol-drivers.md) | Modbus, S7, AbCip, AbLegacy, TwinCAT, FOCAS, OpcUaClient, **MQTT, MTConnect, Sql, Calculation** (+Contracts, +Cli) |
| 06 | [Gateway integrations](06-gateway-integrations.md) | Driver.Galaxy(+Browser,+Contracts), Driver.Historian.Gateway, continuous-historization recorder |
| 07 | [Client, tooling & engineering system](07-client-tooling-engineering.md) | 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 `EditForm`s 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.