Files
lmxopcua/archreview/plans/R2-07-surgical-pure-adds-plan.md
T
Joseph Doherty ded0595ca7
v2-ci / build (pull_request) Successful in 4m3s
v2-ci / unit-tests (pull_request) Failing after 9m47s
test+docs: mixed-deploy survival, STATUS/P1 close-out (R2-07 T14)
2026-07-13 12:24:50 -04:00

51 KiB
Raw Blame History

Design & Implementation Plan — R2-07: Surgical pure-adds (and removals) in the address-space applier

Status (2026-07-12): NOT STARTED — this is the round-2 plan. The declared prerequisite (03/U2 reflection-exhaustive deferred-sink forwarding guard, DeferredSinkForwardingReflectionTests) is merged to master and auto-covers every new sink capability member this plan adds, so the F10b/PR#423 inertness class is retired and the work is unblocked.

  • Source reports: archreview/03-server-runtime.md (P1 — the primary finding), archreview/01-core-composition.md (applier/planner context, S-1, C-7), archreview/00-OVERALL.md prioritized action #7 ("Surgical pure-adds in the applier — Large effort, High impact, biggest operational perf win").
  • Round-1 plan extended: archreview/plans/03-server-runtime-plan.md §P1 (this plan supersedes its two-phase sketch with a full three-phase staged design).
  • Review commit: f6eaa267 · Plan verified against tree at: f6eaa267 (master, clean tree; archreview/*.md working-tree updates from the 2026-07-12 re-review read in-place).
  • Scope: src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ (AddressSpaceApplier, AddressSpacePlan/AddressSpacePlanner, OtOpcUaNodeManager, SdkAddressSpaceSink), src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ (ISurgicalAddressSpaceSink, DeferredAddressSpaceSink, IOpcUaAddressSpaceSink), src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs, plus tests under tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/, tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/, tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/.

Verification summary

Every claim below re-verified by direct read at f6eaa267:

# Claim Verified at
1 Any add/remove of Equipment / ScriptedAlarm / EquipmentTag / EquipmentVirtualTag forces structuralRebuildSafeRebuild() AddressSpaceApplier.cs:134-154 — the predicate ORs every Added*/Removed* count
2 RebuildAddressSpace() detaches + removes every NodeState (variables, conditions, folders, notifier refs) under one Lock OtOpcUaNodeManager.cs:1690-1736
3 Recreated nodes are new NodeState instances ⇒ existing client MonitoredItems go permanently dead SDK binds monitored items to NodeState instances; confirmed by the F10b design history and ISurgicalAddressSpaceSink doc (ISurgicalAddressSpaceSink.cs:9-17)
4 The surgical path today covers ONLY changed-tag attribute edits (TagDeltaIsSurgicalEligible) + folder DisplayName renames (RenamedFolders) — not adds, not removes AddressSpaceApplier.cs:155-199, 630-663
5 The four Materialise passes run unconditionally after Apply returns, over the FULL composition, and are idempotent (EnsureFolder early-return OtOpcUaNodeManager.cs:1290, EnsureVariable early-return :1376, MaterialiseAlarmCondition drop-and-recreate guard :596-604) OpcUaPublishActor.cs:335-354 (passes at 338/343/349/354, gated on the plan.IsEmpty short-circuit at :328)
6 RaiseNodesAddedModelChange (Part 3 GeneralModelChangeEvent, verb NodeAdded, built under Lock, reported outside it) already exists, is on IOpcUaAddressSpaceSink, is forwarded by DeferredAddressSpaceSink, and is guard-covered OtOpcUaNodeManager.cs:1592-1654, IOpcUaAddressSpaceSink.cs:88-94, DeferredAddressSpaceSink.cs:49; NodeManagerModelChangeOnAddTests.cs exists
7 The U2 prerequisite guard is merged: DeferredSinkForwardingReflectionTests (Commons.Tests) reflects over every member of every forwarding interface of DeferredAddressSpaceSink via a DispatchProxy recorder that auto-implements future members, plus a guard-of-the-guard pinning the interface set tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredSinkForwardingReflectionTests.cs:29-75
8 FeedHistorizedRefs + ProvisionHistorizedTags operate on the plan's add/remove/change deltas and fire at the end of Apply unconditionally — independent of rebuild-vs-surgical routing AddressSpaceApplier.cs:208-213, 226-353
9 The driver-side routing maps (_nodeIdByDriverRef / _driverRefByNodeId / _alarmNodeIdByDriverRef) and desired-subscription sets are rebuilt every apply by DriverHostActor.PushDesiredSubscriptions, fully independent of the node-manager rebuild DriverHostActor.cs:107-158 (doc: "Rebuilt every apply")
10 Variable NodeIds are folder-scoped strings derived deterministically from composition (EquipmentNodeIds.Variable/SubFolder), all in the one server namespace (NamespaceIndex) — adds cannot shift existing NodeIds EquipmentNodeIds.cs, OtOpcUaNodeManager.cs:1299, 1391
11 The applier already writes a terminal condition state (RemovedConditionState) to removed equipment/alarm condition nodes before the teardown AddressSpaceApplier.cs:83-98, 665-681
12 03/P1 is STILL OPEN at f6eaa267 ("guard merged, work not started") archreview/03-server-runtime.md prior-finding table row P1 + §2 P1

Conclusion: the finding fully reproduces. Adding one tag to one equipment still tears down and recreates every node on the server, killing every client subscription server-wide. The striking verification result (claims 5+6) is that Phase 1 (pure-adds) requires no new sink capability at all — the idempotent Materialise passes already create exactly the new nodes when the rebuild is skipped; the only missing pieces are (a) a classifier that routes a pure-add plan away from SafeRebuild() and (b) a post-materialise NodeAdded model-change announcement. Removals (Phase 2) are where new sink members are needed.


Restatement

AddressSpaceApplier.Apply treats every topology delta identically: any added or removed equipment, scripted alarm, equipment tag, or virtual tag sets structuralRebuild = true, which calls sink.RebuildAddressSpace()OtOpcUaNodeManager.RebuildAddressSpace() clears all five node maps and detaches every NodeState. The subsequent Materialise passes recreate the world as new NodeState instances. OPC UA monitored items are bound to the old instances, so every client subscription on the server goes dead — for a deploy that added a single tag. On a production SCADA server with dozens of clients (including ScadaBridge's Data Connection Layer), every authoring action that touches topology is a server-wide subscription outage. The F10b surgical path (ISurgicalAddressSpaceSink.UpdateTagAttributes / UpdateFolderDisplayName) proves in-place mutation preserves subscriptions, but covers only in-place attribute edits and UNS folder renames.

Verification

Confirmed — see the table above. Key mechanics re-checked at f6eaa267:

  • The rebuild predicate (AddressSpaceApplier.cs:134-140): AddedEquipment.Count > 0 || RemovedEquipment.Count > 0 || ChangedEquipment.Count > 0 || AddedAlarms/RemovedAlarms/ChangedAlarms || AddedEquipmentTags/RemovedEquipmentTags || ChangedEquipmentTags.Any(!TagDeltaIsSurgicalEligible) || AddedEquipmentVirtualTags/RemovedEquipmentVirtualTags || ChangedEquipmentVirtualTags.Any(!VtagDeltaIsNodeIrrelevant). Pure adds land in this branch.
  • The apply pipeline (OpcUaPublishActor.HandleRebuild, OpcUaPublishActor.cs:290-364): load artifact → ParseCompositionAddressSpacePlanner.Compute(_lastApplied, composition)plan.IsEmpty short-circuit (:328 — the known IsEmpty-gate: the Materialise passes do NOT run on an empty plan, so anything the plan is blind to never materialises) → _applier.Apply(plan) → the four Materialise passes over the full composition (:338-354). A pure-add plan is non-empty, so the passes always run for it.
  • Idempotency of the passes: EnsureFolder/EnsureVariable early-return on an existing NodeId; MaterialiseAlarmCondition drops-and-recreates per id (idempotent but not subscription-preserving for that one condition node — see Design, native-alarm note). MaterialiseHierarchy/MaterialiseEquipmentTags/MaterialiseEquipmentVirtualTags/MaterialiseScriptedAlarms therefore add exactly the new nodes and no-op existing ones when no rebuild cleared the maps.
  • Folder-scoped NodeIds (the second known gotcha): variables materialise at EquipmentNodeIds.Variable(equipmentId, folderPath, name) — NOT the driver FullName — so add-only deploys cannot collide with or re-key existing nodes; the driver value router (_nodeIdByDriverRef) is rebuilt per apply from the same composition and needs no change.
  • The guard: DeferredSinkForwardingReflectionTests.Every_forwarding_interface_method_reaches_the_inner_sink iterates ForwardingInterfaces = { IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink } via GetMethods() — a member added to ISurgicalAddressSpaceSink in Phase 2 is picked up with zero test edits and fails the guard if DeferredAddressSpaceSink does not forward it.

Root cause

The applier's rebuild predicate conflates three delta classes with very different minimal-mutation requirements:

  1. In-place changes (attribute edits, renames) — already handled surgically (F10b).
  2. Pure additions — need no teardown at all: the idempotent Materialise passes add exactly the new nodes; RaiseNodesAddedModelChange exists to announce them. The rebuild is pure collateral damage.
  3. Removals / node-affecting mutations — genuinely need teardown, but only of the affected subtree, not the world.

The predicate collapses all three into one boolean. The fix is a classifier that names the classes and routes each to its minimal mutation, with full rebuild as the safety valve for anything else.


Proposed design

The delta classifier

A new pure static AddressSpaceChangeClassifier in ZB.MOM.WW.OtOpcUa.OpcUaServer (same project as the planner — it is a pure function over AddressSpacePlan, unit-testable without any sink), returning:

/// <summary>How AddressSpaceApplier must mutate the address space for a given plan.</summary>
public enum AddressSpaceChangeKind
{
    /// <summary>Plan.IsEmpty — nothing to do.</summary>
    Empty,
    /// <summary>Only surgical-eligible changed tags, node-irrelevant vtag changes, and/or folder
    /// renames — the existing F10b in-place path. No adds, no removes.</summary>
    AttributeOnly,
    /// <summary>Only additions (any mix of Equipment/Alarms/Tags/VirtualTags/Drivers) on top of
    /// at-most-AttributeOnly changes — no removals, no node-affecting changes. Phase 1.</summary>
    PureAdd,
    /// <summary>Only removals on top of at-most-AttributeOnly changes — no additions, no
    /// node-affecting changes. Phase 2.</summary>
    PureRemove,
    /// <summary>Additions AND removals, still no node-affecting changes. Phase 3 applies
    /// removes-then-adds surgically; until Phase 3 it maps to Rebuild.</summary>
    AddRemoveMix,
    /// <summary>Anything with a node-affecting change (ChangedEquipment, ChangedAlarms,
    /// non-surgical ChangedEquipmentTags, node-relevant ChangedEquipmentVirtualTags) — the
    /// unclassifiable-safe default. Full rebuild.</summary>
    Rebuild,
}

public static class AddressSpaceChangeClassifier
{
    public static AddressSpaceChangeKind Classify(AddressSpacePlan plan);
}

Classification algorithm (evaluated top-down; first match wins):

Order Condition Kind
1 plan.IsEmpty Empty
2 ChangedEquipment.Count > 0 or ChangedAlarms.Count > 0 or ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d)) or ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d)) Rebuild
3 hasAdds && hasRemoves AddRemoveMix
4 hasAdds PureAdd
5 hasRemoves PureRemove
6 otherwise (only surgical tag deltas / node-irrelevant vtag deltas / folder renames / driver deltas) AttributeOnly

