diff --git a/archreview/03-server-runtime.md b/archreview/03-server-runtime.md index 66b212fe..dc2e934f 100644 --- a/archreview/03-server-runtime.md +++ b/archreview/03-server-runtime.md @@ -25,7 +25,7 @@ Every finding from the 2026-07-08 review, verified against the actual code (not | S9 | No server-cert renewal/expiry monitoring | **STILL OPEN.** `OpcUaApplicationHost.cs:305-317` unchanged. | | S10 | SDK start failure swallowed; node runs with Null sinks, invisible to /health | **STILL OPEN.** `OtOpcUaServerHostedService.cs:110-129` unchanged. | | S11 | `AuditWriterActor` unbounded buffer, drops batches on outage | **STILL OPEN** (and still moot — see U3). | -| P1 | Any structural deploy change → full rebuild severs all monitored items | **STILL OPEN.** `AddressSpaceApplier.cs:134-154` / `OtOpcUaNodeManager.cs:1690-1736` unchanged. The U2 reflection guard (now merged) was the declared prerequisite gate for the surgical pure-adds work; the work itself has not started. | +| P1 | Any structural deploy change → full rebuild severs all monitored items | **REMEDIATED (offline; live gates deferred)** on `r2/07-surgical-pure-adds` (R2-07 Phases 1–3, `bb226f45`…`e2b47638`). New pure `AddressSpaceChangeClassifier` routes each deploy delta to its minimal mutation in `AddressSpaceApplier.Apply`: **PureAdd/AttributeOnly** skip `RebuildAddressSpace` (the idempotent Materialise passes add exactly the new nodes; `AnnounceAddedNodes` fires a Part 3 NodeAdded per parent), **PureRemove/AddRemoveMix** tear down only the affected subtree in place via 3 new `ISurgicalAddressSpaceSink` remove members (guard-first, negative-control observed) + `OtOpcUaNodeManager` scoped teardown with `NodeDeleted` model-change, and **Rebuild** stays the default-closed safety valve (any node-affecting change, plus every surgical false/throw ⇒ full-rebuild ratchet). So an add-one-tag / remove-one-tag / mixed deploy no longer severs unrelated subscriptions. OpcUaServer.Tests 336/336, Runtime.Tests 398/398, Commons forwarding guard green. **Deferred:** the over-the-wire `SubscriptionSurvivalTests` (add/remove/mixed) + the docker-dev live-`/run` gates (`kind=PureAdd/PureRemove/AddRemoveMix, rebuild=False`) — authored + compile-green, run in the serial heavy pass. | | P2 | Per-value global-lock write, no batching | **STILL OPEN.** `OtOpcUaNodeManager.cs:266-281` unchanged. Same guard-unblocked, not-started status as P1. | | P3 | HistoryRead-Events unpaged, `0` → `int.MaxValue` | **STILL OPEN.** `OtOpcUaNodeManager.cs:1914-1954` unchanged. | | P4 | Deploy re-runs four Materialise passes (low) | **STILL OPEN.** `OpcUaPublishActor.cs:338-354` unchanged. | diff --git a/archreview/plans/R2-07-surgical-pure-adds-plan.md b/archreview/plans/R2-07-surgical-pure-adds-plan.md index ff9b466f..090533f3 100644 --- a/archreview/plans/R2-07-surgical-pure-adds-plan.md +++ b/archreview/plans/R2-07-surgical-pure-adds-plan.md @@ -369,3 +369,32 @@ Tasks are bite-sized TDD increments. **Classification:** `high-risk` = touches t - 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. 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 dbfd5a76..2d9cd149 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 @@ -121,10 +121,11 @@ { "id": "T14", "subject": "Phase 3: mixed-deploy integration test + live-/run mixed gate + STATUS.md/P1 close-out \u2014 plan complete", - "status": "pending", + "status": "deferred-live", "blockedBy": [ "T13" - ] + ], + "note": "mixed-deploy SubscriptionSurvivalTests authored + compiles (dotnet build green); docs close-out DONE (STATUS.md + 03/P1 marked REMEDIATED + plan deviations). Live docker-dev mixed gate (one deploy +1/-1; kind=AddRemoveMix, rebuild=False; survivor uninterrupted) MUST run in the serial heavy pass \u2014 F10b inertness risk." } ] } diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md index ae2f7d6a..59c8fd7f 100644 --- a/archreview/plans/STATUS.md +++ b/archreview/plans/STATUS.md @@ -178,7 +178,7 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an | **R2-11** | 01/C-1, 01/P-1 + 05/CONV-2, UNDER-1, UNDER-6 | `r2/11-tagconfig-consolidation` (off `46fedda3`) | first `2e14fe1f` … (24 task commits) | **Code-complete, offline-verified.** **Part 1 (C-1/P-1):** single `TagConfigIntent.Parse` + `DeviceConfigIntent` in Commons replace the four byte-parity TagConfig parse copies (composer/artifact/walker/DraftValidator) — composer now parses once per tag (P-1); a permanent `Runtime.Tests/TagConfigCorpusParityTests` (30-blob corpus) guards compose≡decode; `grep "MUST parse identically" src/` = 0. **Part 2 (CONV-2):** shared strict-capable `TagConfigJson` readers in Core.Abstractions replace the six copy-pasted `ReadEnum`s; all six driver parsers gain `Inspect()` warnings + honour the `writable` key (FOCAS **forced read-only**, 05/UNDER-1); FOCAS equipment tags run the capability-matrix pre-flight (05/UNDER-6); Modbus probe parses the factory DTO shape (`timeoutMs` no longer dropped). New `EquipmentTagConfigInspector` in ControlPlane + `AdminOperationsActor` deploy-gate wiring with `Deployment:TagConfigValidationMode` (**Warn default | Error opt-in**), proven through the actor (F10b consuming test). AdminUI: `writable` checkbox in the six typed editors. **Phase-C follow-up (deferred, recorded here):** flip the runtime parsers to strict (typo'd enum ⇒ `BadNodeIdUnknown` instead of wrong-width `Good`) once fleets have run `Error` mode clean — behavior change deliberately not shipped in this plan. **Deferred (T22/T24):** docker-dev `/run` of one typed editor's Writable checkbox + whole-solution serial sweep. | -| **R2-07 (Phases 1+2)** | 03/P1 (action #7, surgical pure-adds/removes) | `r2/07-surgical-pure-adds` (off `1a698cbb`) | `bb226f45`… (T1–T12) | **Phases 1+2 code-complete, offline-verified.** New pure `AddressSpaceChangeClassifier` (Empty/AttributeOnly/PureAdd/PureRemove/AddRemoveMix/Rebuild; first-match-wins, default-closed Rebuild; the F10b tag/vtag surgical-eligibility predicates moved here as `internal static`). `AddressSpaceApplier.Apply` routes by kind. **Phase 1 (PureAdd/AttributeOnly):** skip `RebuildAddressSpace` (the idempotent Materialise passes create exactly the added nodes; existing subscriptions survive); adds announced via `ComputeAddAnnouncements`+`AnnounceAddedNodes` (dedup affected-parent → existing guard-covered `RaiseNodesAddedModelChange`), fired by `OpcUaPublishActor` after the 4 passes gated on `!outcome.RebuildCalled`; `MaterialiseAlarmCondition` made idempotent (skip-if-present-and-same-kind keeps the condition NodeState alive). **NO new sink members in Phase 1.** **Phase 2 (PureRemove):** 3 new `ISurgicalAddressSpaceSink` members (`RemoveVariableNode`/`RemoveAlarmConditionNode`/`RemoveEquipmentSubtree`) — guard-first with the **observed negative control** (`DeferredSinkForwardingReflectionTests` fails member-named until `DeferredAddressSpaceSink` forwards) + `SdkAddressSpaceSink` forwards + `OtOpcUaNodeManager` in-place teardown (scoped map cleanup, notifier demotion via `RemoveRootNotifier`, `BuildNodesRemovedModelChange` verb=NodeDeleted reported outside `Lock`). Applier `PureRemove` arm: terminal Bad / `RemovedConditionState` writes (closing the removed-alarm-tag today-gap) → subsumption partition (removed-equipment children folded into the subtree remove) → in-place removes → any false/throw ⇒ full-rebuild ratchet; non-surgical sink ⇒ rebuild. `AddRemoveMix` still full-rebuild until Phase 3. **Guard stays green (Commons.Tests 17/17), OpcUaServer.Tests 334/334, Runtime.Tests 398/398.** **Deferred-live (T5, T6, T12):** `SubscriptionSurvivalTests` add + remove cases (authored + compiles; NOT run — `*.IntegrationTests` memory constraint) and the docker-dev live-`/run` add + remove gates (`kind=PureAdd/PureRemove, rebuild=False`; survivor subscription uninterrupted; removed node → `BadNodeIdUnknown`) — MUST run in the serial heavy pass (F10b inertness risk). Phase 3 (mixed add+remove) still pending. | +| **R2-07 (Phases 1+2+3)** | 03/P1 (action #7, surgical pure-adds/removes/mixed) | `r2/07-surgical-pure-adds` (off `1a698cbb`) | `bb226f45`…`e2b47638` (T1–T14) | **Phases 1+2 code-complete, offline-verified.** New pure `AddressSpaceChangeClassifier` (Empty/AttributeOnly/PureAdd/PureRemove/AddRemoveMix/Rebuild; first-match-wins, default-closed Rebuild; the F10b tag/vtag surgical-eligibility predicates moved here as `internal static`). `AddressSpaceApplier.Apply` routes by kind. **Phase 1 (PureAdd/AttributeOnly):** skip `RebuildAddressSpace` (the idempotent Materialise passes create exactly the added nodes; existing subscriptions survive); adds announced via `ComputeAddAnnouncements`+`AnnounceAddedNodes` (dedup affected-parent → existing guard-covered `RaiseNodesAddedModelChange`), fired by `OpcUaPublishActor` after the 4 passes gated on `!outcome.RebuildCalled`; `MaterialiseAlarmCondition` made idempotent (skip-if-present-and-same-kind keeps the condition NodeState alive). **NO new sink members in Phase 1.** **Phase 2 (PureRemove):** 3 new `ISurgicalAddressSpaceSink` members (`RemoveVariableNode`/`RemoveAlarmConditionNode`/`RemoveEquipmentSubtree`) — guard-first with the **observed negative control** (`DeferredSinkForwardingReflectionTests` fails member-named until `DeferredAddressSpaceSink` forwards) + `SdkAddressSpaceSink` forwards + `OtOpcUaNodeManager` in-place teardown (scoped map cleanup, notifier demotion via `RemoveRootNotifier`, `BuildNodesRemovedModelChange` verb=NodeDeleted reported outside `Lock`). Applier `PureRemove` arm: terminal Bad / `RemovedConditionState` writes (closing the removed-alarm-tag today-gap) → subsumption partition (removed-equipment children folded into the subtree remove) → in-place removes → any false/throw ⇒ full-rebuild ratchet; non-surgical sink ⇒ rebuild. **Phase 3 (AddRemoveMix):** removes-then-adds within one apply — the remove pass runs in `Apply` (no rebuild), then the publish actor's Materialise passes create the adds + `AnnounceAddedNodes` announces them; an id reused across the remove+add sets is remove-then-recreated as a fresh NodeState (correct — a different tag). **Guard stays green (Commons.Tests 17/17), OpcUaServer.Tests 336/336, Runtime.Tests 398/398.** **03/P1 marked REMEDIATED in `archreview/03-server-runtime.md`.** **Deferred-live (T5, T6, T12, T14):** `SubscriptionSurvivalTests` add + remove + mixed cases (authored + compile-green; NOT run — `*.IntegrationTests` memory constraint) and the docker-dev live-`/run` add/remove/mixed gates (`kind=PureAdd/PureRemove/AddRemoveMix, rebuild=False`; survivor subscription uninterrupted; removed node → `BadNodeIdUnknown`) — MUST run in the serial heavy pass (F10b inertness risk). | **Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1) `01-core-composition.md` U-6 anchor `DriverResilienceOptions.cs:23-25` → the two bulkhead properties sat at diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs index 36dccc30..432c77cb 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SubscriptionSurvivalTests.cs @@ -225,6 +225,94 @@ public sealed class SubscriptionSurvivalTests } } + /// R2-07 Phase 3 — a subscription on tag A survives a MIXED deploy (−B, +C in one apply): A + /// stays alive, B is gone (BadNodeIdUnknown), and C is browsable + readable. + [Fact] + public async Task Subscription_survives_mixed_add_remove_deploy() + { + var pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-subsurvive-mix-{Guid.NewGuid():N}"); + var port = AllocateFreePort(); + var ct = TestContext.Current.CancellationToken; + + try + { + var options = new OpcUaApplicationHostOptions + { + ApplicationName = ServerUri, + ApplicationUri = ServerUri + ".Mixed", + OpcUaPort = port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = pkiRoot, + EnabledSecurityProfiles = new List { OpcUaSecurityProfile.None }, + AutoAcceptUntrustedClientCertificates = true, + }; + var server = new OtOpcUaSdkServer(); + await using var host = new OpcUaApplicationHost(options, NullLogger.Instance); + await host.StartAsync(server, ct); + + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + + var withAB = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-b", "eq-1", "drv", FolderPath: "", Name: "B", DataType: "Int32", FullName: "2", Writable: false, Alarm: null)); + applier.MaterialiseHierarchy(withAB); + applier.MaterialiseEquipmentTags(withAB); + var nodeIdAString = Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "A"); + sink.WriteValue(nodeIdAString, 10, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + + using var session = await OpenSessionAsync($"opc.tcp://127.0.0.1:{port}/OtOpcUa", ct); + var ns = (ushort)session.NamespaceUris.GetIndex(OtOpcUaNodeManager.DefaultNamespaceUri); + var nodeIdA = new NodeId(nodeIdAString, ns); + + var receivedA = new List(); + var gate = new object(); + var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 }; + session.AddSubscription(subscription); + await subscription.CreateAsync(ct); + var miA = new MonitoredItem(subscription.DefaultItem) { StartNodeId = nodeIdA, AttributeId = Attributes.Value, SamplingInterval = 50, QueueSize = 10 }; + miA.Notification += (item, _) => { foreach (var v in item.DequeueValues()) lock (gate) receivedA.Add(v); }; + subscription.AddItem(miA); + await subscription.ApplyChangesAsync(ct); + await WaitUntilAsync(() => { lock (gate) return receivedA.Count >= 1; }, TimeSpan.FromSeconds(5)); + + // MIXED apply: remove B, add C (previous={A,B}, next={A,C}). + var withAC = CompositionWith( + new EquipmentTagPlan("tag-a", "eq-1", "drv", FolderPath: "", Name: "A", DataType: "Int32", FullName: "1", Writable: false, Alarm: null), + new EquipmentTagPlan("tag-c", "eq-1", "drv", FolderPath: "", Name: "C", DataType: "Int32", FullName: "3", Writable: false, Alarm: null)); + var plan = AddressSpacePlanner.Compute(withAB, withAC); + plan.RemovedEquipmentTags.Count.ShouldBe(1); + plan.AddedEquipmentTags.Count.ShouldBe(1); + + lock (gate) receivedA.Clear(); + var outcome = applier.Apply(plan); + outcome.RebuildCalled.ShouldBeFalse(); // AddRemoveMix — removes in place, no rebuild + // Publish-actor sequence: removes ran inside Apply; now materialise the adds + announce. + applier.MaterialiseEquipmentTags(withAC); + applier.AnnounceAddedNodes(plan); + + // A stays alive. + sink.WriteValue(nodeIdAString, 11, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + await WaitUntilAsync(() => { lock (gate) return receivedA.Any(v => Equals(v.Value, 11)); }, TimeSpan.FromSeconds(5)); + + // B is gone. + var nodeIdB = new NodeId(Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "B"), ns); + (await session.ReadValueAsync(nodeIdB, ct)).StatusCode.ShouldBe((StatusCode)StatusCodes.BadNodeIdUnknown); + + // C is browsable + readable. + var nodeIdC = new NodeId(Commons.OpcUa.EquipmentNodeIds.Variable("eq-1", "", "C"), ns); + sink.WriteValue(nodeIdC.Identifier.ToString()!, 9, Commons.OpcUa.OpcUaQuality.Good, DateTime.UtcNow); + var cRead = await session.ReadValueAsync(nodeIdC, ct); + cRead.StatusCode.ShouldNotBe((StatusCode)StatusCodes.BadNodeIdUnknown); + cRead.Value.ShouldBe(9); + } + finally + { + if (Directory.Exists(pkiRoot)) Directory.Delete(pkiRoot, recursive: true); + } + } + private static AddressSpaceComposition CompositionWith(params EquipmentTagPlan[] tags) => new( UnsAreas: Array.Empty(),