fix(runtime): comms loss marks raw + UNS value nodes BadNoCommunication, value retained (archreview 03/S15)

archreview/plans/R3-02-stale-good-family-plan.md

@@ -678,3 +678,15 @@ test could only pass once the production change landed.
 - 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**, both
+  `Timeout 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 in `OnDriverConnectivityChanged`; `HandleRestartDriver` and the watchdog are untouched.

archreview/plans/R3-02-stale-good-family-plan.md.tasks.json

@@ -55,7 +55,7 @@
     {
       "id": "T8",
       "subject": "DriverHostActor.FanCommsLossToValueNodes: one BadNoCommunication status batch over raw + UNS nodes on ConnectivityChanged(false); nothing on reconnect (new method — R3-03 boundary)",
-      "status": "pending",
+      "status": "done",
       "blockedBy": [
         "T7"
       ]

src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs

@@ -1244,13 +1244,28 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
     ///         the source had died. <see cref="FanCommsLossToDependencyMux"/> closes that by pushing a
     ///         synthetic Bad for every value ref the driver owns.
     ///     </para>
+    ///     <para>
+    ///         <b>Archreview 03/S15 — and the EXTERNAL data plane, which the two layers above still left
+    ///         out.</b> Conditions and internal consumers learned of the outage; the driver's raw value
+    ///         variables and their UNS mirrors — what an OPC UA client actually subscribes to — did not, so
+    ///         they served the last value at <c>Good</c> for the whole outage.
+    ///         <see cref="FanCommsLossToValueNodes"/> closes that with one status-only batch that keeps each
+    ///         node's value and changes only its status. All three fan-outs run on the disconnect edge only;
+    ///         the reconnect edge writes nothing to values (recovery is the driver's own re-publish) and
+    ///         re-annotates conditions Good, which is a statement about the LINK, not about any tag's value.
+    ///     </para>
     /// </summary>
     private void OnDriverConnectivityChanged(DriverInstanceActor.ConnectivityChanged msg)
     {
         // #481: the mux fan-out is independent of the OPC UA sink — a node with scripted alarms but no
         // publish actor (the mux-only test/dev path) must still see comms loss. Done before the early
         // return for exactly that reason.
-        if (!msg.Connected) FanCommsLossToDependencyMux(msg.DriverInstanceId);
+        if (!msg.Connected)
+        {
+            FanCommsLossToDependencyMux(msg.DriverInstanceId);
+            // 03/S15: and the EXTERNAL data plane, which #481 stopped one seam short of.
+            FanCommsLossToValueNodes(msg.DriverInstanceId);
+        }
 
         if (_opcUaPublishActor is null) return;
 
@@ -1282,6 +1297,67 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
                 _localNode, msg.DriverInstanceId, msg.Connected ? "connected" : "disconnected", annotated, statusCode);
     }
 
+    /// <summary>
+    ///     Archreview <b>03/S15</b> — the value-node half of the comms-loss fan-out: stamp
+    ///     <see cref="OpcUaStatus.BadNoCommunication"/> on every raw NodeId <b>and</b> every referencing UNS
+    ///     NodeId the just-disconnected driver owns, as ONE status batch through the same publish actor the
+    ///     value fan-out uses.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         <b>Why it was missing.</b> #481 built the per-driver ref index and wired comms loss to the
+    ///         internal consumers (the mux) and to Part 9 conditions, and stopped one seam short of the
+    ///         external OPC UA data plane. A client subscribed to a value node therefore saw the last value
+    ///         at <b>Good</b> quality for the whole outage, for every driver whose failure shape is "the
+    ///         reader stops producing" rather than "publish a Bad snapshot per tag". (Modbus does the
+    ///         latter, quickly, which is why the gap looked smaller than it was — a coalesced-read throw, a
+    ///         dropped session on a push driver, or simply a driver whose last act was a Good publish all
+    ///         held Good indefinitely.)
+    ///     </para>
+    ///     <para>
+    ///         <b>Status-only, so the VALUE is retained.</b> This actor holds no value cache — v3's
+    ///         single-source design deliberately removed the independent buffer — so only the node can keep
+    ///         the last known value, which is what
+    ///         <see cref="OpcUa.OpcUaPublishActor.AttributeStatusBatchUpdate"/> exists for. A Bad node still
+    ///         carrying its last reading is OPC UA Part 8's convention and is what a SCADA consumer expects.
+    ///     </para>
+    ///     <para>
+    ///         <b>Nothing is written on reconnect.</b> Publishing Good would assert a quality this actor has
+    ///         not observed; recovery is the driver's own re-publish (see
+    ///         <see cref="FanCommsLossToDependencyMux"/>'s remark on the detach that makes the poll engine
+    ///         force-raise, and MQTT's initial push).
+    ///     </para>
+    ///     <para>
+    ///         Ungated by redundancy role, like the condition annotation: a Secondary keeps its address
+    ///         space honest for failover. Re-stamping an already-Bad node is harmless — the SDK's
+    ///         <c>StatusValue</c> trigger dedups an unchanged status at the monitored item.
+    ///     </para>
+    /// </remarks>
+    /// <param name="driverInstanceId">The driver child that just reported comms loss.</param>
+    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));
+            }
+        }
+
+        if (writes.Count == 0) return;
+
+        _opcUaPublishActor.Tell(new OpcUa.OpcUaPublishActor.AttributeStatusBatchUpdate(writes));
+
+        _log.Info(
+            "DriverHost {Node}: driver {Driver} disconnected — {Count} value node(s) marked BadNoCommunication (value retained)",
+            _localNode, driverInstanceId, writes.Count);
+    }
+
     /// <summary>
     ///     #481 (Layer 4) — push a synthetic <see cref="OpcUaStatus.BadNoCommunication"/> into the dependency
     ///     mux for every value ref the just-disconnected driver owns, so scripted alarms and virtual tags see

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorCommsLossValueNodesTests.cs

@@ -0,0 +1,126 @@
+using Akka.Actor;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
+using ZB.MOM.WW.OtOpcUa.Commons.Types;
+using ZB.MOM.WW.OtOpcUa.Runtime.Drivers;
+using ZB.MOM.WW.OtOpcUa.Runtime.OpcUa;
+using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
+using static ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness.V3ArtifactSeeder;
+
+namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers;
+
+/// <summary>
+///     Archreview <b>03/S15</b> — the value-node half of the comms-loss fan-out. Until this, a driver
+///     declared comms-lost annotated its Part 9 conditions and pushed a synthetic Bad into the dependency
+///     mux, but its <b>raw and UNS value variables</b> — the product's external surface — got nothing: an
+///     OPC UA client subscribed to a value node saw the last value at <b>Good</b> quality for the whole
+///     outage, for every driver whose failure shape is "reader stops producing" rather than "per-tag Bad
+///     snapshot".
+///
+///     <para><b>Status-only, value retained.</b> The write goes through
+///     <see cref="OpcUaPublishActor.AttributeStatusBatchUpdate"/> so the node keeps its last known value
+///     under <c>BadNoCommunication</c> (OPC UA Part 8's convention). The host holds no value cache — v3's
+///     single-source design — so only the node itself can retain it.</para>
+///
+///     <para><b>Nothing is published on reconnect</b>, deliberately: publishing Good would assert a
+///     quality this actor has not observed. Recovery is the driver's own re-publish (the poll engine's
+///     <c>forceRaise</c> on a fresh subscription, or the push-model initial push). That contract is
+///     end-to-end tested in <see cref="DriverHostCommsLossRecoveryTests"/>.</para>
+/// </summary>
+public sealed class DriverHostActorCommsLossValueNodesTests : RuntimeActorTestBase
+{
+    private static readonly NodeId TestNode = NodeId.Parse("driver-commsloss-values");
+    private static readonly RevisionHash RevA = RevisionHash.Parse(new string('c', 64));
+
+    /// <summary>
+    ///     A disconnect produces ONE status batch naming exactly the disconnected driver's raw nodes plus
+    ///     every referencing UNS mirror, all <c>BadNoCommunication</c>. The second driver's ref is the
+    ///     control: a fan-out that walked every value ref on the node instead of the per-driver index would
+    ///     mark a perfectly healthy driver's nodes Bad, and no assertion about the first driver's nodes
+    ///     would notice.
+    /// </summary>
+    [Fact]
+    public void Disconnect_marks_the_drivers_raw_and_uns_value_nodes_BadNoCommunication()
+    {
+        var db = NewInMemoryDbFactory();
+        var deploymentId = SeedV3Deployment(db, RevA,
+            rawTags: new[]
+            {
+                (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false),
+                (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "temp", DataType: "Double", Writable: false),
+                (Driver: "drv-2", DriverName: "S7", Device: "dev2", Tag: "other", DataType: "Double", Writable: false),
+            },
+            unsRefs: new[] { (Area: "filling", Line: "line1", Equip: "station1", BackingTag: "speed", Effective: (string?)null) });
+
+        var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA);
+
+        actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: false));
+
+        var batch = publish.ExpectMsg<OpcUaPublishActor.AttributeStatusBatchUpdate>(PresenceBudget);
+        batch.Items.Select(i => i.NodeId).ShouldBe(new[]
+        {
+            "Plant/Modbus/dev1/speed",
+            "filling/line1/station1/speed",
+            "Plant/Modbus/dev1/temp",
+        }, ignoreOrder: true);
+
+        batch.Items.ShouldAllBe(i => i.StatusCode == OpcUaStatus.BadNoCommunication);
+        batch.Items.Single(i => i.NodeId == "Plant/Modbus/dev1/speed").Realm.ShouldBe(AddressSpaceRealm.Raw);
+        batch.Items.Single(i => i.NodeId == "filling/line1/station1/speed").Realm.ShouldBe(AddressSpaceRealm.Uns);
+
+        // The control: the OTHER driver's node is untouched by drv-1's outage.
+        batch.Items.ShouldNotContain(i => i.NodeId == "Plant/S7/dev2/other");
+
+        // ONE batch, not one message per node.
+        publish.ExpectNoMsg(TimeSpan.FromMilliseconds(300));
+    }
+
+    /// <summary>
+    ///     A reconnect publishes <b>nothing</b> to the value nodes. Writing Good here would claim a quality
+    ///     the host has not observed — the device may be answering the transport and still returning
+    ///     garbage — and it would race the driver's own first real value.
+    /// </summary>
+    [Fact]
+    public void Reconnect_publishes_no_value_or_status_message()
+    {
+        var db = NewInMemoryDbFactory();
+        var deploymentId = SeedV3Deployment(db, RevA,
+            rawTags: new[]
+            {
+                (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false),
+            },
+            unsRefs: Array.Empty<(string, string, string, string, string?)>());
+
+        var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA);
+
+        actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: false));
+        publish.ExpectMsg<OpcUaPublishActor.AttributeStatusBatchUpdate>(PresenceBudget);
+
+        actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-1", Connected: true));
+
+        publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
+    }
+
+    /// <summary>
+    ///     A driver that owns no value refs produces no message at all — an empty batch would cost a hop
+    ///     and a sink call for nothing on every flap of an alarm-only or freshly-added driver.
+    /// </summary>
+    [Fact]
+    public void A_driver_with_no_value_refs_publishes_nothing()
+    {
+        var db = NewInMemoryDbFactory();
+        var deploymentId = SeedV3Deployment(db, RevA,
+            rawTags: new[]
+            {
+                (Driver: "drv-1", DriverName: "Modbus", Device: "dev1", Tag: "speed", DataType: "Double", Writable: false),
+            },
+            unsRefs: Array.Empty<(string, string, string, string, string?)>());
+
+        var (actor, publish, _) = SpawnHostAndApply(this, db, TestNode, deploymentId, RevA);
+
+        actor.Tell(new DriverInstanceActor.ConnectivityChanged("drv-unknown", Connected: false));
+
+        publish.ExpectNoMsg(TimeSpan.FromMilliseconds(500));
+    }
+}