where hasAdds = AddedEquipment.Count + AddedAlarms.Count + AddedEquipmentTags.Count + AddedEquipmentVirtualTags.Count > 0 and hasRemoves = RemovedEquipment.Count + RemovedAlarms.Count + RemovedEquipmentTags.Count + RemovedEquipmentVirtualTags.Count > 0. Driver deltas (Added/Removed/ChangedDrivers) are node-inert — they never touch the address space today (excluded from the current predicate, routed to DriverSpawnPlanner in Runtime) and stay excluded from hasAdds/hasRemoves. Note a PureAdd/PureRemove/AddRemoveMix plan may also carry surgical tag deltas and folder renames — the applier applies those via the existing F10b path in the same pass (they are orthogonal in-place mutations).

The two eligibility predicates (TagDeltaIsSurgicalEligible, VtagDeltaIsNodeIrrelevant, currently private statics at AddressSpaceApplier.cs:630-663) move to the classifier (as internal static, applier keeps thin delegating calls or references them directly) so the whole classification is one pure, table-testable unit. Their whitelist-via-with-expression technique is preserved byte-for-byte (01 report calls it out as a positive).

Phase staging

Phase 1 — pure-adds (the ~90% win, no new sink members). When Classify == PureAdd:

  1. Apply does not call SafeRebuild(). It still applies any coincident folder renames + surgical tag deltas via the existing ISurgicalAddressSpaceSink path (any false/throw there → SafeRebuild() fallback, exactly as today).
  2. The four Materialise passes in OpcUaPublishActor.HandleRebuild then run as they always do and create exactly the added nodes (folders/variables/conditions), idempotently no-op'ing everything that already exists. No applier change needed for the creation itself — this is the load-bearing simplification. The only pass with a wrinkle is MaterialiseScriptedAlarms/native-alarm materialisation: MaterialiseAlarmCondition is drop-and-recreate per id, so on a pure-add apply it would drop-and-recreate every existing enabled condition node (killing monitored items on those condition nodes only). Fix: make MaterialiseAlarmCondition skip-if-present-and-same-kind on the node-manager side — add an idempotent early-return when _alarmConditions already holds the id with the same isNative flag (drop-and-recreate stays for the kind-swap/re-severity case, which only reaches this path via a rebuild where the map was just cleared). This keeps existing alarm subscriptions alive across a pure-add deploy and is behavior-identical on the rebuild path.
  3. Announcement: after the Materialise passes complete, OpcUaPublishActor calls a new applier.AnnounceAddedNodes(plan) (Phase-1 applier method, no new sink member — it uses the existing, guard-covered RaiseNodesAddedModelChange). It computes a deduplicated set of affected parent ids: for each added tag/vtag → its materialise parent (EquipmentId or EquipmentNodeIds.SubFolder(EquipmentId, FolderPath)); for each added scripted alarm → its EquipmentId; for each added equipment → its UnsLineId when present, else the equipment's own id (the root has no announceable id; announcing the new folder itself is valid Part 3 — verb NodeAdded, Affected = the added node). One RaiseNodesAddedModelChange per distinct id. Ordering is correct by construction: Apply returns before the passes run, and the announce runs after them, so the nodes exist when clients re-browse. The announce is gated on !outcome.RebuildCalled (after a full rebuild the announcement is moot — subscriptions are dead anyway, unchanged behavior).
  4. Existing-subscription behavior on add: none affected — no existing NodeState is touched; per OPC UA Part 4 monitored items are unaffected by address-space growth; model-aware clients get the Part 3 GeneralModelChangeEvent and re-browse.

