Files
lmxopcua/archreview/03-server-runtime.md
T
Joseph Doherty 44f6408e1c docs(archreview): round-3 re-review at 12b0035a — 7 domain reports + 00-OVERALL rewritten in place, STATUS.md round-3 section
Every round-2 finding re-verified against code (73 closed / 31 partial / 97 carried);
matrix 19/28 cells up, one down. New Critical 03/S14 (AdminUI FallbackPolicy 401s every
mesh gRPC map on a login-enabled central; masked on every rig gate by DisableLogin=true).
New Highs 03/S15, 05/STAB-19, 05/STAB-18, 03/S16 — the stale-Good family at four seams.
2026-08-20 09:09:46 -04:00

46 KiB
Raw Blame History

Architecture Review — Server & Runtime Subsystem

  • Date: 2026-08-19
  • Commit: 12b0035a (master, clean tree)
  • Updates: the 2026-07-12 re-review at f6eaa267 (which updated the 2026-07-08 original at 9cad9ed0). This is the second update. Since f6eaa267, ~431 commits touched src/Server alone (+43.9k/−9.7k lines): the per-cluster mesh program Phases 1–7, the ClusterClient→gRPC migration, ConfigSource/ConfigServe fetch-and-cache, the Phase 5 gRPC telemetry stream, Phase 4 DB-less driver nodes, LocalDb registration (incl. the 2026-08-18 conditional register/deregister), the entire API-key + ACL workstream (#520, 8 stages), the comms-loss watchdog (#527), surgical deploys (2026-08-15), the batched value pipeline, and the continuous-historization Primary gate (#537) + poison-tag quarantine (#538).
  • Scope: src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer, .Host, .Runtime, .ControlPlane, .Security, plus src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa, src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization, src/Core/ZB.MOM.WW.OtOpcUa.Cluster, and tests/Server for coverage assessment.
  • Dimensions: Stability · Performance · Conventions · Underdeveloped areas

Prior-finding status (f6eaa267 → 12b0035a)

Every finding from the 2026-07-12 report, verified against the actual code at HEAD (never against STATUS.md or remediation claims). Line numbers are at 12b0035a.

ID One-liner Verified status
S2 LDAP auth blocks SDK session-activation thread CLOSED (superseded by #520), one residual. LdapOpcUaUserAuthenticator is deleted (only historical comments remain — Security/Ldap/LdapOptions.cs:95, LdapOptionsValidator.cs:52-55 confirm the four S2 resilience knobs went with it). The data plane registers ApiKeyOpcUaUserAuthenticator (Host/Program.cs:428,441; deny-all Null at :448) — one indexed lookup + one HMAC, FixedTimeEquals (Core/Authorization/ApiKeySecret.cs:124). The impersonation callback still block-bridges with CancellationToken.None (OpcUaApplicationHost.cs:317-319, 375-376), documented as SDK-irreducible. AdminUI LDAP survives, entirely outside the impersonation path. Residual: on admin/fused nodes the key store is a per-activation SQL read — see S18 below.
S3 HistoryRead block-bridges gateway per node, sequentially CLOSED (R2-08, verified). All four arms async-ified with bounded per-node fan-out (RunBounded, OtOpcUaNodeManager.cs:3772-3789, semaphore = HistoryReadBatchParallelism default 4 at :393), exactly one sync bridge at the arm boundary over contractually non-throwing work items (:3788), a process-wide batch limiter degrading to BadTooManyOperations (:3804-3835, default 16 at :401), and a per-request deadline CTS → BadTimeout per node (:3861-3862, :3914-3919, default 60 s). Options flow verified end-to-end (OtOpcUaServerHostedService.cs:103-105, 260-262).
S4 Primary-gate default-allow while role unknown (dual-primary window) CLOSED (R2-04, verified + live-gated). PrimaryGatePolicy (Runtime/Drivers/PrimaryGatePolicy.cs:21-27): known role wins; unknown role allows only when driver member count ≤ 1. Wired at all three DriverHostActor gates (:1378-1381 alarm emit, :1470-1477 write, :1557-1573 native ack) plus the scripted-alarm emit (ScriptedAlarmHostActor.cs:490-501); denial meter PrimaryGateDenied with site/reason tags (Commons/Observability/OtOpcUaTelemetry.cs:109). The #537 capture gate deliberately inverts to unknown-captures with a mesh-partner scope predicate (PrimaryGatePolicy.cs:79-125) — consistent and documented. Residual: the gate methods' XML summaries still describe the old default-allow — see C6.
S5 Redundancy NodeId string-match, no mismatch diagnostic PARTIALLY CLOSED. A once-per-process Warning naming the snapshot nodes was added when the snapshot omits this node on a multi-driver cluster — but only in DriverHostActor.OnRedundancyStateChanged (DriverHostActor.cs:1648-1657). OpcUaPublishActor still silently demotes to ServiceLevel 0 (OpcUaPublishActor.cs:934, 938-941) and ScriptedAlarmHostActor silently stays role-unknown (:540-544). The three-source string-match seam itself is unchanged (ClusterRoleInfo.cs:41 vs RedundancyStateActor.cs:214-215).
S6 Driver-child restart loses desired-subscription state; no backoff supervision STILL OPEN (partially mitigated). Zero SupervisorStrategy overrides anywhere under src/Server (grep-verified); a restarted child's _desiredRefs reset to empty (DriverInstanceActor.cs:241-247) and the #488 periodic reconcile only re-subscribes when _desiredRefs.Count > 0 (:625-630) — it heals a lost handle within one incarnation, not a restart. The new operator-restart face of this is S16.
S7 PostStop blocks on ShutdownAsync(CancellationToken.None) STILL OPEN. DriverInstanceActor.cs:1327 — try/caught but synchronous and uncancellable.
S8 Part 9 commands / native acks at-most-once, client sees Good STILL OPEN. HandleAlarmCommand returns Good unconditionally (OtOpcUaNodeManager.cs:1409-1413, native acks :1444-1450); host routers log-and-drop (OtOpcUaServerHostedService.cs:154-170, 182-209). Still no drop counter/metric anywhere on the path.
S9 No server-cert renewal/expiry monitoring STILL OPEN. Cert checked once at boot (OpcUaApplicationHost.cs:411-423, 12-month SDK default); the health surface (Host/Health/HealthEndpoints.cs:34-95) watches configdb/akka/cluster-primary/localdb-replication — nothing watches the certificate.
S10 SDK start failure swallowed; node runs with Null sinks, invisible to /health STILL OPEN (deliberate, unchanged). OtOpcUaServerHostedService.cs:129-144. No health check reflects OPC UA server state.
S11 AuditWriterActor unbounded buffer, drops batches on outage PARTIALLY CLOSED. The buffer now snapshots-and-clears per flush (ControlPlane/Audit/AuditWriterActor.cs:90-91) so it is bounded per outage window, but a DB outage still drops the batch with only an Error log (:113-116). No longer moot — U3 gained producers.
S12 Write dispatch hardcodes isIdempotent: true; WriteIdempotentAttribute has zero readers CLOSED (R2-02, verified). Dispatch passes isIdempotent: false with the rationale comment (DriverInstanceActor.cs:802-811); the parser force-zeroes Write/AlarmAcknowledge/Initialize retries (Core/Resilience/DriverResilienceOptionsParser.cs:149-158, 165-169). Per-tag opt-in remains an unshipped forward path, now honestly documented.
S13 Respawn-on-ResilienceConfig-change stale-pipeline re-cache race CLOSED (R2-02 + 2026-08-08). Interlocked options-generation in PipelineKey (DriverCapabilityInvokerFactory.cs:75, DriverResiliencePipelineBuilder.cs:208); Invalidate is cleanup-only by design (DriverCapabilityInvokerFactory.cs:26-28); the tracker is invalidated with the pipelines (:83) — the R2-10 live-gate fix.
P1 Any structural deploy change → full rebuild severs all monitored items SUBSTANTIALLY CLOSED (R2-07 + surgical deploys 2026-08-15). AddressSpaceChangeClassifier is first-match-wins, default-closed Rebuild; the with-equality whitelist makes any FUTURE record field rebuild by construction (AddressSpaceChangeClassifier.cs:156-185); every surgical false/throw ratchets to SafeRebuild() (AddressSpaceApplier.cs:312, 464-486). Everyday shapes (tag add/remove/attribute edit, UNS reference add/remove/re-point, raw-tag rename) are subscription-preserving. Realistic residual rebuild shapes: alarm-bearing tag deltas (justified — notifier rewiring), raw container rename/remove (ApplyPureRemove hard-returns false on RemovedRawContainers — AddressSpaceApplier.cs:443 — so deleting one empty TagGroup still severs everything), ChangedEquipment/ChangedAlarms, protocol-driver DataType edits. Plus the new P5 subsumption gap, which turns equipment-with-references removal back into a rebuild.
P2 Per-value global-lock write, no batching CLOSED. The whole pipeline is batched: DriverHostActor.ForwardBatchToMux is the only value ingress and expands the raw+UNS-mirror fan-out inside one batch (DriverHostActor.cs:1145-1199); OpcUaPublishActor.HandleAttributeBatchUpdate makes one sink.WriteValues call (OpcUaPublishActor.cs:348-369); OtOpcUaNodeManager.WriteValues takes the Lock once per 512-item chunk with validation outside it (OtOpcUaNodeManager.cs:562, 577-641); DeferredAddressSpaceSink forwards it and the reflection guard covers it.
P3 HistoryRead-Events unpaged, 0 → int.MaxValue PARTIALLY CLOSED. The 0 → no limit translation is now a deliberate, documented Part 4/11 reading and a positive client cap is honoured (OtOpcUaNodeManager.cs:3665-3675, 3626); the Raw arm gained real tie-safe paging (ServeRawPagedAsync :3965+). Events still issue no continuation points (:3635-3636) — a huge window is one response, now damage-limited by the S3 deadline + batch limiter.
P4 Deploy re-runs four Materialise passes over the full composition CLOSED (2026-08-15). MaterialiseNodes branches every pass on fullComposition: outcome.RebuildCalled and passes the plan's delta subsets on a non-rebuild apply (OpcUaPublishActor.cs:508-509, 629-701); only MaterialiseHierarchy runs full (O(areas+lines+equipment), rationale :504-507). Branching on RebuildCalled rather than the classifier kind is itself correctness-load-bearing (:609-613).
C1 ServerHistorian bound imperatively in five places PARTIALLY CLOSED. AddValidatedOptions<ServerHistorianOptions, ServerHistorianOptionsValidator> is real (Host/Program.cs:195-196) so the section fails fast at boot; but four imperative GetSection(...).Get<>() sites remain (Runtime/ServiceCollectionExtensions.cs:237, 273; Program.cs:199; OtOpcUaServerHostedService.cs:104).
C2 Two-tier options validation; DevStubMode only log-warned in prod PARTIALLY CLOSED. Fail-fast now covers ServerHistorian (consumer-gated, AlarmHistorian-only corner included — ServerHistorianOptionsValidator.cs:58-98), OpcUa host options (Program.cs:454), and LDAP incl. plaintext-transport boot fail. DevStubMode still exists and is still only log-warned (OtOpcUaLdapAuthService.cs:79-88) but its blast radius shrank to AdminUI login. ContinuousHistorization remains warn-only, with a silent CommitMode fallback (Program.cs:265-280).
C3 Static Serilog logger islands + node-manager CS0618 pragmas STILL OPEN — grew slightly. Runtime SCE now has 4 static-logger sites (Runtime/ServiceCollectionExtensions.cs:158, 189, 204, 241) plus DpsScriptLogPublisher.cs:61; the node manager carries 14 CS0618 pragma blocks. One case is documented-deliberate: LocalDbSetup.OnReady runs inside the AddZbLocalDb singleton factory where DI resolution would nest inside singleton construction (Host/Configuration/LocalDbSetup.cs:93-100).
C4 IHistorianProvisioning resolve has no missing-registration warning CLOSED. AddOtOpcUaRuntime seeds NullHistorianProvisioning.Instance so the resolve is never silently null (Runtime/ServiceCollectionExtensions.cs:64, 501-505); the analogous missing-deps cases now log explicit Warnings (:382, :489-490).
U1 CLAUDE.md "KNOWN LIMITATION 2" stale CLOSED (and the underlying #491 gate itself ran and closed 2026-08-04, incl. restart-convergence live).
U2 Deferred-sink forwarding trap half test-guarded CLOSED — and extended. DeferredSinkForwardingReflectionTests now covers ISurgicalAddressSpaceSink (all five surgical members incl. UpdateTagAttributes) via the DispatchProxy recorder, keeps the interface-set guard-of-the-guard, and adds a realm-discriminator surface lock (tests/Core/.../DeferredSinkForwardingReflectionTests.cs:29-33, 56-93).
U3 Structured audit pipeline has zero producers PARTIALLY CLOSED. Producers now exist: ManualFailoverService (ControlPlane/Redundancy/ManualFailoverService.cs:61-75) and AdminUI cert ops (AdminUI/Audit/CertAuditEvents.cs:43-51). But AuditOutcomeMapper still has zero production readers and its "dormant" remark is now stale in the other direction (AuditOutcomeMapper.cs:13-17); the #520 ACL workstream did not become a producer (no OpcUaAccessDenied emit site).
U4 FleetStatusBroadcaster heartbeat dead code CLOSED — deleted (Gitea #542). No trace remains except the deletion comment at ControlPlane/ServiceCollectionExtensions.cs:141-143.
U5 Native Part 9 conditions Acknowledge-only to driver PARTIALLY CLOSED. Native Acknowledge now routes to the owning driver (OtOpcUaNodeManager.cs:1179-1182 → 1432-1451 → NativeAlarmAckRouter). But Confirm/AddComment/Shelve on a native condition are sent to the scripted engine's router ("H6c scope is Acknowledge only", :1177-1199) — SDK-local effect only, no upstream reach; native Enable/Disable still BadNotSupported (:1226-1231).
U6 Test gaps concentrate on the fragile seams PARTIALLY CLOSED. Since f6eaa267: real-DPS delivery is tested (PrimaryGateFailoverTests, in-process 2-node cluster with a built-in negative control); OpcUaServer.IntegrationTests gained the over-the-wire AclSubscriptionEnforcementTests; recorder restart-convergence was live-gated (#491). Still open: no actor supervision/restart test anywhere (S6), and the F1/S14 meta-gap — every mesh live gate ran on a rig whose DisableLogin auto-auth neutralizes the exact middleware that breaks the mesh in production (see S14 and theme 1).
U7 IHistoryWriter Null-wired; H2 HistoryUpdate unimplemented STILL OPEN. NullHistoryWriter.Instance is the only registration (Runtime/ServiceCollectionExtensions.cs:68, 388); no HistoryUpdate override; NodePermissions.cs:32-35 still reserves the bit.
U8 BuildSecurityPolicies doc claims a log that doesn't exist PARTIALLY CLOSED (mitigated). The method still logs nothing and the comment still lies (OpcUaApplicationHost.cs:479-524), but OpcUaApplicationHostOptionsValidator now fails host start on an empty profile list (OpcUaApplicationHostOptionsValidator.cs:31, wired Program.cs:454), making the silent fallback unreachable through the Host path. Fix the comment.
U9 EnsureVariable silently ignores changed historize-intent CLOSED (by surgical deploys), stale comment remains. UpdateTagAttributes (OtOpcUaNodeManager.cs:2187-2230) updates Historizing + historian-tagname binding (add and remove), AccessLevel, and DataType/shape in place; the classifier whitelists IsHistorized/HistorianTagname deltas onto it; UNS mirrors repair via Backing* fields. EnsureVariable's doc-comment (:2064-2068) still describes the old silent ignore.
U10 DriverResilienceStatusTracker has no reader CLOSED. R2-10 reader chain live; the deferred T10 breaker-OPEN live gate ran 2026-08-08 (Initialize wrapped as DriverCapability.Initialize, breaker threshold cadence-corrected to 2, tracker invalidated with pipelines — DriverCapabilityInvokerFactory.cs:83).

Architecture overview (what changed since f6eaa267)

The subsystem was re-platformed around split topology: the fleet is three independent 2-node Akka meshes; everything that used to assume one gossip ring now rides explicit gRPC contracts. Central→node commands (NodeCommandService) and node→central acks (CentralCommandService) are per-node unary dials behind per-cluster PSK interceptors; driver nodes serve a telemetry stream central dials into (TelemetryStreamGrpcService + TelemetryDialSupervisor); config reaches driver-only nodes via DeploymentArtifactGrpcService fetch-and-cache (SHA-256-verified against RevisionHash, gzip-aware, null-fetch = failed apply, never an empty config); Phase 4 removed ConfigDb from driver-only nodes entirely (Program.cs:158-161), with DB-less ServiceLevel semantics (OpcUaPublishActor.cs:953-962). SplitTopologyTransportValidator and ConfigSourceOptionsValidator enforce the split-safe combinations at boot via ValidateOnStart on effective roles. The redundancy singleton is cluster-scoped, elected as oldest-Up driver member (RedundancyStateActor.cs:124-177), with scope derived from AkkaClusterOptions (pure config — the Phase 6 stack-overflow landmine is structurally avoided, ControlPlane/ServiceCollectionExtensions.cs:234-248). The opt-in bootstrap guard closes the simultaneous-cold-start split (ClusterBootstrapGuard.cs:112-115, ClusterBootstrapCoordinator.cs:118-158).

The data plane replaced LDAP with API keys (#520): ApiKeyOpcUaUserAuthenticator (no directory I/O; peppered HMAC; uniform deny), keys/grants ride the artifact to driver-only nodes (OpcUaPublishActor.FeedDataPlaneAuthority installs ahead of the plan.IsEmpty early-return, :477-483), and Security:Acl:Enabled gates full per-node enforcement: DataPlaneAuthorizer (fail-closed on every path, key revocation re-checked outside the cache on every call — DataPlaneAuthorizer.cs:238-248), the sharded per-key SessionAclDecisionCache with per-shard generation stamps, the monotonic-counter auth generation (DataPlaneAuthority.cs:304-317), browse transit incl. the shared-root anchor fix at HEAD (DataPlaneAuthorizer.cs:278-289, 382-405 — Raw-realm-qualified, Browse-only, fail-closed in the degenerate collision case), and deploy-triggered monitored-item revalidation that detaches rather than disables (OtOpcUaNodeManager.cs:3235-3286, denial queued before detach).

Deploys became surgical (classifier + ISurgicalAddressSpaceSink, default-closed, ratchet-to-rebuild), the value hot path became batched end-to-end, and the runtime gained the comms-loss watchdog (#527), the alarm-S&F drain gate, and the historization capture gate (#537) / quarantine (#538).


Findings

Severity scale: Critical (correctness/availability broken in a realistic scenario) · High (significant risk or debt, plan a fix) · Medium (real but bounded) · Low (hygiene). Closed findings live only in the status table; still-open findings keep their IDs; new findings continue the scheme (S14+, P5+, C5+, U11+).

1. Stability

S14 — CRITICAL (NEW): The AdminUI cookie FallbackPolicy blocks every mesh gRPC ingress on a login-enabled admin node — and every live gate ran where the blind spot is masked. AddOtOpcUaAuth sets a global FallbackPolicy = RequireAuthenticatedUser under the cookie scheme (Security/ServiceCollectionExtensions.cs:143-148), registered on hasAdmin nodes (Host/Program.cs:520-523) with UseAuthentication/UseAuthorization in the pipeline (:732-733). ASP.NET Core applies the fallback policy to every endpoint without explicit authorization metadata — gRPC endpoints included — and none of the mesh gRPC maps carry AllowAnonymous: MapZbLocalDbSync (:749), DeploymentArtifactGrpcService + CentralCommandGrpcService (:757, :767), TelemetryStreamGrpcService + NodeCommandGrpcService (:782-783). The PSK bearer is not an ASP.NET authentication scheme and gRPC interceptors run inside the endpoint invocation, i.e. after the authorization middleware — so a correctly-keyed node call is challenged by the cookie handler before the interceptor's verdict matters. Consequences on a production (login-enabled) central: every FetchAndCache site node's artifact fetch fails on all endpoints → every fleet deploy fails (fail-closed to last-known-good, per #485 — loud, not corrupting, but a fleet-wide deploy-plane outage); every gRPC ReportApply ack is dropped → deploys seal TimedOut; central↔fused-pair telemetry and LocalDb sync fail. The codebase already knows this exact trap — app.MapStaticAssets().AllowAnonymous() exists because the fallback "401s every CSS/JS request" (:724-728), and the deploy API is explicitly exempted (AdminUI/Api/DeployApiEndpoints.cs:22) — the gRPC maps just never got the same treatment. Why nothing caught it: the docker-dev rig runs Security__Auth__DisableLogin=true, which registers an always-succeeding handler under the cookie scheme name so the FallbackPolicy passes (Security/ServiceCollectionExtensions.cs:82-98) — every mesh live gate therefore ran with the blocking middleware neutralized; and MeshGrpcBoundaryTests builds a minimal pipeline with interceptors but no auth stack (tests/Server/.../MeshGrpcBoundaryTests.cs:334-346). Driver-only site nodes are unaffected (no auth services → no authorization middleware), which makes central's serve side the fleet-wide choke point. Recommendation: .AllowAnonymous() on each mesh gRPC map (the fail-closed interceptors are the real gate, exactly as the map-site comments at :754-778 already argue), plus one rig/gate run with DisableLogin=false to prove the production middleware stack end-to-end.

S15 — HIGH (NEW): Driver comms loss never marks value variables Bad — external OPC UA clients hold stale-Good for the whole outage. OnDriverConnectivityChanged (DriverHostActor.cs:1248-1283) annotates the driver's Part 9 alarm conditions (BadNoCommunication) and fans a synthetic Bad batch to the dependency mux so scripts/virtual tags see the outage (FanCommsLossToDependencyMux, :1334-1359 — the #481 fix). The plain raw/UNS value variables get nothing, and the code's own rationale names the motivating shape: "a poll driver whose device goes unreachable publishes nothing at all" (:1240-1245). For such a driver, an external client subscribed to a value node holds the last Good value with a stale timestamp for the entire outage, until a successful reconnect republishes. Internal consumers were fixed; the external data plane — the product's whole point — was not. Serving stale-Good to SCADA clients is the classic quality-flag failure this repo's own status-code workstreams (#497/#519) exist to prevent. Recommendation: extend the comms-loss fan-out to write BadNoCommunication (carrying last-known value) to the driver's raw NodeIds + UNS mirrors through the same batched sink path, and force-republish on recovery (the value-dedup trap from the Batch 4 live gate is the known hazard to avoid).

S16 — HIGH (NEW): Operator "Restart driver" produces a subscription-less child until the next deploy; _lastComposition is write-only. HandleRestartDriver (DriverHostActor.cs:2916-2932) stops and respawns from the cached spec but never re-sends SetDesiredSubscriptions — the only sender is the apply-time loop (:2660-2672). The fresh child starts with empty _desiredRefs, the #488 reconcile can't help (DriverInstanceActor.cs:625 requires a non-empty set), so the driver reads/subscribes nothing and raises no alarms until the next deployment dispatch. Tellingly, _lastComposition ("the composition from the most-recent apply") is declared at :349, assigned at :2709, and read nowhere — it is exactly the state a restart re-push was meant to consume and never was: the house built-but-never-wired shape, and the live face of S6. Recommendation: re-run PushDesiredSubscriptions for the instance after SpawnChild in the restart path (the state is already cached); then S6's remaining exposure is only the exception-restart path.

S17 — MEDIUM (NEW): The comms-loss watchdog's post-reconnect grace collapses to one health tick. DriverInstanceActor.cs:574: var lastEvidence = observed.LastSuccessfulRead ?? _connectedSinceUtc;. Drivers deliberately preserve LastSuccessfulRead across Degraded/Faulted writes (e.g. ModbusDriver.cs:213, 511, 518 — that preservation is the #527 design), so after a watchdog trip and reconnect against a half-broken device (TCP connects, reads still fail), the first Connected tick sees non-Healthy + a read stamp frozen from before the outage — already > 90 s — and re-trips immediately. The comment at :458-459 ("measured from THIS connect, not from a stale earlier one") describes max(LastSuccessfulRead, _connectedSinceUtc); the code implements ??, which only helps a never-read driver. Result: reconnect churn at health-tick cadence (~30 s) instead of the designed 90 s grace per connection. Otherwise the watchdog checks out exactly as documented (conjunction at :571-576, single GetHealth observation feeding snapshot + watchdog :514-521, disabled at ≤ 0, throwing GetHealth never advances it). Recommendation: one-line fix — take the max.

S18 — MEDIUM (NEW): API-key lookup is a per-activation SQL round-trip on admin/fused nodes, inside the SessionManager-wide lock, with no timeout. The authenticator's "no network I/O on this path at all" (ApiKeyOpcUaUserAuthenticator.cs:22) is strictly true only on driver-only nodes (ArtifactApiKeyStore, in-memory). On admin/fused nodes ConfigDbApiKeyStore.FindAsync/FindGrantsAsync (ConfigDbApiKeyStore.cs:90-115) run an EF query per activation, block-bridged under the SDK's SessionManager event lock with CancellationToken.None (OpcUaApplicationHost.cs:317-319), and none of the LDAP-era bound/concurrency/outage-circuit machinery survived the #520 deletion (OpcUaApplicationHost.cs:269-275 records the removal). A hung — not refusing — ConfigDb serializes all session activations up to the SqlClient timeout each. Far smaller exposure than the LDAP bind this replaced, but the structural hazard S2 described is not zero on fused centrals. Recommendation: a short per-call timeout in ConfigDbApiKeyStore, or a small TTL cache (revocation stays immediate for ACL-enforced deployments because DataPlaneAuthorizer.Evaluate re-checks the key per operation — DataPlaneAuthorizer.cs:238-248).

S19 — MEDIUM (NEW): Key expiry never reaches live monitored items between deploys. RevalidateMonitoredItems is deploy-triggered only — the code says so explicitly ("this is the only trigger", OtOpcUaNodeManager.cs:320-328). Request-shaped operations re-check expiry per call and the decision cache deliberately excludes the key check for exactly this reason (SessionAclDecisionCache.cs:20-28), but a subscription authorized under a then-valid key keeps streaming after ExpiresUtc passes, until the next deployment happens to advance the generation. On a quiet fleet that is indefinite — and streaming is how machine consumers actually read this server (the workstream's own rationale for stage 6 revalidation). Revocation-by-deploy is covered; revocation-by-clock is not. Recommendation: a low-frequency timer (minutes) calling RevalidateMonitoredItems — the pass is already built, memo is already pass-local (the comment at :322-325 anticipates exactly this).

S5 — MEDIUM (carried, narrowed): redundancy-identity mismatch diagnostic exists in one consumer of three. See the status table; add the same once-per-process Warning to OpcUaPublishActor (ServiceLevel silently 0 is the historically expensive silent state) and ScriptedAlarmHostActor.

S6 — MEDIUM (carried): no supervision strategy / restart re-push anywhere; see S16 for the operator-visible face and the ready-made fix.

S7 — MEDIUM (carried): PostStop block-bridge (DriverInstanceActor.cs:1327); slightly more load-bearing than ever since stop+respawn is the path for ResilienceConfig changes and operator restarts.

S8 — MEDIUM (carried): at-most-once Part 9 command routing with optimistic Good and no drop metric (OtOpcUaNodeManager.cs:1409-1413; OtOpcUaServerHostedService.cs:154-209).

S9 — MEDIUM (carried): no certificate expiry monitoring; the health-endpoint surface would be the natural home (Host/Health/HealthEndpoints.cs:34-95).

S10 — LOW (carried): SDK start failure swallowed, invisible to /health (OtOpcUaServerHostedService.cs:129-144).

S11 — LOW (carried, narrowed): audit flush drops batches on DB outage (AuditWriterActor.cs:90-116); now non-moot since producers exist.

S20 — LOW (NEW): RemoveEquipmentSubtree never severs cross-realm inverse Organizes references. The d787502d fix (SeverOutgoingReferences, OtOpcUaNodeManager.cs:2640, 2663-2676) covers only the single-node RemoveVariableNode; the subtree path (:2725-2738) removes UNS reference variables leaving the surviving raw node with a dangling inverse edge — the exact accumulation hazard the fix's own comment (:2629-2639) describes. Currently masked because P5 forces a rebuild whenever the scenario arises; fixing P5 without adding severing here exposes S20 immediately — fix them together.

S21 — LOW (NEW): TOCTOU on _authorizer during shutdown. AclEnforced (OtOpcUaNodeManager.cs:372) reads the volatile field once; gated bodies re-dereference _authorizer! (:1480-1483, :2865-2867, :3284, :3330, :3337, :3414) while StopAsync nulls it (OtOpcUaServerHostedService.cs:297) — an in-flight Read/Browse during stop can NRE out of an SDK service call. Snapshot to a local per seam.

S22 — LOW (NEW): terminal-Bad write failures are counted into failedNodes inconsistently — counted at AddressSpaceApplier.cs:102, 107, 300, 374, 430; discarded at :380, :389, :419, :435. The :300 counting can needlessly hold back _lastApplied and trigger the re-diff→ratchet rebuild for a node whose remove succeeded; the discarding sites under-report. Pick one semantics.

S23 — LOW (NEW): #538 quarantine counters never age. _tagWriteFailures clears only on that tag's own success (ContinuousHistorizationRecorder.cs:752), so N isolated blips weeks apart eventually count as "consecutive" and abandon samples that were never a poison pattern; abandoned-tag entries are retained forever (bounded only by tag-name cardinality). The core #538 discriminator logic itself is exactly as designed (outage pass touches nothing :767-775; counters advance only when a sibling wrote :789-798; abandonment deferred until every failing tag is over budget :800-806; shutdown-cancellation rethrown :707-716).

S24 — LOW (NEW): the two node→central endpoint walkers fail over only on RpcException. A non-Rpc throw (e.g. GrpcChannel.ForAddress on a malformed endpoint — the validator checks only the scheme prefix, MeshTransportOptionsValidator.cs:107-117) escapes GrpcApplyAckSender.PostAsync's per-endpoint catch (Runtime/Communication/GrpcApplyAckSender.cs:212-217) and skips the remaining endpoints; same shape in GrpcDeploymentArtifactFetcher.FetchAsync (Runtime/Deployment/GrpcDeploymentArtifactFetcher.cs:104-109, bounded there by the null-fetch failure mapping).

Positive stability notes (worth preserving): The #520 core is the best-defended code in the subsystem: fail-closed on every authorizer path with an explicit allow-is-possible test posture; key revocation deliberately outside and above the decision cache (DataPlaneAuthorizer.cs:238-248); the monotonic generation counter with the hash-derivation trap documented and pinned (DataPlaneAuthority.cs:304-317); per-shard generation stamps making the cache's rollover window inert (SessionAclDecisionCache.cs:35-43, 483-496); trie-owned-by-authority-state making the never-installed-trie mistake unrepresentable (Program.cs:380-386); browse transit as a non-accumulating marker rather than synthesized grants (PermissionTrie.cs:84-96), with the HEAD shared-root fix correctly realm-qualified and fail-closed in its degenerate collision case; revalidation queues the denial before detaching and is reversible (OtOpcUaNodeManager.cs:3260-3277). The PSK perimeter is uniformly fail-closed (unset/whitespace key rejects all — the #543 fix; FixedTimeEquals everywhere; no key material logged; interceptor registration exactly matches service mapping, Program.cs:586-602 vs :747-784) — S14 is a middleware-ordering defect around it, not a hole in it. ConfigSource honors "no answer ≠ empty config" end-to-end (GrpcDeploymentArtifactFetcher.cs:89-97; DriverHostActor.cs:1905-1928; gzip sniff + 512 MiB guard, DeploymentArtifact.cs:133, 164-196). TelemetryDialSupervisor is generation-stamped, leak-free on node removal, and never gives up (AdminUI/Telemetry/TelemetryDialSupervisor.cs:204-286, 354-362). The bootstrap guard, singleton scoping (options-not-services), and Phase 4 DB-less wiring all check out against their documented rationale.

2. Performance

P5 — MEDIUM (NEW): ApplyPureRemove lacks the subsumption filter for UNS reference variables, so removing any equipment that holds references self-defeats into a full rebuild. The equipment-subtree remove runs first (AddressSpaceApplier.cs:402-405) and subsumes the equipment's UNS reference-variable nodes; the planner independently emits those nodes in RemovedUnsReferenceVariables (AddressSpacePlan.cs:216-219), and the loop over them (:417-421) has no NotSubsumed filter — unlike tags/vtags/alarms (:367, :386, :395). SafeRemoveVariable on the already-removed node returns false ⇒ ratchet ⇒ SafeRebuild(). Correct but maximally expensive: every equipment-with-references removal severs every client subscription — exactly what the surgical-deploys work exists to avoid. Fix: apply the subsumption filter (by the reference's equipment) to the :417 loop — and land S20's severing in the same change, since P5's rebuild is what currently masks S20.

P6 — MEDIUM (NEW, filing the known gap): AnnounceAddedNodes is Uns-only — surgically-added raw containers/tags are materialised but never announced. ComputeAddAnnouncements collects only UNS parents and the announce call hardcodes AddressSpaceRealm.Uns (AddressSpaceApplier.cs:519-531, 576); the sink member is already realm-aware (RaiseNodesAddedModelChange(string, AddressSpaceRealm), OtOpcUaNodeManager.cs:2316), so the fix is confined to the applier. Values publish and writes route fine (the routing maps rebuild from the full composition every apply, DriverHostActor.cs:2594-2605), but added raw nodes are undiscoverable to connected model-aware clients until a manual re-browse. The code comment itself says "a real follow-up, not an accepted limitation" (OpcUaPublishActor.cs:562-563) — no tracking issue exists; file one.

P3 — MEDIUM (carried, narrowed): HistoryRead-Events still returns the full window in one response with no continuation points (OtOpcUaNodeManager.cs:3635-3675), now bounded by the S3 deadline + limiter.

Positive performance notes: the batched value pipeline is the right shape end-to-end (single ingress, fan-out inside the batch, chunked lock holds with validation outside — OtOpcUaNodeManager.cs:555-641); the apply path parses the artifact once (OpcUaPublishActor.cs:465-475), reads Direct-mode artifacts off the host mailbox with an explicit timeout, and dedups stashed dispatches with an Applying-wedge watchdog (warn-only, honestly); delta materialisation makes adding one tag O(delta) instead of O(address space); pinned-dispatcher throughput raised 1→100 without ordering impact.

3. Conventions

C5 — MEDIUM (NEW): host role wiring reads only OTOPCUA_ROLES while the mesh reads effective Cluster:Roles — divergence boots a role-less host that gossips roles. Program.cs:61-63 derives hasAdmin/hasDriver solely from the env var; the effective Akka roles prefer Cluster:Roles with env fallback (Cluster/RoleParser.cs:115-119). A node configured only via Cluster:Roles (the exact knob the wonder-app-vd03 field fix hand-added, per RoleParser.cs:96-103) joins the mesh carrying driver/cluster-* — passing SplitTopologyTransportValidator and ConfigSourceOptionsValidator, which read effective roles — yet registers no runtime actors, no ConfigDb/LocalDb, and the deny-all authenticator. Central counts it as a driver node (deploys time out against it) and it can even be the oldest role-carrying member for singleton placement without running the singleton manager. This is the exact inverse of the #532 incident, and no validator cross-checks the two sources. Recommendation: fail (or Error-log) at boot when the Cluster:Roles-derived set ≠ the OTOPCUA_ROLES-derived set actually wired.

C6 — LOW (NEW): the primary-gate XML summaries still document the pre-R2-04 default-allow. HandleRouteNodeWrite's summary says "default-allow while the role is unknown, so single-node deploys + the boot window never reject" (DriverHostActor.cs:1449-1451; native ack :1542), contradicting the multi-driver default-DENY the bodies implement (:1465-1468, :1553-1556). A future editor reading only the summaries could "restore" the S4 bug believing it documented behavior. Same class: EnsureVariable's stale historize-intent comment (U9), AuditOutcomeMapper's stale dormancy remark (U3), BuildSecurityPolicies' phantom log (U8).

C7 — LOW (NEW): endpoint routing is listener-agnostic — no RequireHost anywhere, so every mapped gRPC service is also reachable on the main HTTP surface (and via Traefik on fused nodes), and health/metrics are served unauthenticated on the dedicated h2c ports (Program.cs:786-787; AllowAnonymous by design in the shared libs). No auth bypass — interceptors are path-scoped, not listener-scoped — but the exposure surface is wider than the port-per-purpose docs imply; one doc line for firewall authors.

C1 — MEDIUM (carried, narrowed): four imperative ServerHistorian binds remain despite the validator. C2 — MEDIUM (carried, narrowed): ContinuousHistorization still warn-only with a silent CommitMode fallback; DevStubMode still log-warn-only (AdminUI-scope now). C3 — LOW (carried): the static-logger island grew slightly (4 sites in Runtime SCE + DpsScriptLogPublisher.cs:61; 14 node-manager pragma blocks), with LocalDbSetup the one documented-legitimate case.

Positive conventions notes: the new perimeter code is uniformly disciplined — every PSK interceptor shares the same fail-closed template with the same header parsing (GrpcRequestHeaders.cs:44-69); the options-not-services rule (AkkaClusterOptions, never IClusterRoleInfo, at composition time) is now stated and honored at three seams (ConfigDbApiKeyStore.cs:72-78, ControlPlane/ServiceCollectionExtensions.cs:234-248, Program.cs:419-427); validators name the key and allowed values on failure (MeshTransportOptionsValidator.cs:49-55); the classifier's with-equality whitelist makes future-field safety structural rather than reviewed-in; and LocalDbSetup's registration-follows-config with idempotent deregister self-heal (LocalDbSetup.cs:171-179, 209-248) is the pattern at its best.

4. Underdeveloped areas

U11 — MEDIUM (NEW): the Backing*-field parity invariant is convention-only. The UNS-mirror staleness repair depends on every raw-tag attribute the surgical whitelist covers having a Backing* counterpart on UnsReferenceVariable (so the planner emits the coincident UnsReferenceDelta). The mechanism is verified and pinned for the four current fields (AddressSpacePlannerDualNamespaceTests.cs:233-269), but nothing mechanical ties the classifier whitelist (AddressSpaceChangeClassifier.cs:174-185) to the Backing* set — CLAUDE.md's own warning ("omitting one silently reopens the staleness") is enforced by comments alone. Mitigating factor: widening the whitelist also widens the fixed-arity UpdateTagAttributes signature, so a human touches the seam — but that is precisely the "plan names N−1 dispatch sites" trap this repo has already paid for. Recommendation: a reflection guard in the DeferredSinkForwardingReflectionTests style — assert every non-identity whitelisted field has a same-named Backing* property, or a planner property test perturbing each whitelisted field and asserting a coincident delta.

U12 — MEDIUM (NEW): a surgical historize-flip or tagname change never fires EnsureTags provisioning. ProvisionHistorizedTags iterates plan.AddedRawTags only (AddressSpaceApplier.cs:597), while FeedHistorizedRefs does handle changed tags (:699-706) and starts the recorder capturing the new ref. Flipping isHistorized on an existing tag — now a routine first-class surgical edit — captures values against a historian tag that may not exist; WriteLiveValues fails until the #538 quarantine abandons the tag's entries. Pre-existing (rebuilds also only provisioned adds; the documented Issue-5 limitation), but surgical deploys made it routine-reachable. Extend provisioning to the changed-ref add side of the delta.

U13 — LOW (NEW): the telemetry dial's "connected" pill requires a first received event (TelemetryDialSupervisor.cs:236-243; TelemetryLocalHub.cs:16-22 replays only what was emitted), so a healthy stream from an empty node reads as down. Cosmetic.

U14 — LOW (NEW): RevalidateMonitoredItems counts restorations but never reports them (OtOpcUaNodeManager.cs:3241, 3264 vs :3282-3286 — only stand-downs are logged); grant-restored feed resumptions are invisible to operators.

U3 — MEDIUM (carried, narrowed): the audit pipeline has producers now but the mapper has none, and the new producers bypass it — decide whether AuditOutcomeMapper is the contract or dead weight; the ACL gate remains a natural producer that was never wired.

U5 — MEDIUM (carried, narrowed): native Confirm/Shelve route to the scripted engine (upstream no-op) and Enable/Disable stay BadNotSupported (OtOpcUaNodeManager.cs:1177-1231).

U6 — MEDIUM (carried, reshaped): the two biggest prior gaps (DPS delivery, ACL over-the-wire) are now tested; the remaining structural gaps are (1) still no actor supervision/restart test anywhere (S6/S16 would both have been caught), and (2) the DisableLogin masking problem — the production auth middleware stack has never been exercised against the mesh (S14). A single gate run with DisableLogin=false retires an entire class.

U7 — LOW (carried): IHistoryWriter Null-wired; H2 HistoryUpdate unimplemented. U8 — LOW (carried, mitigated): fix the BuildSecurityPolicies comment.


Maturity ratings

Dimension 2026-07-12 2026-08-19 Justification
Stability 3 3 The three carried Highs all closed for real (S2 by deletion, S3 by async fan-out, S4 by default-deny — each verified in code and live-gated), and the new watchdog/gates/quarantine machinery is carefully built. But the fresh code contributed one Critical (S14 — fleet-wide deploy-plane outage on a login-enabled central, masked by every rig gate) and two Highs (S15 stale-Good on comms loss; S16 restart yields a dead driver), all three in the house failure-mode families. Better where it was weak; newly exposed where it grew.
Performance 3 4 P1 substantially closed (surgical deploys, default-closed with a rebuild ratchet), P2 closed (end-to-end batching with bounded lock holds), P4 closed (delta materialisation). Residuals are bounded and enumerable (P5 subsumption gap, P6 announce gap, P3 events paging, container removes). The hot paths are now designed rather than inherited.
Conventions 4 4 The new perimeter code is the pattern at its best (uniform fail-closed interceptor template, options-not-services stated at three seams, structural future-field safety in the classifier). Held down by the growing doc-drift class (C6 — stale summaries that document reverted behavior are a regression vector, not hygiene), the C5 dual-role-source divergence, and the slightly grown static-logger island.
Underdeveloped 4 3.5 Strong closes: U4 deleted, U9/U10 closed, U2 extended to the surgical members, audit gained producers, DPS delivery + over-the-wire ACL now tested. But the dominant new theme is guards that cannot see their own blind spot: every mesh live gate ran with the auth middleware neutralized (S14), the Backing*-parity invariant is comment-enforced (U11), _lastComposition shipped write-only (S16), and provisioning silently skips the newly-routine edit shape (U12). The half-step down reflects that the class, retired in round 2 for sinks and DI, re-emerged in the new subsystems.

Cross-cutting themes

  1. A dev-mode auth bypass in the rig makes every live gate blind to the production middleware. S14 is the sharpest instance of the house "verification can't see its own blind spot" mode this review has recorded: DisableLogin=true registers a pass-everything handler under the cookie scheme name, so the FallbackPolicy — the exact thing that breaks the mesh in production — passes on every rig run, and the boundary tests build pipelines without the auth stack at all. The rig's convenience feature is load-bearing for the gap. One gate run with DisableLogin=false (and a standing CI variant of MeshGrpcBoundaryTests that includes UseAuthentication/UseAuthorization + the FallbackPolicy) retires the class.
  2. Deleting a subsystem deletes its protections; audit what still needed them. #520 removed the LDAP authenticator and, with it, the timeout/concurrency/outage-circuit machinery — correctly, for the in-memory artifact store, but the ConfigDb store re-introduced network I/O into the same synchronous, lock-holding callback with none of the guards (S18). When a replacement changes the I/O profile per-node-role, the old protections need a per-role disposition, not a wholesale delete.
  3. Doc drift is now a regression vector, not hygiene. The gates' XML summaries still describe default-allow (C6), EnsureVariable still claims the silent ignore (U9), the audit mapper claims dormancy that is no longer true (U3). Each stale comment describes the exact behavior a past fix reverted, which is the worst kind: it invites a confident restoration of the bug. A doc-sweep pass over the three named sites is cheap; the habit of updating the summary in the same commit as the body is the real fix.
  4. The fail-closed ratchet is doing its job — now trim what it over-catches. P5, S22, and the container-remove hard-false all resolve to "correct but rebuilds": the one-way ratchet made every surgical gap safe, which is the design working. The next increment of value is not new machinery but subsumption/accounting fixes (P5+S20 together, S22) so the ratchet fires only when it must.
  5. Convention-enforced invariants in the new subsystems need the mechanical guards the old ones got. Round 2 retired the built-but-never-wired class for sinks (reflection forwarding) and DI (registration guards). The new code re-created the shape in three places — Backing* parity (U11), _lastComposition (S16), announce realm coverage (P6) — each of which admits the same style of cheap structural guard that already proved itself.