From ebf2f1dd7abac5220c64d24eee8ac626223752b7 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Mon, 15 Jun 2026 10:12:11 -0400
Subject: [PATCH] fix(vtags): prune _planByVtag on child termination +
 crash-then-change test (H1b review follow-up)

---
 .../VirtualTags/VirtualTagHostActor.cs        | 12 ++++---
 .../VirtualTags/VirtualTagHostActorTests.cs   | 34 +++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs
index ce34b2cc..fc6f79d5 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagHostActor.cs
@@ -39,8 +39,9 @@ public sealed class VirtualTagHostActor : ReceiveActor
     private readonly Dictionary<string, IActorRef> _children = new(StringComparer.Ordinal);
     // vtagId -> folder-scoped OPC UA NodeId the materialiser placed the variable at.
     private readonly Dictionary<string, string> _nodeIdByVtag = new(StringComparer.Ordinal);
-    // vtagId -> the plan its currently-spawned child was built from, so we can detect an in-place
-    // change (edited Expression/DependencyRefs) and stop+respawn the child to adopt it.
+    // vtagId -> the plan its live child was built from, so we can detect an in-place change (edited
+    // Expression/DependencyRefs) and stop+respawn the child to adopt it. Kept in lock-step with
+    // _children: rebuilt to the desired set at the end of every apply, and pruned in OnChildTerminated.
     private readonly Dictionary<string, EquipmentVirtualTagPlan> _planByVtag = new(StringComparer.Ordinal);
 
     /// <summary>Factory method to create Props for a VirtualTagHostActor.</summary>
@@ -161,8 +162,11 @@ public sealed class VirtualTagHostActor : ReceiveActor
         foreach (var id in stale)
         {
             _children.Remove(id);
-            // NodeId map is rebuilt on the next ApplyVirtualTags; leaving the mapping is harmless
-            // (no child will publish for it until respawned). A dead child is respawned on next apply.
+            // Keep _planByVtag in lock-step with _children so it never holds a plan for a dead child.
+            // (The next apply also resets it, and the change-detect guard short-circuits on the missing
+            // _children entry — so this is defensive consistency, not a live-bug fix.) NodeId map is
+            // rebuilt on the next ApplyVirtualTags; leaving that mapping is harmless. Dead child respawns.
+            _planByVtag.Remove(id);
             _log.Warning("VirtualTagHost: child for vtag {VirtualTagId} terminated; will respawn on next apply", id);
         }
     }
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs
index c7cd3642..43461e72 100644
--- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagHostActorTests.cs
@@ -243,6 +243,40 @@ public sealed class VirtualTagHostActorTests : RuntimeActorTestBase
         mux.ExpectNoMsg(TimeSpan.FromMilliseconds(400));
     }
 
+    /// <summary>
+    /// H1b crash-then-change: after a child terminates unexpectedly AND the next apply carries a
+    /// CHANGED plan for the same vtagId, the replacement child must adopt the NEW plan (refs "B"),
+    /// not the stale one (refs "A"). This pins that OnChildTerminated prunes _planByVtag so the
+    /// change-detect guard can't be confused by a dead child's old plan.
+    /// </summary>
+    [Fact]
+    public void Respawn_after_termination_with_a_changed_plan_adopts_the_new_refs()
+    {
+        var publish = CreateTestProbe();
+        var mux = CreateTestProbe();
+        var host = Sys.ActorOf(VirtualTagHostActor.Props(publish.Ref, mux.Ref, new StubEvaluator()));
+
+        // First apply — child registers interest in "A".
+        host.Tell(new VirtualTagHostActor.ApplyVirtualTags(
+            new[] { PlanWithRefs("vt-1", "eq-1", "speed-rpm", "ctx.GetTag(\"A\")", "A") }));
+        mux.ExpectMsg<DependencyMuxActor.RegisterInterest>().TagRefs.ShouldContain("A");
+        var firstChild = mux.LastSender;
+
+        // Kill the child deterministically and drain its PostStop UnregisterInterest.
+        Watch(firstChild);
+        Sys.Stop(firstChild);
+        ExpectTerminated(firstChild);
+        mux.ExpectMsg<DependencyMuxActor.UnregisterInterest>(TimeSpan.FromSeconds(5));
+
+        // Re-apply with a CHANGED plan (refs "B"). The replacement must register the NEW refs.
+        host.Tell(new VirtualTagHostActor.ApplyVirtualTags(
+            new[] { PlanWithRefs("vt-1", "eq-1", "speed-rpm", "ctx.GetTag(\"B\")", "B") }));
+        var reg2 = mux.ExpectMsg<DependencyMuxActor.RegisterInterest>(TimeSpan.FromSeconds(5));
+        reg2.TagRefs.ShouldContain("B");
+        reg2.TagRefs.ShouldNotContain("A");
+        mux.LastSender.ShouldNotBe(firstChild);
+    }
+
     /// <summary>Deterministic no-op evaluator: keeps spawned children inert so tests drive the host's
     /// OnResult path directly via synthetic EvaluationResults.</summary>
     private sealed class StubEvaluator : IVirtualTagEvaluator