feat(commons): add surgical remove members to ISurgicalAddressSpaceSink + deferred forwarding (guard-verified) (R2-07 T7)

archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json

@@ -63,7 +63,7 @@
     {
       "id": "T7",
       "subject": "Phase 2: add RemoveVariableNode/RemoveAlarmConditionNode/RemoveEquipmentSubtree to ISurgicalAddressSpaceSink + DeferredAddressSpaceSink forwarding \u2014 guard-first with observed negative control (high-risk)",
-      "status": "pending",
+      "status": "completed",
       "blockedBy": [
         "T6"
       ]

src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs

@@ -68,4 +68,21 @@ public sealed class DeferredAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgical
     public bool UpdateFolderDisplayName(string folderNodeId, string displayName)
         => _inner is ISurgicalAddressSpaceSink surgical
            && surgical.UpdateFolderDisplayName(folderNodeId, displayName);
+
+    // R2-07 T7 — surgical remove forwards. Same capability-sniffing pattern as UpdateTagAttributes /
+    // UpdateFolderDisplayName: forward when the inner sink supports the surgical capability; return false
+    // otherwise (before the real SdkAddressSpaceSink is swapped in, or any non-surgical inner) so the caller
+    // (AddressSpaceApplier) falls back to a full rebuild. Without these forwards the surgical remove path is
+    // inert on every driver-role host — the F10b / PR#423 trap the reflection guard exists to catch.
+    /// <inheritdoc />
+    public bool RemoveVariableNode(string variableNodeId)
+        => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveVariableNode(variableNodeId);
+
+    /// <inheritdoc />
+    public bool RemoveAlarmConditionNode(string alarmNodeId)
+        => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveAlarmConditionNode(alarmNodeId);
+
+    /// <inheritdoc />
+    public bool RemoveEquipmentSubtree(string equipmentNodeId)
+        => _inner is ISurgicalAddressSpaceSink surgical && surgical.RemoveEquipmentSubtree(equipmentNodeId);
 }

src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs

@@ -34,4 +34,34 @@ public interface ISurgicalAddressSpaceSink
     /// <param name="displayName">The new display name to apply.</param>
     /// <returns>True when the in-place update was applied; false when the folder is missing (caller rebuilds).</returns>
     bool UpdateFolderDisplayName(string folderNodeId, string displayName);
+
+    /// <summary>Remove ONE existing value-variable node IN PLACE (detach from its parent, drop it from the
+    /// predefined-node set, drop any historized-tagname registration), then raise a Part 3
+    /// GeneralModelChangeEvent (verb=NodeDeleted) outside the lock — so subscribers of OTHER nodes are
+    /// untouched and their MonitoredItems survive. The caller has already written a final Bad quality to the
+    /// node so in-flight MonitoredItems on the removed node observe the removal; after the node is gone,
+    /// re-subscription attempts get BadNodeIdUnknown from the SDK. Returns false when the node id is unknown
+    /// (the node-manager maps drifted from the plan — caller falls back to a full rebuild).</summary>
+    /// <param name="variableNodeId">The folder-scoped node id of the variable to remove in place.</param>
+    /// <returns>True when the node was removed; false when the id is unknown (caller rebuilds).</returns>
+    bool RemoveVariableNode(string variableNodeId);
+
+    /// <summary>Remove ONE existing Part 9 alarm-condition node IN PLACE (detach, drop from the
+    /// predefined-node set, clear the native-alarm flag), then raise NodeDeleted outside the lock. The
+    /// caller has already written the terminal RemovedConditionState (Retain=false) so a removed condition
+    /// stops replaying on ConditionRefresh. Returns false when the condition id is unknown (caller
+    /// rebuilds).</summary>
+    /// <param name="alarmNodeId">The alarm condition node id (== ScriptedAlarmId, or a native alarm tag's folder-scoped id).</param>
+    /// <returns>True when the condition was removed; false when the id is unknown (caller rebuilds).</returns>
+    bool RemoveAlarmConditionNode(string alarmNodeId);
+
+    /// <summary>Remove an equipment folder AND its entire descendant subtree (sub-folders, value variables,
+    /// condition nodes) IN PLACE: per-descendant map/registration cleanup (variables, historized tagnames,
+    /// conditions, native flags), notifier demotion (sever the Server↔folder root-notifier ref +
+    /// notifier-folder + event-notifier-source registrations for the equipment id), then one NodeDeleted for
+    /// the equipment folder outside the lock. Returns false when the folder id is unknown (caller
+    /// rebuilds).</summary>
+    /// <param name="equipmentNodeId">The equipment folder node id whose entire subtree to remove.</param>
+    /// <returns>True when the subtree was removed; false when the id is unknown (caller rebuilds).</returns>
+    bool RemoveEquipmentSubtree(string equipmentNodeId);
 }

