lmxopcua/archreview/03-server-runtime.md

# Architecture Review — Server & Runtime Subsystem

- **Date:** 2026-07-12
- **Commit:** `f6eaa267` (master, clean tree)
- **Updates:** the 2026-07-08 review at `9cad9ed0`. Since then all arch-review remediation branches merged to master (~40 commits), including the S1 SBR work + #11 premise correction, the #9 hard-kill failover test, the U2 deferred-sink reflection guard, and the #10/#13 CapabilityInvoker dispatch wiring + per-instance ResilienceConfig.
- **Scope:** `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer`, `.Host`, `.Runtime`, `.ControlPlane`, `.Security`, plus the sink/publisher wrappers in `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa`, cluster/SBR behavior in `src/Core/ZB.MOM.WW.OtOpcUa.Cluster`, and `tests/Server` for coverage assessment.
- **Dimensions:** Stability · Performance · Conventions · Underdeveloped areas

---

## Prior-finding status (9cad9ed0 → f6eaa267)

Every finding from the 2026-07-08 review, verified against the actual code (not just STATUS.md). Production code in `OpcUaServer`, `ControlPlane`, `Security`, and `Commons/OpcUa` is **byte-identical** since 9cad9ed0 (only `Host/Drivers/DriverFactoryBootstrap.cs`, `Host/Engines/*`, `Runtime/Drivers/*`, `Runtime/ServiceCollectionExtensions.cs`, and `Cluster/*` changed), so most findings carry over verbatim with shifted line numbers noted.