Phase 2 — pure-removes (new sink capability). When Classify == PureRemove:

New members on ISurgicalAddressSpaceSink (existing interface — the guard's ForwardingInterfaces set needs no change; the DispatchProxy recorder auto-covers new members):

/// <summary>Remove ONE existing value-variable node in place (detach from parent, drop from the
/// predefined-node set, drop the historized-tagname registration), then raise a Part 3
/// GeneralModelChangeEvent (verb=NodeDeleted) outside the lock. The caller has already written a
/// final Bad quality to the node so in-flight MonitoredItems observe the removal. Returns false
/// when the node id is unknown (caller falls back to a full rebuild — map-drift safety valve).</summary>
bool RemoveVariableNode(string variableNodeId);

/// <summary>Remove ONE existing Part 9 alarm-condition node in place (detach, drop from the
/// predefined-node set, drop the native-alarm flag), then raise NodeDeleted outside the lock. The
/// caller has already written the terminal RemovedConditionState (Retain=false). Returns false
/// when the condition id is unknown (caller rebuilds).</summary>
bool RemoveAlarmConditionNode(string alarmNodeId);

/// <summary>Remove an equipment folder AND its entire descendant subtree (sub-folders, variables,
/// condition nodes) in place: per-descendant map/registration cleanup (variables, historized
/// tagnames, conditions, native flags), notifier demotion (RemoveRootNotifier + 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>
bool RemoveEquipmentSubtree(string equipmentNodeId);

Implemented on OtOpcUaNodeManager (all mutation under Lock; the NodeDeleted model-change event built under Lock and reported after release, mirroring RaiseNodesAddedModelChange / ReportNodeShapeChangedEvent), forwarded by SdkAddressSpaceSink, capability-sniff-forwarded by DeferredAddressSpaceSink (_inner is ISurgicalAddressSpaceSink s && s.X(...) — false pre-swap ⇒ rebuild fallback, same as UpdateTagAttributes), and no-op'd nowhere else (NullOpcUaAddressSpaceSink doesn't implement the surgical interface — unchanged).