src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs

@@ -1746,6 +1746,19 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         }
     }
 
+    // R2-07 Phase 2 (T7 placeholder) — surgical single-node / subtree removal. These return false (the
+    // rebuild-fallback contract) so the tree stays shippable at T7 while T8/T9/T10 implement the real
+    // in-place teardown (mirroring the per-node loops inside RebuildAddressSpace, scoped to the id/subtree,
+    // with a NodeDeleted model-change reported outside Lock). Implemented in T8/T9/T10.
+    /// <inheritdoc cref="ISurgicalAddressSpaceSink.RemoveVariableNode"/>
+    public bool RemoveVariableNode(string variableNodeId) => false;
+
+    /// <inheritdoc cref="ISurgicalAddressSpaceSink.RemoveAlarmConditionNode"/>
+    public bool RemoveAlarmConditionNode(string alarmNodeId) => false;
+
+    /// <inheritdoc cref="ISurgicalAddressSpaceSink.RemoveEquipmentSubtree"/>
+    public bool RemoveEquipmentSubtree(string equipmentNodeId) => false;
+
     private FolderState ResolveParentFolder(string? parentNodeId)
     {
         EnsureAddressSpaceCreated();

src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs

@@ -48,6 +48,18 @@ public sealed class SdkAddressSpaceSink : IOpcUaAddressSpaceSink, ISurgicalAddre
     public bool UpdateFolderDisplayName(string folderNodeId, string displayName)
         => _nodeManager.UpdateFolderDisplayName(folderNodeId, displayName);
 
+    /// <inheritdoc />
+    public bool RemoveVariableNode(string variableNodeId)
+        => _nodeManager.RemoveVariableNode(variableNodeId);
+
+    /// <inheritdoc />
+    public bool RemoveAlarmConditionNode(string alarmNodeId)
+        => _nodeManager.RemoveAlarmConditionNode(alarmNodeId);
+
+    /// <inheritdoc />
+    public bool RemoveEquipmentSubtree(string equipmentNodeId)
+        => _nodeManager.RemoveEquipmentSubtree(equipmentNodeId);
+
     /// <inheritdoc />
     public void RebuildAddressSpace() => _nodeManager.RebuildAddressSpace();
 

tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs

@@ -114,6 +114,44 @@ public class DeferredAddressSpaceSinkTests
         surgical.FolderRenameCalled.ShouldBeTrue();
     }
 
