lmxopcua/archreview/00-OVERALL.md

OtOpcUa — Deep Architecture Review: Overall Report

Date: 2026-07-12 Commit: f6eaa267 (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 first update of the 2026-07-08 review at 9cad9ed0: every prior finding was re-verified against code (not against remediation claims), and each domain got a fresh-eyes pass with extra scrutiny on the newly merged remediation code itself. Per-domain reports carry a Prior-finding status (9cad9ed0 → f6eaa267) table; remediation history is in plans/STATUS.md.

Delta reviewed: all 11 arch-review remediation branches merged and pushed to origin/master (4 Criticals, 3 systemic guards, #10 CapabilityInvoker dispatch wiring, #13 ResilienceConfig artifact plumbing, #11 Crit-1 premise correction, docs branch, docker-dev rig fix), plus the fixdocs XML-doc sweep (3b5ef439) and the HistorianGateway Client/Contracts 0.1.0→0.2.0 bump (f6eaa267). 84 files, +9218/−217.

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; redundancy, LDAP, node manager
04	AdminUI (Blazor)	AdminUI pages, ScriptAnalysis, tag editors, authorization
05	Protocol drivers	Modbus, S7, AbCip, AbLegacy, TwinCAT, FOCAS, OpcUaClient (+Contracts, +Cli)
06	Gateway integrations	Driver.Galaxy(+Browser,+Contracts), Driver.Historian.Gateway
07	Client, tooling & engineering system	Client.CLI/Shared/UI, Analyzers, build/CPM, CI, test architecture, repo hygiene

Consolidated maturity matrix (1–5, previous score in parentheses)

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

Read of the matrix: the remediation round did exactly what it targeted. The verification system moved most (07 stability 2.5→3.5, conventions 3→4: whole-solution CI, fail-on-skip, tree-wide analyzer — all verified live with negative controls), and the two engine gaps closed (02 underdeveloped 2→3; 03 underdeveloped 3→4 via the failover test + forwarding guard). Nothing regressed at the matrix level. What kept scores from rising further: the untouched High backlog (failure visibility, AdminUI authz, driver perf/concurrency) and new second-order findings inside the remediation code itself — the sharpest of which are one High regression (S7) and one operator-authorable driver-brick (ResilienceConfig).

---

Status of the original four Criticals

#	Original Critical	Verdict at f6eaa267
1	Split-brain resolver never activated	Premise corrected (#11) + closed — SBR was already active on master (Akka.Cluster.Hosting applies SplitBrainResolverOption.Default → pre-existing akka.conf keep-oldest). Now explicit in code (KeepOldestOption{DownIfAlone=true}) and guarded by the hard-kill failover integration test (HardKillFailoverTests, runs in CI).
2	VT script timeout ineffective + ALC cache leak	Fixed (7fd44f0f) — production path routes through TimedScriptEvaluator; CompiledScriptCache + apply-boundary clear. Residuals: new 02/S13 (failure/timeout leaves the node stale-Good, below) and 02/S12 (clear-vs-in-flight race).
3	S7 no reconnect	Fixed (25c0c6f6, live-verified vs python-snap7) — but the fix carries a new High regression, 05/STAB-14 (below).
4	TwinCAT orphaned ADS subscriptions	Fixed (af318fb4, fake-client verified; no live fixture exists) — residual 05/STAB-16 (replay failure is log-only).

Critical/High findings — updated ranking

New in this round (all inside or activated by the remediation delta):

1. 05/STAB-14 (High, regression) — S7's ensure-wrapper rethrows connect-timeout OperationCanceledException unconditionally, and PollLoopAsync's bare catch (OCE) { return; } then permanently, silently kills every S7 subscription poll after an unreachable-host outage (PLC dead-to-SYN, not refusing). The live bounce test can't see it — docker restart yields connection-refused, not timeout. Two-line filter fix + a fake-factory connect-timeout test (S7Driver.cs:488, 1000, 1383, 1404, 1206-1208).
2. 01/S-6 (High) — a valid-JSON ResilienceConfig with timeoutSeconds: 0 (or out-of-range) passes the parser but makes Polly throw ValidationException at pipeline build on every capability call — an operator-authorable permanent driver-brick (the AdminUI form has no range validation), violating the parser's explicit never-brick contract (DriverResilienceOptionsParser.cs:91-95, DriverResiliencePipelineBuilder.cs:111-114, ResilienceFormModel.cs:32-37).
3. 05/STAB-15 (High) — S7's IsS7ConnectionFatal misses framing/desync errors and cancellation-during-I/O (half-read PDU kept → permanent stream desync — the Modbus STAB-3 failure mode reproduced), and the probe catch never marks the handle dead (S7Driver.cs:1239-1251, 1542).
4. 03/S12 = 01/S-8 (Medium, two reviewers independently) — write dispatch hardcodes isIdempotent: true; WriteIdempotentAttribute has zero readers, so the moment an operator enables Write retries via #13, command-shaped writes get retried (DriverInstanceActor.cs:597-601).
5. 02/S13 (Medium) — the timeout fix converted a visible hang into an invisible fault: a script that starts failing/timing out never degrades node quality, leaving the last Good value stale-Good forever (VirtualTagActor.cs:123-128; the dormant engine maps to BadInternalError, the live path doesn't).
6. 01/U-6 (Medium) — the resilience bulkhead is a fully dead knob: documented, defaulted, parsed, and authorable in AdminUI, but Build never adds a concurrency limiter (DriverResiliencePipelineBuilder.cs:99-177).
7. 06/S-11 (Medium) — an enabled-but-misconfigured ServerHistorian still crash-loops the Host with a raw UriFormatException (proven live in the #12 rig session; only the config side was fixed) (HistorianGatewayClientAdapter.cs:45).
8. 06/U-7 (Medium) — the 0.2.0 bump's proto3-optional HistorianTagDefinition.DataType change silently alters GatewayTagProvisioner semantics (Boolean was wire-0 → provisioned as Float under 0.1.0, honored as Int1 under 0.2.0); pre-bump tags mismatch and the live EnsureTags leg has never run on 0.2.0 (GatewayTagProvisioner.cs:57).
9. 04/C-7 + 04/U-5 + 03/U10 (Medium) — the resilience authoring/observability surface is half-built now that #10/#13 made it production-live: the AdminUI form round-trip is lossy (malformed JSON silently overwritten, unknown keys stripped), and DriverResilienceStatusTracker has no reader anywhere (Admin /hosts Stream E.2/E.3 never shipped) — operators author live retry/breaker config blind.

Carried open from 2026-07-08 (re-verified, no remediation branch targeted them):

• Failure-visibility group — applier swallows sink failures / no failure field in AddressSpaceApplyOutcome (01/S-1); Galaxy write success optimistic (06/S-1); primary gate default-allow on unknown role (03/S4). STATUS.md queues these as "next"; still the top carried batch.
• 04/C-1 (High) — AdminUI's largest mutating surface still bare [Authorize] (GlobalUns, EquipmentPage, cluster/driver editors, Reservations); three-idiom authz mix. Stakes rose this round: the ungated surface now includes live resilience config.
• Availability — LDAP block-bridging (03/S2), HistoryRead block-bridging (03/S3), full-rebuild-per-tag-add killing all subscriptions (03/P1 — now unblocked by the U2 forwarding guard, work not started).
• Driver fleet — Modbus timeout/TxId desync (05/STAB-3), AbCip shared-handle race / FOCAS dictionary race, ResolveHost mis-keying, dead onError sink, ReadEnum ×6, silent enum defaulting on TagConfig paths.
• Repo hygiene — 7 AVEVA DLLs in lib/, Wonderware husk dirs, tracked planning files, 5 undocumented CLI commands — all untouched (07).

---

Cross-cutting themes — updated

1. "Built-but-never-wired" — the guards work, and the pattern still recurred

The two systemic guards shipped and are live (analyzer tree-wide with negative control; reflection-exhaustive sink-forwarding guard; whole-solution CI + fail-on-skip). The analyzer immediately earned its keep by surfacing the RESILIENCE-DISPATCH-GAP, which was then genuinely closed (bacea1a4 — pragmas removed, not suppressed). But the pattern reproduced inside the remediation itself: the bulkhead knob parses-but-never-builds (01/U-6), WriteIdempotentAttribute has zero readers (01/S-8), and DriverResilienceStatusTracker has no reader (03/U10). The guards catch interface-forwarding and unwrapped-dispatch inertness; they don't catch option-parsed-but-unapplied inertness. A cheap next guard: a test asserting every parsed DriverResilienceOptions property influences the built pipeline.

2. Fixes that can't be seen by their own verification

The round's sharpest lesson: 05/STAB-14 sits exactly in the blind spot of the live test that validated the fix (bounce = refused; the bug needs timeout). Same genre: the TwinCAT replay is fake-client-only verified (no fixture exists), and 02/S13 turned a loud failure (hang) into a silent one (stale-Good). When a fix targets a fault path, the verification must enumerate the fault variants (refused vs timeout vs desync vs cancel), not just one reproduction.

3. Optimistic success — unchanged, and now with new instances

The carried trio (applier, Galaxy writes, primary gate) remains open, and the round added stale-Good VT values (02/S13), log-only TwinCAT replay failure (05/STAB-16), and blind-authored resilience config (04/U-5). This is now the largest coherent batch of open work.

4. The verification system is materially better

Biggest mover: all 47 test projects gate CI, skips can't masquerade as green (strict on the unit leg), the analyzer is load-bearing, and the failover + forwarding guards exist with negative controls. Remaining: E2E workflow still gates zero tests (07/S-3), integration matrix runs redundant fixtureless legs (07/P-5), driver suites still carry ~100 fixed sleeps now exposed in CI, no bUnit (04/U-1), and behavioral wiring tests cover only 2 of 6 dispatch sites (03/U6).

5. Documentation drift — mostly closed, one new instance

CLAUDE.md's Known Limitation 2 and the ScriptAnalysis policy drift are fixed and verified. New drift: archreview/plans/STATUS.md merged to master still claiming "all LOCAL — nothing pushed" (corrected alongside this report), and the C6 "Phase 7 plan" residue survived fixdocs.

6. The resilience seam is the new second-class citizen

Six of this round's nine notable new findings live on the #10/#13 resilience surface. The seam's architecture is clean (correct Polly-free layering, null objects, actor-context preservation — all verified), but its edges are unfinished: no range validation (S-6), dead bulkhead (U-6), hardcoded idempotency (S-8), lossy form (C-7), no status reader (U-5), stale-pipeline respawn race (S-7/S13-03). One focused hardening pass closes all of them.

---

Prioritized action list (updated)

#	Action	Source	Effort	Impact
1	S7: filter connect-timeout OCE in the ensure-wrapper + poll-loop catches; broaden fatal classification (framing/cancel); mark handle dead in probe catch. Add fake-factory connect-timeout test	05 STAB-14/15	Small	Critical-adjacent — regression silently kills S7 subscriptions
2	ResilienceConfig hardening pass: range-validate in parser (clamp + diagnostic, never brick) and AdminUI form; wire or delete the bulkhead knob; read WriteIdempotentAttribute (or default writes non-idempotent); fix lossy form round-trip	01 S-6/U-6/S-8, 04 C-7	Small-Medium	High — operator-authorable driver-brick; command writes retried
3	Publish Bad quality on VT script failure/timeout (match the dormant engine's BadInternalError); optionally gate the apply-boundary cache clear on changed scripts	02 S13/P7/S12	Small	High — silent stale-Good data
4	Failure-visibility trio: AddressSpaceApplyOutcome failure field; Galaxy fail-closed writes; primary-gate default-deny	01 S-1, 06 S-1, 03 S4	Medium	High — carried from round 1
5	Gate AdminUI mutating pages with real policies (stakes rose: resilience config is on the ungated surface)	04 C-1	Medium	High
6	ServerHistorian fail-fast options validation (misconfig → clear startup error, not crash-loop); assess 0.2.0 DataType provisioning delta + run the live EnsureTags leg	06 S-11/U-7	Small	Medium-High
7	Surgical pure-adds in the applier (prerequisite guard in place since round 1)	03 P1	Large	High — biggest operational perf win
8	Async LDAP + configurable timeout; channel-ize HistoryRead bridging	03 S2/S3	Medium	Medium-High
9	Driver fleet carried batch: Modbus fatal-classification, AbCip/FOCAS locking, ResolveHost, onError, TwinCAT replay retry + Bad-quality	05	Medium	Medium-High
10	Resilience observability: a /hosts reader for DriverResilienceStatusTracker (Stream E.2/E.3)	03 U10, 04 U-5	Medium	Medium — operators currently blind
11	TagConfig parse consolidation (TagConfigIntent.Parse in Commons) + ReadEnum unification	01 C-1/P-1, 05	Medium	Medium
12	Hygiene: delete lib/ AVEVA DLLs + Wonderware husks, untrack planning files, document 5 CLI commands, E2E workflow or delete it, dedupe CI integration matrix	07	Small	Medium — license risk + doc trust

Overall assessment

The remediation round was real: all four Criticals are verifiably closed in code (one by premise-correction), both systemic guards are live and already caught one genuine gap, and the verification system went from "proves ~10%" to whole-solution with skip-detection — the single biggest maturity move available, and it happened. The honest counterweight is that this round's scrutiny of the fixes found what fixes always carry: one High regression (S7 timeout-vs-refused), one High operator-authorable brick (ResilienceConfig validation), and a cluster of half-wired edges on the new resilience seam — the house "built-but-never-wired" pattern reproducing inside the code meant to eliminate it. Items 1–3 above are small and close everything new-and-sharp from this round; items 4–5 are the carried Highs that have now waited two rounds and should not wait a third.