| ID | One-liner | Verified status |
|---|---|---|
| S1 | SBR "configured but never activated"; cluster runs NoDowning | **INVALID PREMISE, GAP CLOSED ANYWAY (#11 + #9).** The original Critical was factually wrong: Akka.Cluster.Hosting's `WithClustering` applies `SplitBrainResolverOption.Default` when the typed option is null, which registers the SBR downing provider that reads the pre-existing `akka.conf` `keep-oldest` block — so SBR **was already active on master** and hard-crash failover already worked; the cluster was never NoDowning. Proven by the #9 negative control (only an explicit `downing-provider-class = ""` breaks failover). What *was* real: the strategy was implicit-in-framework-default and hard-kill failover was untested. Both closed: `BuildClusterOptions` now sets `KeepOldestOption { DownIfAlone = true }` explicitly (`Cluster/ServiceCollectionExtensions.cs:108-116`, `a81dea10`) with corrected-premise docs in the XML comment + `akka.conf:40-47` (`eaf78aad`), and `HardKillFailoverTests` + `TwoNodeClusterHarness.HardKillNodeAAsync` (transport shutdown → B downs the alone oldest and takes over as `driver` role-leader, ~35 s, stable-streak polled) guard the failover *outcome* regardless of activation path (`a25c9ed0`). The Failover trait is not excluded by the CI filter (`v2-ci.yml:61`), so it runs in the whole-solution leg. |
| S2 | LDAP auth blocks SDK session-activation thread, no timeout | **STILL OPEN.** `OpcUaApplicationHost.cs:271-274` still block-bridges with `CancellationToken.None`; `LdapOpcUaUserAuthenticator.cs:30-48` adds no timeout; `LdapOptions.ToLibraryOptions()` (`Security/Ldap/LdapOptions.cs:94-116`) still projects no timeout option. No remediation branch existed. |
| S3 | HistoryRead block-bridges gateway per node, sequentially | **STILL OPEN.** `OtOpcUaNodeManager.cs:1911, 2059, 2171, 2203` unchanged — per-node `.GetAwaiter().GetResult()` with `CancellationToken.None`, bounded only by the 30 s gateway `CallTimeout` against `MaxRequestThreadCount = 100`. |
| S4 | Primary-gate default-allow while role unknown (dual-primary window) | **STILL OPEN.** `DriverHostActor.cs:977` (alarm emit), `:1039` (write gate), `:1095` (ack gate) still gate on `_localRole is Secondary or Detached` with null → allow; `OnRedundancyStateChanged` `:1126`. Listed in STATUS.md's "suggested next", not started. |
| S5 | Redundancy NodeId string-match, no mismatch diagnostic | **STILL OPEN.** `OpcUaPublishActor.cs:512` unchanged; no log when the local node is absent from a snapshot. |
| S6 | Driver-child restart loses desired-subscription state; no backoff supervision | **STILL OPEN.** Repo-wide grep: still zero `SupervisorStrategy` overrides in Runtime/ControlPlane; desired-subs still arrive post-spawn into actor state (`DriverInstanceActor.cs:84, 190-203, 912`). Still no supervision/restart test. |
| S7 | `PostStop` blocks on `ShutdownAsync(CancellationToken.None)` | **STILL OPEN.** Now at `DriverInstanceActor.cs:1009`. |
| S8 | Part 9 commands / native acks at-most-once, client sees Good | **STILL OPEN.** `OtOpcUaServerHostedService.cs:139-194` routers unchanged; no drop metric. |
| S9 | No server-cert renewal/expiry monitoring | **STILL OPEN.** `OpcUaApplicationHost.cs:305-317` unchanged. |
| S10 | SDK start failure swallowed; node runs with Null sinks, invisible to /health | **STILL OPEN.** `OtOpcUaServerHostedService.cs:110-129` unchanged. |
| S11 | `AuditWriterActor` unbounded buffer, drops batches on outage | **STILL OPEN** (and still moot — see U3). |
| P1 | Any structural deploy change → full rebuild severs all monitored items | **REMEDIATED (offline; live gates deferred)** on `r2/07-surgical-pure-adds` (R2-07 Phases 1–3, `bb226f45`…`e2b47638`). New pure `AddressSpaceChangeClassifier` routes each deploy delta to its minimal mutation in `AddressSpaceApplier.Apply`: **PureAdd/AttributeOnly** skip `RebuildAddressSpace` (the idempotent Materialise passes add exactly the new nodes; `AnnounceAddedNodes` fires a Part 3 NodeAdded per parent), **PureRemove/AddRemoveMix** tear down only the affected subtree in place via 3 new `ISurgicalAddressSpaceSink` remove members (guard-first, negative-control observed) + `OtOpcUaNodeManager` scoped teardown with `NodeDeleted` model-change, and **Rebuild** stays the default-closed safety valve (any node-affecting change, plus every surgical false/throw ⇒ full-rebuild ratchet). So an add-one-tag / remove-one-tag / mixed deploy no longer severs unrelated subscriptions. OpcUaServer.Tests 336/336, Runtime.Tests 398/398, Commons forwarding guard green. **Deferred:** the over-the-wire `SubscriptionSurvivalTests` (add/remove/mixed) + the docker-dev live-`/run` gates (`kind=PureAdd/PureRemove/AddRemoveMix, rebuild=False`) — authored + compile-green, run in the serial heavy pass. |
| P2 | Per-value global-lock write, no batching | **STILL OPEN.** `OtOpcUaNodeManager.cs:266-281` unchanged. Same guard-unblocked, not-started status as P1. |
| P3 | HistoryRead-Events unpaged, `0` → `int.MaxValue` | **STILL OPEN.** `OtOpcUaNodeManager.cs:1914-1954` unchanged. |
| P4 | Deploy re-runs four Materialise passes (low) | **STILL OPEN.** `OpcUaPublishActor.cs:338-354` unchanged. |
| C1 | `ServerHistorian` bound imperatively in five places | **STILL OPEN.** `Program.cs:121`, `Runtime/ServiceCollectionExtensions.cs:132, 168` (+ `AddAlarmHistorian`), `OtOpcUaServerHostedService.cs:89` — all unchanged. |
| C2 | Two-tier options validation; `DevStubMode` only log-warned in prod | **STILL OPEN.** `OtOpcUaLdapAuthService.cs:79-88` unchanged. |
| C3 | Static Serilog logger islands + node-manager CS0618 pragmas | **STILL OPEN.** `Runtime/ServiceCollectionExtensions.cs:90, 100, 136` unchanged. (The *new* resilience code in `DriverFactoryBootstrap` correctly uses `ILoggerFactory` — the convention is honored by new code, not retrofitted.) |
| C4 | `IHistorianProvisioning` resolve has no missing-registration warning | **STILL OPEN.** Now `Runtime/ServiceCollectionExtensions.cs:303` (shifted +5). The new invoker-factory resolve (`:219-220`) is likewise silent, but deliberately so (admin nodes legitimately lack it) and is backstopped by the new DI guard test — see positive notes. |
| U1 | CLAUDE.md "KNOWN LIMITATION 2" stale (ref-feed gap closed in code) | **FIXED.** Rewritten on the docs branch (`9fadead6`, merged `b67bd9e8`): now reads "value-capture path is code-complete… treat continuous value-capture as unproven-in-production rather than absent". The residual recommendation (explicit restart-convergence test) remains open — folded into U6. |
| U2 | Deferred-sink forwarding trap half test-guarded | **FIXED.** `DeferredSinkForwardingReflectionTests` (`tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/`, `a65c2ced`, merged `1d7e7caa`): reflects over every method of every forwarding interface of `DeferredAddressSpaceSink` + `DeferredServiceLevelPublisher`, invokes each against a `DispatchProxy` recording inner (auto-covers future members), asserts arrival, plus a guard-of-the-guard pinning the wrapper's interface set. Negative control verified per STATUS. This retires the F10b/PR#423 trap class and unblocks P1/P2. |
| U3 | Structured audit pipeline has zero producers | **STILL OPEN.** `AuditOutcomeMapper.cs:12-18` / `Security/Audit/AuditActor.cs:18-24` unchanged. |
| U4 | `FleetStatusBroadcaster` heartbeat dead code + host-only NodeId key | **STILL OPEN.** `FleetStatusBroadcaster.cs:38, 110-121, 152-159` unchanged. |
| U5 | Native Part 9 conditions Acknowledge-only to driver | **STILL OPEN.** `OtOpcUaNodeManager.cs:647-658, 698-703` unchanged. |
| U6 | Test gaps concentrate on the fragile seams | **PARTIALLY FIXED.** Closed: hard-kill failover (#9, the biggest single blind spot); whole-solution CI leg + fail-on-skip gate + CLI deflake (`10b89830`, `v2-ci.yml:58-61` + `scripts/ci/assert-not-all-skipped.sh`) — green-because-all-skipped is now detected. Still open: DPS delivery of redundancy/ServiceLevel state untested over a real mediator; no actor supervision/restart test (S6); outbox restart-durability untested; recorder restart-convergence untested (from U1); `OpcUaServer.IntegrationTests` still contains exactly one test (`DualEndpointTests`). |
| U7 | `IHistoryWriter` Null-wired; H2 HistoryUpdate unimplemented; `LdapAuthFailure` overload | **STILL OPEN** (tracked backlog). |
| U8 | `BuildSecurityPolicies` doc claims a log that doesn't exist | **STILL OPEN.** `OpcUaApplicationHost.cs:376-418` unchanged. |
| U9 | `EnsureVariable` historize-intent invariant lives in comments | **STILL OPEN.** `OtOpcUaNodeManager.cs:1345-1349` unchanged. |

---

## Architecture overview

### Host composition

`Program.cs` (`src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs`) is a single fused binary whose wiring is driven by the `OTOPCUA_ROLES` env var (`admin`, `driver`, or both). Notable mechanics: per-role appsettings overlays with env-vars/args re-appended so deployment overrides keep precedence (`Program.cs:61-73`); CWD anchored to `AppContext.BaseDirectory` for Windows-service path sanity (`:83`); Serilog via the shared `AddZbSerilog` with the static `Log.Logger` assigned post-build so Akka's `SerilogLogger` routes actor logs (`:305`).

On `driver` nodes the host registers the historian stack (three config-gated layers plus the ContinuousHistorization outbox/writer pair), the driver-factory registry (8 drivers, `DriverFactoryBootstrap.cs`), **the Phase 6.1 resilience infrastructure** (new since 9cad9ed0: process-singleton `DriverResilienceStatusTracker` + `DriverResiliencePipelineBuilder` + the `IDriverCapabilityInvokerFactory` closing over `DriverFactoryRegistry.GetTier`, `DriverFactoryBootstrap.cs:53-71`), the LDAP data-plane authenticator, and the late-binding seams: `DeferredAddressSpaceSink` / `DeferredServiceLevelPublisher` are DI singletons that actors capture at construction; `OtOpcUaServerHostedService` swaps real SDK-backed implementations in after `StandardServer` start and Nulls back on stop (`OtOpcUaServerHostedService.cs:131, 226-231, 241-253`).

### OPC UA server & node manager

Unchanged since 9cad9ed0. `OtOpcUaSdkServer` (a `StandardServer`) creates the single `OtOpcUaNodeManager` (`CustomNodeManager2`, 2 424 lines) owning the entire writable address space: UNS folders, typed equipment-tag variables, Part 9 `AlarmConditionState` nodes, historized-node registration, four HistoryRead override arms. Reverse paths are Akka-free delegates set by the Host at start (`AlarmCommandRouter`, `NativeAlarmAckRouter`, `NodeWriteGateway`, `HistorianDataSource`). Inbound writes are role-gated (`WriteOperate`), dispatched fire-and-forget under the SDK `Lock`, optimistically applied, self-corrected on failed device outcome.

### Runtime actor topology (per driver node)

`WithOtOpcUaRuntimeActors` (`Runtime/ServiceCollectionExtensions.cs:196-357`) spawns: `DbHealthProbeActor` → `DependencyMuxActor` → (gated) `ContinuousHistorizationRecorder` → `OpcUaPublishActor` (pinned dispatcher, owns `AddressSpaceApplier`) → `PeerProbeSupervisor` → `DriverHostActor` → `HistorianAdapterActor`. `DriverHostActor` spawns one `DriverInstanceActor` per deployed driver plus the VirtualTag/ScriptedAlarm sub-hosts.

**New since 9cad9ed0 — the resilience dispatch seam (#10/#13):** `DriverInstanceActor` now routes all **six** of its driver-capability call sites through an injected `IDriverCapabilityInvoker` (Core.Abstractions; Runtime stays deliberately Polly-free — verified: the csproj still references only `Core.Abstractions`, never `Core`): Write via `ExecuteWriteAsync` (`DriverInstanceActor.cs:597`), alarm-ack `AlarmAcknowledge` (`:650`), subscribe/unsubscribe `Subscribe` (`:693, :722`), alarm-subscribe `AlarmSubscribe` (`:803`), discover `Discover` (`:868`). Single-reference calls key the per-host breaker via `IPerCallHostResolver` (`:957`; implemented by Modbus/AbCip/AbLegacy/FOCAS/TwinCAT with a safe fall-back-to-instance-id); bulk calls key on the instance id. Per-instance `ResilienceConfig` from the deploy artifact (`DeploymentArtifact.cs:171-185`) is layered onto tier defaults by `DriverCapabilityInvokerFactory.Create`, which also `Invalidate`s the instance's cached pipelines so a respawn with changed options rebuilds them; `DriverSpawnPlanner` forces stop+respawn on a ResilienceConfig change while a pure DriverConfig change stays an in-place delta (`DriverSpawnPlan.cs:45-56`). Generation-suffixed actor names (`DriverHostActor.cs:1666-1670`, pre-existing) make the stop→respawn name-collision-safe, and the apply path re-pushes desired subscriptions after reconcile (`:1215-1225`), so a respawned child converges. The OTOPCUA0001 analyzer (wired tree-wide via `Directory.Build.props:48`) is the compile-time guard that every guarded call stays wrapped; `ResilienceInvokerFactoryRegistrationTests` guards that production DI binds the *real* factory, not the pass-through.

**No actor in Runtime overrides `SupervisorStrategy`** — everything still runs under Akka's default one-for-one unlimited restart decider (S6).

### Redundancy model

Non-transparent warm redundancy, unchanged in code; the *understanding* of it is corrected (S1/#11). `RedundancyStateActor` derives Primary/Secondary from Akka's `driver` role-leader, debounces 250 ms, publishes on DPS `redundancy-state`, re-publishes on a 10 s heartbeat. The split-brain resolver is **active and now explicit**: `BuildClusterOptions` sets `KeepOldestOption { DownIfAlone = true }` (correct for a 2-node pair; `keep-majority`/`static-quorum` would be wrong), consistent with the `akka.conf` block (`stable-after = 15s`, which only HOCON can express). Hard-crash failover is proven by `HardKillFailoverTests` (~35 s in-process). There is still no lease or fencing token — correctness rests on Akka's single-role-leader guarantee within a converged cluster, and the boot-window default-allow (S4) remains the data-plane soft spot.

---

## 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). Fixed/invalidated findings live only in the status table above; still-open findings keep their IDs; new findings continue the scheme.

### 1. Stability

**S2 — HIGH: LDAP authentication blocks the OPC UA session-activation thread with an effectively non-cancellable, non-configurable timeout.**
The SDK invokes `ImpersonateUser` synchronously; `OpcUaApplicationHost.HandleImpersonation` block-bridges the authenticator (`OpcUaApplicationHost.cs:271-274`) with `CancellationToken.None`. The doc comment shifts timeout responsibility to the authenticator, but `LdapOpcUaUserAuthenticator` adds none (`Host/OpcUa/LdapOpcUaUserAuthenticator.cs:30-48`), and the shared library's `AuthenticateAsync` is synchronous-in-`Task.FromResult`; its 10 s connect timeout is a library default that `LdapOptions.ToLibraryOptions()` does not project or expose (`Security/Ldap/LdapOptions.cs:94-116`). Every authentication opens a fresh TCP connection + service-account bind + search + user bind. Under a directory outage, session activations stall serially ~10-20 s each on SDK threads.
*Recommendation:* enforce a hard timeout in `LdapOpcUaUserAuthenticator` (`Task.WhenAny` + deny on timeout), surface `Security:Ldap:TimeoutMs`, consider a short negative-cache. Fail-closed semantics are otherwise correct end-to-end.

**S3 — HIGH: HistoryRead block-bridges the gateway per node, sequentially, on SDK request threads.**
All four HistoryRead arms call `.GetAwaiter().GetResult()` per node handle with `CancellationToken.None` (`OtOpcUaNodeManager.cs:1911, 2059, 2171, 2203`). Safe w.r.t. the node-manager `Lock` (the overrides run outside it — correctly documented), but bounded only by the gateway client's 30 s `CallTimeout`. A single request naming N historized nodes against a slow historian holds one SDK request thread up to N × 30 s against `MaxRequestThreadCount = 100` / `MaxQueuedRequestCount = 200` (`OpcUaApplicationHost.cs:329-331`) — a handful of misbehaving history clients can exhaust the pool and degrade all OPC UA services.
*Recommendation:* parallelize per-node reads within a batch (bounded), pass a per-request deadline token, add a server-side concurrent-HistoryRead limiter.

**S4 — HIGH: Primary-gate "default-allow while role unknown" opens a dual-primary window on the data plane.**
`DriverHostActor` gates native-alarm emission, `HandleRouteNodeWrite`, and `HandleRouteNativeAlarmAck` on `_localRole is Secondary or Detached` and deliberately defaults to *allow* until the first redundancy snapshot arrives (`DriverHostActor.cs:977, 1039, 1095; OnRedundancyStateChanged :1126`). Combined with at-most-once DPS delivery heartbeat-repaired every 10 s, a freshly-booted or snapshot-missed **secondary services device writes and emits alerts as if primary** for up to the heartbeat interval — and indefinitely if the NodeId identity mismatch (S5) bites. The boot-window rationale (single-node deploys) is sound, but the same signal gates shared-field-device writes. The now-proven SBR failover (S1) does not narrow this window — it is a delivery/boot-order gap, not a downing gap.
*Recommendation:* default-*deny* for the write/ack gates once the cluster has >1 driver member, or require one received snapshot before servicing writes on multi-node clusters. (Named in STATUS.md's "suggested next" — not started.)

**S5 — MEDIUM: Redundancy NodeId identity is string-matched across three independently-derived sources with no diagnostic on mismatch.**
`OpcUaPublishActor` matches `n.NodeId == _localNode.Value` (`OpcUaPublishActor.cs:512`); the snapshot side derives `host:port` from the gossiped `Member.Address` (`RedundancyStateActor.cs:147-148`), the local side from configured `PublicHostname:Port`. Divergence (DNS vs IP, container advertised name) silently computes ServiceLevel 0 / stale role — the shape of the historical "silently inert delivery" bug, indistinguishable from legitimate Detached.
*Recommendation:* log-once (Warning) when the local node is absent from a received snapshot; startup identity self-check.

**S6 — MEDIUM: A crashed-and-restarted `DriverInstanceActor` loses its message-delivered state (desired subscriptions) until the next deploy.**
Still zero `SupervisorStrategy` overrides in Runtime (grep re-confirmed at f6eaa267): a throwing driver child restarts one-for-one, unlimited, no backoff. Restart re-runs `PreStart` but the desired-subscription set arrives post-spawn via `SetDesiredSubscriptions` stored in actor state (`DriverInstanceActor.cs:84, 190-203, 912`); a restart wipes it and the parent has no restart-detection to re-send. A persistently-throwing driver hot-loops. Note the new invoker does *not* change this: retries happen inside a single message handler; a handler that ultimately throws still escalates to the default decider.
*Recommendation:* `BackoffSupervisor` for driver children + child-requests-state-on-restart (or parent push on `PostRestart`). Still no supervision test (U6).

**S7 — MEDIUM: `DriverInstanceActor.PostStop` blocks the actor shutdown path on driver shutdown.**
`_driver.ShutdownAsync(CancellationToken.None).GetAwaiter().GetResult()` (`DriverInstanceActor.cs:1009`) remains the only synchronous block in the Runtime actors; a hung protocol stack stalls stop/re-deploy of the whole child set. Slightly more load-bearing now: #13 makes stop+respawn the path for every ResilienceConfig edit.
*Recommendation:* bound it (`WaitAsync(timeout)`), log-and-abandon on expiry.

**S8 — MEDIUM: Inbound Part 9 commands and native acks are at-most-once; the client sees `Good` even if the command is lost.**
Unchanged (`OtOpcUaServerHostedService.cs:139-194`; `OtOpcUaNodeManager.cs:769-772`). Deliberate (non-blocking under `Lock`), but invisible in operation.
*Recommendation:* counter/metric for dropped routes; engine-side reconciliation sweep.

**S9 — MEDIUM: Certificate lifecycle has no renewal or expiry monitoring.**
Unchanged (`OpcUaApplicationHost.cs:305-317`): self-signed, 12-month lifetime, checked only at boot. ~12 months after first deploy, Sign/SignAndEncrypt endpoints and UserName-token encryption fail with no warning.
*Recommendation:* startup + periodic expiry check surfaced via health/metric; rotation runbook.

**S10 — LOW: SDK start failure is swallowed and the node keeps running with Null sinks** (`OtOpcUaServerHostedService.cs:110-129`); nothing surfaces it through `/health`.

**S11 — LOW: `AuditWriterActor` buffer is unbounded between flushes and drops whole batches on DB outage** (`ControlPlane/Audit/AuditWriterActor.cs:35, 83, 113-116`). Explicitly best-effort; currently moot (U3: no producers).

**S12 — MEDIUM (NEW): The write dispatch site hardcodes `isIdempotent: true`, bypassing the per-tag `WriteIdempotentAttribute` contract.**
`DriverInstanceActor.HandleWriteTagAttribute` routes through `_invoker.ExecuteWriteAsync(ResolveHost(msg.TagId), isIdempotent: true, …)` (`DriverInstanceActor.cs:597-601`). The `WriteIdempotentAttribute` (`Core.Abstractions/WriteIdempotentAttribute.cs`) — whose whole purpose is per-tag opt-in to write retries, and which `DriverCapability.Write`'s doc and `ExecuteWriteAsync`'s non-idempotent arm are built around — has **zero readers anywhere in the tree**. Today this is behavior-neutral (the Write tier default is `RetryCount = 0`, and the site comment says so), but the moment an operator sets a Write retry in a per-instance `ResilienceConfig` (#13 made exactly this reachable), retries apply to **every** write on that instance — including command-shaped tags (write-1-to-pulse, PLC auto-resets) where a duplicate write double-triggers. The hardcode silently converts a per-tag safety contract into an instance-wide one.
*Recommendation:* either resolve idempotence per tag (plumb the attribute/tag-config through `WriteTagAttribute`) before honoring a configured Write retry, or document loudly on the `ResilienceConfig` authoring surface that Write.RetryCount > 0 retries all writes on the instance. Until then the safest change is `isIdempotent: false` with per-tag opt-in later — that keeps the non-idempotent no-retry arm authoritative.

**S13 — LOW (NEW): Respawn-on-ResilienceConfig-change has a narrow stale-pipeline re-cache race.**
`DriverCapabilityInvokerFactory.Create` invalidates the instance's cached pipelines (`DriverCapabilityInvokerFactory.cs:71`) before the *old* child has terminated (`StopChild` is an async `Context.Stop`, `DriverHostActor.cs:1278-1280, 1658-1664`). An old-child handler already dequeued and mid-await can reach its invoker call *after* the invalidate; `GetOrCreate` then re-caches a pipeline built from the old child's captured options snapshot under the same `(instance, host, capability)` key — and the cache ignores options on a hit (`DriverResiliencePipelineBuilder.cs:75-79`), so the new child silently serves the stale pipeline until the next respawn. Window is milliseconds (the mailbox suspends on Stop; a pipeline is resolved once per `ExecuteAsync` at call start), but the failure is the exact "options-blind cache hit" the invalidate was added to prevent.
*Recommendation:* include an options fingerprint in `PipelineKey` (removes the invalidate-ordering dependency entirely), or invalidate from the *new* invoker lazily on first use after the old child's `Terminated`.

**Positive stability notes (worth preserving):** the node-manager locking discipline remains exemplary (every mutation under `Lock`, every `ReportEvent` outside it, write-dispatch continuation forced off the SDK thread, revert guarded by `ShouldRevert`). The new resilience wiring is carefully done: exception transparency holds (pipeline exceptions — `TimeoutRejectedException`, `BrokenCircuitException` — surface through the same `catch (Exception)` arms the raw calls used, so failure paths like `WriteAttributeResult(false, …)` and Reconnecting transitions are unchanged); retry `ShouldHandle` excludes `OperationCanceledException`, so the actor's outer `cts` deadlines (5 s write/ack/unsub, 10 s subscribe, 30 s discover) still hard-bound mailbox suspension even with retries enabled; the Discover site's ConfigureAwait analysis (invoker-internal `ConfigureAwait(false)` does not leak to the caller's actor-context resume) is correct and covered by a real-`CapabilityInvoker` actor-context test; pipeline order Timeout→Retry→Breaker means the tier timeout bounds the *total* including retries. The `TwoNodeClusterHarness.HardKillNodeAAsync` design (transport shutdown as crash sim, stable-streak takeover polling, diagnostic timeout message naming the inactive-resolver signature) is a model integration test.

### 2. Performance

**P1 — HIGH: Any structural deploy change triggers a full address-space teardown/rebuild that severs every client's monitored items server-wide.**
Unchanged (`AddressSpaceApplier.cs:134-154`; `OtOpcUaNodeManager.cs:1690-1736`): any added/removed equipment, alarm, tag, or virtual tag forces `RebuildAddressSpace()`; recreated nodes are new `NodeState` instances, so existing subscriptions go dead. The F10b surgical path covers only attribute edits and renames. Adding one tag still drops every subscription on a production SCADA server. **The declared prerequisite (U2 reflection forwarding guard) is now merged — the surgical pure-adds work is unblocked but not started.**
*Recommendation:* extend the surgical path to pure-add deltas first (`EnsureFolder`/`EnsureVariable` are already idempotent; `RaiseNodesAddedModelChange` exists), then scope removals per-equipment. Still the highest-leverage item in the subsystem.

**P2 — MEDIUM: The value hot path is one global lock acquisition + one actor message per value, with no batching.**
Unchanged: driver child `Tell` → `ForwardToMux` → `OpcUaPublishActor` → `sink.WriteValue` taking the global node-manager `Lock` per value (`OtOpcUaNodeManager.cs:266-281`). Same unblocked-by-U2, not-started status as P1.
*Recommendation:* batched `WriteValues(IReadOnlyList<…>)` sink call — the reflection guard now auto-covers the new member's forwarding.

**P3 — MEDIUM: HistoryRead-Events has no paging and translates `NumValuesPerNode == 0` to `int.MaxValue`** (`OtOpcUaNodeManager.cs:1944-1954`; no continuation points `:1914-1921`). Unchanged.

**P4 — LOW: Every deploy re-runs the four `Materialise*` passes over the full composition** (`OpcUaPublishActor.cs:338-354`) — acceptable (early-return on existing ids); fold into P2 if it profiles hot. Positive: Raw-paging tie-cluster over-fetch stays bounded; `HandleDiscoveredNodes` unchanged-plan short-circuit preserved. New-code note: the resilience pipeline adds one `GetOrCreate` dictionary hit + options-snapshot closure per capability call on *dispatch* paths only (write/ack/subscribe/discover) — the per-value publish hot path is untouched by #10, which is the right call.

### 3. Conventions

**C1 — MEDIUM: The `ServerHistorian` section is bound imperatively in five places with no `IOptions` registration.**
Unchanged: `Program.cs:121`, `AddServerHistorian`/`AddAlarmHistorian`/`AddHistorianProvisioning` (`Runtime/ServiceCollectionExtensions.cs:132, 168` et al.), `OtOpcUaServerHostedService.cs:89`.
*Recommendation:* one `AddValidatedOptions<ServerHistorianOptions>` + `IOptions<>` everywhere, matching the existing `OpcUa`/`Ldap` pattern.

**C2 — MEDIUM: Options validation is two-tier and the tiers disagree.**
Unchanged: `OpcUa`/`Security:Ldap` get fail-fast `ValidateOnStart` validators; `ServerHistorian`/`ContinuousHistorization`/`AlarmHistorian` get advisory warning lists; `DevStubMode=true` is merely log-warned in production (`OtOpcUaLdapAuthService.cs:79-88`).
*Recommendation:* promote the historian sections to the validator pattern; environment-gate `DevStubMode`.

**C3 — LOW: Library code logs via the static Serilog logger** (`Runtime/ServiceCollectionExtensions.cs:90, 100, 136`) and the node manager keeps six `#pragma warning disable CS0618` `Utils.LogError` sites. Unchanged — though notably the *new* resilience registrations do it right (`ILoggerFactory`-created categorized loggers, `DriverFactoryBootstrap.cs:58-70`), so the island isn't growing.

**C4 — LOW: `WithOtOpcUaRuntimeActors` service-locator fallbacks are inconsistently warned.** `resolver.GetService<IHistorianProvisioning>()` (`Runtime/ServiceCollectionExtensions.cs:303`) still has no missing-registration warning despite being the seam that shipped dormant once. The new `IDriverCapabilityInvokerFactory` resolve (`:219-220`) is likewise silent-fallback, but is the better-defended instance of the pattern: the fallback is *deliberate* for admin-only nodes, and `ResilienceInvokerFactoryRegistrationTests` pins the driver-node bind to the real factory — the guard the provisioning seam still lacks.

**Positive conventions notes:** the layering discipline held under pressure — #10's filed plan (thread `CapabilityInvoker` directly into Runtime) would have broken the Polly-free boundary, and the shipped seam (`IDriverCapabilityInvoker`/`IDriverCapabilityInvokerFactory` in Core.Abstractions, mirroring `IDriverFactory`, Null-object defaults, DI-resolved with pass-through fallback) is exactly this codebase's established pattern; the Runtime csproj still references only `Core.Abstractions`. The corrected S1 docs are a conventions win too: `BuildClusterOptions`'s XML comment and `akka.conf` now state precisely *why* the typed option exists, what activates SBR, the keep-oldest-for-2-nodes rationale, and the HOCON/typed-option consistency constraint. Layer inventory unchanged otherwise: Akka-free `OpcUaServer`, SDK-free `Runtime`, pure calculators in Core, uniform Props factories + registry keys, scoped-mapper handling in the singleton authenticator.

### 4. Underdeveloped areas

**U3 — MEDIUM: The structured audit pipeline is fully built, tested, and has zero producers.** Unchanged (`ControlPlane/Audit/AuditOutcomeMapper.cs:12-18`; `Security/Audit/AuditActor.cs:18-24`). Wire producers or delete.

**U4 — MEDIUM: `FleetStatusBroadcaster.DriverHostStatusHeartbeat` is dead code with a latent NodeId-mismatch repeat.** Unchanged (`FleetStatusBroadcaster.cs:38, 110-121`; host-only key `:152-159` vs `host:port` everywhere else).

**U5 — MEDIUM: Native Part 9 conditions support Acknowledge-to-driver only.** Unchanged (`OtOpcUaNodeManager.cs:647-658`; Enable/Disable `BadNotSupported` `:698-703`). Shelving a Galaxy alarm from a Part 9 client remains silently ineffective upstream.

**U6 — MEDIUM: Test-coverage gaps still concentrate on the fragile seams — though the two worst are now closed.**
Closed since 9cad9ed0: **hard-kill failover** (`HardKillFailoverTests`, in-process 2-node, CI-runnable — the single biggest prior blind spot) and **green-because-skipped** (whole-solution CI leg + `assert-not-all-skipped.sh` fail-on-skip gate). Still open, in priority order:
- **DPS delivery of redundancy/ServiceLevel state is untested** — `RedundancyStateActorTests` stub the broadcast; `ServiceLevelEndToEndTests` inject state directly; the harness (even hard-kill) asserts cluster membership, not that `RedundancyStateChanged` *arrives* and flips ServiceLevel/gates on the survivor. The known "unit tests can't catch it" blind spot survives the failover test.
- **No actor supervision/restart test** anywhere (S6).
- **Outbox durability across process restart** and recorder **restart-convergence** (the U1 residual) untested.
- `OpcUaServer.IntegrationTests` still contains **one test** (`DualEndpointTests`) — no over-the-wire security-mode matrix, subscription, or HistoryRead round-trip.
- **Resilience wiring behavioral coverage is 2 of 6 sites** (`DriverInstanceActorResilienceWiringTests`: Subscribe + Write-with-resolved-host). The analyzer proves the other four *are wrapped* but not that they pass the right `DriverCapability`/host key — a transposed enum member at the ack or alarm-subscribe site would compile, pass the analyzer, and ship.

**U7 — LOW:** `IHistoryWriter` permanently Null-wired; H2 HistoryUpdate unimplemented (`OtOpcUaNodeManager.cs:1797-1801`); `LdapAuthFailure` lacks `DirectoryUnavailable`. Tracked backlog, unchanged.

**U8 — LOW: `BuildSecurityPolicies` doc-comment claims the empty-profile fallback is "logged and very visible" but the method logs nothing** (`OpcUaApplicationHost.cs:376-418`). Unchanged.

**U9 — LOW: `EnsureVariable` silently ignores changed historize-intent on an existing node** (`OtOpcUaNodeManager.cs:1345-1349`) — invariant still lives in two files' comments rather than an assert.

**U10 — LOW (NEW): The `DriverResilienceStatusTracker` has no reader — pipeline observability is log-only.** — ✅ **REMEDIATED** (R2-10, branch `r2/10-resilience-observability`). The tracker now has a production reader: a periodic (5 s) full-snapshot publisher (`DriverResilienceStatusPublisherService`, driver nodes, registered next to the tracker with a wiring guard `ResilienceStatusReaderRegistrationTests`) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → a resilience section on `DriverStatusPanel.razor` (breaker-OPEN / consecutive-failures / in-flight / staleness chips, read in-process per the self-HubConnection ban). Two tracker truthfulness gaps fixed along the way: `RecordBreakerClose` + `IsBreakerOpen` (breaker-open state is now derivable) and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is now a true consecutive counter). Mirrors the driver-health flow one-for-one. *(Original finding:)* The tracker was registered, fed by every retry/breaker event, and consumed by nothing (Admin `/hosts` Stream E.2/E.3 never shipped); the interim mitigation was retry/breaker logging in `DriverResiliencePipelineBuilder.cs`.
*Live gate (T10) pending:* docker-dev breaker-open observation on both centrals + staleness dim.

---

## Maturity ratings

| Dimension | 2026-07-08 | 2026-07-12 | Justification |
|---|---|---|---|
| Stability | **3** | **3** | The prior Critical dissolved (S1's premise was wrong — SBR was always active — and hard-kill failover is now integration-proven), which removes the cluster-wide failure mode from the ledger. But the three untouched Highs (S2 LDAP thread-stall, S3 HistoryRead pool exhaustion, S4 dual-primary boot window) are exactly the realistic production exposures the band describes, and the new S12 idempotence bypass adds a loaded footgun. Better-evidenced, not yet safer where it counts. |
| Performance | **3** | **3** | Nothing on the hot paths changed. P1 (rebuild severs all subscriptions) and P2 (per-value global lock) remain; their prerequisite guard merged, so the score is now held down purely by unstarted work. The resilience pipeline correctly stayed off the per-value publish path. |
| Conventions | **4** | **4** | The new code is the pattern at its best (Polly-free seam mirroring `IDriverFactory`, Null-object + DI-guard, corrected load-bearing docs), and the static-logger island stopped growing — but C1/C2/C3/C4 themselves are all still open, so the score holds rather than rises. |
| Underdeveloped | **3** | **4** | The two systemic traps are retired with durable guards (U2 reflection forwarding test closes the F10b/PR#423 class; fail-on-skip CI closes green-because-skipped), U1's doc drift is fixed, the once-dormant CapabilityInvoker is wired end-to-end with analyzer + DI + behavioral guards, and the worst coverage blind spot (hard-kill failover) is tested. Remaining: dormant audit/heartbeat code (U3/U4), the DPS-delivery and supervision test gaps, and the reader-less tracker (U10). |

---

## Cross-cutting themes

1. **The review's own premises need negative controls too.** S1 was filed as Critical on a correct-looking static read (no `downing-provider-class` anywhere) that missed a framework default; only the #9 negative control (force `downing-provider-class = ""`) exposed it. The corrected framing is now pinned in code comments, `akka.conf`, and a test that guards the *outcome* regardless of activation path — the right durable form. Treat "config X is inert" findings as unproven until a negative control flips the observed behavior.
2. **The Deferred/Null seam pattern's trap class is now guarded, not just known.** The reflection-exhaustive forwarding test (U2) auto-covers future sink/publisher members via `DispatchProxy` plus a guard-of-the-guard on the interface set — new capabilities (P1 surgical adds, P2 `WriteValues`) can no longer ship inert through the wrappers. The same "built-but-never-wired" failure mode got a second guard at the DI layer (`ResilienceInvokerFactoryRegistrationTests`). The remaining unguarded instance of the pattern is the silent `IHistorianProvisioning` fallback (C4).
3. **"Default-allow until told otherwise" gates remain the compounding risk.** S4 (primary write gate) + at-most-once DPS delivery + the untested delivery path (U6) still stack; the SBR work hardened membership convergence but none of the delivery-dependent gates. S12 joins this family: `isIdempotent: true` is a default-permissive stance on a safety contract, dormant only because a *different* default (RetryCount=0) currently masks it.
4. **Unit-test greenness overstating distributed correctness is being paid down in the right order** — hard-kill failover (done), fail-on-skip (done), DPS delivery / supervision restarts / outbox durability (still open, now the top of the list). The pattern to keep: every guard shipped with a verified negative control.