fix(runtime): VT script failure/timeout degrades node quality to Bad once per transition (02/S13)

archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json

@@ -18,7 +18,7 @@
     {
       "id": "R2-03-T3",
       "subject": "VirtualTagActor failure/throw publishes Bad EvaluationResult once per transition, gated on all dependencyRefs arrived; recovery bypasses value-dedup; update Evaluator_failure_publishes_ScriptLogEntry_warning expectation (turns T1 green)",
-      "status": "pending",
+      "status": "completed",
       "blockedBy": [
         "R2-03-T2"
       ]

src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs

@@ -13,8 +13,17 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags;
 /// Wraps a single virtual-tag expression. Receives dependency-tag updates, recomputes the
 /// expression via an injected <see cref="IVirtualTagEvaluator"/>, and emits an
 /// <see cref="EvaluationResult"/> to the parent (the publish actor) whenever the value
-/// actually changes. Script failures publish a Warning <see cref="ScriptLogEntry"/> on the
+/// actually changes. Script failures publish a <see cref="ScriptLogEntry"/> on the
 /// <c>script-logs</c> DPS topic so operators see the diagnostic in the live tail.
+///
+/// <para>
+///     02/S13 — a persistent script failure/timeout also degrades the node's OPC UA quality to
+///     <see cref="OpcUaQuality.Bad"/> (carrying the last-known value), published <b>once per
+///     transition</b> (Good→Bad) and gated on every declared dependency having arrived at least once
+///     — so a broken script no longer serves its last Good value forever, and a fresh/respawned
+///     multi-dep script does not flash Bad during deploy warm-up. Recovery to a Good value clears the
+///     Bad state and is force-published even when the value equals the pre-failure value.
+/// </para>
 /// </summary>
 public sealed class VirtualTagActor : ReceiveActor
 {
@@ -41,6 +50,9 @@ public sealed class VirtualTagActor : ReceiveActor
 
     private bool _hasLastValue;
     private object? _lastValue;
+    // 02/S13: true once a Bad EvaluationResult has been published and not yet cleared by a Good
+    // recovery. Gates Bad publishes to once-per-transition and force-publishes the recovery value.
+    private bool _lastPublishedBad;
 
     /// <summary>Factory method to create Props for a VirtualTagActor.</summary>
     /// <param name="virtualTagId">Unique identifier for the virtual tag.</param>
@@ -122,27 +134,28 @@ public sealed class VirtualTagActor : ReceiveActor
         catch (Exception ex)
         {
             _log.Warning(ex, "VirtualTag {Id}: evaluator threw", _virtualTagId);
-            OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair<string, object?>("outcome", "fail"));
-            PublishLog("Error", $"evaluator threw: {ex.Message}");
+            OnEvaluationFailed($"evaluator threw: {ex.Message}", "Error", msg.TimestampUtc);
             return;
         }
 
         if (!result.Success)
         {
-            OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair<string, object?>("outcome", "fail"));
-            PublishLog("Warning", result.Reason ?? "evaluator failure");
+            OnEvaluationFailed(result.Reason ?? "evaluator failure", "Warning", msg.TimestampUtc);
             return;
         }
 
         // Skip no-change results. Real evaluator returns Ok(value); Null returns NoChange — both
-        // safe because Null never produces a fresh value.
+        // safe because Null never produces a fresh value. NoChange is "no fresh value", NOT a recovery,
+        // so it must stay above the dedup and must NOT clear _lastPublishedBad.
         if (ReferenceEquals(result, VirtualTagEvalResult.NoChange))
         {
             OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair<string, object?>("outcome", "skip"));
             return;
         }
 
-        if (_hasLastValue && Equals(_lastValue, result.Value))
+        // Value dedup — but bypass it while recovering from Bad, so a recovery whose value equals the
+        // pre-failure value still republishes Good (otherwise the node would stay Bad forever).
+        if (!_lastPublishedBad && _hasLastValue && Equals(_lastValue, result.Value))
         {
             OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair<string, object?>("outcome", "skip"));
             return;
@@ -150,11 +163,41 @@ public sealed class VirtualTagActor : ReceiveActor
 
         _hasLastValue = true;
         _lastValue = result.Value;
+        _lastPublishedBad = false;
         OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair<string, object?>("outcome", "ok"));
         var evalResult = new EvaluationResult(_virtualTagId, result.Value, msg.TimestampUtc, CorrelationId.NewId());
         Context.Parent.Tell(evalResult);
     }
 
+    /// <summary>02/S13 failure tail shared by the evaluator-threw catch and the <c>!Success</c> branch.
+    /// Always meters + publishes the per-failure script-log entry (the pre-existing operator
+    /// diagnostic); additionally publishes a Bad <see cref="EvaluationResult"/> — carrying the
+    /// last-known value if any — once per Good→Bad transition, and only after every declared dependency
+    /// has arrived (the inputs-ready gate that suppresses the cold-start/respawn warm-up flash).</summary>
+    /// <param name="reason">The failure reason for the script-log entry.</param>
+    /// <param name="level">The script-log level ("Warning" for a Failure result, "Error" for a throw).</param>
+    /// <param name="timestampUtc">The triggering dependency change's timestamp, stamped on the Bad result.</param>
+    private void OnEvaluationFailed(string reason, string level, DateTime timestampUtc)
+    {
+        OtOpcUaTelemetry.VirtualTagEval.Add(1, new KeyValuePair<string, object?>("outcome", "fail"));
+        PublishLog(level, reason);
+
+        if (_lastPublishedBad || !AllDependenciesArrived())
+        {
+            return;
+        }
+
+        _lastPublishedBad = true;
+        Context.Parent.Tell(new EvaluationResult(
+            _virtualTagId, _hasLastValue ? _lastValue : null, timestampUtc, CorrelationId.NewId(), OpcUaQuality.Bad));
+    }
+
+    /// <summary>Inputs-ready gate (mirrors the dormant <c>VirtualTagEngine.AreInputsReady</c>): true once
+    /// every declared dependency ref has been seen at least once. Called once per failure — the ref list
+    /// is small, so no caching. A vacuously-true empty ref list means a no-dependency script degrades on
+    /// its first failure.</summary>
+    private bool AllDependenciesArrived() => _dependencyRefs.All(_dependencies.ContainsKey);
+
     private void PublishLog(string level, string message)
     {
         var entry = new ScriptLogEntry(

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs

@@ -4,6 +4,7 @@ using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Commons.Engines;
 using ZB.MOM.WW.OtOpcUa.Commons.Messages.Logging;
+using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
 using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
 using ZB.MOM.WW.OtOpcUa.Runtime.VirtualTags;
 
@@ -80,7 +81,10 @@ public sealed class VirtualTagActorTests : RuntimeActorTestBase
             entry.VirtualTagId.ShouldBe("vt-1");
         }, duration: TimeSpan.FromMilliseconds(500));
 
-        parent.ExpectNoMsg(TimeSpan.FromMilliseconds(100));
+        // 02/S13: the failure now ALSO degrades the node — with no declared dependencyRefs the
+        // inputs-ready gate is vacuously satisfied, so a Bad EvaluationResult reaches the parent (this
+        // replaces the old ExpectNoMsg, which encoded the stale-Good bug).
+        parent.ExpectMsg<VirtualTagActor.EvaluationResult>().Quality.ShouldBe(OpcUaQuality.Bad);
     }
 
     /// <summary>Test evaluator that sums integer dependency values.</summary>