+    // ---------- R2-07 Phase 2: surgical remove forwarding ----------
+
+    [Fact]
+    public void Remove_members_return_false_for_non_surgical_inner()
+    {
+        var sink = new DeferredAddressSpaceSink();
+        sink.SetSink(new SpySink());
+
+        sink.RemoveVariableNode("v-1").ShouldBeFalse();
+        sink.RemoveAlarmConditionNode("a-1").ShouldBeFalse();
+        sink.RemoveEquipmentSubtree("eq-1").ShouldBeFalse();
+    }
+
+    [Fact]
+    public void Remove_members_forward_to_surgical_inner()
+    {
+        var surgical = new SpySurgicalSink();
+        var sink = new DeferredAddressSpaceSink();
+        sink.SetSink(surgical);
+
+        sink.RemoveVariableNode("v-1").ShouldBeTrue();
+        sink.RemoveAlarmConditionNode("a-1").ShouldBeTrue();
+        sink.RemoveEquipmentSubtree("eq-1").ShouldBeTrue();
+
+        surgical.RemoveVariableCalled.ShouldBeTrue();
+        surgical.RemoveAlarmCalled.ShouldBeTrue();
+        surgical.RemoveSubtreeCalled.ShouldBeTrue();
+    }
+
+    [Fact]
+    public void Before_SetSink_remove_members_return_false()
+    {
+        var sink = new DeferredAddressSpaceSink();
+        sink.RemoveVariableNode("v-1").ShouldBeFalse();
+        sink.RemoveAlarmConditionNode("a-1").ShouldBeFalse();
+        sink.RemoveEquipmentSubtree("eq-1").ShouldBeFalse();
+    }
+
     // ---------- SetSink(null) reverts to null sink ----------
 
     [Fact]
@@ -183,5 +221,13 @@ public class DeferredAddressSpaceSinkTests
             FolderRenameCalled = true;
             return true;
         }
+
+        public bool RemoveVariableCalled { get; private set; }
+        public bool RemoveAlarmCalled { get; private set; }
+        public bool RemoveSubtreeCalled { get; private set; }
+
+        public bool RemoveVariableNode(string variableNodeId) { RemoveVariableCalled = true; return true; }
+        public bool RemoveAlarmConditionNode(string alarmNodeId) { RemoveAlarmCalled = true; return true; }
+        public bool RemoveEquipmentSubtree(string equipmentNodeId) { RemoveSubtreeCalled = true; return true; }
     }
 }

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs

@@ -2013,6 +2013,35 @@ public sealed class AddressSpaceApplierTests
             return FolderRenameReturns;
         }
 
+        /// <summary>Gets the queue of surgical remove calls (kind + node id) in call order (R2-07 Phase 2).</summary>
+        public ConcurrentQueue<(string Kind, string NodeId)> RemoveQueue { get; } = new();
+        /// <summary>Gets the list of recorded surgical remove calls.</summary>
+        public List<(string Kind, string NodeId)> RemoveCalls => RemoveQueue.ToList();
+        /// <summary>When false, the Remove* members report the node missing (return false), driving the
+        /// applier's rebuild fallback. Defaults to true (node present, removal succeeds).</summary>
+        public bool RemoveReturns { get; init; } = true;
+
+        /// <summary>Records a surgical variable-node removal; returns <see cref="RemoveReturns"/>.</summary>
+        public bool RemoveVariableNode(string variableNodeId)
+        {
+            RemoveQueue.Enqueue(("var", variableNodeId));
+            return RemoveReturns;
+        }
+
+        /// <summary>Records a surgical alarm-condition-node removal; returns <see cref="RemoveReturns"/>.</summary>
+        public bool RemoveAlarmConditionNode(string alarmNodeId)
+        {
+            RemoveQueue.Enqueue(("alarm", alarmNodeId));
+            return RemoveReturns;
+        }
+
+        /// <summary>Records a surgical equipment-subtree removal; returns <see cref="RemoveReturns"/>.</summary>
+        public bool RemoveEquipmentSubtree(string equipmentNodeId)
+        {
+            RemoveQueue.Enqueue(("equipment", equipmentNodeId));
+            return RemoveReturns;
+        }
+
         /// <summary>Gets the queue of alarm condition write calls.</summary>
         public ConcurrentQueue<(string NodeId, AlarmConditionSnapshot State)> AlarmQueue { get; } = new();
         /// <summary>Gets the queue of folder creation calls.</summary>

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DeferredAddressSpaceSinkTests.cs

@@ -174,6 +174,40 @@ public sealed class DeferredAddressSpaceSinkTests
         ((ISurgicalAddressSpaceSink)deferred).UpdateFolderDisplayName("area-1", "X").ShouldBeFalse();
     }
 
