94 KiB
Design + Implementation Plan — R3-02 The Stale-Good Family
- Source reports:
archreview/03-server-runtime.md§S15 ·archreview/05-protocol-drivers.md§STAB-19 + §STAB-18 (and §Recommendations item 1) ·archreview/02-scripting-alarms.md§S14 (and §Recommendations item 2) ·archreview/00-OVERALL.mdtheme 1 ("The stale-Good family") — 2026-08-19 round-3 re-review at12b0035a - Prior plan consumed:
plans/R2-03-vt-failure-quality-plan.md(02/S13 — the first member of the family, closed in round 2; its transition-once / force-republish-on-recovery / inputs-ready-gate decisions are inherited, not re-litigated here) - Plan verified against tree at: master
7c77a8c7(code byte-identical to12b0035a; the merge added only the archreview docs) — every anchor re-opened and re-checked 2026-08-20 - Scope: OVERALL prioritized action item #2 — "Stale-Good family: comms-loss fan-out writes
BadNoCommunicationto raw + UNS value variables with force-republish on recovery; Calculation threads the realstatusCode; MQTT re-subscribe initial push (or suppression reset); VT path honours input quality. One contract test per seam" (03 S15 High, 05 STAB-19 High, 05 STAB-18 High, 02 S14 Medium). - Cross-cutting requirement (OVERALL theme 1): state the contract once and pin it with one test per seam that holds a Bad input and asserts on the derived node. The contract, as this plan states it, is in §"The quality contract" below and is quoted verbatim into the one doc section every seam links to.
- Sibling-plan boundary (R3-03, 03/S16 + 03/S17): R3-03 edits
DriverHostActor.HandleRestartDriver(:2916-2932) and the watchdog's??atDriverInstanceActor.cs:574. This plan does not touch either method. Every S15 host edit lives in new, separately-named methods (FanCommsLossToValueNodes,TrackCommsLatch*) plus one newReceiveregistration, and the onlyDriverInstanceActoredit is an additivePostStopTell. Land R3-03 first (it is smaller and its S16 re-subscribe is what makes this plan's recovery contract hold for an operator-restarted driver); rebase this branch over it — the textual overlap is zero by construction, so the rebase is a no-op.
Verification summary
All four findings CONFIRMED at 7c77a8c7 — none stale, none partially fixed. One finding is under-stated (S15: Modbus goes Bad per tag on its own, so the stale-Good window on Modbus is shorter than "the whole outage" — but the S15 mechanism is still absent and every non-per-tag failure shape still holds Good indefinitely; see Finding 1). Anchor drift status:
| Finding | Report anchor | Status at 7c77a8c7 |
|---|---|---|
| 03/S15 | DriverHostActor.cs:1248-1283 (OnDriverConnectivityChanged), :1334-1359 (FanCommsLossToDependencyMux), :1240-1245 (rationale), :1310-1318 (forceRaise recovery remark) |
Exact. The handler writes AlarmQualityUpdate per native condition (:1267-1277) and one AttributeValueBatchPublished of BadNoCommunication/null to the mux (:1346-1353). Nothing targets _opcUaPublishActor with a value/status write. The per-driver index it needs already exists: _valueRefsByDriver (:305, DriverInstanceId → [(RawPath, HashSet<NodeRealmRef>)], node set held by reference so it covers the raw node AND every UNS mirror). The host has no value cache (single-source fan-out by design, :253-258), so "keep the last value" can only be done at the node manager. |
| 03/S15 publish path | ForwardBatchToMux :1166-1200 → OpcUaPublishActor.AttributeValueBatchUpdate (OpcUaPublishActor.cs:57, handled :348-368) → IOpcUaAddressSpaceSink.WriteValues (IOpcUaAddressSpaceSink.cs:35-44; SdkAddressSpaceSink.cs:31-32; DeferredAddressSpaceSink.cs:33) → OtOpcUaNodeManager.WriteValues :577-640 → WriteValueLocked :663-680 |
Traced; the node manager does NOT dedup — WriteValueLocked sets Value/StatusCode/Timestamp unconditionally and calls ClearChangeMasks (:672-677). The only dedup on the value path is driver-side: PollGroupEngine.PollOnceAsync raises only on forceRaise || value differs || status differs (PollGroupEngine.cs:404), with forceRaise: true on the initial poll of every new subscription (:259) and LastValues per SubscriptionState (:467). ModbusDriver adds a deadband cache _lastPublishedByRef (:55, ShouldPublish :244-259) that is cleared in TeardownAsync (:1810). SDK-level: the UA stack's DataChangeMonitoredItem (default trigger StatusValue) does not queue a notification when value AND status are unchanged — a timestamp-only rewrite is invisible to subscribers, which is why a Bad write must actually change the node's status for the later Good to be visible (it does). |
| 03/S15 watchdog | DriverInstanceActor.cs:567-584 (EvaluateCommsLossWatchdog), :463-470 (Connected DisconnectObserved handler), :633-658 (Reconnecting) |
Exact. Trip ⇒ DetachSubscription() (:467, nulls the handle + unhooks handlers, does not unsubscribe the driver) → Become(Reconnecting) → Reconnecting() Tells ConnectivityChanged(false) (:637) → RetryConnect runs InitializeAsync(teardownFirst: true) (:640) which calls driver.ShutdownAsync first (:701-725) → on InitializeSucceeded: Become(Connected) (Tells ConnectivityChanged(true), :456) then ResubscribeDesired() (:657 → :1171-1177 self-Tell Subscribe). For every PollGroupEngine driver, ShutdownAsync disposes the engine's subscriptions (PollGroupEngine.DisposeAsync :428-457, clears _subscriptions; the engine is reusable afterwards) so the re-subscribe creates a fresh SubscriptionState with empty LastValues ⇒ the initial poll runs forceRaise: true ⇒ every tag re-publishes with its real quality even if unchanged. This is the recovery mechanism S15 relies on, and it is sound for the seven poll drivers (Modbus, FOCAS, TwinCAT, AbLegacy, S7, Sql, AbCip construct a PollGroupEngine). It is exactly what MQTT lacks (STAB-18). |
| 03/S15 sink surface | — | New fact: IOpcUaAddressSpaceSink has 16 implementers (3 in src: the interface's defaults, DeferredAddressSpaceSink, SdkAddressSpaceSink; 13 test fakes). A new abstract member costs 13 fake edits; a default-implemented member costs none, and the DeferredSinkForwardingReflectionTests guard (tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/) forces the deferred forwarder to be written. Design below uses a default-implemented member. |
| 05/STAB-19 | CalculationDriver.cs:330-352 (OnDependencyValue), DependencyConsumerMuxAdapter.cs:71-72, CalculationEvaluator.cs:181-187 (BuildReadCache), DependencyMuxActor.cs:114, :150 |
Exact for the first, second and fourth (OnDependencyValue is :329-352, _arrivedDeps.Add at :336, statusCode parameter never read; adapter comment :71 "Quality is stripped at the mux boundary" + statusCode: 0u at :72; mux forwards msg.StatusCode at :114/:150). Minor drift on the third: BuildReadCache is :176-188, the StatusCode: 0u stamp at :185. Also verified: _depValues is Dictionary<string, object?> (:85) — no per-dep status storage exists; the passthrough fast path (:88-93) bypasses the read cache entirely; MarkEvaluationSucceeded :549-559 already bypasses value-dedup while LastPublishedBad (the R2-03 recovery idiom), which this plan generalises. Precedent in the same file: CalculationDriver.SubscribeAsync :356-371 performs an initial-data callback for each subscribed ref — the push-model forceRaise STAB-18 asks MQTT to adopt already exists in-tree. |
| 05/STAB-18 | MqttSubscriptionManager.cs:325-365 (SubscribeAsync), :762-767 (suppression), SparkplugIngestor.cs:470-506 (SubscribeAsync) |
Exact (suppression compare at :762-766, Values.Update then if (!unchanged) Raise). Verified the cache lifetime claim: LastValueCache is a driver-owned readonly field (MqttDriver.cs:127), handed to every manager/ingestor the driver builds (CreateSubscriptionManager :1147-1161, cache: _values at :1154; Sparkplug :1193), and MqttDriver.TeardownAsync :1387-1411 never touches it — so it survives the watchdog's ShutdownAsync + InitializeAsync and every redeploy. Verified there is no connection-lost hook: MqttConnection exposes only Reconnected (:350) and MessageReceived (:363); OnDisconnectedAsync :932-948 flips state to Reconnecting (:941) and wakes the supervisor — nothing stales values. SparkplugIngestor.ResetSessionState documents the opposite policy explicitly ("Values are deliberately not staled here … Only a death … stales a tag", :583-588) — this plan overrides that remark, with the reason recorded below. |
| 05/STAB-18 rebuild shape (b) | — | Confirmed at the host: every deploy re-sends SetDesiredSubscriptions, and the Connected handler (DriverInstanceActor.cs:483-486) always self-Tells Subscribe (unsubscribe-then-subscribe in HandleSubscribeAsync). A poll driver answers with a fresh initial push; MqttSubscriptionManager.SubscribeAsync/SparkplugIngestor.SubscribeAsync record handles and push nothing — a rebuilt node reseeded BadWaitingForInitialData (OtOpcUaNodeManager.cs:2125/:2220) stays there until the plant publishes. |
| 02/S14 | VirtualTagActor.cs:33-38 (DependencyValueChanged doc: "the virtual-tag engine ignores it"), :187-191 (OnDependencyChanged), RoslynVirtualTagEvaluator.cs:165-177 (BuildReadCache) |
Exact (BuildReadCache is :165-178, StatusCode: 0u at :174). _dependencies is Dictionary<string, object?> (:75); OnDependencyBatch :200-217 has the same shape; Evaluate :223-266 dedups on value only (:254); OnEvaluationFailed :277-289 is the R2-03 transition-once tail gated on AllDependenciesArrived (:295); OnReassertValue :179-185 re-emits _lastPublishedBad ? BadInternalError : Good. VirtualTagHostActor.OnResult :214-242 bridges result.StatusCode verbatim to AttributeValueUpdate (:227-228) and historizes non-reassert results with that status (:238-241) — so a propagated sub-code needs no bridge change. IVirtualTagEvaluator.Evaluate takes IReadOnlyDictionary<string, object?> (IVirtualTagEvaluator.cs:20) and has 6 implementers (Roslyn + 5 test fakes across 4 Runtime.Tests files). |
| Parity reference | ScriptedAlarmEngine.cs:746-765 (worst-of-input computed BEFORE the readiness short-circuit; a Bad input holds the prior condition state and annotates quality out of band), AreInputsReady :829-840 (null value OR Bad ⇒ not ready; Uncertain IS ready), WorstInputStatus :923-941 (skips StatusNeverPushed = BadWaitingForInitialData) |
Exact. This is the in-tree policy the VT and Calculation seams are aligned to. |
Supporting facts verified: OpcUaStatus constants (Commons/OpcUa/OpcUaStatus.cs: Good, Uncertain 0x40000000, Bad, BadWaitingForInitialData 0x80320000 :39, BadNoCommunication 0x80310000 :43, BadInternalError :47, Severity/IsGood/IsUncertain/IsBad :52-66); Commons references Core.Abstractions (so DataValueSnapshot is usable from Commons interfaces); Driver.Calculation references Core.Abstractions + Commons; Core.ScriptedAlarms references Core.Abstractions only. Existing test scaffolding: DriverHostActorCommsLossTests (real DependencyMuxActor, SeedV3Deployment with raw tags + UNS refs, PresenceBudget), DriverHostActorBatchFanOutTests (publish-actor probe asserting AttributeValueBatchUpdate raw+UNS fan-out), DriverInstanceActorCommsLossWatchdogTests (injectable commsLossTimeout, stable-health stub), CalculationDependencyFlowTests, VirtualTagActorTests/VirtualTagHostActorTests (R2-03 transition tests), MqttSubscriptionManagerTests/SparkplugIngestorTests/MqttConnectionTests (+ MiniBroker), LastValueCacheTests. Modbus fixture (tests/Drivers/…Modbus.IntegrationTests/Docker/profiles/standard.json): HR[0..31] = address-as-value (constant, config-seeded — survives a container restart), HR[100] = auto-increment — the constant/changing pair the live gate needs.
Design choices resolved in this plan
- One contract, stated once (§below). Every per-seam section derives its behaviour from it rather than re-deciding.
- S15 keeps the last value via a new status-only sink write (
WriteStatuses) rather thanWriteValueswithnull— the host holds no value, the node does; and an OPC UA client reading a Bad node with its last-known value is the Part 8 "last known value" convention SCADA clients expect. The null-through-WriteValuesroute is kept as the documented fallback if the sink change is judged too wide. - S15 publishes nothing Good on reconnect. Recovery rides the driver's own re-publish (the poll engine's
forceRaise, or the push-model initial push STAB-18 adds). The host instead audits recovery: refs still latched Bad 30 s afterConnectedare logged + metered, which is the operator-visible signature of any future driver that forgets its initial push. - STAB-18 lands BEFORE S15 (or in the same PR with MQTT first in the commit order). Today an MQTT ref latched Bad by the watchdog is stuck only at the mux; once S15 latches the value nodes too, an MQTT driver without STAB-18 would show every external client Bad until the plant value changes. Ordering is the mitigation.
- STAB-18 fixes both halves: (A) an initial push of cached observed snapshots on every
SubscribeAsync(rebuild shape), and (B) aConnectionLoststale of every observed ref toBadNoCommunication(value kept) so the reconnect's identical retained/birth replay differs in status and is no longer suppressed — and so MQTT marks Bad on its own disconnect edge like every poll driver does per tag, not only after the 90 s watchdog. "Clear the suppression memory" alone was rejected: it recovers retained topics but leaves non-retained ones serving stale-Good from the cache — the exact contract violation. - Calculation + VT quality policy: hold on Bad, propagate the worst input's status; evaluate through Uncertain and stamp the output with it. Alternatives weighed in Finding 2; this is the alarm engine's existing policy (
AreInputsReady+ out-of-band worst-of-input quality) and the only one that is honest for a comms-lost input (an Uncertain output would claim the value is merely imprecise; it may be arbitrarily old). - No new message-contract field on the mux path.
DependencyValueChanged.StatusCodealready carries the sub-code since #478/#519; only the consumers change (adapter forwards it; VT/calc store it).IVirtualTagEvaluatorgains a snapshot overload with a default implementation so the 5 fakes compile unchanged. - Worst-of-input helper lives once, in
Core.Abstractionsnext toDataValueSnapshot, and the alarm engine's private copy is re-pointed at it (theme 4: "one rule stated twice diverges").
The quality contract
A value consumer never reports a quality better than its source's. A node derived from a driver value (the raw node, every UNS mirror, a Calculation output, a virtual tag, a scripted condition's quality) carries, at worst, the worst status among its inputs: a Bad input makes the derived node Bad with that input's sub-code and the last-known value; an Uncertain input makes it Uncertain; a source that has stopped communicating is a Bad input (
BadNoCommunication) for everything downstream of it, including the external OPC UA data plane. Recovery is a publish, never an assumption: the first post-recovery value must land even when it equals the value held before the outage, so every dedup on the path — driver caches, the node's own status, a consumer's last-published memory — must be keyed on (value, status), not value alone.
Pinned per seam by a test that holds a Bad input and asserts on the derived node (never on the signal):
| Seam | Derived node asserted | Test (new) |
|---|---|---|
| S15 host fan-out | raw value node + its UNS mirror (publish-actor probe) | DriverHostActorCommsLossValueNodesTests + recovery DriverHostCommsLossRecoveryTests |
| STAB-18 MQTT | the ref's raised snapshot after reconnect / resubscribe | MqttSubscriptionManagerRecoveryTests, SparkplugIngestorRecoveryTests |
| STAB-19 Calculation | the calc tag's OnDataChange snapshot |
CalculationDriverInputQualityTests |
| 02/S14 VT | the VT's EvaluationResult (status + value) at the host bridge |
VirtualTagActorInputQualityTests |
Plus one falsification control for the dedup trap per recovering seam (see Finding 1 §Tests "vacuity"): the recovery assertion must be run with a source that re-publishes the identical value, and the plan names the variant that must FAIL.
Finding 1 — 03/S15: Driver comms loss never marks raw/UNS value variables Bad (High)
Restatement
When a driver is declared comms-lost (watchdog trip, failed connect, operator Force Reconnect — anything that enters Reconnecting), DriverHostActor.OnDriverConnectivityChanged annotates its Part 9 conditions BadNoCommunication and fans a synthetic Bad batch to the dependency mux, so scripted alarms and virtual tags see the outage. The driver's raw value nodes and their UNS mirrors — the product's external surface — get nothing. A client subscribed to a value node sees the last value with a stale timestamp and Good quality for as long as the driver publishes nothing, i.e. the whole outage for any driver whose failure shape is "reader throws / no data" rather than "per-tag Bad snapshot".
Verification
Confirmed (summary table). Two refinements the fix must respect:
- Modbus already degrades per tag, quickly.
ModbusDriver.ReadAsyncturns a per-tag read exception intoDataValueSnapshot(null, BadCommunicationError 0x80050000, …)(:511,:517; coalesced path:995), the poll engine raises it (status changed,PollGroupEngine.cs:404), and the host fans it to raw + UNS nodes within one poll interval. So on the docker-dev Modbus rig the value nodes go Bad (0x80050000, value null) within seconds of stopping the fixture — before the 90 s watchdog. What S15 adds for Modbus is theBadNoCommunicationannotation on the watchdog edge (idempotent over an already-Bad node) and, critically, coverage for the shapes where the driver raises nothing: a reader that throws as a whole (RequireTransport()with a null transport, a coalesced-read throw —PollGroupEnginecatches and callsonErrorwithout raising), S7 (whose reader "never throws for device faults" per 05/STAB-21 but can also stall), TwinCAT/OpcUaClient/Galaxy/MTConnect session drops that stop the push without a Bad, and any driver whose last act before going quiet was a Good publish. No double-handling harm: the S15 write is status-only over whatever value the node holds (null after Modbus's own Bad), and the Bad→Bad re-stamp is exactly the "unchanged code suppresses" shapeWriteAlarmQualityalready documents for conditions — here it is a secondClearChangeMasks, which the SDK'sStatusValuetrigger dedups away at the monitored item (status unchanged). - Ordering on the way down is safe. The Connected-state
DisconnectObservedhandler callsDetachSubscription()beforeBecome(Reconnecting)(:467-469), andReconnectinghas noDataChangeForwardreceive — so any driver value already in the mailbox is dropped as unhandled, and theConnectivityChanged(false)the new behaviour Tells (:637) is the last value-affecting message the host sees for that driver until recovery. The S15 Bad write cannot be overtaken by a late Good. - Ordering on the way up relies on the driver.
Connected()TellsConnectivityChanged(true)(:456) beforeResubscribeDesired()(:657), so the host learns "connected" before any value arrives. Publishing Good there would assert a quality the host has not observed (the existing remark at:1310-1318says so for the mux; it holds for nodes). The first real value arrives from the fresh subscription's initial push.
Root cause
#481 built the per-driver ref index and the comms-loss fan-out for internal consumers (mux) and conditions (Part 9), and stopped one seam short of the external data plane. The host has no value cache (by design — single source, no drift), so "mark Bad keeping the value" needs a node-manager primitive that did not exist; the absence of that primitive is why the value seam was skipped.
Proposed design
1. Sink primitive — status-only batch write (keep value).
// IOpcUaAddressSpaceSink.cs (additive, default-implemented)
/// <summary>One status-only write within a <see cref="IOpcUaAddressSpaceSink.WriteStatuses"/> batch:
/// the node's current VALUE is retained; only StatusCode + SourceTimestamp change.</summary>
public readonly record struct StatusWrite(string NodeId, uint StatusCode, DateTime SourceTimestampUtc, AddressSpaceRealm Realm);
/// <summary>Batched status-only write — the comms-loss annotation for value variables (03/S15). A sink
/// that holds the node's current value MUST retain it (the SDK sink does). This DEFAULT cannot — it holds
/// no value — so it degrades to <see cref="WriteValue"/> with a null value, which is the honest statement
/// for a value-less sink and keeps every fake compiling. Same per-item isolation contract as
/// <see cref="WriteValues"/>; an unknown/unmaterialised NodeId is a no-op, not a create (a status has
/// nothing to create a node from).</summary>
void WriteStatuses(IReadOnlyList<StatusWrite> statuses)
{
ArgumentNullException.ThrowIfNull(statuses);
for (var i = 0; i < statuses.Count; i++)
{
var s = statuses[i];
WriteValue(s.NodeId, null, s.StatusCode, s.SourceTimestampUtc, s.Realm);
}
}
OtOpcUaNodeManager.WriteStatuses(new, besideWriteValues :577): same outside-the-lock pre-pass + 512-per-lock chunking; per itemif (_variables.TryGetValue(key, out var v)) { v.StatusCode = new StatusCode(s.StatusCode); v.Timestamp = s.SourceTimestampUtc; v.ClearChangeMasks(SystemContext, false); }— neverCreateVariable; a missing node is skipped (counted in a Debug tally, not a failure — the index can name a node the address space dropped on a surgical remove). SameValueWriteBatchExceptionaggregate on thrown items.SdkAddressSpaceSink.WriteStatuses→_nodeManager.WriteStatuses.DeferredAddressSpaceSink.WriteStatuses→_inner.WriteStatuses(the reflection guard turns red until this exists — run it first).
2. Publish-actor message. OpcUaPublishActor.AttributeStatusBatchUpdate(IReadOnlyList<StatusWrite> Items) (beside AttributeValueBatchUpdate :57), handled by HandleAttributeStatusBatchUpdate — a copy of HandleAttributeBatchUpdate :348-368 calling _sink.WriteStatuses, metered OpcUaSinkWrite kind:"status", same partial-failure Warning.
3. Host fan-out — new method, called from the existing trigger.
// DriverHostActor.cs — NEW method (R3-03 boundary: HandleRestartDriver / the watchdog are untouched)
/// <summary>03/S15 — the value-node half of the comms-loss fan-out. Stamps BadNoCommunication (value
/// retained) on every raw NodeId AND every referencing UNS NodeId the disconnected driver owns, as ONE
/// status batch through the same publish actor the value fan-out uses. Nothing is written on reconnect:
/// recovery is the driver's own re-publish (poll forceRaise / push initial push); see TrackCommsLatch.</summary>
private void FanCommsLossToValueNodes(string driverInstanceId)
{
if (_opcUaPublishActor is null) return;
if (!_valueRefsByDriver.TryGetValue(driverInstanceId, out var entries) || entries.Count == 0) return;
var ts = DateTime.UtcNow;
var writes = new List<StatusWrite>(entries.Count);
foreach (var (_, nodes) in entries)
foreach (var n in nodes)
writes.Add(new StatusWrite(n.NodeId, OpcUaStatus.BadNoCommunication, ts, n.Realm));
_opcUaPublishActor.Tell(new OpcUaPublishActor.AttributeStatusBatchUpdate(writes));
TrackCommsLatchSet(driverInstanceId, entries); // recovery audit, step 4
_log.Info("DriverHost {Node}: driver {Driver} disconnected — {Count} value node(s) marked BadNoCommunication", …);
}
Wired by one added line in OnDriverConnectivityChanged next to the existing mux call (:1253): if (!msg.Connected) { FanCommsLossToDependencyMux(id); FanCommsLossToValueNodes(id); } else TrackCommsLatchArmAudit(id);. Ungated by redundancy role (same reason as the condition annotation: a Secondary keeps its address space honest for failover). Every entry into Reconnecting reaches it — watchdog, failed initial connect, Force Reconnect — because all of them run Reconnecting()'s ConnectivityChanged(false) Tell. (The one path that does NOT is a child being stopped: PostStop detaches silently. Task T11 makes DriverInstanceActor.PostStop Tell ConnectivityChanged(false) to its parent so an operator Restart (S16) and a supervisor respawn latch Bad too; after R3-03's re-subscribe the fresh child's initial push recovers them. On a deploy that removes the driver the index has already been rebuilt without it, so the Tell is a no-op.)
4. Recovery audit (the dedup-trap tripwire, not a Good write). _commsLatched: Dictionary<string, HashSet<string>> (driver → RawPaths latched). Set in step 3; each item routed by ForwardBatchToMux removes its FullReference from the owning driver's set (one TryGetValue + Remove on the hot path — only while the set is non-empty, so steady state costs one dictionary miss per burst, keyed by driver not by item). On ConnectivityChanged(true) a single timer comms-audit:{driver} fires CommsRecoveryAudit(driver) after CommsRecoveryAuditDelay (30 s, ctor-injectable for tests): if the set is still non-empty ⇒ _log.Warning("… {Count} ref(s) still BadNoCommunication {Delay} after reconnect — the driver did not re-publish them (missing initial push / dedup) …") + meter otopcua.driver.comms_latched_refs (tag driver_type). A second disconnect before the audit re-arms it. This is what turns a future STAB-18-class regression into a log line instead of a silent stale-Bad.
Alternatives considered:
- Write
null+BadNoCommunicationthrough the existingWriteValues— zero sink change, and it is what Modbus itself publishes per tag. Rejected as the primary design because the action item asks to keep the last value and because a Part 8 last-known-value is more useful to a SCADA consumer than a null; retained as the fallback ifWriteStatusesis judged too wide during review — the host-side change is then a two-line difference. - Publish
Goodwith the node's current value on reconnect — rejected (asserts an unobserved quality; the existing remark at:1310-1318already rules it out for the mux). - Host-side last-value cache to re-publish on reconnect — rejected: it recreates the independent buffer the v3 single-source design removed, and it would re-publish a value the device may no longer hold.
- Status-only write keyed by the driver's
DriverStaterather thanConnectivityChanged— rejected:Degradedmeans connected-with-some-bad-data (#527); the Reconnecting edge is the only honest "no communication".
Implementation steps
src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/IOpcUaAddressSpaceSink.cs—StatusWrite+ defaultWriteStatuses.DeferredAddressSpaceSink.cs— forward.src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs—WriteStatuses(+WriteStatusLocked);SdkAddressSpaceSink.cs— forward.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs—AttributeStatusBatchUpdate+ handler.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs—FanCommsLossToValueNodes,_commsLatched+TrackCommsLatch*,CommsRecoveryAuditmessage +Receive, one-line call inOnDriverConnectivityChanged;Commons/Observability/OtOpcUaTelemetry.cs—DriverCommsLatchedRefscounter.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs—PostStop:Context.Parent.Tell(new ConnectivityChanged(_driverInstanceId, Connected: false))before the existing detach (T11; additive, not in the watchdog method).- Update the xmldoc at
DriverHostActor.cs:1223-1247(the handler summary now covers values) and the remark at:1310-1318(recovery contract now names the audit + MQTT's initial push).
Tests
tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OtOpcUaNodeManagerWriteStatusesTests.cs(new): materialise +WriteValue(42, Good);WriteStatuses(BadNoCommunication)⇒ node readsValue == 42,StatusCode == 0x80310000, timestamp updated; unknown NodeId ⇒ no node created, no throw; a throwing item does not drop the tail (ValueWriteBatchExceptionsplit).tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests— goes RED on the new member until forwarded (the guard is the test; no new file).tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorStatusBatchTests.cs(new):AttributeStatusBatchUpdatereachessink.WriteStatuses; partial failure logs Warning and keeps the tally.tests/Server/…Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs(new, publish-actor probe,SeedV3Deploymentwith 2 raw tags + 1 UNS ref likeDriverHostActorBatchFanOutTests):ConnectivityChanged("drv-1", false)⇒ ONEAttributeStatusBatchUpdatewhose items are exactly {raw speed (Raw), raw temp (Raw), UNS mirror of speed (Uns)} withBadNoCommunication; control: a second driver's refs are absent;ConnectivityChanged(true)⇒ no value/status message at all (ExpectNoMsg).tests/Server/…Runtime.Tests/Drivers/DriverHostCommsLossRecoveryTests.cs(new — the contract + vacuity test): realDriverInstanceActorunder the host with a scriptedISubscribable+IDriverfake (commsLossTimeout= 200 ms,HealthPollIntervalshort, asDriverInstanceActorCommsLossWatchdogTestsdoes): fake publishesspeed = 5 Good, then reportsDegradedwith frozenLastSuccessfulRead⇒ host emits the status batch (raw + UNS); fake'sInitializeAsyncthen succeeds and itsSubscribeAsyncperforms an initial push of the IDENTICAL value5 Good⇒ assert anAttributeValueBatchUpdatecarrying5 / Goodfor raw + UNS arrives after the status batch; assert nocomms_latched_refsincrement and no audit Warning.- ⚠️ Vacuity rule: a recovery test whose fake pushes a different value (or whose assertion is only "some Good arrives") passes against a build with no force-republish anywhere and is not the contract test — call it out in the file header and do not ship it as the guard. The identical-value push is load-bearing.
- Falsification control (same file,
[Fact]named…_control_silent_resubscribe_trips_the_audit): the same scenario with a fake whoseSubscribeAsyncpushes nothing (the pre-STAB-18 MQTT shape) ⇒ no Good batch arrives, the audit timer (injected 300 ms) fires,otopcua.driver.comms_latched_refsincrements by 1 (the latch is keyed per RawPath — raw node + UNS mirror share one ref; the Warning reports refs, not nodes) and the Warning names the driver. This proves the contract test can fail and that the audit catches what it misses.
DriverInstanceActorTestsaddition: stopping a Connected child TellsConnectivityChanged(false)to its parent (T11).- Existing:
DriverHostActorCommsLossTests(mux half) andDriverHostActorBatchFanOutTestsunchanged and green.
Effort + Risk
Effort: M (sink primitive S, host S, recovery test M). Risk: medium — a new sink member (guarded by reflection), a new hot-path dictionary probe in ForwardBatchToMux (one lookup per burst while a latch exists; zero-cost when _commsLatched is empty — pin with a microbench-free assertion that the set is removed when it empties), and a client-visible behaviour change: value nodes of a comms-lost driver now go Bad, which is the point, but any consumer (ScadaBridge) that treated a Bad value as "drop the tag" will see more Bads. Called out in the docs task.
Finding 2 — 05/STAB-19: Calculation launders dependency quality to Good (High)
Restatement
DependencyConsumerMuxAdapter.OnDependencyValueChanged forwards statusCode: 0u behind a comment that is false (the mux has carried the real status since #478/#519), and CalculationDriver.OnDependencyValue never reads its statusCode anyway. A comms-lost input arrives as arrived, Good, null: the warm-up gate is satisfied by a value that never arrived, (double)ctx.GetTag("x").Value NREs per trigger (a "script threw" storm), and a null-tolerant script publishes a Good output computed from a dead input — a slow input that dies leaves the calc output stale-Good indefinitely. CalculationEvaluator.BuildReadCache additionally stamps every dep Good with eval-time timestamps, so even a script that checks .StatusCode is lied to.
Verification
Confirmed exact (table). Additional: the _arrivedDeps gate (:442) is a "has ever been pushed" set with no quality dimension; MarkEvaluationFailed :569-577 publishes BadInternalError once per transition with the last value (R2-03 parity already in place) — so the output side already has the transition-once + force-republish-on-recovery idiom; only the input side is missing. CalculationDriver.GetHealth is Healthy while running (the watchdog never trips it; correct) — so the calc driver's own quality must come entirely from its inputs. Passthrough scripts (:88-93) bypass the read cache, so a passthrough of a Bad input currently returns the Bad input's null value as Ok(null) Good — the worst case of the laundering.
Root cause
The adapter was written when the mux did strip quality; the contract moved under it (#478/#519) and the comment outlived the fact (theme 6). The calc driver then inherited the mux-strips-quality assumption in its storage shape (Dictionary<string, object?>).
Proposed design
Policy (chosen): hold on Bad, propagate worst status; evaluate through Uncertain, stamp the output with the worst input status. Concretely, per calc tag with definition deps D:
- Store
DataValueSnapshotper dep (_depValues : Dictionary<string, DataValueSnapshot>);_arrivedDepsstays (arrival ≠ usability). worst = InputQuality.WorstOf(snapshots of D)(shared helper, see below), where a never-pushed dep isBadWaitingForInitialData.- Readiness = all of
Darrived and!IsBad(worst)(alarm-engine parity: Uncertain is usable, Bad is not). - On any dependency push: if not all arrived ⇒ do nothing (today's warm-up; the node stays at the materialiser's
BadWaitingForInitialData). If all arrived andIsBad(worst)⇒ do not evaluate; publish(LastValue-or-null, worst)once per change ofworst(TagState.LastPublishedStatusreplacesLastPublishedBad; a Bad→different-Bad change re-publishes soBadNoCommunicationis not masked by an earlierBadInternalError, and vice versa). If usable ⇒ evaluate; on success publish(value, worst)(Good or the Uncertain sub-code), bypassing value-dedup wheneverLastPublishedStatus != worst(generalises the existingLastPublishedBadbypass at:551) — this is the force-republish that makes an identical-value recovery land; on script failure keepMarkEvaluationFailed(BadInternalError, last value) unchanged. ReadOne(:390-399) returnsLastPublishedStatusinstead of the two-state mapping, soIReadableand theSubscribeAsyncinitial-data callback agree with what was published.CalculationEvaluator.Evaluategains an overload takingIReadOnlyDictionary<string, DataValueSnapshot>;BuildReadCachepasses the snapshots through (real status, real source timestamps), and the passthrough fast path returnsOk(snapshot.Value)— its quality is then stamped by the caller asworst, which for a single-dep passthrough IS the input's status. The oldobject?overload stays as a thin adapter for the existing tests.
Why hold rather than evaluate-and-degrade: the NRE storm (b) is caused by running the script against a null; "degrade the output" still runs it. The alarm engine made the same call for the same reason (ScriptedAlarmEngine.cs:751-756). Why Bad rather than Uncertain: BadNoCommunication on the output tells the operator why and is honest about age; Uncertain says "imprecise", which a two-hour-old value is not. Why propagate the sub-code rather than a fixed BadInternalError: the sub-code is the diagnostic (BadNoCommunication = go look at the PLC; BadWaitingForInitialData = the input never arrived; BadInternalError = the script). Why evaluate through Uncertain: the alarm engine treats Uncertain as ready; a calc over an Uncertain input is still computable, and stamping the output Uncertain keeps the contract without holding perfectly good arithmetic hostage.
Null guard: with hold-on-Bad the evaluator is never handed a Bad/null-from-Bad dep, so the (double)ctx.GetTag("x").Value NRE class disappears without touching scripts. A Good null (a driver that legitimately publishes null) still reaches the script as today — not this finding's concern.
Shared helper (T1):
// src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs (new)
/// <summary>The one worst-of-input rule every derived-value seam uses (scripted alarms, virtual tags,
/// Calculation). Severity is the OPC UA Part 4 top-two bits; among equal severity the FIRST seen wins
/// (stable, so a re-evaluation over the same inputs reports the same sub-code).</summary>
public static class InputQuality
{
public static uint WorstOf(IEnumerable<DataValueSnapshot?> inputs); // null entry ⇒ BadWaitingForInitialData
public static bool IsUsable(uint statusCode) => (statusCode >> 30) < 2; // Good or Uncertain
}
ScriptedAlarmEngine.WorstInputStatus :923-941 becomes a call to WorstOf (its StatusNeverPushed skip is preserved by the alarm engine passing null for never-pushed entries, or — simpler — by keeping its skip loop and delegating only the comparison; choose the smaller diff and keep ScriptedAlarmEngineTests green).
Adapter: DependencyConsumerMuxAdapter.OnDependencyValueChanged forwards msg.StatusCode; delete the false comment; xmldoc states the contract.
Alternatives considered: evaluate on Bad inputs and publish Uncertain — rejected (NRE storm persists; Uncertain is dishonest for comms loss). Hold on Bad but keep the output's prior status — that is the bug. A per-script opt-in ([TolerateBadInputs]) — defer; nobody has asked, and the alarm engine has no such knob.
Implementation steps
src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs(new) +ScriptedAlarmEngine.cs:923-941re-point.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyConsumerMuxAdapter.cs:70-73— forwardmsg.StatusCode.src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationDriver.cs—_depValuessnapshots;PrepareEvaluationreadiness + worst;RunEvaluation/MarkEvaluation*status-keyed dedup;TagState.LastPublishedStatus;ReadOne; a newMarkInputsBadpublishing(last, worst)once per status change; xmldoc at:405-437updated.src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation/CalculationEvaluator.cs— snapshot overload + pass-throughBuildReadCache; passthrough fast path on snapshots.docs/drivers/Calculation.md— "Input quality" section (policy table).
Tests
tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InputQualityTests.cs(new; the project exists): truth table Good/Uncertain/Bad/never-pushed, stable tie-break.tests/Server/…Runtime.Tests/VirtualTags/DependencyConsumerMuxAdapterTests(new or extend): aDependencyValueChangedwithBadNoCommunicationreaches the consumer'sstatusCodeparameter verbatim (batch path too).tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverInputQualityTests.cs(new — the contract test; evaluator spy countingEvaluatecalls):- deps
a,bGood ⇒ output Good (regression); thenaarrivesBadNoCommunication/null ⇒ no evaluation, exactly ONE publish(lastValue, 0x80310000); a second Bad push fora⇒ no publish (transition-once); no script-log entry (the storm is gone); - recovery:
areturns Good with the identical value ⇒ evaluation runs, output re-published Good even though the computed value equals the pre-outage value (the dedup bypass) — this is the vacuity-sensitive assertion; a recovery with a different value is not the guard; aUncertain ⇒ evaluation runs, output stamped witha's Uncertain code; back to Good ⇒ Good republished;- cold start with one dep never pushed and the other Bad ⇒ no publish (node stays
BadWaitingForInitialData); once both have arrived (one Bad) ⇒ publishBadNoCommunication; - passthrough script over a Bad input ⇒ Bad output, never
Ok(null) Good; ReadAsyncafter a Bad publish reports the same status the subscriber saw.
- deps
CalculationEvaluatorTestsaddition:ctx.GetTag("a").StatusCodeinside a script reads the real (Uncertain) code and the real source timestamp, not eval-timeGood.- Existing
CalculationDriverTests/CalculationDependencyFlowTestsgreen (the Good-only paths are byte-identical).
Effort + Risk
Effort: S-M. Risk: low-medium — driver-local state-shape change; behaviour change only when an input is non-Good (today that path is wrong by construction). The one regression to watch: a script that relied on receiving a null for a Bad input (null-tolerant scripts) now holds instead of computing; the docs task names this.
Finding 3 — 05/STAB-18: MQTT has no push-model forceRaise; a Bad-latched ref recovers only when the plant value changes (High)
Restatement
The host's comms-loss recovery contract is "the driver re-publishes everything on re-subscribe". MQTT's SubscribeAsync (plain and Sparkplug) records handles and pushes nothing; the plain-mode Deliver suppresses a message whose value+status equal the driver-owned cache, and that cache survives reconnects and redeploys. So (a) after a watchdog reconnect the retained-seed replay carries the identical value ⇒ suppressed ⇒ every mux ref latched BadNoCommunication by #481 — and, after S15, every value node — stays Bad until the plant value changes; (b) after an address-space rebuild, nodes reseeded BadWaitingForInitialData are never refilled for quiet topics/metrics (Sparkplug: a rebuild triggers no rebirth).
Verification
Confirmed exact (table), plus: DegradeTopic :664-690 already shows the "stale + Raise BadCommunicationError" idiom for a refused filter, so the codebase has the per-ref degrade shape — it is just never applied on a connection loss. MqttDriver.GetHealth :425-443 maps connection Reconnecting to DriverState.Reconnecting (non-Healthy), so the #527 watchdog does trip for a dead broker after 90 s of no messages — but in the common case MQTTnet's own supervisor reconnects well inside 90 s and the host never learns anything; the driver is the only party that can mark the outage.
Root cause
Push-model drivers inherit none of the poll engine's edge behaviours (initial push, fresh per-subscription last-value state); the MQTT manager re-implemented the steady-state dedup (ValuesAreDifferent, deliberately duplicated from the engine) without the edge behaviours that make that dedup safe.
Proposed design
Two halves, both in the driver, no host change:
(A) Initial push on every SubscribeAsync — both MqttSubscriptionManager (:325-365) and SparkplugIngestor (:470-506): after registering the handle, for each resolved def whose Values.Read(def.Name) is an observed snapshot (status ≠ BadWaitingForInitialData), Raise(def.Name, snapshot) attributed to the new handle. Mirrors CalculationDriver.SubscribeAsync :356-371 and the poll engine's forceRaise: true. Fixes shape (b) (a rebuilt node gets its last observed value + status back immediately) and completes the recovery of shape (a) once (B) has staled the cache. Cost: one raise per subscribed ref per (re)subscribe — the same burst a poll driver already produces.
(B) Stale on connection loss — MqttConnection gains public event Action? ConnectionLost;, fired from OnDisconnectedAsync :932-948 on the Connected → Reconnecting edge only (it runs on MQTTnet's dispatcher; subscribers must be non-blocking — the manager's handler is dictionary work + OnDataChange invokes, the same work Deliver already does on that thread). MqttSubscriptionManager.OnConnectionLost() and SparkplugIngestor.OnConnectionLost(): for every authored ref whose cached snapshot is observed and not already Bad, Values.Update(ref, snapshot with { StatusCode = BadNoCommunication, ServerTimestampUtc = now }) (value retained) and Raise it. After this, the reconnect's retained seed / NBIRTH-DBIRTH replay carries the same value with Good ⇒ unchanged == false ⇒ raised ⇒ recovery — no suppression-memory surgery needed. A non-retained topic stays BadNoCommunication with its last value until the plant publishes, which is the honest answer. AttachTo subscribes the handler beside Reconnected (:232); DetachFrom/dispose unsubscribes.
Sparkplug policy override, recorded: ResetSessionState's remark (:583-588) argues values are not staled on a session change because "the connection's own health surface already reports the outage". That is precisely the stale-Good shape this family is about — health is not quality, and an external client subscribed to the metric's node sees neither the health page nor the outage. The remark is rewritten to state the new rule and to keep the part that is still right: a session reset for a config rebuild (same live connection) does not stale — only ConnectionLost does. Deaths still stale as before (they already use BadNoCommunication, :97-102).
Dedup-trap test discipline for this seam: the recovery test MUST replay the identical payload after the stale and assert a Good raise; a test that replays a different payload passes against today's code and is vacuous. The control is the inverse: without (B), replaying the identical payload after a host-style latch raises nothing (assert this in a [Fact] that documents it as the pre-fix behaviour by calling the manager's Deliver directly with an unchanged payload and no prior stale — it must stay suppressed, proving suppression still exists for the steady state).
Alternatives considered: Clear suppression memory on reconnect only — rejected (non-retained topics keep stale-Good in the cache and in reads). Re-raise cached snapshots on Reconnected as Good — rejected (asserts Good for a value the session has not re-observed). Poll-style periodic re-publish — rejected (unconfigurable traffic class, the #523 lesson). Stale on ShutdownAsync/teardown — rejected: teardown also runs for a config reinit on a healthy broker and would flip every non-retained tag Bad on each redeploy.
Implementation steps
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs—ConnectionLostevent + fire inOnDisconnectedAsyncon the Connected→Reconnecting edge; xmldoc states the dispatcher-thread constraint.src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs—SubscribeAsyncinitial push;OnConnectionLost;AttachTo/detach wiring; the suppression comment at:752-761gains "initial push + connection-loss stale are what make this safe".src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs— same two; rewrite theResetSessionStateremark.src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/LastValueCache.cs— optionalSnapshot()/ForEachObservedenumerator so the stale pass does not need the authored table (keep it minimal).docs/drivers/Mqtt.md— "Quality on connection loss and re-subscribe" section.
Tests
(tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; manager tests drive Deliver/SubscribeAsync directly as the existing suite does; connection tests use MiniBroker.)
MqttSubscriptionManagerRecoveryTests.cs(new):- seed cache via
Deliver(payload 42);SubscribeAsync([ref])⇒OnDataChangeraised once with42 Goodattributed to the new handle (initial push); a second identicalDeliver(42)⇒ not raised (steady-state suppression intact — the control); OnConnectionLost()⇒ raised42 / BadNoCommunication(value retained), cache reads the same;Deliver(42)(the retained replay) ⇒ raised42 / Good— the identical-value recovery; a ref never observed is not raised by the stale;- unsubscribe + re-
SubscribeAsync⇒ initial push again (rebuild shape).
- seed cache via
SparkplugIngestorRecoveryTests.cs(new): DBIRTH seeds metricm=7;SubscribeAsync⇒ initial push7 Good;OnConnectionLost()⇒7 / BadNoCommunication; reconnect path (OnReconnectedAsync+ replayed DBIRTH withm=7) ⇒7 / Goodraised; DDEATH still stales as before (regression).MqttConnectionTestsaddition (MiniBroker): broker drops the session ⇒ConnectionLostfires exactly once per Connected→Reconnecting edge, not on initial-connect failure, not after dispose.MqttDriverSessionLifecycleTestsaddition: end-to-end throughMqttDriver— connect, deliver, kill broker ⇒ driverOnDataChangecarriesBadNoCommunication; broker back + retained replay of the identical payload ⇒ Good raised.
Effort + Risk
Effort: M (two managers + transport event + four test files). Risk: medium — behaviour change on the MQTT data plane (tags now flip Bad on a broker drop, and re-subscribe produces a burst). The initial-push burst on a large Sparkplug group is bounded by the authored set, identical in size to a rebirth. The ConnectionLost handler runs on MQTTnet's dispatcher — keep it allocation-light and never await in it (pin with a test that the handler completes synchronously).
Finding 4 — 02/S14: VT path stamps every dependency Good (Medium)
Restatement
The virtual-tag actor stores dependency values only, evaluates unconditionally, and the Roslyn evaluator's read cache fabricates Good + eval-time timestamps for every input — so a VT over a comms-lost input keeps publishing fresh-Good results, and a script author cannot detect it because ctx.GetTag(path).StatusCode reads the fabricated Good. R2-03 fixed "broken script looks Good"; "broken input looks Good" is the same lie through the other door.
Verification
Confirmed exact (table). R2-03 decisions this plan keeps: Bad published once per transition with the last-known value; recovery bypasses value-dedup; the inputs-ready gate (AllDependenciesArrived) suppresses Bad on cold start/respawn; EvaluationResult.StatusCode rides verbatim through the host bridge and is historized for non-reassert results; ReassertValue re-emits the last published value/status after a deploy reset. R2-03 explicitly left the mux "untouched — quality flows child→parent→publish actor; the mux only fans values into children"; this plan does not change the mux either — it changes what the child does with the status the mux already delivers.
Root cause
Same as STAB-19: the consumer's storage shape (Dictionary<string, object?>) predates the mux carrying status, and the doc at :36 turned the omission into a stated behaviour.
Proposed design
Apply the same policy as Finding 2 (hold on Bad / propagate worst / evaluate through Uncertain), expressed in the VT actor's existing R2-03 vocabulary:
_dependencies : Dictionary<string, DataValueSnapshot>(value + status + timestamps).OnDependencyChanged/OnDependencyBatchstore snapshots.Evaluate(ts):worst = InputQuality.WorstOf(deps of _dependencyRefs)before anything else. If!AllDependenciesArrived()⇒ return (unchanged warm-up). IfIsBad(worst)⇒OnInputsBad(worst, ts): publishEvaluationResult(last value or null, worst)once per change of the published status (generalise_lastPublishedBad: bool→_lastPublishedStatus: uint), meterVirtualTagEval outcome:"inputs-bad", no script-log entry (inputs being Bad is not a script fault; the script log stays the per-failure diagnostic R2-03 defined). Otherwise evaluate; on success publish(value, worst)with dedup keyed on (value, status) —if (_lastPublishedStatus == worst && _hasLastValue && Equals(_lastValue, result.Value)) skip— which is R2-03's "bypass while recovering from Bad" made general (it now also covers Uncertain→Good and Bad-subcode changes). Script failure keepsOnEvaluationFailed(BadInternalError) unchanged.OnReassertValuere-emits_lastPublishedStatus(so a deploy reset of a VT whose input is comms-lost re-assertsBadNoCommunication, notGood).IVirtualTagEvaluatorgainsEvaluate(id, expr, IReadOnlyDictionary<string, DataValueSnapshot>)with a default implementation that projects to values and calls the existing overload (the 5 fakes compile unchanged; the actor calls the new overload).RoslynVirtualTagEvaluator.BuildReadCache :165-178passes snapshots through — real status, real source timestamps — soctx.GetTag(path).StatusCodeis honest for Uncertain inputs (Bad never reaches the script under hold-on-Bad).- Fix the
DependencyValueChangeddoc (:33-38): it no longer says the VT engine ignores it.
Alternatives considered: same set as Finding 2, same resolution. One VT-specific option — publish the computed value with Uncertain on Bad inputs (the report's option b) — rejected for the same NRE/honesty reasons, and because it would make the VT path disagree with the Calculation driver, whose scripts share VirtualTagContext and the Monaco editor; one policy for both keeps docs/ScriptEditor.md true for every ctx.GetTag call site.
Implementation steps
src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IVirtualTagEvaluator.cs— snapshot overload (default-implemented).src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs— override;BuildReadCachepass-through.src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs— snapshot map, worst-of gate,_lastPublishedStatus,OnInputsBad, dedup key, reassert, docs.docs/ScriptEditor.md(ctx.GetTag(...).StatusCodeis real; scripts are not run while an input is Bad; outputs inherit the worst input quality) +docs/Uns.mdVT section pointer to the contract.
Tests
tests/Server/…Runtime.Tests/VirtualTags/VirtualTagActorInputQualityTests.cs(new — the contract test, fake evaluator counting calls):- deps arrive Good ⇒
EvaluationResult Good(regression);aarrivesBadNoCommunication/null ⇒ no evaluator call, ONEEvaluationResult(lastValue, 0x80310000); repeat Bad ⇒ nothing; Bad with a different sub-code ⇒ one more result with the new code; - recovery with the identical computed value ⇒
EvaluationResult Goodpublished (the (value,status) dedup key) — the vacuity-sensitive assertion; a same-value recovery that is NOT republished is exactly the v3-gate "Bad forever" trap, so this test is the guard and must be run once against a build with the dedup key reverted to value-only to see it fail before merge; - Uncertain input ⇒ evaluated, result stamped Uncertain;
ctx.GetTag("a").StatusCodeobserved by a capturing fake evaluator equals the input's code (not 0); - cold start:
bnever arrives,aBad ⇒ no result (inputs-ready gate preserved); after a deployReassertValuere-emits the last status (Bad input ⇒ Bad reassert); - R2-03 script-failure cases (
VirtualTagActorTeststransition tests) unchanged and green.
- deps arrive Good ⇒
VirtualTagHostActorTestsaddition: aBadNoCommunicationresult is bridged verbatim toAttributeValueUpdateand historized with that status (parity with S13's Bad); a reassert is not historized (unchanged).
Effort + Risk
Effort: S-M. Risk: low — actor-local; the interface change is default-implemented; every Good-path assertion in the existing suites stays byte-identical.
Live gate (docker-dev) — required before merge
Rig: docker-dev/docker-compose.yml (AdminUI http://localhost:9200, login disabled; OPC UA opc.tcp://localhost:4840 central-1 / :4841 central-2 — rebuild both centrals, :9200 round-robins). Modbus fixture on the shared host: ssh dohertj2@10.100.0.35 'cd /opt/otopcua-modbus && docker compose --profile standard up -d' (container otopcua-pymodbus-standard, 10.100.0.35:5020, reachable from the rig's containers).
Why this gate is non-vacuous: HR[5] in the standard profile is config-seeded to the constant 5 and restored to 5 on a container restart, so its value is identical before and after the outage — a Good re-publish of HR[5] can only come from a force-republish, never from a value change. HR[100] auto-increments and is the positive control (it would recover even with the dedup trap present; if HR[100] recovers and HR[5] does not, the trap is present).
Authoring (AdminUI, or the headless POST /api/deployments + X-Api-Key recipe from the MTConnect memory):
/raw→ MAIN cluster → FolderPlant→ DriverModbus(Modbus, endpoint10.100.0.35:5020, unit 1, poll 1000 ms) → Devicesim→ tagsHR5(holding register 5, UInt16) andHR100(holding register 100, UInt16). No deadband./uns→ AreaA/ LineL/ EquipmentE1→ Tags tab → "+ Add reference" →Plant/Modbus/sim/HR5(UNS mirror)./raw→ DriverCalc(Calculation) → tagHR5x2with scriptreturn (int)ctx.GetTag("Plant/Modbus/sim/HR5").Value * 2;(dependency on HR5)./uns→ onE1a VirtualTagVT5=return (int)ctx.GetTag("Plant/Modbus/sim/HR5").Value + 1;.- Deploy; confirm all four nodes read Good:
dotnet run --project src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI -- read -u opc.tcp://localhost:4840 -n "ns=<raw>;s=Plant/Modbus/sim/HR5"(and the UNSs=A/L/E1/HR5, raws=Plant/Calc/…/HR5x2, UNSs=A/L/E1/VT5). Usebrowse -r -d 4to get the exact ids/namespace indexes.
Run:
- Start four
subscribesessions (or one with-rovers=Plant+ one overs=A) at-i 500; note HR5 = 5 Good, HR100 counting. ssh dohertj2@10.100.0.35 'docker stop otopcua-pymodbus-standard'. Expected, in order: within ~1–2 s HR5/HR100 (raw + UNS) go Bad 0x80050000 (BadCommunicationError), value null — Modbus's own per-tag degrade (pre-existing behaviour, confirms the publish path); the calcHR5x2andVT5go Bad 0x80050000 with their last value (10 / 6) — STAB-19 + S14 propagating the input's sub-code; within ≤ 90 s central-1's log showsdeclaring comms lost and reconnectingand the value nodes re-stampBadNoCommunication0x80310000 (S15 — visible as a status change on the subscription), calc/VT follow withBadNoCommunication./hostsshows the driver Reconnecting. Both the status and the fact that the last value is retained on the calc/VT nodes are the assertions.- Hold the outage > 2 min (two watchdog windows) to see no churn beyond the retry log lines (S17 is R3-03's; note but do not fail on a re-trip).
ssh dohertj2@10.100.0.35 'docker start otopcua-pymodbus-standard'. Expected within ~15 s (retry 10 s + first poll): HR5 re-publishes5 Goodon raw AND UNS without its value having changed (the force-republish);HR5x2re-publishes10 Good,VT56 Good— same values as before the outage. HR100 recovers too (positive control). Nocomms_latched_refsWarning in the log.- Falsification leg (required): temporarily set a
deadbandof 1000 on HR5 in/rawand redeploy; repeat 7→9. Modbus clears its deadband cache inTeardownAsync, so HR5 must still recover — if it does not, the driver-side cache survived the reconnect and the audit Warning… still BadNoCommunication 30s after reconnect …must appear in the log with count 1 andotopcua.driver.comms_latched_refsincremented. Either outcome is informative; a silent non-recovery with no Warning fails the gate. - Operator restart leg (after R3-03 is merged):
/hosts→ Restart driver onModbus⇒ nodes goBadNoCommunication(T11's PostStop Tell) then recover Good with unchanged values (R3-03's re-subscribe + the poll initial push). - MQTT leg (
docker-dev/docker-compose.mqtt.yml+ the Mosquitto fixture): author one retained JSON topic tag;docker stopthe broker's network path (ordocker pause otopcua-mosquitto) ⇒ tagBadNoCommunicationwithin the MQTTnet keep-alive;unpause⇒ retained replay with the identical payload ⇒Goodre-publish. Then redeploy (rebuild) with the broker quiet ⇒ the rebuilt node shows the cached value immediately (initial push), notBadWaitingForInitialData.
Record outcomes + status-code captures in archreview/plans/STATUS.md.
Task breakdown
Branch: fix/archreview-r3-02-stale-good-family off master, after R3-03 merges (zero textual overlap by construction; if R3-03 slips, land this first and R3-03 rebases — the only shared file regions are the Receive table in DriverHostActor, which both plans append to). Commits grouped per block; blocks B (MQTT), C (S15), D (Calculation), E (VT) touch disjoint files and may run in parallel worktrees but merge in the order B → C, D/E anywhere (resolved choice #4).
Verification bar: every task has a deterministic unit guard; the recovery tasks (T5, T10, T13, T16) carry an explicit vacuity note and a named falsification variant; T19 is the rig gate. TDD throughout: write the failing test, see it FAIL, implement, see it PASS.
T1 — InputQuality.WorstOf shared helper + alarm-engine re-point
Classification: standard (pure helper; alarm engine refactor behind its suite)
Estimated implement time: 5 min
Parallelizable with: T2–T11 (different files); T12–T16 depend on it
Files: src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/InputQuality.cs (new), src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs:923-941, tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/InputQualityTests.cs (new)
- Truth-table Theory (all-Good ⇒ Good; one Uncertain ⇒ that code; Bad beats Uncertain; null entry ⇒
BadWaitingForInitialData; equal severity ⇒ first seen;IsUsableGood/Uncertain true, Bad false).dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests --filter "FullyQualifiedName~InputQualityTests"→ FAIL. - Implement; re-point
WorstInputStatus(keep its never-pushed skip). Filter +ScriptedAlarmEngineTests→ PASS. - Commit:
feat(core): InputQuality.WorstOf — the one worst-of-input rule (archreview R3-02 contract).
T2 — MQTT plain-mode initial push on SubscribeAsync
Classification: medium-risk (data-plane burst on subscribe)
Estimated implement time: 5 min
Parallelizable with: T1, T6–T16
Files: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs, tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerRecoveryTests.cs (new)
- Tests: seeded cache +
SubscribeAsync⇒ one raise per observed ref with the cached snapshot on the new handle; never-observed ref ⇒ no raise; identicalDeliverafterwards ⇒ suppressed (control). FilterFullyQualifiedName~MqttSubscriptionManagerRecoveryTests→ FAIL. - Implement the push after handle registration. → PASS. Commit:
fix(mqtt): initial push of cached snapshots on (re)subscribe — the push-model forceRaise (05/STAB-18 shape b).
T3 — MqttConnection.ConnectionLost + plain-mode stale to BadNoCommunication
Classification: HIGH-RISK (transport event on MQTTnet's dispatcher; data-plane Bad on broker drop)
Estimated implement time: 5 min + 5 min tests
Parallelizable with: T1, T6–T16; after T2
Files: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs, MqttSubscriptionManager.cs, MqttConnectionTests.cs, MqttSubscriptionManagerRecoveryTests.cs
- Tests: MiniBroker drop ⇒
ConnectionLostonce per Connected→Reconnecting edge (not on initial failure / after dispose); managerOnConnectionLost()⇒ each observed ref raisedvalue-retained / BadNoCommunication, cache agrees; then identicalDeliver⇒ raised Good (the recovery; identical payload is load-bearing — a different payload is vacuous). → FAIL. - Implement event + handler +
AttachTowiring; handler must not await (assert completes synchronously). → PASS. Commit:fix(mqtt): stale observed refs BadNoCommunication on connection loss so the identical replay recovers (05/STAB-18 shape a).
T4 — Sparkplug ingestor: initial push + connection-loss stale
Classification: medium-risk
Estimated implement time: 5 min
Parallelizable with: T1, T6–T16; after T3 (uses the event)
Files: Sparkplug/SparkplugIngestor.cs, tests/…/SparkplugIngestorRecoveryTests.cs (new)
- Tests: DBIRTH seeds;
SubscribeAsyncinitial push;OnConnectionLost⇒BadNoCommunicationvalue retained; reconnect + identical DBIRTH replay ⇒ Good; DDEATH regression. → FAIL. - Implement; rewrite the
ResetSessionStateremark (:583-588). → PASS. Commit:fix(mqtt/sparkplug): initial push on subscribe + BadNoCommunication on connection loss.
T5 — MQTT driver-level recovery test + docs/drivers/Mqtt.md
Classification: standard (test + docs)
Estimated implement time: 5 min
Parallelizable with: T6–T16; after T4
Files: tests/…/MqttDriverSessionLifecycleTests.cs, docs/drivers/Mqtt.md
- End-to-end through
MqttDriverwith MiniBroker: deliver → broker drop →BadNoCommunicationraised → broker back + identical retained replay → Good raised. Vacuity note in the test header. → PASS (after T2–T4). - Docs section "Quality on connection loss and re-subscribe". Commit:
test+docs(mqtt): end-to-end Bad-then-identical-replay recovery.
T6 — Sink primitive WriteStatuses (interface default + node manager + SDK sink + deferred forward)
Classification: HIGH-RISK (new sink contract member; node-manager SDK mutation under Lock)
Estimated implement time: 5 min interface + 5 min node manager
Parallelizable with: T1–T5, T12–T16; T7–T10 depend on it
Files: Commons/OpcUa/IOpcUaAddressSpaceSink.cs, Commons/OpcUa/DeferredAddressSpaceSink.cs, OpcUaServer/OtOpcUaNodeManager.cs, OpcUaServer/SdkAddressSpaceSink.cs, tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OtOpcUaNodeManagerWriteStatusesTests.cs (new)
- Add
StatusWrite+ defaultWriteStatusesto the interface; rundotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests --filter "FullyQualifiedName~DeferredSinkForwardingReflectionTests"→ FAIL (guard sees an unforwarded member). Forward inDeferredAddressSpaceSink→ PASS. - Node-manager tests (value retained, status+timestamp change, unknown id no-op, tail-isolation) → FAIL; implement
WriteStatuses/WriteStatusLocked+SdkAddressSpaceSinkforward → PASS; fullOpcUaServer.Tests. - Commit:
feat(opcua): IOpcUaAddressSpaceSink.WriteStatuses — status-only batch write retaining the node value (03/S15).
T7 — OpcUaPublishActor.AttributeStatusBatchUpdate
Classification: standard
Estimated implement time: 5 min
Parallelizable with: T1–T5, T12–T16; after T6
Files: Runtime/OpcUa/OpcUaPublishActor.cs, tests/Server/…Runtime.Tests/OpcUa/OpcUaPublishActorStatusBatchTests.cs (new)
- Test: message ⇒
sink.WriteStatuseswith the same items; partial failure ⇒ Warning + tally. → FAIL. Implement handler +OpcUaSinkWrite kind:"status". → PASS. Commit:feat(runtime): publish actor routes status-only batches to the sink.
T8 — DriverHostActor.FanCommsLossToValueNodes (raw + UNS, one batch)
Classification: HIGH-RISK (data-plane behaviour change: value nodes go Bad on comms loss)
Estimated implement time: 5 min
Parallelizable with: T1–T5, T12–T16; after T7. R3-03 boundary: new method + one-line call in OnDriverConnectivityChanged; HandleRestartDriver and the watchdog untouched.
Files: Runtime/Drivers/DriverHostActor.cs, tests/Server/…Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs (new)
- Tests (publish probe, 2 raw tags + 1 UNS ref): disconnect ⇒ ONE
AttributeStatusBatchUpdate= {raw×2 (Raw), UNS mirror (Uns)} allBadNoCommunication; second driver's refs absent (control); reconnect ⇒ExpectNoMsg. → FAIL. - Implement + xmldoc updates at
:1223-1247. → PASS;DriverHostActorCommsLossTests+DriverHostActorBatchFanOutTestsunchanged green. Commit:fix(runtime): comms loss marks raw + UNS value nodes BadNoCommunication, value retained (archreview 03/S15).
T9 — Recovery audit: _commsLatched + CommsRecoveryAudit timer + otopcua.driver.comms_latched_refs
Classification: medium-risk (hot-path dictionary probe in ForwardBatchToMux)
Estimated implement time: 5 min
Parallelizable with: T1–T5, T12–T16; after T8
Files: DriverHostActor.cs, Commons/Observability/OtOpcUaTelemetry.cs, DriverHostActorCommsLossValueNodesTests.cs
- Tests (audit delay injected 300 ms): disconnect then reconnect with a value batch covering ALL latched refs ⇒ no Warning, no meter; covering NONE ⇒ Warning names driver + count, meter increments,
_commsLatchedentry removed afterwards (assert via a second disconnect re-latching cleanly). → FAIL. - Implement (set on loss, prune per routed item only while non-empty, single timer per driver keyed
comms-audit:{id}). → PASS. Commit:feat(runtime): audit refs still BadNoCommunication after a reconnect (dedup-trap tripwire).
T10 — S15 end-to-end recovery contract test (real DriverInstanceActor, identical-value initial push) + falsification control
Classification: high-risk verification (guards T8/T9 against the v3-gate dedup trap)
Estimated implement time: ~15 min (the one deliberately larger task — scripted fake driver + watchdog timing)
Parallelizable with: T1–T5, T12–T16; after T9
Files: tests/Server/…Runtime.Tests/Drivers/DriverHostCommsLossRecoveryTests.cs (new)
- Scripted
IDriver+ISubscribablefake (stable health;LastSuccessfulReadcontrollable;SubscribeAsyncpushes a configured initial snapshot): publish5 Good→ freeze read + report Degraded → host status batch observed → fake connect succeeds + initial push5 Good(identical) ⇒AttributeValueBatchUpdate5 Goodfor raw + UNS arrives after the status batch; no audit Warning. - Control
[Fact]: fake with a silentSubscribeAsync⇒ no Good batch, audit Warning + meter. Header comment states the vacuity rule (a different-value push is not the guard). - Commit:
test(runtime): comms-loss → identical-value recovery contract + silent-resubscribe control (03/S15).
T11 — DriverInstanceActor.PostStop announces comms loss
Classification: medium-risk (additive Tell in PostStop; interacts with R3-03's restart re-subscribe)
Estimated implement time: 5 min
Parallelizable with: all; after T8. Not in the watchdog method; after R3-03 merges so the restart leg of the live gate recovers.
Files: Runtime/Drivers/DriverInstanceActor.cs (PostStop), DriverInstanceActorTests.cs
- Test: stop a Connected child ⇒ parent probe receives
ConnectivityChanged(id, false)beforeTerminated. → FAIL. Implement. → PASS. Commit:fix(runtime): a stopping driver child announces comms loss so restart/respawn latches its nodes Bad.
T12 — DependencyConsumerMuxAdapter forwards the real StatusCode
Classification: standard (one-line fix + stale-comment removal, theme 6)
Estimated implement time: 5 min
Parallelizable with: T1–T11; T13 depends on it
Files: Runtime/VirtualTags/DependencyConsumerMuxAdapter.cs:70-73, tests/Server/…Runtime.Tests/VirtualTags/DependencyConsumerMuxAdapterTests.cs (new or extend)
- Test: single + batch delivery with
BadNoCommunicationreaches the consumer verbatim. → FAIL. Implement; delete the "Quality is stripped" comment. → PASS. Commit:fix(runtime): mux adapter forwards the dependency's real status code (05/STAB-19).
T13 — CalculationDriver input quality: hold on Bad, propagate worst, status-keyed dedup
Classification: HIGH-RISK (calc output semantics change on non-Good inputs)
Estimated implement time: 5 min per case — split across commits if needed
Parallelizable with: T2–T11, T15–T16; after T1 + T12
Files: Driver.Calculation/CalculationDriver.cs, tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Calculation.Tests/CalculationDriverInputQualityTests.cs (new)
- Tests per Finding 2 §Tests (no-eval on Bad; transition-once; identical-value recovery republished — vacuity-sensitive; Uncertain evaluated + stamped; cold-start gate; passthrough over Bad;
ReadAsyncparity). → FAIL. - Implement snapshots,
LastPublishedStatus,MarkInputsBad, readiness,ReadOne. → PASS;CalculationDriverTests+CalculationDependencyFlowTestsgreen. Commit:fix(calculation): hold evaluation on Bad inputs and propagate the worst input status; status-keyed dedup (05/STAB-19).
T14 — CalculationEvaluator snapshot overload (honest ctx.GetTag().StatusCode/timestamps)
Classification: standard
Estimated implement time: 5 min
Parallelizable with: T2–T11, T15–T16; after T13
Files: Driver.Calculation/CalculationEvaluator.cs, CalculationEvaluatorTests.cs
- Test: script reading
ctx.GetTag("a").StatusCodesees the Uncertain code + the real source timestamp. → FAIL. Implement overload + pass-throughBuildReadCache+ passthrough fast path. → PASS. Commit:fix(calculation): read cache carries real input status + timestamps.
T15 — IVirtualTagEvaluator snapshot overload + Roslyn BuildReadCache pass-through
Classification: medium-risk (Commons interface; default-implemented so fakes compile)
Estimated implement time: 5 min
Parallelizable with: T2–T14; T16 depends on it
Files: Commons/Engines/IVirtualTagEvaluator.cs, Host/Engines/RoslynVirtualTagEvaluator.cs:165-178, Host engine tests
- Test: Roslyn evaluator given a snapshot map — script reads the real status/timestamp. → FAIL. Implement. → PASS; all 5 fakes compile untouched. Commit:
feat(engines): IVirtualTagEvaluator snapshot overload; Roslyn read cache stops fabricating Good (02/S14).
T16 — VirtualTagActor honours input quality (hold on Bad / worst-of / (value,status) dedup / reassert status)
Classification: HIGH-RISK (VT output semantics on non-Good inputs; touches R2-03's transition machinery)
Estimated implement time: 5 min per case — split across commits
Parallelizable with: T2–T14; after T1 + T15
Files: Runtime/VirtualTags/VirtualTagActor.cs, tests/Server/…Runtime.Tests/VirtualTags/VirtualTagActorInputQualityTests.cs (new), VirtualTagHostActorTests.cs (historize parity case)
- Tests per Finding 4 §Tests (no-eval on Bad; once per status change; identical-value recovery republished — run once against a value-only dedup key to see it FAIL; Uncertain evaluated + stamped; inputs-ready gate; reassert carries status; host bridge historizes the Bad-input result). → FAIL.
- Implement; fix the
DependencyValueChangeddoc (:33-38). → PASS;VirtualTagActorTests/VirtualTagHostActorTests(R2-03) green. Commit:fix(runtime): virtual tags hold on Bad inputs and inherit the worst input status (archreview 02/S14).
T17 — Docs: the contract stated once + per-seam pointers + CLAUDE.md touch
Classification: standard (docs)
Estimated implement time: 5 min
Parallelizable with: all
Files: docs/DriverLifecycle.md (new §"Quality contract" quoting §The quality contract verbatim; the comms-loss fan-out section gains the value-node half + the audit meter), docs/Raw.md + docs/Uns.md (one paragraph each, link), docs/ScriptEditor.md (GetTag status is real; scripts do not run on Bad inputs), docs/drivers/Calculation.md, docs/drivers/Mqtt.md (if not done in T5), CLAUDE.md (one bullet under the comms-loss watchdog section pointing at the contract + the comms_latched_refs meter + the "value nodes now go Bad" consumer note)
Commit: docs: the stale-Good quality contract, stated once (R3-02).
T18 — Full-suite sweep + STATUS/report bookkeeping
Classification: standard
Estimated implement time: 5 min (+ suite wall-clock)
Parallelizable with: nothing (after T1–T17)
Files: archreview/plans/STATUS.md, the four source reports' prior-finding tables
dotnet test ZB.MOM.WW.OtOpcUa.slnx (unit legs); Runtime.Tests run 3× for the known flake profile (#500); record. Commit: docs(archreview): R3-02 status.
T19 — LIVE GATE: docker-dev comms-loss → identical-value recovery (Modbus const HR[5] + calc + VT + MQTT retained) — operator/rig task
Classification: HIGH-RISK verification gate (merge-blocking) Estimated implement time: ~45 min rig session Parallelizable with: nothing (needs the built branch; restart leg needs R3-03 merged) Files: none (findings recorded in STATUS.md)
Run the twelve-step recipe in §Live gate. Pass criteria: (7) value nodes + calc + VT Bad with retained values and the BadNoCommunication re-stamp at the watchdog; (9) HR[5] = 5 re-published Good on raw AND UNS with no value change, calc/VT follow with unchanged values; (10) deadband leg recovers or is caught by the audit Warning; (12) MQTT identical retained replay recovers and a rebuild shows the cached value. Record status-code captures from subscribe output.
Sequencing summary
T1 ───────────────────────────────┐
T2 → T3 → T4 → T5 (B: MQTT — MUST merge before C)
T6 → T7 → T8 → T9 → T10 (C: S15 host; T11 after T8 AND after R3-03 merges)
T1 + T12 → T13 → T14 (D: Calculation)
T1 + T15 → T16 (E: VT)
T17 anytime; T18 after all; T19 last
Overall effort: ~2 dev-days code+unit, plus the rig session. The three HIGH-RISK data-plane slices (T3 MQTT stale, T8 value-node Bad, T13/T16 hold-on-Bad) each land behind their full suites; the two recovery contract tests (T10, T16's identical-value case) must each be seen to FAIL once against a reverted dedup before merge — that is the whole reason this family re-appeared.
Execution notes
Branch fix/r3-02-stale-good-family (worktree .claude/worktrees/agent-a696a39981023fa0b), off master
72329eab. Executed in plan block order T1, B (T2–T5), C (T6–T10), D (T12–T14), E (T15–T16), T17, T18.
T11 and T19 are deliberately NOT in this branch — T11 waits for the sibling R3-03 branch to merge
(it is the only edit that would touch DriverInstanceActor lifecycle alongside R3-03's restart work),
and T19 is the operator rig gate.
Red-before observations are recorded per task below: what was run, and the failure text that proves the test could only pass once the production change landed.
T1 — InputQuality.WorstOf
- RED:
dotnet test tests/Core/…Core.Abstractions.Tests --filter "FullyQualifiedName~InputQualityTests"⇒ build failure, 8 ×error CS0103: The name 'InputQuality' does not exist in the current context. - GREEN: 13 passed.
Core.ScriptedAlarms.Tests94/94 green withWorstInputStatusre-pointed (the never-pushed/null skips stay in the alarm engine — they are alarm policy, not the shared rule).
T2 — MQTT plain-mode initial push on SubscribeAsync
- RED:
--filter "FullyQualifiedName~MqttSubscriptionManagerRecoveryTests"⇒ 4 failed, each onraised.Count(0 observed, 1 expected) /raised.Select(r => r.Reference)—SubscribeAsyncraised nothing at all for an already-observed ref. - GREEN: 77
MqttSubscriptionManager*tests pass, incl. the suppression control (an identical message after the push is still swallowed). FullDriver.Mqtt.Tests: 790/790.
T3 — MqttConnection.ConnectionLost + plain-mode stale
- RED:
--filter "…MqttSubscriptionManagerRecoveryTests|…ConnectionLost"⇒ build failure,CS1061: 'MqttSubscriptionManager' does not contain a definition for 'OnConnectionLost'(×5) andCS1061: 'MqttConnection' does not contain a definition for 'ConnectionLost'(×3). - GREEN: 11/11 (8 manager recovery + 3 connection edge). Full
Driver.Mqtt.Tests: 797/797. - The event fires inside the existing
State == Connectedguard inOnDisconnectedAsync, which is what makes "not on an initial-connect failure" and "not on dispose" true by construction rather than by a second condition —DisposeAsyncsetsDisposedbefore tearing the client down.
T4 — Sparkplug ingestor: initial push + connection-loss stale
- RED:
--filter "…SparkplugIngestorRecoveryTests"⇒ build failure,CS1061: 'SparkplugIngestor' does not contain a definition for 'OnConnectionLost'(×2). - GREEN: 6/6. Full
Driver.Mqtt.Tests: 803/803. - ⚠️ Deviation from the plan's test rationale (recorded, not silently accepted): the plan asks for
the identical-DBIRTH-replay assertion as this seam's vacuity guard. It is present, but it is not a
falsifier here — Sparkplug ingestion has no unchanged-value suppression at all (births/data
publish directly, never through
Deliver), so that replay was already raised Good before the change. That is now asserted explicitly by an added control,An_identical_birth_replay_is_raised_even_without_a_preceding_stale, and the suite header says so. The load-bearing assertion on this seam is the stale itself. - The stale reports
BadNoCommunicationwhile a DDEATH keepsBadCommunicationError: the death is the source announcing its own offline state, the stale is our transport losing the broker (and matches what the host's S15 fan-out writes). The existing const's "no driver emits BadNoCommunication at all" remark was corrected rather than left to rot.
T5 — MQTT driver-level end-to-end recovery + docs
- The test is green on first run by construction (T2–T4 already landed), so the plan's TDD bar was met
the other way: the wiring was deliberately broken and the test seen to fail. Deleting
connection.ConnectionLost += OnConnectionLost;fromMqttSubscriptionManager.AttachTo⇒ABrokerDrop_MarksCachedRefsBadNoCommunication_AndTheIdenticalReplayRecoversFAILED withTimed out waiting for the broker drop to publish a BadNoCommunication snapshot(15 s). Restored, green. That is the point of this test: the manager-level suites driveOnConnectionLost()directly and stay green against an unforwarded seam. - The outage edge is a real socket drop through MiniBroker → MQTTnet
DisconnectedAsync; ingest is fed throughSubscriptions.HandleMessagebecause MiniBroker answers CONNECT/PINGREQ only and cannot PUBLISH. - Docs:
docs/drivers/Mqtt.md§"Quality on connection loss and re-subscribe" (links the contract inDriverLifecycle.md, records the two distinct "not communicating" codes and the operator-visible behaviour change).
T6 — IOpcUaAddressSpaceSink.WriteStatuses (interface default + node manager + both sinks)
- RED (the guard, first): adding the member to the interface and running
--filter "…DeferredSinkForwardingReflectionTests"⇒Every_forwarding_interface_method_reaches_the_inner_sinkFAILED. Note the failure surfaced asSystem.ArgumentNullException : Value cannot be null. (Parameter 'statuses')rather than the guard's "did NOT forward" message — the reflection harness passesnullfor anIReadOnlyList<>parameter, so the interface default ran on the wrapper and threw. Red for the right reason (the member was not forwarded); green onceDeferredAddressSpaceSink.WriteStatusesforwards. This is the memory'd "Deferred sink doesn't forward the new member" trap, and here the default is not merely slower — it writes a null value, erasing the very value the annotation exists to keep. - RED (node manager):
--filter "…OtOpcUaNodeManagerWriteStatusesTests"⇒ build failure,CS1061: 'OtOpcUaNodeManager' does not contain a definition for 'WriteStatuses'(×5). - GREEN: 5/5 new; full
OpcUaServer.Tests495 passed / 1 skipped;Commons.Tests348/348. - An unmaterialised NodeId is skipped and not counted as a failure (Debug tally via the SDK static
trace — the manager has no
ILogger): the caller's per-driver index racing a surgical remove is an expected shape, not a fault.
T7 — OpcUaPublishActor.AttributeStatusBatchUpdate
- RED:
--filter "…OpcUaPublishActorStatusBatchTests"⇒ build failure,CS0426: The type name 'AttributeStatusBatchUpdate' does not exist in the type 'OpcUaPublishActor'(×3). - GREEN: 58
OpcUaPublishActor*tests pass (3 new). - Metered
OpcUaSinkWrite kind:"status"so a status annotation is separable from a value write on the same counter; the recording fake asserts zero value writes, since a handler falling through to the value path would null every node.
T8 — DriverHostActor.FanCommsLossToValueNodes
- RED:
--filter "…DriverHostActorCommsLossValueNodesTests"⇒ 2 failed, bothTimeout 00:00:15 while waiting for a message of type …AttributeStatusBatchUpdate(the disconnect produced no value-node write at all — the finding, reproduced). - GREEN: 10 passed across the new suite +
DriverHostActorCommsLossTests(mux half) +DriverHostActorBatchFanOutTests, all unchanged. - Deviation (sequencing, not design): the plan's snippet calls
TrackCommsLatchSet(...)from inside this method, but that helper is T9's — calling it here would not compile. The call is added in T9, where the latch and its audit timer land together. R3-03 boundary respected: this is a new method plus one branch inOnDriverConnectivityChanged;HandleRestartDriverand the watchdog are untouched.
T9 — recovery audit (_commsLatched + timer + otopcua.driver.comms_latched_refs)
- RED:
--filter "…DriverHostActorCommsLossValueNodesTests"⇒ build failure,CS1739: The best overload for 'Props' does not have a parameter named 'commsRecoveryAuditDelay'. - GREEN: 6/6 in the suite; 20 passed across it +
DriverHostActorCommsLossTests+DriverHostActorBatchFanOutTests+DriverHostActorPrimaryGateTests. - Falsification of the control (the plan's discipline, applied to the audit rather than to a dedup
key): deleting the prune loop in
ForwardBatchToMux⇒A_driver_that_republishes_every_ref_trips_no_auditFAILED withReceived 1 message too many. Expected 0 messages but received 1 that matched filter [Warning when Message contains "still BadNoCommunication"]. Restored, green. So the "no audit fires" assertion is falsifiable and the audit is not merely dormant. - Meter assertions filter on a per-test driver tag, not on the instrument name alone (#503: a
MeterListenersees every measurement in the process). - The latch counts RawPaths, so the raw node and its UNS mirrors report as one ref — the operator is told how many tags did not come back, not how many nodes.
T10 — S15 end-to-end recovery contract + silent-resubscribe control
- RED:
--filter "…DriverHostCommsLossRecoveryTests"⇒ 2 failed,Timeout 00:00:15 while waiting for a message of type …AttributeValueBatchUpdate(beforeAttributeStatusBatchUpdateeven existed in the first draft, and again while the fake's instance id disagreed — see below). - GREEN: 2/2 (33 s wall clock — see the reconnect-interval note).
Runtime.Tests~Driversfilter: 333 passed / 3 skipped. - Two harness facts worth keeping (both cost a red run to find):
DriverInstanceActortakes its instance id fromdriver.DriverInstanceId, not from the spec the host spawned it with. A fake returning a different id publishes under a key the host's ref map does not contain, and every value is silently dropped as unmapped. The fake now takes the id.- The fake's push-on-subscribe fires on the FIRST subscribe too, so the pre-outage value must come from that push rather than from a separate seeding call — otherwise the probe holds an extra batch.
- Deviation: the plan sketched driving the outage with the #527 watchdog (
commsLossTimeout200 ms). The host spawns its children with the defaultreconnectInterval/healthPollIntervaland exposes no seam for either, so the outage is driven by the productionReconnectDriver(operator Force Reconnect) command instead — sameReconnectingentry, sameConnectivityChanged(false), and it does not touchHandleRestartDriver(R3-03's). The watchdog's own trip condition stays covered byDriverInstanceActorCommsLossWatchdogTests. The 10 sDefaultReconnectIntervalis why these two tests take ~15 s each. - The identical-value rule is honoured: the driver pushes 5.0 Good before and after the outage, so the
recovery assertion cannot be satisfied by a value change. The control (
InitialPush = null) assertsExpectNoMsg— no Good ever arrives — and that the audit fires with exactly 1 latched ref.
T12 — DependencyConsumerMuxAdapter forwards the real status code
- RED:
--filter "…DependencyConsumerMuxAdapterTests"⇒ 2 failed, bothShouldly.ShouldAssertException : consumer.Received[…].StatusCode should be 2150694912u(0x80310000) — the consumer received0(Good) for aBadNoCommunicationinput, on both the single and the batch path. That is the finding, reproduced at the seam. - GREEN: 11 passed (
DependencyConsumerMuxAdapterTests+DependencyMuxActorTests). - The false "quality is stripped at the mux boundary" comment is deleted and replaced with why it was false (theme 6: the contract moved at #478/#519 and the comment outlived the fact).
T13 — CalculationDriver input quality (hold on Bad, propagate worst, status-keyed dedup)
- RED:
--filter "…CalculationDriverInputQualityTests"⇒ 5 failed:Snapshot.StatusCode should be 2150694912u(BadNoCommunication) on the Bad-input, cold-start and passthrough cases;should be 1073741824u(Uncertain) on the Uncertain case;published.Count should be 1on the sub-code-change case. That is the laundering, reproduced from three directions. - GREEN: whole
Driver.Calculation.Testsproject 61/61 — the 9 new plus every pre-existing case unchanged (the Good-only paths are byte-identical). - Vacuity check performed (the plan's requirement): reverting the dedup key to value-only
(
if (st.HasValue && Equals(st.LastValue, value)) return null;) ⇒An_identical_value_is_republished_when_recovering_from_a_bad_inputFAILED withpublished.Count should be 1— i.e. the recovery was swallowed, exactly the "Bad forever" trap. Restored, green. TagState.LastPublishedBad(two-state) becameHasPublished+LastPublishedStatus(uint), which is what lets a Bad→different-Bad transition republish and what makesReadAsyncagree with the subscriber. The evaluator is still handed bare values here — T14 hands it the snapshots.
T14 — CalculationEvaluator snapshot overload
- RED:
--filter "…CalculationEvaluatorTests"⇒ build failure,CS1503: cannot convert from 'Dictionary<string, DataValueSnapshot>' to 'IReadOnlyDictionary<string, object?>'(×3) — no snapshot overload existed. - GREEN:
Driver.Calculation.Tests64/64. BuildReadCacheis deleted, not fixed: the snapshots now pass straight through as the read cache. The value-only overload survives (tests use it) but is now the one that fabricates Good, and its xmldoc says so and points at the snapshot overload.- Docs:
docs/drivers/Calculation.mdgains an "Input quality" section (the policy table + the behaviour change for null-tolerant scripts) and the dedup bullet now says (value, status).
T15 — IVirtualTagEvaluator snapshot overload + Roslyn read-cache pass-through
- RED:
--filter "…RoslynVirtualTagEvaluatorTests"⇒ build failure,CS1503: cannot convert from 'Dictionary<string, DataValueSnapshot>' to 'IReadOnlyDictionary<string, object?>'(×3). - GREEN: 24/24 in that suite; full solution build clean (0 errors), so all five existing
IVirtualTagEvaluatorfakes compile untouched — which is the point of default-implementing the overload. A test pins that fallback explicitly (ValueOnlyEvaluator). RoslynVirtualTagEvaluator.BuildReadCacheis deleted; the snapshots are the read cache. The value-only overload now projects up into the snapshot one, so there is a single evaluation body.
T16 — VirtualTagActor honours input quality
- RED:
--filter "…VirtualTagActorInputQualityTests"⇒ 7 failed, incl.bad.StatusCode should be 2150694912u,result.StatusCode should be 1073741824u(Uncertain),reassert.StatusCode should be 2150694912u, andExpected no messages … instead we received EvaluationResult { … StatusCode = 0 }for the cold-start case — i.e. a Bad input produced a fresh Good result. - GREEN: 58 passed across
VirtualTag*(the 7 new + the R2-03 transition suites + the host bridge's new historize-parity case), all pre-existing cases unchanged. - Vacuity check performed (the plan requires this one explicitly): reverting the dedup key to
value-only ⇒
An_identical_value_is_republished_when_recovering_from_a_bad_inputFAILED withTimeout 00:00:15 while waiting for a message of type …EvaluationResult— the recovery was swallowed entirely. Restored, green. - ⚠️ Deviation from the plan's design, deliberate and load-bearing. The plan says the worst-of-input
walk should treat a never-pushed dependency as
BadWaitingForInitialDataand hold. Implemented literally, that HOLDS evaluation during ordinary warm-up — but this actor (unlike the Calculation driver) has always evaluated on partial arrival and published the partial result, and two existing tests assert exactly that (VirtualTagActorTests.Evaluator_result_flows_to_parent_as_EvaluationResultandDependencyMuxActorTests.VirtualTagActor_PreStart_registers_deps_…_end_to_end, which both went red under the literal reading). The worst-of walk therefore covers arrived deps only — the same skipScriptedAlarmEngine.WorstInputStatushas always made — and cold start stays governed by R2-03's existingAllDependenciesArrivedgate on the publish tails. The plan's own risk note ("every Good-path assertion in the existing suites stays byte-identical") is what this preserves. _lastPublishedBadbecame_hasPublished+_lastPublishedStatus, which is also what makesReassertValuere-emitBadNoCommunicationafter a deploy instead of guessing Good.- Docs:
docs/ScriptEditor.md§"Input quality: what a script sees, and when it runs at all".