58 KiB
Design + Implementation Plan — R2-04 Failure-Visibility Trio
- Source reports:
archreview/01-core-composition.md§S-1 ·archreview/06-gateway-integrations.md§S-1 ·archreview/03-server-runtime.md§S4 (2026-07-12 re-review, working-tree versions) - Round-1 designs consumed:
plans/01-core-composition-plan.md§1 (S-1) ·plans/06-gateway-integrations-plan.md§P0-A (S-1) ·plans/03-server-runtime-plan.md§S4 - Plan verified against tree at: master
f6eaa267(clean) — every anchor re-opened and re-checked 2026-07-12 - Scope: OVERALL prioritized action item #4 — "Failure-visibility trio:
AddressSpaceApplyOutcomefailure field; Galaxy fail-closed writes; primary-gate default-deny" (01 S-1, 06 S-1, 03 S4). Carried from round 1; no branch has ever targeted these. - Cross-cutting requirement (OVERALL theme 3, "optimistic success"): every swallowed failure must surface somewhere operator-visible. Concrete pick per finding: 01/S-1 → Error-level Serilog + a new dedicated meter (
otopcua.opcua.apply.failed); 06/S-1 → Bad write status that feeds the existing #5 node-revert + Part 8 audit event, plus two new Galaxy meters (galaxy.writes.advise_failed/galaxy.writes.unconfirmed); 03/S4 → the rejected write rides the existing optimistic-write revert + Bad-blip +AuditWriteUpdateEventsurface, plus a new denial meter (otopcua.redundancy.primary_gate_denied) and Warning logs.
Verification summary
All three findings CONFIRMED at f6eaa267 — none stale, none partially fixed. Anchor drift status:
| Finding | Report anchor | Status at f6eaa267 |
|---|---|---|
| 01/S-1 | AddressSpaceApplier.cs:148-153 (rebuilt=true after SafeRebuild) |
Minor drift: the structural block is now :150-154 (SafeRebuild() at :152, rebuilt = true at :153). Report also under-counts: there are three rebuilt-set sites — :150-154 (structural), :191 (surgical fallback if (!allApplied) { SafeRebuild(); rebuilt = true; }), :196-198 (sink lacks surgical capability). All three must carry the failure flag. |
| 01/S-1 | AddressSpaceApplier.cs:373-383 (SafeRebuild), :612-622 (SafeEnsureFolder/Variable), :677-687 (SafeWriteAlarmCondition/SafeMaterialiseAlarmCondition), :691 (outcome record) |
Exact. Record is AddressSpaceApplyOutcome(int RemovedNodes, int AddedNodes, int ChangedNodes, bool RebuildCalled) at :691-695; construction sites :80 (empty plan) and :215 (main return). |
| 01/S-1 consumer | OpcUaPublishActor.cs:335 (Apply), :338-354 (four Materialise passes), :357-358 (Info log only), :360-363 (catch-all) |
Exact (round-1 plan refs still hold). Confirmed no ApplyAck/DeploymentFailed coupling: DriverHostActor.cs:1218-1222 sends ApplyAck(Applied) before Telling RebuildAddressSpace — the deploy seals independently of apply health, exactly as reported. |
| 06/S-1 | GatewayGalaxyDataWriter.cs:163-169 (comment), :234-243 (advise-failure fall-through), :281-302 (TranslateReply) |
Exact (the "value never reaches the galaxy" comment spans :161-166; advise-failure TryRemove+warn+proceed at :234-243; empty-statuses→Good at :295-301). Verified the Bad-status chain the fix relies on: DriverInstanceActor.IsGoodStatus (:950, top-2-bits check) → WriteAttributeResult(false, "StatusCode=0x…") (:607-608) → NodeWriteResult → NodeWriteOutcome(false) → OtOpcUaNodeManager.RevertOptimisticWriteIfNeeded (:842-846, :1048, :1095) — so a Bad return from the writer does fire the #5 revert + audit event today. |
| 06/S-1 test constraint | round-1 plan: "fake session records calls" | INFEASIBLE as written — GatewayGalaxyDataWriterTests.cs:17-99 documents that the MxGatewaySession SDK types are sealed + internal-ctor and cannot be faked; the existing 8 tests are cache-level only (SeedHandleCachesForTest). Resolved below with an internal IMxWriteOps seam. |
| 03/S4 | DriverHostActor.cs:977 (alarm-emit gate), :1039 (write gate), :1095 (ack gate), OnRedundancyStateChanged :1126 |
Exact (re-review refs are current). _localRole field is now at :201 (round-1 plan said :195); the round-1 plan's gate refs (:1018-1026, :1074-1083, :956-984, :1109-1116) drifted +6…+17 lines — use :1035-1043, :1091-1100, :955-1002, :1126-1133. DPS subscribe to redundancy-state confirmed in PreStart (:385-393). The same default-allow pattern also exists in ScriptedAlarmHostActor.cs:314 (scripted-alarm alerts-emit gate, _localRole at :109, OnRedundancyStateChanged :327-338) — not cited by the finding, brought in scope as a consistency extension (same policy helper). |
Supporting facts verified: OtOpcUaTelemetry central meter (Commons/Observability/OtOpcUaTelemetry.cs:19-63, meter name ZB.MOM.WW.OtOpcUa); Galaxy driver meter name ZB.MOM.WW.OtOpcUa.Driver.Galaxy is a public const on EventPump (EventPump.cs:36) while GalaxyTelemetry.cs is ActivitySource-only (no Meter); RuntimeActorTestBase runs a real single-node Akka cluster (self-join, ClusterActorRefProvider, no roles) so Cluster.Get is safe in Runtime.Tests and the "driver"-role member count there is 0; role string "driver" per Cluster/RoleParser.cs:7; existing gate coverage in DriverHostActorWriteRoutingTests.cs asserts the exact reason string "not primary" on a Secondary (:76-102) and asserts a no-snapshot write is serviced (:49-74) — both must keep passing; TwoNodeClusterHarness exists in tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/ for the delivery-path integration test; applier test fixtures RecordingSink/ThrowingSink at AddressSpaceApplierTests.cs:1761/:1906 (ThrowingSink currently only supports throwOnAlarmWrite).
Round-1 open design choices — resolved in this plan
- 01/S-1 failure-tally exposure (round-1 offered out-param vs instance counter): resolved to return
intfrom eachMaterialise*method (option a) — no hidden state, the publish actor sums. - 01/S-1 operator surface: resolved to a new dedicated counter
otopcua.opcua.apply.failedinstead of round-1'sOpcUaSinkWrite+kind:"apply-failed"tag —OpcUaSinkWriteis documented as "writes that landed", and a failure is not a landed write. Plus_log.Error. No deploy abort / no ApplyAck change (confirmed round-1's deferral: the ack is already sent before the rebuild message is even Told; deploy-status coupling belongs to a separate ControlPlane work item). - 01/S-1 record-shape compatibility: new fields are defaulted trailing parameters (
bool RebuildFailed = false, int FailedNodes = 0) so both construction sites and every existing test compile unchanged. - 06/S-1 status choice: confirmed round-1's
BadCommunicationError(Bad, not Uncertain) on advise failure — verified above that Bad feeds the #5 revert so the phantom-Good node is corrected. Empty-statuses staysGood+ metered (round-1's interim), because whether the gateway guarantees a status row on success is a gateway-contract question — deferred to the cross-repoOnWriteCompletecorrelation follow-up (EventPump.cs:200-206filters that event family out today). - 06/S-1 testability: round-1's fake-session test is impossible (sealed SDK types) — resolved with an internal
IMxWriteOpsseam (production adapter over(MxGatewaySession, serverHandle); tests inject a fake). Advise failure is judged on protocol status only (current semantics preserved; interpreting MX status rows on an advise reply risks fail-closing healthy writes on an unverified contract — noted for the cross-repo follow-up). - 03/S4 grace-window semantics (reject vs queue vs reject-with-BadNotWritable): resolved to reject. Queueing is rejected outright: an operator write to an OT field device executed seconds after the operator issued it is surprise actuation, and OPC UA write semantics are synchronous. "Reject-with-BadNotWritable" does not fit the server's write pipeline:
OnWriteValuereturns optimistic Good and the asyncNodeWriteOutcome(false)fires the sanctioned self-correction surface — node reverts to prior value/status with a transient Bad-quality blip + a Part 8AuditWriteUpdateEvent(OtOpcUaNodeManager.cs:842-846). The boot-window rejection uses a distinct reason string"not primary (role unknown)"so operators can tell a boot-window denial from a steady-state Secondary denial. - 03/S4 membership signal: confirmed round-1's cluster-member-count default (single-driver cluster ⇒ allow; ≥2
driver-role Up members ⇒ deny while role unknown), implemented behind an injectableFunc<int>? driverMemberCountProviderProps parameter (deterministic unit tests; the default readsCluster.Get(Context.System).State.Membersguarded by try/catch → 0 on non-cluster systems, preserving legacy/test harnesses). - 03/S4 gate scope: uniform policy across all three
DriverHostActorsites (write, ack, alarm-emit) andScriptedAlarmHostActor's alerts-emit gate. For the emit gates the trade-off is: deny-on-unknown may drop an alerts fan-out publish during the ≤10 s boot window (bounded by theRedundancyStateActorheartbeat) vs. default-emit produces duplicate cluster-wide alert rows historized durably into AVEVA. Chosen: deny — the OPC UA condition write stays ungated (the node's own data is never lost; only the fleet-wide fan-out defers to the peer that knows it is Primary), and the double-row bug has real history (see memory: redundancy-state delivery was double-broken; single-row/alertswas an explicit verification criterion).
Finding 1 — 01/S-1: Applier swallows every sink failure; deploy reports applied even when the address space is broken (High)
Restatement
AddressSpaceApplier wraps every sink call in catch-all Safe* helpers that log-and-continue. AddressSpaceApplyOutcome carries only Added/Removed/Changed counts + RebuildCalled — no failure channel. If _sink.RebuildAddressSpace() throws, rebuilt is still reported true and OpcUaPublishActor logs the outcome at Info; the deployment seals (the ApplyAck(Applied) was already sent at DriverHostActor.cs:1218 before the rebuild was even requested) while the running server holds a partially-materialised or entirely stale address space. Per-node EnsureFolder/EnsureVariable/MaterialiseAlarmCondition failures during the four materialise passes vanish into per-node Warnings.
Verification
Confirmed (see summary table). Precisely:
AddressSpaceApplier.cs:691-695— outcome record, no failure fields; constructed at:80(empty plan) and:215(main return).:373-383—SafeRebuildcatches everything, returns nothing. Callers at:152/:191/:196all setrebuilt = trueregardless.:612-616/:618-622—SafeEnsureFolder/SafeEnsureVariablereturnvoid, swallow to Warning.:677-681/:683-687— same for the alarm-condition writes. The fourMaterialise*passes (:393-415,:432-488,:543-580,:592-610) andMaterialiseDiscoveredNodes(:501-528) have no failure output.- Consumer:
OpcUaPublishActor.HandleRebuild(:290-364) —Applyat:335, four materialise calls:338-354, single_log.Infoat:357-358, all inside a catch-all that logs (:360-363). Nothing downstream reads the outcome. - Note: the surgical branch (
:155-199) already logs per-itemLogErrorand falls back to a full rebuild on any surgical failure — a recovered failure. Only an unrecovered rebuild throw / node failure needs to surface.
Root cause
The never-fail-a-deploy posture — correct for the detached hooks (ProvisionHistorizedTags, FeedHistorizedRefs, both deliberately fire-and-forget) — was applied uniformly to the structural materialisation that is the deploy's core contract. The outcome record has no channel to express failure, so the publish actor could not act even if it wanted to.
Proposed design
Give the applier a truthful failure surface and route it to Error-level Serilog + a dedicated meter. Visibility-only in v1: no deploy abort, no ApplyAck change (the ack has already been sent by the time the publish actor runs; converting apply health into deploy status is ControlPlane-scoped follow-on work). A partially-materialised address space is still better than none; the goal is that an operator can see it happened.
Outcome shape (exact):
/// <summary>Summary of one apply pass. Useful for tests + audit-log entries on the deploy path.
/// RebuildCalled means a rebuild was ATTEMPTED; RebuildFailed means the attempt threw (the sink's
/// address space is stale/partial). FailedNodes counts swallowed per-node sink failures in Apply's
/// own passes (removal condition writes). The Materialise* passes report their own failed-node
/// tallies via their int returns — the publish actor sums.</summary>
public sealed record AddressSpaceApplyOutcome(
int RemovedNodes,
int AddedNodes,
int ChangedNodes,
bool RebuildCalled,
bool RebuildFailed = false,
int FailedNodes = 0);
Defaulted trailing params keep :80, :215, and every existing test construction compiling.
Applier changes:
SafeRebuild()→private bool SafeRebuild()returningfalseon catch (log staysLogError). All three call sites thread the flag:if (structuralRebuild) { rebuildFailed = !SafeRebuild(); rebuilt = true; }(:150-154), and the same at:191and:196-198.SafeEnsureFolder/SafeEnsureVariable/SafeWriteAlarmCondition/SafeMaterialiseAlarmCondition→ returnbool(false on catch; keep Warning logs).Applytallies its own removal-passSafeWriteAlarmConditionfailures intoFailedNodesand returnsnew AddressSpaceApplyOutcome(removedCount, addedCount, changedCount, rebuilt, rebuildFailed, failedNodes).- The five materialise methods change
void→int(count of failed sink calls in that pass). Source-compatible for any caller that ignores the return.
Publish-actor surfacing (OpcUaPublishActor.HandleRebuild):
var outcome = _applier.Apply(plan);
_lastApplied = composition;
var failedNodes = outcome.FailedNodes;
failedNodes += _applier.MaterialiseHierarchy(composition);
failedNodes += _applier.MaterialiseScriptedAlarms(composition);
failedNodes += _applier.MaterialiseEquipmentTags(composition);
failedNodes += _applier.MaterialiseEquipmentVirtualTags(composition);
OtOpcUaTelemetry.OpcUaSinkWrite.Add(1, new KeyValuePair<string, object?>("kind", "rebuild"));
if (outcome.RebuildFailed || failedNodes > 0)
{
OtOpcUaTelemetry.OpcUaApplyFailed.Add(1,
new KeyValuePair<string, object?>("kind", outcome.RebuildFailed ? "rebuild" : "nodes"));
_log.Error(
"OpcUaPublish: address-space apply DEGRADED (correlation={Correlation}, rebuildFailed={RebuildFailed}, failedNodes={FailedNodes}, added={Added}, removed={Removed}, changed={Changed}) — the running address space may be stale or partial",
msg.Correlation, outcome.RebuildFailed, failedNodes,
outcome.AddedNodes, outcome.RemovedNodes, outcome.ChangedNodes);
}
else
{
_log.Info("OpcUaPublish: applied rebuild (correlation={Correlation}, added={Added}, removed={Removed}, changed={Changed}, rebuild={Rebuild})",
msg.Correlation, outcome.AddedNodes, outcome.RemovedNodes, outcome.ChangedNodes, outcome.RebuildCalled);
}
HandleMaterialiseDiscovered gets the same treatment for its single pass (Error + meter when the returned count > 0).
New instrument (OtOpcUaTelemetry):
/// <summary>Address-space apply passes that swallowed at least one sink failure (kind=rebuild|nodes).</summary>
public static readonly Counter<long> OpcUaApplyFailed =
Meter.CreateCounter<long>("otopcua.opcua.apply.failed", unit: "{apply}",
description: "Apply/materialise passes with swallowed sink failures (kind=rebuild|nodes).");
Alternatives considered (carried + re-affirmed from round 1):
- Throw from
Applyon rebuild failure — rejected: the actor's catch (:360-363) would swallow it, it aborts the still-viableMaterialise*passes, and it loses the counts. - Full ApplyAck→DeploymentFailed wiring — rejected for this pass: the ack is sent before the rebuild message (
DriverHostActor.cs:1218-1222); re-sequencing deploy acknowledgment around apply health is a ControlPlane design change with its own blast radius. Tracked as a follow-on note, not here. - Overload
OpcUaSinkWritewithkind:"apply-failed"— rejected in favor of a dedicated counter (semantic purity; a dashboard alerting on apply failures shouldn't parse a tag off a success counter). - Alert row on
/alerts— rejected: the alerts topic is the alarm fan-out and is historized into AVEVA byHistorianAdapterActor; deploy-infrastructure failures would pollute durable alarm history.
DeferredAddressSpaceSink interplay: no new IOpcUaAddressSpaceSink members are added (only the applier's own types change), so the reflection forwarding guard (DeferredSinkForwardingReflectionTests) is unaffected. Note for the record: a rebuild routed through a deferred sink before the inner sink attaches no-ops silently rather than throwing — that pre-attach window is startup-scoped and out of scope here.
Implementation steps
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs— record +Safe*bool returns + three rebuilt sites + removal-pass tally +Materialise*/MaterialiseDiscoveredNodesint returns.src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs—OpcUaApplyFailedcounter.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs— sum + branch Error/meter inHandleRebuildandHandleMaterialiseDiscovered.
Tests
tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs(new; extendThrowingSinkwiththrowOnRebuild/throwOnEnsureVariableknobs or a local fixture):- rebuild throws ⇒
RebuildCalled == true,RebuildFailed == true; - N
EnsureVariablethrows inMaterialiseEquipmentTags⇒ return == N; - happy path ⇒
RebuildFailed == false,FailedNodes == 0, allMaterialise*return 0 (regression); - removal-pass
WriteAlarmConditionthrow ⇒FailedNodescounts it (upgrade of the existing:105ThrowingSink case).
- rebuild throws ⇒
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs(new, TestKit): wire a throwing sink + real applier throughPropsForTests, driveRebuildAddressSpace, assert viaEventFilter.Error(...)that the DEGRADED line fires, and viaMeterListenerthatotopcua.opcua.apply.failedincremented; assert the happy path stays Info-only.- Full existing suites stay green:
AddressSpaceApplierTests(construction sites untouched thanks to defaulted params).
Effort + Risk
Effort: M. Risk: medium — public record + five public method signatures change (void→int is source-compatible; the record change is compile-guarded by defaults). Blast radius: OpcUaServer + Runtime + their tests. Classified high-risk (data-contract) in the task breakdown; land first and run the two full test projects.
Finding 2 — 06/S-1: Galaxy write success is optimistic; a committed-write failure can never surface (High)
Restatement
GatewayGalaxyDataWriter.TranslateReply defaults to Good when the reply's statuses array is empty, and the gateway's write execution is fire-and-forget past dispatch (the reply reflects command acceptance, not the COM-side commit). Worse: when AdviseSupervisory fails, the code logs a warning and issues the raw Write anyway — while the file's own comment (:161-166) states a non-advised raw Write "doesn't throw (reply looks OK) but the value never reaches the galaxy". That is a knowingly-lost write returned as Good to the OPC UA client, and because the status is Good, the #5 write-outcome self-correction (master 1d797c1c) structurally never fires for Galaxy — the node keeps a phantom-Good value indefinitely.
Verification
Confirmed exact at f6eaa267 (see summary). Key chain fact re-verified: a Bad status returned from the writer does propagate — GalaxyDriver.WriteAsync (GalaxyDriver.cs:785-803) returns the writer's results verbatim → DriverInstanceActor.HandleWriteAsync (:602-608) maps non-Good to WriteAttributeResult(false, "StatusCode=0x…") → DriverHostActor.HandleRouteNodeWrite → ActorNodeWriteGateway → NodeWriteOutcome(false) → OtOpcUaNodeManager reverts the optimistic node value and raises the Part 8 audit event. So making the advise-failure path return Bad is sufficient to activate the entire existing self-correction surface for this failure class — no new plumbing.
Test-seam constraint verified: MxGatewaySession (SDK package ZB.MOM.WW.MxGateway.Client) is sealed with internal ctors; the existing 8 GatewayGalaxyDataWriterTests never execute WriteOneAsync. The round-1 "fake session" test design cannot be implemented as written.
Root cause
Two independent optimistic defaults: (1) empty Statuses treated as success although it only proves worker-side command acceptance; (2) the supervisory-advise precondition is best-effort — on failure the code still issues a Write it knows cannot commit and reports the (empty/OK) reply as Good.
Proposed design
Three in-repo layers (fail-closed + metered), plus a tracked cross-repo follow-up:
- Fail-closed on advise failure.
EnsureSupervisoryAdvisedAsync→Task<bool>(false on non-OK protocol status; theTryRemove+ warning stay). InWriteOneAsync, when a non-secured write's advise returns false: do not issue the raw Write; incrementWriteAdviseFailed; returnnew WriteResult(StatusCodeMap.BadCommunicationError). Rationale forBadCommunicationErroroverUncertain: it is already this writer's "couldn't reach the device" status (:184,:290), and only a Bad status triggers the #5 revert, killing the phantom-Good node. Advise failure is judged on protocol status only — matching current semantics; whether the worker emits MX status rows forAdviseSupervisoryis an unverified gateway contract, and misreading it would fail-close healthy writes. - Meter the empty-statuses Good.
TranslateReplykeeps returningGoodon an empty statuses array (changing it to Uncertain needs the gateway to guarantee a status row on success — a contract question) but incrementsWriteUnconfirmedand the XML doc gains a "provisional pending WriteComplete correlation" note. This makes the unconfirmed-write rate operator-visible today. - Counters. New static Meter + counters in the driver (meter name reuses the pump's so one host listener catches both):
// GatewayGalaxyDataWriter.cs (or GalaxyTelemetry.cs — same assembly)
private static readonly Meter WriterMeter = new(EventPump.MeterName); // "ZB.MOM.WW.OtOpcUa.Driver.Galaxy"
private static readonly Counter<long> WriteAdviseFailed =
WriterMeter.CreateCounter<long>("galaxy.writes.advise_failed", unit: "{write}",
description: "Writes short-circuited to Bad because AdviseSupervisory failed (value could not have committed).");
private static readonly Counter<long> WriteUnconfirmed =
WriterMeter.CreateCounter<long>("galaxy.writes.unconfirmed", unit: "{write}",
description: "Writes reported Good off an EMPTY gateway statuses array (command accepted; commit unconfirmed).");
- Testability seam (replaces round-1's infeasible fake session). Extract the per-write gateway operations behind an internal interface; production adapts the real session, tests inject a fake:
internal interface IMxWriteOps
{
string SessionId { get; }
int ServerHandle { get; }
Task<int> AddItemAsync(string fullRef, CancellationToken ct);
Task<MxCommandReply> InvokeAsync(MxCommandRequest request, CancellationToken ct);
Task<MxCommandReply> WriteRawAsync(int itemHandle, MxValue value, int userId, CancellationToken ct);
}
WriteAsync builds a SessionMxWriteOps(session, serverHandle) once per batch; WriteOneAsync / EnsureItemHandleAsync / EnsureSupervisoryAdvisedAsync / InvokeWriteSecuredAsync take IMxWriteOps instead of (MxGatewaySession, int). Add internal Task<WriteResult> WriteOneForTestAsync(IMxWriteOps ops, WriteRequest request, SecurityClassification classification, CancellationToken ct) so tests drive the real pipeline. Behavior-preserving refactor; the public IGalaxyDataWriter surface is untouched.
Cross-repo (mxaccessgw) follow-up — the real fix, out of this plan's scope: gateway-side WriteComplete correlation so the unary reply's Statuses row carries the actual COM commit outcome (the OnWriteComplete event family already exists in the proto and is filtered out at EventPump.cs:200-206). File on the MxAccessGateway backlog; link from the in-source comment at GatewayGalaxyDataWriter.cs:161-166. Until then, layer 2's meter is the honest signal.
Alternatives considered: return Uncertain on advise failure (report's option a) — rejected: Uncertain does not fire the #5 revert, leaving the phantom-Good node; Bad is strictly more truthful ("this write did not reach the device"). Fail TranslateReply empty-statuses to Bad now — rejected: would fail-close every Galaxy write if the gateway legitimately returns empty statuses on success (contract unverified); metered-Good is the safe interim. Public seam / new abstraction project — rejected: internal + InternalsVisibleTo (already in place for the existing internal test seams) is sufficient.
Implementation steps
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs—IMxWriteOps+SessionMxWriteOpsadapter (same file, internal), thread through the four private methods,EnsureSupervisoryAdvisedAsync→Task<bool>, short-circuit branch, counters,TranslateReplymeter + doc note,WriteOneForTestAsyncseam.- Update the comment block at
:161-166to state the new fail-closed behavior + cross-repo link. docs/drivers/Galaxy notes (if the driver doc mentions write semantics) — one paragraph on fail-closed advise + the unconfirmed meter.
Tests
New tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterFailClosedTests.cs (fake IMxWriteOps records calls):
- advise reply carries non-OK protocol status ⇒ result is
BadCommunicationError,WriteRawAsyncwas never invoked,galaxy.writes.advise_failedincremented (MeterListener), supervised-handle cache does NOT retain the handle; - advise OK + write reply with empty statuses ⇒
Goodreturned ANDgalaxy.writes.unconfirmedincremented; - advise OK + write reply with non-OK protocol status ⇒
BadCommunicationError(regression); - advise OK + write reply with a Bad MX status row ⇒ mapped via
StatusCodeMap.FromMxStatus(regression); - secured-write path (
SecuredWriteclassification) never calls advise (regression); - second write to the same handle after a successful advise skips the advise round-trip (cache regression, now assertable through the fake).
Existing 8 cache-level tests stay green (--filter "FullyQualifiedName~GatewayGalaxyDataWriterTests"). Live gateway smoke: extend the skip-gated GatewayGalaxyLiveReopenAndWriteTests with an assertion that a normal live write still returns Good (guard against over-eager fail-closing); recipe unchanged:
KEY=$(docker exec otopcua-dev-central-1-1 printenv GALAXY_MXGW_API_KEY)
MXGW_ENDPOINT=http://10.100.0.48:5120 GALAXY_MXGW_API_KEY="$KEY" \
dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests \
--filter FullyQualifiedName~GatewayGalaxyLiveReopenAndWrite
Effort + Risk
Effort: S-M (the seam refactor is the bulk; the behavior change is ~15 lines). Risk: low-medium — the only behavior change is advise-failed writes returning Bad instead of phantom-Good (strictly more correct, and it enables the #5 revert); the seam refactor is mechanical and covered by the new pipeline tests. Live-verify normal writes still Good before merging.
Finding 3 — 03/S4: Primary gate defaults to allow while role unknown — dual-primary data-plane window (High)
Restatement
DriverHostActor gates inbound operator writes (:1039), native alarm acks (:1095), and the native-alarm alerts publish (:977) on _localRole is Secondary or Detached — null (no snapshot yet) falls through to allow. A freshly-booted secondary on a multi-node cluster services shared-field-device writes and emits fleet-wide alerts as if it were Primary for up to the 10 s RedundancyStateActor heartbeat — and indefinitely if the snapshot's NodeId never matches this node (the 03/S5 identity-mismatch shape; OnRedundancyStateChanged at :1126-1133 leaves the role unchanged when the snapshot omits this node). Compounds with at-most-once DPS delivery. This delivery path has real history: it was double-broken once (NodeId host:port mismatch + late-subscriber missing the heartbeat) and was invisible to unit tests — the fix must carry a delivery-level verification, not just TestKit guards.
Verification
Confirmed exact (see summary table). Additional facts: DPS subscribe to redundancy-state in PreStart (:385-393); the boot-window rationale is written into the _localRole doc (:193-201): "default-allow while unknown so single-node deploys + the boot window never reject". Existing tests pin behavior: DriverHostActorWriteRoutingTests — Secondary write rejected with reason exactly "not primary" (:76-102), and a write before any snapshot is serviced (:49-74, single-node test cluster). The same default-allow gate exists in ScriptedAlarmHostActor.cs:314 (alerts-emit for scripted alarms). RuntimeActorTestBase runs a real single-node cluster with no roles, so a default member-count of "Up members with role driver" evaluates to 0 there ⇒ the existing no-snapshot test keeps passing untouched.
Root cause
The gate uses the role signal to gate shared-device writes, and the role signal is under-determined at boot: null cannot distinguish "single node — no snapshot will ever demote me" from "multi-node — my snapshot just hasn't arrived". The correct discriminator (cluster membership) is already available in-process and is authoritative much earlier than DPS delivery.
Proposed design
Cluster-membership-aware default-deny, one policy in one place:
// src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs (new)
/// <summary>Single decision point for the Primary data-plane gates (inbound device writes, native
/// alarm acks, fleet-wide alerts publishes). Known role wins outright; an UNKNOWN role (no snapshot
/// yet, or the snapshot never mentioned this node) is resolved by cluster membership: a single-driver
/// cluster stays default-allow (no peer exists — the boot-window/single-node posture), a multi-driver
/// cluster is default-DENY (a real Primary peer exists; do not touch the shared field device until
/// the redundancy snapshot proves this node is it).</summary>
public static class PrimaryGatePolicy
{
public static bool ShouldServiceAsPrimary(RedundancyRole? localRole, int driverMemberCount) =>
localRole switch
{
RedundancyRole.Primary => true,
RedundancyRole.Secondary or RedundancyRole.Detached => false,
_ => driverMemberCount <= 1, // unknown: allow only when no driver peer exists
};
}
DriverHostActor wiring:
- New optional Props/ctor parameter
Func<int>? driverMemberCountProvider = null(test seam, mirrors the existing override-parameter idiom). Default implementation:
private int DriverMemberCount()
{
if (_driverMemberCountProvider is not null) return _driverMemberCountProvider();
try
{
return Akka.Cluster.Cluster.Get(Context.System).State.Members
.Count(m => m.Status == MemberStatus.Up && m.Roles.Contains("driver"));
}
catch (Exception) { return 0; } // non-cluster ActorRefProvider (legacy harnesses) ⇒ single-node posture
}
private bool ShouldServiceAsPrimary() => PrimaryGatePolicy.ShouldServiceAsPrimary(_localRole, DriverMemberCount());
- All three gate sites route through
ShouldServiceAsPrimary(); reasons + visibility:HandleRouteNodeWrite(:1035-1043): on deny, replyNodeWriteResult(false, _localRole is null ? "not primary (role unknown)" : "not primary")— the existing exact-string test keeps passing, boot-window denials are distinguishable — and increment the denial meter (site:"write",reason:"role-unknown"|"secondary"|"detached"). The rejection then rides the established surface:ActorNodeWriteGatewayWarning log (:60) →NodeWriteOutcome(false)→ node revert + Bad blip +AuditWriteUpdateEvent. No queueing (see resolved choice #6).HandleRouteNativeAlarmAck(:1091-1100): on deny, drop as today but elevate the role-unknown case from Debug to Warning (the operator's ack silently not reaching the upstream alarm system is precisely a swallowed failure) + meter (site:"ack").ForwardNativeAlarm(:955-1002): computeShouldServiceAsPrimary()once before the nodeIds loop; on denycontinueas today + meter (site:"alarm-emit", Debug log). The OPC UA condition write above the gate stays UNGATED (warm standby keeps its address space current — no local data loss; only the fleet-wide duplicate publish is suppressed).
- S5 hook (log-once): in
OnRedundancyStateChanged, when the snapshot does not mention this node andDriverMemberCount() > 1, log a one-time Warning naming_localNodeand the snapshot's node ids (the silent-identity-mismatch diagnostic the S5 finding asks for); abool _warnedSnapshotMissingLocalNodefield debounces it.
New instrument (OtOpcUaTelemetry):
/// <summary>Inbound operations denied by the Primary data-plane gate.</summary>
public static readonly Counter<long> PrimaryGateDenied =
Meter.CreateCounter<long>("otopcua.redundancy.primary_gate_denied", unit: "{denial}",
description: "Operations denied by the Primary gate (site=write|ack|alarm-emit, reason=secondary|detached|role-unknown).");
ScriptedAlarmHostActor consistency extension: the emit gate at :314 adopts the same PrimaryGatePolicy + optional driverMemberCountProvider Props param + meter (site:"alarm-emit"). Same trade-off resolution as the native emit gate (resolved choice #8).
Grace-window semantics, explicitly: during boot on a multi-node cluster (role unknown), inbound writes are rejected — the OPC UA client sees the sanctioned optimistic-write self-correction (node reverts to prior value with a transient Bad blip; a Part 8 audit event records the rejected write; the CLI/client's own write call still returned Good synchronously, exactly as any failed device write behaves since #5). Acks are dropped with a Warning. Alerts publishes are skipped (the Primary peer publishes the single fleet copy). The window closes on the first received snapshot (≤ ~10 s heartbeat; typically the 250 ms-debounced initial publish). Writes are not queued and no synchronous BadNotWritable is introduced — both rejected above.
Alternatives considered (round-1 set, re-affirmed): (a) unconditional default-deny — breaks single-node deploys (no snapshot ever arrives to enable writes); rejected. (b) fixed boot grace-timer flipping to deny — fragile, wrongly denies on a healthy multi-node cluster with a slow snapshot, and wrongly allows during the timer on a fast dual-boot; rejected. (c) lease/fencing token — the architecturally complete answer but a redundancy-design change (report notes "no lease or fencing"); out of scope. (d) subscribing to ClusterEvent.IMemberEvent and caching the count — rejected in favor of reading _cluster.State.Members per gate call: the state read is cheap, always fresh, and avoids a new message-handling surface in an already-large actor.
Implementation steps
src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs— new pure policy class.src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs—PrimaryGateDeniedcounter.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs— Props/ctor param,DriverMemberCount(),ShouldServiceAsPrimary(), rewire:977/:1039/:1095, reasons + meters, S5 log-once inOnRedundancyStateChanged, update the_localRolexmldoc (:193-201) and the three gate xmldocs (:1017-1034,:1073-1090,:946-953) to the new default-deny-on-multi-node semantics.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs— same policy at:314+ Props param.docs/Redundancy.md— document the gate semantics (single-node allow / multi-node deny-until-snapshot, what a client sees during the window).
Tests
- Pure:
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/PrimaryGatePolicyTests.cs— the full truth table (Primary/Secondary/Detached/null × count 0,1,2). - Unit (TestKit),
DriverHostActorPrimaryGateTests.cs(new, alongsideDriverHostActorWriteRoutingTests):- role unknown + provider→2 ⇒
RouteNodeWritereplies(false, "not primary (role unknown)")and the recording driver saw no write; - role unknown + provider→1 ⇒ serviced (boot-window/single-node preserved);
- Primary snapshot then write ⇒ serviced; Secondary snapshot ⇒
(false, "not primary")(existing tests already pin this — keep them untouched as the regression net); - role unknown + provider→2 ⇒
RouteNativeAlarmAckdropped (probe on the child sees nothing) and Warning logged (EventFilter); - role unknown + provider→2 ⇒
ForwardNativeAlarmpublishes noAlarmTransitionEventto the alerts topic (DPS probe) while theAlarmStateUpdateto the publish actor still flows (ungated condition write); - meter assertions via
MeterListener(site/reasontags). - Existing
DriverHostActorWriteRoutingTestsall green unchanged (single-node test cluster ⇒ default provider counts 0 driver members ⇒ allow).
- role unknown + provider→2 ⇒
- ScriptedAlarm: mirror emit-gate test with provider→2 in the ScriptedAlarmHostActor suite.
- Integration (in-process 2-node, delivery-path — the "unit tests can't catch delivery" guard): new
PrimaryGateFailoverTests(or extendHardKillFailoverTests' harness usage) intests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTestsonTwoNodeClusterHarness: after both nodes join and snapshots deliver, assert the secondary'sDriverHostActorrejectsRouteNodeWritewith"not primary"while the primary services it — proving the DPS-delivered snapshot (not a test-injected message) actually drives the gate end-to-end. - Live 2-node rig gate (required before trusting the change — see delivery-history note): docker-dev rig, both centrals rebuilt (
:9200round-robins central-1/2 — rebuild BOTH):docker compose -f docker-dev/docker-compose.yml up -d --build central-1 central-2; confirm roles via ServiceLevel — Client.CLIreadofServer_ServiceLevelonopc.tcp://localhost:4840vs:4841shows 240 / 100.docker restart otopcua-dev-central-2-1; during its boot window issue a Client.CLIwriteagainstopc.tcp://localhost:4841to a known writable equipment node (bind anopc-writeop/multi-roleLDAP user; write reads-first per the #5 recipe). Expected: the write does not reach the device — central-2 log showsOperator write to … rejected: not primary (role unknown)(ornot primaryif the snapshot already landed) + the node reverts (Bad blip) — andotopcua.redundancy.primary_gate_deniedincrements.- After ~10 s (snapshot heartbeat), a write against
:4841rejects with plainnot primary, and the same write against:4840(primary) succeeds end-to-end. - Regression: single
/alertsrow per native/scripted alarm transition (no duplicates, no missing rows) with both nodes steady — re-run the alarm-transition recipe from the native-alarms verification.
Effort + Risk
Effort: S/M code + M verification (the live rig gate is the long pole). Risk: medium-high — this changes write-admission policy on multi-node clusters (actor + data-plane behavior). A policy bug could deny legitimate Primary writes during a snapshot gap; mitigations: the deny branch requires a real driver peer (count ≥2), the single-node path is bit-identical to today, the known-role branches are unchanged, and the exact-string regression tests pin existing behavior. The delivery-path history mandates the in-process 2-node test AND the live rig gate before merge.
Task breakdown
Branch: one branch fix/archreview-r2-04-failure-visibility off master, commits grouped per finding (the three findings touch disjoint files; if implementing in parallel, worktree-isolate per the shared-tree/git-index gotcha). Ordering: T1-block (01/S-1) first (data-contract), T2-block (06/S-1) and T3-block (03/S4) parallel after — or fully parallel in worktrees.
Verification bar: every task has a deterministic unit guard; T3 additionally has the wiring/delivery assertions (in-process 2-node) and the live 2-node rig gate. Follow TDD: write the failing test, see it FAIL, implement, see it PASS.
T1 — Extend AddressSpaceApplyOutcome + SafeRebuild failure flag
Classification: HIGH-RISK (public data-contract change on the deploy path)
Estimated implement time: 5 min
Parallelizable with: T5–T13 (different files); NOT T2/T3/T4 (same files)
Files: src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs, tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierFailureSurfaceTests.cs (new)
- Write
AddressSpaceApplierFailureSurfaceTests.Apply_WhenRebuildThrows_ReportsRebuildFailed: applier over a sink whoseRebuildAddressSpacethrows (extendThrowingSinkwiththrowOnRebuild: trueor a local fixture), plan with one added equipment ⇒ assertoutcome.RebuildCalled == true && outcome.RebuildFailed == true; andApply_HappyPath_ReportsNoFailure⇒RebuildFailed == false, FailedNodes == 0. dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~AddressSpaceApplierFailureSurfaceTests"→ FAIL (no such members).- Implement: record gains
bool RebuildFailed = false, int FailedNodes = 0;SafeRebuild()returnsbool; threadrebuildFailedthrough the three sites (:150-154,:191,:196-198); main return passes both fields. - Re-run filter → PASS; then the full existing suite:
dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~AddressSpaceApplierTests"→ PASS (defaulted params keep old constructions compiling). - Commit:
fix(applier): AddressSpaceApplyOutcome carries RebuildFailed/FailedNodes (archreview 01/S-1).
T2 — Safe* helpers return bool; removal-pass tally into FailedNodes
Classification: high-risk (same contract surface) Estimated implement time: 5 min Parallelizable with: T5–T13 Files: same as T1
- Add test
Apply_WhenRemovalConditionWriteThrows_CountsFailedNodes(ThrowingSinkthrowOnAlarmWrite: true, plan with one removed alarm) ⇒outcome.FailedNodes == 1. Run filter → FAIL. - Implement:
SafeWriteAlarmCondition/SafeMaterialiseAlarmCondition/SafeEnsureFolder/SafeEnsureVariable→bool;Applytallies removal-pass failures. - Filter → PASS. Commit:
fix(applier): Safe* helpers report failure; Apply tallies removal-pass failures.
T3 — Materialise* passes return failed-node counts
Classification: medium-risk (public void→int, source-compatible)
Estimated implement time: 5 min
Parallelizable with: T5–T13
Files: same as T1
- Tests:
MaterialiseEquipmentTags_WhenEnsureVariableThrows_ReturnsFailedCount(ThrowingSinkthrowOnEnsureVariable, composition with 3 tags, sink throws on all ⇒ returns 3), plus happy-path-returns-0 for all five methods (Hierarchy, EquipmentTags, EquipmentVirtualTags, ScriptedAlarms, DiscoveredNodes). Run → FAIL (return type). - Implement the five
intreturns (each pass counts itsSafe*falses). - Filter + full applier suites → PASS. Commit:
fix(applier): Materialise* passes return swallowed-failure tallies.
T4 — OpcUaApplyFailed counter + publish-actor surfacing
Classification: medium-risk (actor logging/metric branch; no message-flow change)
Estimated implement time: 5 min
Parallelizable with: T5–T13 (Commons file also touched by T9 — coordinate or sequence)
Files: src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs, src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs, tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyFailureTests.cs (new)
- Test (TestKit + MeterListener + EventFilter): rebuild through a throwing sink ⇒ one
otopcua.opcua.apply.failedincrement + an Error log containingDEGRADED; happy path ⇒ zero increments, Info log. Rundotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~OpcUaPublishActorApplyFailureTests"→ FAIL. - Implement: counter in
OtOpcUaTelemetry;HandleRebuildsumsoutcome.FailedNodes+ four materialise returns, branches Error+meter vs Info (code in Finding 1);HandleMaterialiseDiscoveredsame for its pass. - Filter → PASS; run full
Runtime.Tests. Commit:fix(runtime): surface degraded address-space applies (Error log + otopcua.opcua.apply.failed).
T5 — Galaxy IMxWriteOps seam (behavior-preserving refactor)
Classification: medium-risk (write-path refactor, no behavior change)
Estimated implement time: 5 min
Parallelizable with: T1–T4, T9–T13
Files: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyDataWriter.cs, tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyDataWriterFailClosedTests.cs (new)
- Test-first through the seam:
WriteOne_SecuredClassification_RoutesThroughWriteSecured_NoAdvisewith a fakeIMxWriteOpsreturning an OK reply ⇒ Good,InvokeAsynccalled once withWriteSecured, noAdviseSupervisory. Rundotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests --filter "FullyQualifiedName~GatewayGalaxyDataWriterFailClosedTests"→ FAIL (seam absent). - Implement:
IMxWriteOps+SessionMxWriteOps(internal, same file); thread throughWriteOneAsync/EnsureItemHandleAsync/EnsureSupervisoryAdvisedAsync/InvokeWriteSecuredAsync; addWriteOneForTestAsync. - Filter + existing
GatewayGalaxyDataWriterTests→ PASS. Commit:refactor(galaxy): internal IMxWriteOps seam under GatewayGalaxyDataWriter (sealed SDK session untestable otherwise).
T6 — Galaxy fail-closed on advise failure
Classification: medium-risk (behavior change: knowingly-lost writes now return Bad) Estimated implement time: 5 min Parallelizable with: T1–T4, T9–T13; after T5 Files: same as T5
- Tests:
WriteOne_AdviseFails_ReturnsBadCommunicationError_AndNeverIssuesRawWrite(fake ops: advise reply non-OK protocol status ⇒ assert statusStatusCodeMap.BadCommunicationError,WriteRawAsyncnever called, handle absent from supervised cache);WriteOne_AdviseSucceeds_RawWriteProceeds(regression). Run filter → FAIL. - Implement:
EnsureSupervisoryAdvisedAsync→Task<bool>; short-circuit branch inWriteOneAsync; update the:161-166comment (fail-closed + mxaccessgwWriteCompletefollow-up link). - Filter → PASS. Commit:
fix(galaxy): fail-closed write when AdviseSupervisory fails — Bad status feeds #5 node revert (archreview 06/S-1).
T7 — Galaxy write counters (advise_failed / unconfirmed)
Classification: standard Estimated implement time: 5 min Parallelizable with: T1–T4, T9–T13; after T6 Files: same as T5
- Tests (MeterListener on
ZB.MOM.WW.OtOpcUa.Driver.Galaxy): advise-failure incrementsgalaxy.writes.advise_failed; empty-statuses Good incrementsgalaxy.writes.unconfirmed; a status-row reply increments neither. Run filter → FAIL. - Implement counters (static
MeteronEventPump.MeterName) +TranslateReplyempty-statuses meter + XML "provisional" note. - Filter → PASS. Commit:
feat(galaxy): meter unconfirmed + advise-failed writes.
T8 — Galaxy live smoke guard (skip-gated)
Classification: standard (test-only)
Estimated implement time: 5 min
Parallelizable with: everything except T6/T7 (same test project files)
Files: tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyLiveReopenAndWriteTests.cs
- Add a live case asserting a normal advised write still returns Good post-change (guards against over-eager fail-closing). Skips cleanly without
MXGW_ENDPOINT/GALAXY_MXGW_API_KEY. - Run the skip path locally (
--filter FullyQualifiedName~GatewayGalaxyLiveReopenAndWrite→ skipped); the operator runs it live per the recipe in Finding 2. Commit:test(galaxy): live guard — advised write remains Good under fail-closed advise.
T9 — PrimaryGatePolicy + truth-table tests + denial counter
Classification: standard (pure class + counter)
Estimated implement time: 5 min
Parallelizable with: T1–T8 (coordinate OtOpcUaTelemetry.cs with T4)
Files: src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/PrimaryGatePolicy.cs (new), src/Core/ZB.MOM.WW.OtOpcUa.Commons/Observability/OtOpcUaTelemetry.cs, tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/PrimaryGatePolicyTests.cs (new)
- Truth-table test (Theory: Primary⇒true any count; Secondary/Detached⇒false any count; null⇒true at 0/1, false at 2+).
dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~PrimaryGatePolicyTests"→ FAIL. - Implement policy +
PrimaryGateDeniedcounter. Filter → PASS. Commit:feat(runtime): PrimaryGatePolicy — membership-aware default-deny + denial meter (archreview 03/S4).
T10 — Wire the three DriverHostActor gates through the policy
Classification: HIGH-RISK (actor write-admission policy change on the data plane)
Estimated implement time: 5 min (edit) — verification is T11
Parallelizable with: T1–T8; after T9
Files: src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs
- Add
Func<int>? driverMemberCountProviderProps/ctor param,DriverMemberCount()(try/catch → 0),ShouldServiceAsPrimary(); rewire:977/:1039/:1095with reasons + meters per Finding 3; S5 log-once inOnRedundancyStateChanged; update the four xmldocs. - Regression gate BEFORE new tests:
dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~DriverHostActorWriteRoutingTests"→ PASS unchanged (single-node test cluster ⇒ count 0 ⇒ allow; exact"not primary"string preserved on Secondary). - Commit:
fix(runtime): primary gates default-DENY on multi-driver clusters while role unknown (archreview 03/S4).
T11 — DriverHostActorPrimaryGateTests (TestKit, provider-injected)
Classification: high-risk verification (guards T10)
Estimated implement time: 5 min per case — split across commits if needed
Parallelizable with: T1–T8; after T10
Files: tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorPrimaryGateTests.cs (new)
Cases (each FAIL→PASS against a temporarily-reverted gate is the negative control; run filter FullyQualifiedName~DriverHostActorPrimaryGateTests):
- role unknown + provider→2 ⇒
RouteNodeWrite→(false, "not primary (role unknown)"), recorder driver saw no write. - role unknown + provider→1 ⇒ write serviced.
- Primary snapshot + provider→2 ⇒ serviced; Secondary ⇒
(false, "not primary"). - role unknown + provider→2 ⇒ native ack dropped + Warning (EventFilter).
- role unknown + provider→2 ⇒ no alerts-topic publish from
ForwardNativeAlarm, butAlarmStateUpdatestill reaches the publish-actor probe (ungated condition write). - MeterListener: denial counter tags (
site,reason). Commit:test(runtime): primary-gate boot-window guards (deny-on-unknown multi-node, allow single-node).
T12 — ScriptedAlarmHostActor emit-gate consistency
Classification: medium-risk (actor gate change, emit-only)
Estimated implement time: 5 min
Parallelizable with: T1–T8; after T9
Files: src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs, its test suite
- Test: role unknown + provider→2 ⇒ engine emission NOT published to alerts topic (DPS probe); provider→1 ⇒ published. Run its filter → FAIL.
- Implement: Props param + policy at
:314+ meter. Filter → PASS. Commit:fix(runtime): scripted-alarm alerts emit uses PrimaryGatePolicy (consistency with 03/S4).
T13 — In-process 2-node delivery-path test
Classification: high-risk verification (the "unit tests can't catch delivery" guard)
Estimated implement time: ~15 min (harness plumbing) — the one deliberately-larger task
Parallelizable with: T1–T8; after T10/T11
Files: tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/PrimaryGateFailoverTests.cs (new, on TwoNodeClusterHarness)
- Two-node harness up, deployment applied, redundancy snapshots delivered (await ServiceLevel/role convergence as
HardKillFailoverTestsdoes); AskRouteNodeWriteon the secondary'sDriverHostActor⇒(false, "not primary"); same on the primary ⇒ serviced. This proves the REAL DPS-delivered snapshot drives the gate (not a test-injected message). dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests --filter "FullyQualifiedName~PrimaryGateFailoverTests"→ PASS (and FAIL under a negative control that blocks the redundancy topic subscribe).- Commit:
test(integration): 2-node delivery-path guard for the primary write gate.
T14 — docs/Redundancy.md + CLAUDE.md-adjacent doc touch
Classification: standard (docs)
Estimated implement time: 5 min
Parallelizable with: all
Files: docs/Redundancy.md
Document the gate policy table (role × cluster size), the boot-window client experience (optimistic-Good → revert + Bad blip + audit event), the new meters (otopcua.redundancy.primary_gate_denied, otopcua.opcua.apply.failed, galaxy.writes.*), and the fail-closed Galaxy write semantics. Commit: docs: failure-visibility trio semantics (R2-04).
T15 — LIVE GATE: 2-node docker-dev rig verification (03/S4) — operator/rig task
Classification: HIGH-RISK verification gate (merge-blocking for the S4 slice) Estimated implement time: ~30 min rig session Parallelizable with: nothing (needs the built branch) Files: none (rig session; findings recorded in STATUS.md)
Run the four-step live recipe in Finding 3's Tests section (rebuild BOTH centrals; ServiceLevel 240/100 check; boot-window write against :4841 rejected + reverted; post-convergence write against :4840 succeeds; single /alerts row regression). Also eyeball one degraded-apply Error line is absent on a clean deploy (T4 sanity) and, if the Galaxy gw is reachable, run T8's live smoke. Record outcomes in archreview/plans/STATUS.md.
Sequencing summary
T1 → T2 → T3 → T4 (01/S-1, sequential — same files)
T5 → T6 → T7, T8 (06/S-1, sequential after seam; T8 anytime)
T9 → T10 → T11 → T13 (03/S4 core)
T9 → T12 (03/S4 consistency)
T14 anytime; T15 last (after T10–T13 merge-ready)
Overall effort: ~1.5–2 dev-days code+unit, plus the rig session. The two HIGH-RISK slices (T1 record change, T10 gate change) each land behind their full regression suites before anything else stacks on them.
Execution deviations (R2-04)
- T1–T3 test increments built incrementally. The three tasks edit the same two files (
AddressSpaceApplier.cs+ a new test file). TheOpcUaServer.Testsproject setsTreatWarningsAsErrors=true, so an unused test helper would break the build. To keep every per-task commit green, the newAddressSpaceApplierFailureSurfaceTests.csgrew one task's tests at a time (T1 RebuildFailed, T2 FailedNodes removal-pass, T3 Materialise* int returns) rather than landing all tests up-front. - T11 write-path assertions made retry-tolerant.
Unknown_role_single_driver_services_writeand the Primary arm ofKnown_role_wins_over_member_counttransiently hit a pre-existing driver-connect race ("driver not connected" reply while theRecordingDriverchild's async init lags theApplyAck) — orthogonal to the gate. The gate correctly ALLOWED (reason was never "not primary"). Wrapped the write in anAwaitAssertretry (as a real client retries), asserting the invariant under test: the gate never denies. Verified deterministic across two full-class runs. - T13 (in-process 2-node delivery) implemented as a gate-reason poll, not a tag write. Rather than deploy a known tag and drive its NodeId (brittle: the harness's fixed deploy config + the DriverHostActor path), the test resolves both nodes'
DriverHostActorvia theActorRegistry(DriverHostActorKey) and AsksRouteNodeWritefor an intentionally-unmapped node, polling until the DELIVERED snapshot promotes exactly one node to Primary (the Secondary rejects with"not primary"; the Primary passes the gate and rejects only with a MAPPING reason). This is a faithful delivery-path assertion with a built-in negative control (a broken redundancy-topic subscribe leaves both stuck"not primary (role unknown)"and times out). Build-verified; markeddeferred-live(Host.IntegrationTests memory leak + needs SQL/2-node rig — controller runs it in the serialized heavy pass). - T8 / T13 / T15 marked
deferred-live. T8 Galaxy live smoke skips cleanly offline (its live run is the operator's, matching the plan's acceptance) — keptcompleted(authored + skip-verified). T13 (in-process 2-node integration) and T15 (2-node docker-dev rig gate) require the live rig / heavy suites the plan reserves for the serialized pass. ServiceCollectionExtensions.DriverRolereused for the driver-member-count read in bothDriverHostActor.DriverMemberCount()andScriptedAlarmHostActor.DriverMemberCount()(the canonical"driver"role constant), rather than a new literal.