+    /// <summary>R2-07 Phase 2 — the three surgical remove members forward to a surgical inner with
+    /// arg-fidelity (kind + node id), returning the inner's own result; false when the inner is not surgical
+    /// so the caller falls back to a full rebuild.</summary>
+    [Fact]
+    public void Remove_members_forward_to_a_surgical_inner_sink_with_arg_fidelity()
+    {
+        var deferred = new DeferredAddressSpaceSink();
+        var inner = new SurgicalRecordingSink { Result = true };
+        deferred.SetSink(inner);
+
+        ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("eq-1/A").ShouldBeTrue();
+        ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("alm-1").ShouldBeTrue();
+        ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeTrue();
+
+        inner.RemoveCalls.ShouldBe(new[] { ("var", "eq-1/A"), ("alarm", "alm-1"), ("equipment", "eq-1") });
+    }
+
+    /// <summary>The remove forwards return the inner's own result (false ⇒ id unknown ⇒ caller rebuilds),
+    /// and return false when the inner is not surgical at all.</summary>
+    [Fact]
+    public void Remove_members_return_inner_result_and_false_when_not_surgical()
+    {
+        var deferred = new DeferredAddressSpaceSink();
+        deferred.SetSink(new SurgicalRecordingSink { Result = false });
+        ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("v-1").ShouldBeFalse();
+        ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("a-1").ShouldBeFalse();
+        ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeFalse();
+
+        deferred.SetSink(null);
+        ((ISurgicalAddressSpaceSink)deferred).RemoveVariableNode("v-1").ShouldBeFalse();
+        ((ISurgicalAddressSpaceSink)deferred).RemoveAlarmConditionNode("a-1").ShouldBeFalse();
+        ((ISurgicalAddressSpaceSink)deferred).RemoveEquipmentSubtree("eq-1").ShouldBeFalse();
+    }
+
     /// <summary>Builds a minimal <see cref="AlarmConditionSnapshot"/> for the forwarding tests (the
     /// inner sink only records the node id, so the exact state values don't matter here).</summary>
     private static AlarmConditionSnapshot Snapshot(bool active = false) =>
@@ -240,6 +274,16 @@ public sealed class DeferredAddressSpaceSinkTests
             return Result;
         }
 
+        /// <summary>Gets the recorded surgical remove calls (kind + node id), in call order (R2-07 Phase 2).</summary>
+        public List<(string Kind, string NodeId)> RemoveCalls { get; } = new();
+
+        /// <inheritdoc />
+        public bool RemoveVariableNode(string variableNodeId) { RemoveCalls.Add(("var", variableNodeId)); return Result; }
+        /// <inheritdoc />
+        public bool RemoveAlarmConditionNode(string alarmNodeId) { RemoveCalls.Add(("alarm", alarmNodeId)); return Result; }
+        /// <inheritdoc />
+        public bool RemoveEquipmentSubtree(string equipmentNodeId) { RemoveCalls.Add(("equipment", equipmentNodeId)); return Result; }
+
         /// <inheritdoc />
         public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc) { }
         /// <inheritdoc />

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs

@@ -487,5 +487,23 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
             FolderRenameQueue.Enqueue((folderNodeId, displayName));
             return true;
         }
+        /// <summary>Records a surgical variable-node removal (always succeeds in this recording sink).</summary>
+        public bool RemoveVariableNode(string variableNodeId)
+        {
+            Calls.Enqueue($"RV:{variableNodeId}");
+            return true;
+        }
+        /// <summary>Records a surgical alarm-condition-node removal (always succeeds in this recording sink).</summary>
+        public bool RemoveAlarmConditionNode(string alarmNodeId)
+        {
+            Calls.Enqueue($"RA:{alarmNodeId}");
+            return true;
+        }
+        /// <summary>Records a surgical equipment-subtree removal (always succeeds in this recording sink).</summary>
+        public bool RemoveEquipmentSubtree(string equipmentNodeId)
+        {
+            Calls.Enqueue($"RE:{equipmentNodeId}");
+            return true;
+        }
     }
 }