Applier routing for PureRemove:

  1. Partition removals: removed equipment "own" their child tags/vtags/alarms — a RemovedEquipmentTags/RemovedEquipmentVirtualTags/RemovedAlarms entry whose EquipmentId is in RemovedEquipment is subsumed by the subtree removal and skipped individually.
  2. Pre-teardown terminal writes (extends the existing :83-98 pattern): removed value variables get WriteValue(nodeId, null, Bad, ts); removed condition nodes (scripted alarms AND alarm-bearing tags — the latter is a today-gap: RemovedEquipmentTags with Alarm != null currently get no condition write) get SafeWriteAlarmCondition(nodeId, RemovedConditionState, ts). This is what an in-flight MonitoredItem observes (final Bad notification), which is the spec-friendly signal; after the node is gone, re-subscription attempts get BadNodeIdUnknown from the SDK because the node is no longer in the predefined set.
  3. Teardown order children-first: individual variable/condition removals (for surviving equipment), then RemoveEquipmentSubtree per removed equipment. Empty leftover FolderPath sub-folders on surviving equipment are deliberately kept (browse-visible but harmless; the next full rebuild collects them — documented v1 cut, mirrors EnsureFolder's create-only semantics).
  4. Any false/throw from a remove ⇒ SafeRebuild() (one-way ratchet per apply: once rebuilt, skip remaining surgical work — the rebuild already reconciled everything).
  5. Subscription behavior on remove (per spec): subscribers of other nodes are untouched; subscribers of the removed node see a final Bad value (step 2), the node disappears from browse, and model-aware clients get NodeDeleted. This matches OPC UA Part 3/4 expectations for node deletion.

Phase 3 — mixed add+remove batches. When Classify == AddRemoveMix: apply the Phase-2 remove pass, then let the Materialise passes + Phase-1 announce handle the adds — removes-then-adds within one apply. The one ordering hazard is an id reused across the remove and add sets in the same deploy (e.g. tag removed from equipment E folder F name N while another tag materialises at the identical folder-scoped NodeId): removes run in Apply (before the passes), adds run in the passes (after), so remove-then-recreate is the natural order and the recreated node is a fresh NodeState (correct — it is a different tag). No special-casing needed; covered by a targeted test. Until Phase 3 ships, AddRemoveMix maps to Rebuild (today's behavior), which is why each phase is independently shippable.

NodeId / namespace-index stability constraints

  • All materialised nodes live in the single server namespace (NamespaceIndex on OtOpcUaNodeManager); the index is assigned at server start and is untouched by deploys. No constraint is added or relaxed by this work.
  • Folder/variable/condition NodeIds are deterministic pure functions of composition (UnsAreaId/UnsLineId/EquipmentId, EquipmentNodeIds.SubFolder/Variable, ScriptedAlarmId) — an add cannot re-key an existing node, and the surgical paths reuse the same derivations the Materialise passes use (the applier already does this for UpdateTagAttributes at :177). Any new derivation in the remove path MUST go through EquipmentNodeIds — never hand-rolled strings.
  • Same-NodeId collision on add (two tags materialising to one folder-scoped id) keeps today's first-wins EnsureVariable semantics — no regression, explicitly out of scope.

Interaction with the historian/provisioning/alarm seams (all verified no-change or additive)

  • Historized-ref feed (FeedHistorizedRefs) and historian provisioning (ProvisionHistorizedTags) already consume the plan's add/remove/change deltas and fire unconditionally at the end of Apply — routing a plan surgically instead of via rebuild changes nothing for them. (They are the design template for this work: per-deploy delta consumption, non-blocking, deploy-safe.)
  • Native alarm condition nodes: added alarm-bearing tags materialise via SafeMaterialiseAlarmCondition in MaterialiseEquipmentTags — additive on the pure-add path once the skip-if-present guard (Phase 1 step 2) lands. Removal of an alarm-bearing tag routes to RemoveAlarmConditionNode (its node is a condition, not a value variable). EnsureFolderIsEventNotifier promotion for a first-alarm-on-existing-equipment add is already runtime-safe (the rebuild path performs it on a live server today).
  • Driver subscriptions / value routing: rebuilt every apply by DriverHostActor.PushDesiredSubscriptions independent of this path — adds get subscribed, removes get unsubscribed and un-routed, no change needed.
  • AddressSpaceApplyOutcome: gains nothing structurally in Phase 1 (the existing RebuildCalled=false + AddedNodes counts express the pure-add outcome); the apply log line gains the classifier verdict (kind={Kind}) so live verification can grep for kind=PureAdd, rebuild=False.

Fallback-to-rebuild safety valve

Single principle, enforced by the classifier's shape: anything not positively classified as safe falls into Rebuild (order rule 2 catches every node-affecting change, including any future plan field that makes a changed record unequal — the with-expression whitelists are default-closed). At apply time, every surgical sink call keeps the F10b contract: false or throw ⇒ SafeRebuild() and mark rebuilt. Net: the worst possible outcome of any bug in this work is today's behavior (a full rebuild).

Alternatives considered (and rejected)

  • Re-home monitored items onto recreated nodes after a rebuild — the SDK offers no supported API to re-bind a MonitoredItem to a new NodeState; rejected (round-1 plan concurs).
  • Generic diff-and-patch of every delta class (make ChangedEquipment/ChangedAlarms surgical too) — larger and riskier; changed-alarm re-materialisation and equipment renames can ride a later increment (equipment DisplayName rename is UpdateFolderDisplayName-shaped — noted as follow-up, not in scope). The add/remove split follows the plan's existing typed delta sets.
  • Classify in AddressSpacePlanner (attach the kind to the plan record) — rejected: Compute is a pure diff and the kind is a policy over the diff that the applier owns (and the F10b eligibility predicates it needs already live applier-side). A separate pure classifier keeps the planner untouched and the policy table-testable.
  • Batch remove member (RemoveNodes(IReadOnlyList<…>)) instead of three typed members — rejected: the per-node bool false-means-rebuild contract (the established F10b posture) gets murky on a batch; three members mirror the existing surgical-member shape and the guard covers each independently.
  • Return true (idempotent success) from a remove of an unknown node — rejected: an unknown id on remove means the node-manager maps drifted from what the planner believes; the house posture (F10b UpdateTagAttributes:false) is resync-via-rebuild. Documented in the member xmldoc.
  • Announce adds from inside Apply — impossible: Apply runs before the Materialise passes create the nodes; the announce must be a post-passes step in OpcUaPublishActor (verified ordering OpcUaPublishActor.cs:335-354).

Implementation steps (exact paths)

Phase 1 — pure-adds

  1. src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs (new): AddressSpaceChangeKind enum + Classify(AddressSpacePlan) per the table; move TagDeltaIsSurgicalEligible + VtagDeltaIsNodeIrrelevant here as internal static (from AddressSpaceApplier.cs:630-663).
  2. src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs: replace the structuralRebuild boolean (:134-140) with var kind = AddressSpaceChangeClassifier.Classify(plan); Rebuild/AddRemoveMix/PureRemove (until their phases ship) ⇒ SafeRebuild(); PureAdd/AttributeOnly ⇒ no rebuild + existing surgical renames/tag-deltas (with the existing false⇒rebuild fallback). Add kind to the apply log line. Add public IReadOnlyList<string> ComputeAddAnnouncements(AddressSpacePlan plan) (pure, dedup parent-id set per the design) and public void AnnounceAddedNodes(AddressSpacePlan plan) calling _sink.RaiseNodesAddedModelChange per id (Safe-wrapped). While here, refresh the stale F14-era class doc (:7-26, 01/C-7).
  3. src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs: MaterialiseAlarmCondition (:586-604) — idempotent early-return when _alarmConditions holds the id AND the stored native flag equals isNative (keeps drop-and-recreate only for kind-swaps; requires remembering the flag — _nativeAlarmNodeIds already encodes it).
  4. src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs: in HandleRebuild after the fourth Materialise pass (:354), if (!outcome.RebuildCalled) _applier.AnnounceAddedNodes(plan);.
  5. Live-/run gate (see Tests) + docs: docs/v2/-appropriate note + archreview/plans/STATUS.md row.

Phase 2 — pure-removes

  1. src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs: add the three members (signatures above). Guard-first discipline: the reflection guard auto-covers them the moment they exist — implement DeferredAddressSpaceSink forwarding in the same commit or the guard fails the build's test leg (that failure IS the negative control; observe it once deliberately).
  2. src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs: capability-sniffing forwards for all three (pattern of :58-70).
  3. src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs: implement RemoveVariableNode / RemoveAlarmConditionNode / RemoveEquipmentSubtree (teardown under Lock mirroring the per-node loops inside RebuildAddressSpace :1694-1734, scoped to the id/subtree; clean _variables/_historizedTagnames/_alarmConditions/_nativeAlarmNodeIds/_folders/_notifierFolders(+RemoveRootNotifier)/_eventNotifierSources); internal GeneralModelChangeEventState BuildNodesRemovedModelChange(string affectedNodeId) mirroring BuildNodesAddedModelChange (:1627-1654) with Verb = NodeDeleted; report outside Lock.
  4. src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs: three one-line forwards.
  5. AddressSpaceApplier.cs: PureRemove arm — terminal writes (Bad value / RemovedConditionState, extending :83-98 incl. the alarm-bearing-removed-tag gap), subsumption partition, children-first removal calls, false/throw ⇒ SafeRebuild() ratchet.
  6. Live-/run remove gate + docs.

Phase 3 — mixed batches

  1. AddressSpaceChangeClassifier.cs + AddressSpaceApplier.cs: AddRemoveMix routes to the Phase-2 remove pass then falls through to the pure-add (no-rebuild + announce) behavior; remove-then-recreate same-NodeId test.
  2. Final docs + STATUS.md close-out.

Tests

Ordering rule: guard-set/TDD first in every task (the F10b precedent — 258 unit tests + 3 reviews missed prod-inertness; the guard + live gate are the two nets that catch it).

  • Guard extension (Phase 2, FIRST): no test-code change needed — DeferredSinkForwardingReflectionTests auto-covers new ISurgicalAddressSpaceSink members. The TDD step is a deliberate negative control: add the interface members with a non-forwarding DeferredAddressSpaceSink stub (=> false), run dotnet test --filter "FullyQualifiedName~DeferredSinkForwardingReflectionTests" and observe the member-named FAIL, then implement the forward and observe PASS. Also extend the hand-written DeferredAddressSpaceSinkTests (Commons + OpcUaServer.Tests variants) with arg-fidelity cases for the three members.
  • Classifier (Phase 1): new AddressSpaceChangeClassifierTests (OpcUaServer.Tests) — table-driven ([Theory]/[MemberData]) over the full matrix: empty; each single add class ⇒ PureAdd; each single remove class ⇒ PureRemove; add+remove ⇒ AddRemoveMix; each changed class ⇒ Rebuild; surgical-eligible-changed-only ⇒ AttributeOnly; renames-only ⇒ AttributeOnly; adds+surgical-changes ⇒ PureAdd; adds+non-surgical-change ⇒ Rebuild; driver-only deltas ⇒ AttributeOnly (node-inert).
  • Applier (Phase 1): extend AddressSpaceApplierTests (the RecordingSink : IOpcUaAddressSpaceSink, ISurgicalAddressSpaceSink at :1761 already records RebuildCalls + ModelChangeQueue): pure-add plan ⇒ RebuildCalls == 0, outcome.RebuildCalled == false; AnnounceAddedNodes ⇒ one RaiseNodesAddedModelChange per distinct parent (dedup + subfolder-parent + added-equipment cases); pure-add with coincident surgical tag delta ⇒ no rebuild + UpdateTagAttributes called; surgical-false during a pure-add apply ⇒ rebuild fallback; remove-only and add+remove plans still rebuild (phase-boundary regression pins, updated in Phases 2/3).
  • Node manager (Phase 1): extend NodeManagerSurgicalShapeUpdateTests-style suite (or new NodeManagerAlarmIdempotentMaterialiseTests): re-MaterialiseAlarmCondition same id+kind keeps the same AlarmConditionState instance (FindAlarmCondition reference-equal); kind-swap still recreates.
  • Node manager (Phase 2): new NodeManagerSurgicalRemoveTests: ensure→remove variable ⇒ FindVariable null, VariableCount decremented, TryGetHistorianTagname false, unknown id ⇒ false; alarm-condition remove ⇒ FindAlarmCondition null + native flag cleared; equipment-subtree remove ⇒ descendants gone, notifier demoted (no orphaned root-notifier ref), other equipment untouched; BuildNodesRemovedModelChange populates Verb=NodeDeleted + Affected (mirrors NodeManagerModelChangeOnAddTests).
  • Publish actor (Phase 1): extend tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs: pure-add artifact apply ⇒ sink records NO RebuildAddressSpace call, records RaiseNodesAddedModelChange AFTER EnsureVariable for the added tag (ordering assertion via a sequenced recording sink).
  • Subscription-survival integration (Phase 1, the headline): new SubscriptionSurvivalTests in tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ (reuse DualEndpointTests' in-process server + DefaultSessionFactory session bootstrap): materialise tag A, create a real subscription + monitored item on A, drive a pure-add apply (applier + Materialise passes) adding tag B, then (a) A's monitored item still delivers on a subsequent WriteValue (no BadNodeIdUnknown/dead item), (b) B is browsable + readable. Phase 2 sibling: subscribe A + B, surgical-remove B ⇒ A alive, B's item observes the final Bad then the node is gone from browse; Phase 3 sibling for the mixed case. This also chips at 03/U6's "OpcUaServer.IntegrationTests has one test".
  • Live-/run gate (MANDATORY per phase — the F10b lesson: an optimization path that isn't live-driven can ship inert despite full unit green): on docker-dev, rebuild BOTH central-1 and central-2 (:9200 round-robins them); dotnet run --project src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI -- subscribe -u opc.tcp://localhost:4840 -n "ns=2;s=<existing tag>" -i 500 and keep it running; POST http://localhost:9200/api/deployments (X-Api-Key) a config adding one tag to a different equipment; assert (1) the running subscribe keeps delivering (no dead/Bad transition on the existing node), (2) the central log shows kind=PureAdd, rebuild=False, (3) browse/read finds the new node, (4) the new tag goes Good once the driver publishes. Phase 2: same recipe with a one-tag remove (subscription on a different tag survives; the removed node reads BadNodeIdUnknown after). Phase 3: one deploy carrying +1/1.

Effort + Risk (per phase)

Phase Effort Risk / blast radius Mitigations
1 — pure-adds M (~1.5-2 days incl. integration + live gate) High-leverage, moderate risk: touches the apply routing core, but adds no sink members and no node-manager teardown; the worst regression is "should have rebuilt but didn't" (stale space) — bounded by the default-closed classifier + F10b fallback table-driven classifier tests; recording-sink applier tests; subscription-survival integration test; mandatory live-/run; classifier verdict in the apply log
2 — pure-removes M-L (~2-3 days) Highest risk slice: new node-manager teardown under Lock (leaked notifier refs / map drift / lock-order inversion on the event report) + the deferred-forwarding trap guard auto-coverage + deliberate negative control; teardown mirrors the proven RebuildAddressSpace loops; report-outside-Lock mirrors three existing patterns; false⇒rebuild ratchet; live-/run remove gate
3 — mixed S-M (~0.5-1 day) Low: composition of two shipped paths; the only new surface is remove-then-recreate ordering targeted same-NodeId reuse test; live-/run mixed gate

Overall: Large (matches OVERALL #7), ~4-6 days. Each phase lands independently; between phases, unhandled kinds keep today's rebuild behavior.


Task breakdown

Tasks are bite-sized TDD increments. Classification: high-risk = touches the sink seam / node-manager mutation under Lock / apply routing (review + live-verify weight); low-risk = pure functions, tests, docs. Every task: run its FAIL→PASS filter, then the touched project's full suite before committing. Phase boundaries are hard sequence points (marked). Baseline branch suggestion: feat/r2-07-surgical-pure-adds off master.

── PHASE 1 — pure-adds ──

T1 — Classifier skeleton + table-driven tests (RED→GREEN)

  • Classification: low-risk (pure function)
  • Estimated implement time: 5 min (tests) + 5 min (impl)
  • Parallelizable with: T5 (different files); everything else in Phase 1 depends on it
  • Files: src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceChangeClassifier.cs (new), tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceChangeClassifierTests.cs (new)
  • TDD: write the full [Theory] matrix from the Tests section (reuse AddressSpacePlannerTests' plan-construction helpers); dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~AddressSpaceChangeClassifierTests" → FAIL (type missing) → implement enum + Classify (first-match-wins table) with the two eligibility predicates moved from the applier (AddressSpaceApplier.cs:630-663) as internal static; applier temporarily references them at their new home (compile-only change to the applier) → PASS. Full project suite green.
  • Commit: feat(opcua): add AddressSpaceChangeClassifier — typed delta classification for the applier (R2-07 T1)

T2 — Route Apply through the classifier (behavior-preserving except PureAdd)

  • Classification: high-risk (apply routing core)
  • Estimated implement time: 5 min (tests) + 10 min (impl)
  • Parallelizable with: — (blocks on T1)
  • Files: src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs, tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierTests.cs
  • TDD: add applier tests — pure-add-only plan ⇒ RebuildCalls == 0 + RebuildCalled == false; pure-add + surgical tag delta ⇒ no rebuild + UpdateTagAttributes recorded; pure-add + surgical-returns-false ⇒ rebuild fallback; remove-only / add+remove / changed-non-surgical plans ⇒ still rebuild (phase pins); dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~AddressSpaceApplierTests" → new tests FAIL → replace structuralRebuild with the classifier switch (Rebuild/AddRemoveMix/PureRemove ⇒ SafeRebuild(); PureAdd/AttributeOnly ⇒ surgical path) + kind={Kind} in the apply log → PASS, every pre-existing applier test untouched-green (the behavior-preservation proof).
  • Commit: feat(opcua): route AddressSpaceApplier through the classifier — pure-add deploys no longer rebuild (R2-07 T2)

T3 — ComputeAddAnnouncements + AnnounceAddedNodes on the applier

  • Classification: low-risk (uses the existing guard-covered RaiseNodesAddedModelChange sink member)
  • Estimated implement time: 5 min (tests) + 5 min (impl)
  • Parallelizable with: T4a (blocks T4b)
  • Files: AddressSpaceApplier.cs, AddressSpaceApplierTests.cs
  • TDD: tests — two tags added under the same equipment ⇒ ONE announce for the equipment id; tag with FolderPath ⇒ announce EquipmentNodeIds.SubFolder(...); added scripted alarm ⇒ equipment id; added equipment with UnsLineId ⇒ line id, without ⇒ its own id; empty-add plan ⇒ zero announces; filter as T2 → FAIL → implement (pure compute + Safe-wrapped sink loop) → PASS.
  • Commit: feat(opcua): announce pure-add nodes via Part 3 NodeAdded model-change (R2-07 T3)

T4a — Idempotent MaterialiseAlarmCondition (skip-if-present-and-same-kind)

  • Classification: high-risk (node-manager mutation under Lock)
  • Estimated implement time: 5 min (tests) + 5 min (impl)
  • Parallelizable with: T3
  • Files: src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs, tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerAlarmIdempotentMaterialiseTests.cs (new; crib the server-bootstrap fixture from NodeManagerModelChangeOnAddTests)
  • TDD: tests — re-materialise same id+kind ⇒ FindAlarmCondition returns the same instance (subscription-preserving); kind-swap (isNative flip) ⇒ recreated; post-rebuild re-materialise ⇒ fresh (map was cleared); dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~NodeManagerAlarmIdempotentMaterialise" FAIL → early-return guard → PASS. Run the full NodeManager* + AlarmCommandRouter suites (condition-handler regressions).
  • Commit: feat(opcua): make MaterialiseAlarmCondition idempotent for same-kind re-applies (R2-07 T4a)

T4b — Publish-actor post-materialise announce hook

  • Classification: high-risk (deploy pipeline)
  • Estimated implement time: 5 min (tests) + 5 min (impl)
  • Parallelizable with: — (blocks on T2+T3+T4a)
  • Files: src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs, tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs
  • TDD: extend the rebuild tests with a pure-add artifact case — recording sink shows NO RebuildAddressSpace, and RaiseNodesAddedModelChange recorded AFTER the added tag's EnsureVariable (sequence list); a rebuild-kind artifact records NO announce; dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests --filter "FullyQualifiedName~OpcUaPublishActorRebuildTests" FAIL → add if (!outcome.RebuildCalled) _applier.AnnounceAddedNodes(plan); after the fourth pass → PASS.
  • Commit: feat(runtime): announce added nodes after the materialise passes on non-rebuild applies (R2-07 T4b)

T5 — Subscription-survival integration test (over-the-wire)

  • Classification: high-risk to write, low-risk to land (test-only)
  • Estimated implement time: 15 min
  • Parallelizable with: T1-T4 (can be authored against the design; goes green only after T4b)
  • Files: tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs (new; reuse DualEndpointTests' server + session bootstrap)
  • TDD: subscribe tag A (real monitored item, 250 ms), pure-add apply adds tag B, WriteValue(A) ⇒ notification arrives (item alive), browse finds B. Written FIRST it FAILS on master behavior (rebuild kills the item); goes PASS once T2/T4b land. dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests --filter "FullyQualifiedName~SubscriptionSurvivalTests".
  • Commit: test(opcua): over-the-wire subscription-survival on pure-add deploy (R2-07 T5)

T6 — Phase 1 live-/run gate (docker-dev) — MANDATORY before merging Phase 1

  • Classification: high-risk verification (the F10b inertness net)
  • Estimated implement time: 20 min
  • Parallelizable with:
  • Files: none (evidence into the PR/STATUS notes)
  • Steps: rebuild both central-1 + central-2 images; Client.CLI subscribe on an existing tag (keep running); POST :9200/api/deployments adding ONE tag to a different equipment; assert: subscribe stream uninterrupted; central log kind=PureAdd, rebuild=False; browse/read shows the new node; new tag turns Good on first driver publish. Capture the log lines.
  • Commit: docs(archreview): record R2-07 Phase 1 live-verify evidence in STATUS.md
  • ── PHASE 1 SHIPPABLE BOUNDARY — merge/PR here ──

── PHASE 2 — pure-removes ──

T7 — Sink members + deferred forwarding, guard-first (negative control observed)

  • Classification: high-risk (THE deferred-sink trap; guard-set coverage FIRST)
  • Estimated implement time: 5 min (interface+stub, observe guard FAIL) + 5 min (forward, PASS)
  • Parallelizable with: — (Phase 2 root)
  • Files: src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs, src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/DeferredAddressSpaceSink.cs, tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/OpcUa/DeferredAddressSpaceSinkTests.cs
  • TDD (the guard IS the test): add the three members with DeferredAddressSpaceSink stubs => false (non-forwarding); dotnet test tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests --filter "FullyQualifiedName~DeferredSinkForwardingReflectionTests"FAIL with the member-named diagnostic (negative control — record it) → implement capability-sniffing forwards (pattern DeferredAddressSpaceSink.cs:58-70) → PASS. Add arg-fidelity cases to the hand-written deferred tests. NOTE: SdkAddressSpaceSink + OtOpcUaNodeManager must compile — add node-manager methods returning false (rebuild-fallback semantics ⇒ tree stays shippable) with xmldoc marking T8-T10 as the real implementation.
  • Commit: feat(commons): add surgical remove members to ISurgicalAddressSpaceSink + deferred forwarding (guard-verified) (R2-07 T7)

T8 — RemoveVariableNode on the node manager

  • Classification: high-risk (teardown under Lock)
  • Estimated implement time: 5 min (tests) + 10 min (impl)
  • Parallelizable with: T9 (disjoint methods, same file — coordinate or sequence)
  • Files: OtOpcUaNodeManager.cs, tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerSurgicalRemoveTests.cs (new)
  • TDD: ensure→remove ⇒ FindVariable null + VariableCount down + TryGetHistorianTagname false; unknown id ⇒ false; BuildNodesRemovedModelChange carries Verb=NodeDeleted + Affected (internal seam, mirrors NodeManagerModelChangeOnAddTests); dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~NodeManagerSurgicalRemoveTests" FAIL → implement (detach mirrors RebuildAddressSpace:1694-1701 scoped to one id; build event under Lock, report outside) → PASS.
  • Commit: feat(opcua): surgical RemoveVariableNode with NodeDeleted model-change (R2-07 T8)

T9 — RemoveAlarmConditionNode

  • Classification: high-risk
  • Estimated implement time: 5 + 5 min
  • Parallelizable with: T8 (coordinate on the shared test file)
  • Files: OtOpcUaNodeManager.cs, NodeManagerSurgicalRemoveTests.cs
  • TDD: materialise (native + scripted)→remove ⇒ FindAlarmCondition null + native flag cleared + NodeDeleted built; unknown ⇒ false; filter as T8 FAIL→PASS (teardown mirrors :1703-1711 scoped).
  • Commit: feat(opcua): surgical RemoveAlarmConditionNode (R2-07 T9)

T10 — RemoveEquipmentSubtree (descendants + notifier demotion)

  • Classification: high-risk (the notifier/root-ref leak hazard)
  • Estimated implement time: 10 min (tests) + 15 min (impl)
  • Parallelizable with: — (blocks on T8+T9 for the per-node primitives it reuses)
  • Files: OtOpcUaNodeManager.cs, SdkAddressSpaceSink.cs (the three forwards), NodeManagerSurgicalRemoveTests.cs
  • TDD: equipment with subfolder+variables+condition → remove ⇒ all descendants gone from every map, RemoveRootNotifier invoked (no orphaned Server↔folder ref — assert via _notifierFolders-driven observable or the notifier accessor), _eventNotifierSources entry dropped, sibling equipment fully intact; unknown ⇒ false; filter FAIL→PASS. Wire the three SdkAddressSpaceSink forwards here (replacing T7's false-stubs at the manager) and re-run the Commons guard.
  • Commit: feat(opcua): surgical RemoveEquipmentSubtree with notifier demotion (R2-07 T10)

T11 — Applier PureRemove arm (terminal writes + subsumption + fallback ratchet)

  • Classification: high-risk (apply routing)
  • Estimated implement time: 10 min (tests) + 10 min (impl)
  • Parallelizable with:
  • Files: AddressSpaceApplier.cs, AddressSpaceApplierTests.cs
  • TDD: remove-one-tag plan ⇒ Bad WriteValue then RemoveVariableNode, NO rebuild; removed alarm-bearing tag ⇒ RemovedConditionState write + RemoveAlarmConditionNode (closes the today-gap); removed equipment ⇒ RemoveEquipmentSubtree and NO individual child removals (subsumption); any remove returns false ⇒ SafeRebuild() + no further surgical calls (ratchet); flip the T2 phase-pin ("remove-only ⇒ rebuild") to the new expectation; filter FullyQualifiedName~AddressSpaceApplierTests FAIL→PASS.
  • Commit: feat(opcua): surgical pure-remove deploys — scoped teardown, no full rebuild (R2-07 T11)

T12 — Phase 2 integration + live gate

  • Classification: high-risk verification
  • Estimated implement time: 15 min (test) + 20 min (live)
  • Parallelizable with:
  • Files: SubscriptionSurvivalTests.cs
  • TDD: subscribe A+B → surgical-remove B ⇒ A's item alive; B's item observes final Bad; B unbrowsable (BadNodeIdUnknown on read). Then the live-/run remove recipe (both centrals; subscribe survivor; deploy 1 tag; log shows kind=PureRemove, rebuild=False). Record evidence in STATUS.md.
  • Commit: test(opcua): subscription-survival on pure-remove + live-verify evidence (R2-07 T12)
  • ── PHASE 2 SHIPPABLE BOUNDARY ──

── PHASE 3 — mixed batches ──

T13 — Classifier + applier AddRemoveMix routing (removes-then-adds)

  • Classification: high-risk (apply routing)
  • Estimated implement time: 5 min (tests) + 5 min (impl)
  • Parallelizable with:
  • Files: AddressSpaceApplier.cs, AddressSpaceApplierTests.cs, AddressSpaceChangeClassifierTests.cs
  • TDD: add+remove plan ⇒ removes applied surgically, NO rebuild, RebuildCalled=false (flip the T2 phase pin); remove-then-recreate same folder-scoped NodeId in one plan ⇒ remove recorded before the passes would re-ensure (sequence assertion); any remove failure ⇒ rebuild ratchet still holds; filters as before FAIL→PASS.
  • Commit: feat(opcua): surgical mixed add+remove deploys (R2-07 T13)

T14 — Phase 3 integration + live gate + docs close-out

  • Classification: high-risk verification + low-risk docs
  • Estimated implement time: 15 min (test) + 20 min (live) + 10 min (docs)
  • Parallelizable with:
  • Files: SubscriptionSurvivalTests.cs, archreview/plans/STATUS.md, CLAUDE.md/docs note if the deploy-behavior description warrants it, archreview/03-server-runtime.md P1 row (mark remediated)
  • TDD: mixed-deploy integration case (subscribe A; deploy B/+C; A alive, B gone, C browsable). Live: one deploy carrying +1/1 on docker-dev (both centrals), log kind=AddRemoveMix, rebuild=False, survivor subscription uninterrupted. Update STATUS.md + the P1 finding row.
  • Commit: test+docs: mixed-deploy survival, live evidence, STATUS/P1 close-out (R2-07 T14)
  • ── PHASE 3 / PLAN COMPLETE ──

Execution deviations (R2-07)

  • Applier phase-pin tests were flipped, not "untouched-green". The plan framed T2/T11/T13 as behavior-preserving ("every pre-existing applier test untouched-green"). In reality several pre-existing tests directly encoded the OLD full-rebuild-on-any-topology-change behavior — Added_equipment_*, Removed_equipment_*, Surgical_eligible_tag_delta_mixed_with_added_equipment_rebuilds, Folder_rename_mixed_with_added_equipment_rebuilds, Removed_equipment_tags_and_virtual_tags_only_rebuild_* — and were flipped to assert the new PureAdd/PureRemove/AddRemoveMix no-rebuild behavior (each doc-commented "Supersedes the pre-R2-07 … pin"). The genuinely-behavior-preserving tests (surgical attribute path, folder renames, changed-non-surgical ⇒ rebuild) stayed green untouched.
  • Incidental RebuildCalled assertions repointed. Tests whose subject was not the rebuild decision but which happened to assert RebuildCalled==true on a now-PureAdd plan were adjusted: AddressSpaceApplierFailureSurfaceTests (rebuild-failed surfacing) + OpcUaPublishActorApplyFailureTests (apply.failed meter) were repointed to a rebuild-forcing ChangedEquipment plan so they still exercise the SafeRebuild path; AddressSpaceApplierProvisioningTests RebuildCalled assertions became RebuildCalled==false + AddedNodes>0 (provisioning still fires on a pure-add).
  • T1 predicate references: the two eligibility predicates moved to AddressSpaceChangeClassifier as internal static; the applier references them directly (AddressSpaceChangeClassifier.TagDeltaIsSurgicalEligible) rather than keeping thin delegating wrappers.
  • Integration + live gates deferred-live (memory constraint). Per the executor's memory budget (*.IntegrationTests leak ~16 GB/run, other plans running concurrently), the over-the-wire SubscriptionSurvivalTests (add/remove/mixed) were authored + dotnet build-compile-verified but NOT run, and T6/T12/T14 docker-dev live-/run gates were marked deferred-live. These are the F10b inertness nets and MUST be run in the serial heavy pass before merge/trust.
  • Worktree entry: EnterWorktree refused to switch from the repo root; all work was done against the worktree via absolute paths + git -C, never touching the main checkout.