diff --git a/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json b/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json
index a3db061f..dbfd5a76 100644
--- a/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json
+++ b/archreview/plans/R2-07-surgical-pure-adds-plan.md.tasks.json
@@ -113,7 +113,7 @@
{
"id": "T13",
"subject": "Phase 3: AddRemoveMix routing \u2014 removes-then-adds surgically, same-NodeId remove-then-recreate ordering test (high-risk)",
- "status": "pending",
+ "status": "completed",
"blockedBy": [
"T12"
]
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs
index 61a963a7..a4f72c49 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs
@@ -132,8 +132,11 @@ public sealed class AddressSpaceApplier
// subtree, via the ISurgicalAddressSpaceSink remove members below (each preceded by a terminal
// Bad / RemovedConditionState write so in-flight MonitoredItems observe the removal); subscribers
// of OTHER nodes are untouched. Any false/throw from a remove ⇒ full-rebuild fallback (ratchet).
- // • AddRemoveMix ⇒ full rebuild UNTIL Phase 3 ships (R2-07 T13); mapping it here keeps each phase
- // independently shippable with today's behaviour as the floor.
+ // • AddRemoveMix ⇒ NO full rebuild. Removes-then-adds within one apply: the removed nodes are torn
+ // down IN PLACE here (the PureRemove pass), then OpcUaPublishActor's idempotent Materialise passes
+ // create the added nodes and AnnounceAddedNodes announces them — natural remove-then-recreate
+ // order even when an id is reused across the remove + add sets (the recreated node is a fresh
+ // NodeState, correct — it is a different tag). Any remove false/throw ⇒ full-rebuild fallback.
// • Rebuild ⇒ the default-closed safety valve for any node-affecting change (ChangedEquipment /
// ChangedAlarms / non-surgical ChangedEquipmentTags / node-relevant ChangedEquipmentVirtualTags)
// — the classifier's rule 2, which also catches any future plan field that makes a changed
@@ -141,8 +144,10 @@ public sealed class AddressSpaceApplier
// ChangedDrivers is node-inert (routes through DriverHostActor's spawn plan) — the classifier leaves
// a driver-only plan AttributeOnly, so it never rebuilds here.
var kind = AddressSpaceChangeClassifier.Classify(plan);
- var mustRebuild = kind is AddressSpaceChangeKind.Rebuild
- or AddressSpaceChangeKind.AddRemoveMix;
+ var mustRebuild = kind is AddressSpaceChangeKind.Rebuild;
+ // PureRemove + AddRemoveMix both run the in-place remove pass below (AddRemoveMix's adds are handled
+ // afterward by the publish actor's Materialise passes + announce).
+ var hasRemovePass = kind is AddressSpaceChangeKind.PureRemove or AddressSpaceChangeKind.AddRemoveMix;
var surgicalTagDeltas = plan.ChangedEquipmentTags.Where(AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible).ToList();
// UNS Area / Line renames are surgically applicable (in-place DisplayName swap)
@@ -158,7 +163,7 @@ public sealed class AddressSpaceApplier
rebuildFailed = !SafeRebuild();
rebuilt = true;
}
- else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || kind == AddressSpaceChangeKind.PureRemove)
+ else if (surgicalTagDeltas.Count > 0 || renamedFolders.Count > 0 || hasRemovePass)
{
if (_sink is ISurgicalAddressSpaceSink surgical)
{
@@ -194,10 +199,12 @@ public sealed class AddressSpaceApplier
}
if (!ok) { allApplied = false; break; }
}
- // R2-07 Phase 2 — PureRemove scoped teardown (runs after any coincident surgical
- // attribute updates / renames succeeded). Terminal Bad / RemovedConditionState writes then
- // in-place removes; any false/throw flips allApplied → the rebuild ratchet below.
- if (allApplied && kind == AddressSpaceChangeKind.PureRemove)
+ // R2-07 Phase 2/3 — scoped remove teardown for PureRemove AND AddRemoveMix (runs after any
+ // coincident surgical attribute updates / renames succeeded). Terminal Bad /
+ // RemovedConditionState writes then in-place removes; any false/throw flips allApplied → the
+ // rebuild ratchet below. For AddRemoveMix the adds are materialised afterward by the publish
+ // actor's passes (remove-then-recreate order).
+ if (allApplied && hasRemovePass)
{
allApplied = ApplyPureRemove(surgical, plan, ts, ref failedNodes);
}
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs
index 72021ec8..97dfefc5 100644
--- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs
@@ -927,10 +927,11 @@ public sealed class AddressSpaceApplierTests
sink.RebuildCalls.ShouldBe(1);
}
- /// R2-07 T2 phase pin — an add+remove plan still triggers a full rebuild until Phase 3
- /// (AddRemoveMix) ships. Flipped in T13.
+ /// R2-07 T13 — an add+remove plan is an AddRemoveMix: the applier removes the old node IN PLACE
+ /// (RemoveVariableNode) and does NOT rebuild; the add is materialised afterward by the publish actor's
+ /// passes. (Supersedes the T2 phase pin.)
[Fact]
- public void Add_and_remove_still_rebuild_phase_pin()
+ public void Add_and_remove_is_mixed_removes_in_place_without_rebuild()
{
var sink = new RecordingSink();
var applier = new AddressSpaceApplier(sink, NullLogger.Instance);
@@ -949,6 +950,64 @@ public sealed class AddressSpaceApplierTests
var outcome = applier.Apply(plan);
+ outcome.RebuildCalled.ShouldBeFalse();
+ sink.RebuildCalls.ShouldBe(0);
+ // The removed node is torn down in place; the add is left to the publish actor's materialise passes.
+ sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", EquipmentNodeIds.Variable("eq-1", "", "Old")));
+ outcome.AddedNodes.ShouldBe(1);
+ outcome.RemovedNodes.ShouldBe(1);
+ }
+
+ /// R2-07 T13 — an id REUSED across the remove + add sets in one deploy (a tag removed at the
+ /// same folder-scoped NodeId another tag is added at): the applier removes the old node in place (the
+ /// recreate is the publish actor's job, a fresh NodeState). No rebuild.
+ [Fact]
+ public void Add_and_remove_reusing_same_node_id_removes_the_old_node_without_rebuild()
+ {
+ var sink = new RecordingSink();
+ var applier = new AddressSpaceApplier(sink, NullLogger.Instance);
+
+ var nodeId = EquipmentNodeIds.Variable("eq-1", "", "Slot");
+ var plan = EmptyPlan with
+ {
+ // Different TagId, but both resolve to the SAME folder-scoped NodeId (eq-1/Slot).
+ RemovedEquipmentTags = new[]
+ {
+ new EquipmentTagPlan("tag-old", "eq-1", "drv", FolderPath: "", Name: "Slot", DataType: "Float", FullName: "40001", Writable: false, Alarm: null),
+ },
+ AddedEquipmentTags = new[]
+ {
+ new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "Slot", DataType: "Int32", FullName: "40002", Writable: false, Alarm: null),
+ },
+ };
+
+ var outcome = applier.Apply(plan);
+
+ outcome.RebuildCalled.ShouldBeFalse();
+ sink.RemoveCalls.ShouldHaveSingleItem().ShouldBe(("var", nodeId));
+ }
+
+ /// R2-07 T13 — a remove failure inside an AddRemoveMix still trips the rebuild ratchet.
+ [Fact]
+ public void Add_and_remove_with_remove_returns_false_falls_back_to_rebuild()
+ {
+ var sink = new RecordingSink { RemoveReturns = false };
+ var applier = new AddressSpaceApplier(sink, NullLogger.Instance);
+
+ var plan = EmptyPlan with
+ {
+ AddedEquipmentTags = new[]
+ {
+ new EquipmentTagPlan("tag-new", "eq-1", "drv", FolderPath: "", Name: "New", DataType: "Float", FullName: "40009", Writable: false, Alarm: null),
+ },
+ RemovedEquipmentTags = new[]
+ {
+ new EquipmentTagPlan("tag-old", "eq-1", "drv", FolderPath: "", Name: "Old", DataType: "Float", FullName: "40001", Writable: false, Alarm: null),
+ },
+ };
+
+ var outcome = applier.Apply(plan);
+
outcome.RebuildCalled.ShouldBeTrue();
sink.RebuildCalls.ShouldBe(1);
}