From 5fff597324ab132da368443cf99719ac58031d41 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 28 Jul 2026 13:59:06 -0400 Subject: [PATCH 1/3] revert(drivers): remove the periodic 5-minute device re-browse (#523) Removes the tag-set drift detector shipped in e77c8a35. The design was sound -- the tautology drivers were correctly gated out, and the structurally undetectable "vanished" direction was declared rather than faked -- but none of that is the objection. The feature browsed real plant equipment on a recurring timer, at a frequency no operator could configure (the interval was a constructor parameter with no appsettings key), and that outbound traffic was never once observed against a real PLC, CNC or MTConnect Agent. Removed: TagSetDrift/TagSetDriftDetector; DriverInstanceActor's drift timer, tick message, Props/ctor parameter, gate, handler and leaf-collector; ITagDiscovery.AuthoredDiscoveryRefs + .DiscoveryStreamIncludesAuthoredTags and their four implementations; 14 tests. Kept: ITagDiscovery.SupportsOnlineDiscovery (it predates this and drives the universal browse picker), and the whole IRediscoverable consumption from 09a401b8 -- the driver tells us, we do not poll. CONSEQUENCE, deliberately accepted: AbCip and FOCAS browse a live backend but implement no IRediscoverable, so they now have NO tag-change signal at all -- a PLC re-download is invisible until someone re-browses by hand. Recorded in CLAUDE.md so the next reader does not have to rediscover it, along with the shape to reach for if that coverage is wanted back (event-driven, or triggered by a reconnect/deploy, not a wall-clock timer). Runtime.Tests 476 pass / 13 skipped (skip set unchanged); FOCAS 275, TwinCAT 192, MTConnect 491, AbCip 342, Core.Abstractions 266 all green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo --- CLAUDE.md | 46 +-- deferment.md | 49 +++- .../ITagDiscovery.cs | 28 -- .../AbCipDriver.cs | 11 - .../FocasDriver.cs | 11 - .../MTConnectDriver.cs | 13 - .../TwinCATDriver.cs | 11 - .../Drivers/DriverInstanceActor.cs | 160 +---------- .../Drivers/TagSetDrift.cs | 92 ------ .../DriverInstanceActorDriftCheckTests.cs | 268 ------------------ .../Drivers/TagSetDriftDetectorTests.cs | 113 -------- 11 files changed, 59 insertions(+), 743 deletions(-) delete mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index ee6d2f81..e72a1e37 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -188,38 +188,22 @@ line and injected nothing. Gone with it: `HandleDiscoveredNodes`, `PartitionDisc browse picker drives it through `Commons/Browsing/DiscoveryDriverBrowser`, which never went through the actor. -⛔ **Tag-set drift detection is slated for REMOVAL (Gitea #523).** It is in `master` today and the -description below is accurate, but the periodic device re-browse is being taken back out: it is -recurring, unconfigurable (interval is a constructor parameter, no appsettings key) and never -live-gated traffic to real PLCs/CNCs/Agents. After removal, **AbCip and FOCAS have no tag-change signal -at all** (they implement no `IRediscoverable`); TwinCAT and MTConnect keep their native ones. The -`IRediscoverable` consumption above is unaffected. +**`IRediscoverable` is the ONLY tag-change signal, and four drivers do not raise it.** A periodic +tag-set drift check briefly existed (shipped `e77c8a35`, **removed** by Gitea **#523**): every +`SupportsOnlineDiscovery` driver was re-browsed on a 5-minute timer and the result diffed against its +authored tags. It was taken back out because it generated recurring, **unconfigurable** (interval was a +constructor parameter with no appsettings key), **never-live-gated** outbound traffic to real +PLCs/CNCs/Agents — how often to browse someone's plant equipment is an operational decision, and the +design treated it as an implementation detail. -**Tag-set drift detection (2026-07-28) — the second signal, and it is GATED.** `DriverInstanceActor` -re-browses on a slow timer (`DefaultDriftCheckInterval`, 5 min) and compares what the remote offers -against what an operator authored, raising the same re-browse prompt. This matters most for **AbCip and -FOCAS**, which browse a live backend but implement no `IRediscoverable`, so a periodic compare is their -only way to notice a PLC re-download. - -⚠️ **It runs ONLY for a driver declaring `ITagDiscovery.SupportsOnlineDiscovery` AND reporting -`AuthoredDiscoveryRefs` (nullable, default null = opt out).** Modbus, S7, MQTT, AbLegacy and Sql *echo -authored config* from `DiscoverAsync` rather than browsing the device, so the diff for them is a -tautology that would report "no drift" forever and read as coverage. A check that cannot fail is worse -than no check. Note `AuthoredDiscoveryRefs` is **nullable, not empty-by-default** — an empty collection -legitimately means "nothing authored, so everything discovered is new". - -⚠️ **One direction is structurally undetectable for most drivers.** FOCAS, TwinCAT and AbCip re-emit -their authored tags into the same `DiscoverAsync` stream as device-derived ones (so the browse picker -shows existing tags), which means discovered ⊇ authored **always** and a *vanished* tag can never appear -missing. Those drivers declare `DiscoveryStreamIncludesAuthoredTags => true`, and the detector then -reports only what it can see and **says so** in the operator message rather than implying a clean bill of -health. **MTConnect is currently the only driver where both directions are detectable** — its stream is -built purely from the Agent's probe model. - -Other properties worth knowing: a **failed browse is not drift** (an unreachable device would otherwise -report every authored tag as vanished); a **truncated** capture is skipped for the same reason; an -**unchanged** drift is reported once rather than re-stamping its timestamp every 5 min; and drift that -resolves **clears** the prompt. The timer starts on Connected entry and is cancelled on every exit. +⚠️ **Consequence to know before you rely on rediscovery:** **AbCip and FOCAS browse a live backend but +implement no `IRediscoverable`, so they have no tag-change signal at all** — a PLC program re-download +or a CNC option install is invisible until someone re-browses the device by hand. Galaxy, TwinCAT, +MTConnect and MQTT raise natively and are unaffected. If that coverage matters later, reach for an +event-driven signal, or one that runs when something *else* already indicates change (a reconnect, a +deploy) — not a wall-clock timer. Removed with the feature: `TagSetDrift`/`TagSetDriftDetector`, +`DriverInstanceActor`'s drift timer, and `ITagDiscovery.AuthoredDiscoveryRefs` / +`DiscoveryStreamIncludesAuthoredTags`. ⚠️ **`IHostConnectivityProbe` is still unconsumed.** `GetHostStatuses()` has no production call site and nothing ever writes a `DriverHostStatus` row (the table was re-created in the v3 initial migration, so diff --git a/deferment.md b/deferment.md index dfe60f16..843640ee 100644 --- a/deferment.md +++ b/deferment.md @@ -46,11 +46,11 @@ themes into tracked decisions. What remains: | **#520** node ACLs authored + deployed but **never enforced** | Decision: wire it or retire the surface. Non-enforcement is now stated in docs and warned in the UI, so nobody can author a deny rule believing it works — but reads are still ungated. | §3.1 | | **#521** `IHostConnectivityProbe` / `DriverHostStatus` dead surface | Decision: build the publisher or delete it. The table was deliberately re-created in the v3 initial migration, so deleting on inference would be wrong. | §3.2 | | **#522** driver stability tiers inert | Decision: pass real tiers or delete the machinery. Docs now say every driver runs Tier A. | §3.3 | -| **#523** remove the periodic 5-min device re-browse | **Reversal of work shipped in this remediation.** After removal, AbCip and FOCAS have no tag-change signal at all. | §9 (2026-07-28 entry) | +| ~~**#523** remove the periodic 5-min device re-browse~~ | ✅ **Done** — removed. **AbCip and FOCAS now have no tag-change signal at all**; that is the accepted consequence, not an oversight. | §9 (2026-07-28, #523) | | **G-7** `EquipmentTagConfigInspector` covers 6 of 12 driver types | Pre-existing gap, not a regression; the only §1.2 gap left. | §1.2 | | **13 `EquipmentTagsDarkBatch4` skipped tests** | Still stale — their unblock condition already happened and the path was retired instead. | §4.4 | | 17 other tracker issues | Driver features + deferred work, unchanged by this remediation. | §2 | -| Open live gates | #491 continuous historization, #468 browser tree, archreview R2-03 / R2-10, and the **un-gated drift detector** (moot if #523 lands). | §5 | +| Open live gates | #491 continuous historization, #468 browser tree, archreview R2-03 / R2-10. *(The un-gated drift detector's gate closed by deletion — see #523.)* | §5 | **Three issues are resolved but still open on the tracker** — **#516** (fixed for all 12 drivers), **#518** (the `IRediscoverable` half fixed; the probe half split to #521) and **#507** (injection deleted, @@ -300,7 +300,7 @@ mere env-var presence, so it cannot pass vacuously. | Gate | Status | Evidence | |---|---|---| -| **Tag-set drift detection** (shipped `e77c8a35`) | **OPEN — and likely moot.** Never live-gated: it needs a device whose tag set actually changes. Gitea **#523** removes the feature, which would close this gate by deletion. | +| ~~**Tag-set drift detection** (shipped `e77c8a35`)~~ | ✅ **CLOSED BY DELETION** — removed under Gitea **#523** rather than gated. The gate it needed (a device whose tag set actually changes) required exactly the traffic the removal objected to. | §9, 2026-07-28 (#523) | | **Continuous-historization value capture** (#491) | **OPEN** — code complete, unproven in production | `CLAUDE.md:710`; no gate-passed commit exists | | **Wave-0 universal discovery browser** full tree render (#468) | **OPEN** — fixture-blocked | tracking doc §Wave 0; named as blocking BACnet browse | | **archreview R2-03** — VT Good→Bad on a data-fed rig | **OPEN** — explicitly "live-blocked on this data-less rig" | `archreview/plans/STATUS.md` | @@ -470,7 +470,7 @@ the tree. | 5. Bookkeeping sweep | ✅ **Done** — 13 plan files closed, 11 archreview gates written back | `88ce8df0` | | 6. Tier truth | ✅ **Done** — docs reconciled; keep-or-delete is Gitea **#522** | `88ce8df0` | | follow-up A | ✅ **Done** — Sql/FOCAS in-place re-parse made **atomic** | `5184a2e1` | -| follow-up B | ⛔ **Shipped, then reverted by decision** — tag-set drift detection; removal tracked as Gitea **#523** | `e77c8a35` | +| follow-up B | ⛔ **Shipped, then removed by decision** — tag-set drift detection; **removal landed**, Gitea **#523** | `e77c8a35`, removed below | ### 2026-07-27 — §8.1 ACL non-enforcement made explicit @@ -624,10 +624,9 @@ guarantee. Sql.Tests 226 / FOCAS.Tests 275 pass. ### 2026-07-28 — the two things §8.3 deliberately did NOT build (2 of 2) -> ⛔ **SUPERSEDED SAME DAY — the periodic re-browse is to be REMOVED (owner decision, Gitea #523).** -> It ships in `master` (`e77c8a35`, merged `914d70bf`) and this entry stays as the record of what was -> built and why, but it is **not the intended end state**. The objection is the one this entry does not -> weigh: the check is recurring, **unconfigurable** (the interval is a constructor parameter with no +> ⛔ **REMOVED — the periodic re-browse is gone from the tree (owner decision, Gitea #523; removal +> recorded in the next entry).** It shipped in `master` (`e77c8a35`, merged `914d70bf`) and this entry +> stays as the record of what was built and why. The objection is the one this entry does not weigh: the check is recurring, **unconfigurable** (the interval is a constructor parameter with no > appsettings key) and **never live-gated** outbound traffic to real PLCs, CNCs and MTConnect Agents. > Frequency of an unsolicited device browse is an operational decision, and the design treated it as an > implementation detail. @@ -679,6 +678,40 @@ The comparison is a pure, separately-tested type (`TagSetDriftDetector`) rather tautology-driver test goes red, and it asserts discovery was **never invoked** rather than merely "no prompt raised", which would have passed for the wrong reason if the timer simply never fired. +### 2026-07-28 — #523: the periodic re-browse removed + +The feature described in the entry above is **gone from the tree**. Removed: + +| Removed | Where | +|---|---| +| `TagSetDrift` + `TagSetDriftDetector` | `Runtime/Drivers/TagSetDrift.cs` (file deleted) | +| The drift timer, `DriftCheckTick`, `DefaultDriftCheckInterval`, the `driftCheckInterval` Props/ctor parameter, `QualifiesForDriftCheck`, `Start`/`StopDriftCheck`, `HandleDriftCheckAsync`, `CollectLeafRefs`, `_lastDriftSignature` | `DriverInstanceActor` | +| `ITagDiscovery.AuthoredDiscoveryRefs` + `.DiscoveryStreamIncludesAuthoredTags`, and their four implementations | `Core.Abstractions`; FOCAS, TwinCAT, MTConnect, AbCip | +| `DriverInstanceActorDriftCheckTests` (6), `TagSetDriftDetectorTests` (8) | `Runtime.Tests` | + +**What survives, deliberately.** `ITagDiscovery.SupportsOnlineDiscovery` stays — it predates drift +detection and is what the universal browse picker dispatches on. The `IRediscoverable` consumption +(`09a401b8`) is untouched: the driver tells us, we do not poll, and it costs nothing when nothing +changes. The `/hosts` re-browse chip stays and is now fed by that one source. + +**The consequence, stated plainly rather than buried.** **AbCip and FOCAS now have no tag-change signal +at all.** Both browse a live backend, neither implements `IRediscoverable`, and the periodic compare was +the only thing that would have noticed a PLC re-download or a CNC option install. That is the accepted +trade, not an oversight — it is recorded in `CLAUDE.md` §Change Detection so the next reader does not +have to rediscover it, together with the shape to reach for if the coverage is wanted back +(event-driven, or triggered by a reconnect/deploy, not a wall-clock timer). + +**Why the earlier entry's reasoning did not save it.** That entry defends the *design* — the gate is +real, the tautology drivers are correctly excluded, the undetectable direction is declared rather than +faked. All of that is true and none of it is the objection. The objection is to the feature existing at +all in that shape: it browsed real plant equipment on a recurring timer, at a frequency nobody could +configure, and that behaviour was never once observed against real hardware. A well-built mechanism for +something that should not run is still something that should not run. + +Build clean; `Runtime.Tests` 476 passed / 13 skipped (unchanged skip set — the stale +`EquipmentTagsDarkBatch4` group), FOCAS 275, TwinCAT 192, MTConnect 491, AbCip 342, +`Core.Abstractions` 266, all green. + ### 2026-07-27 — §8.4 dispatch-map parity (G-1 … G-6) **The maps had to become data before they could be guarded.** A Razor `@switch` compiles into diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ITagDiscovery.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ITagDiscovery.cs index 76d2009d..cc713a1b 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ITagDiscovery.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ITagDiscovery.cs @@ -38,32 +38,4 @@ public interface ITagDiscovery /// See docs/plans/2026-07-15-universal-discovery-browser-design.md §5. /// bool SupportsOnlineDiscovery => false; - - /// - /// The device-native references this driver's authored tags bind to — the same vocabulary - /// streams as . Comparing the - /// two sets detects that the remote's tag set has drifted from what an operator authored. - /// Null means "I do not report this", and drift detection is skipped. That is the - /// default on purpose: an empty collection legitimately means "no tags authored, so everything - /// discovered is new", and the two must not be confused. A driver opting in is asserting that its - /// authored refs are directly comparable to what it discovers. - /// Only consulted when is true. For a driver that - /// replays authored tags from config rather than browsing the backend, the comparison is a - /// tautology — it would report "no drift" forever and read as reassurance. - /// - IReadOnlyCollection? AuthoredDiscoveryRefs => null; - - /// - /// True when re-emits this driver's authored tags into the same - /// stream as the device-derived ones — which FOCAS, TwinCAT and AbCip all do, so the browse picker - /// shows an operator their existing tags alongside what the device offers. - /// This makes one half of drift detection structurally undetectable. If authored tags - /// are always re-emitted then the discovered set always contains the authored set, so an authored tag - /// that has VANISHED from the device can never appear missing. Declaring it here means the detector - /// reports only what it can actually see, instead of reporting "nothing vanished" forever and reading - /// as reassurance. - /// False (the default) means the stream is purely device-derived — as MTConnect's is, built - /// from the Agent's probe model — so both directions are detectable. - /// - bool DiscoveryStreamIncludesAuthoredTags => false; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 60aff54f..05ad2904 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -1036,17 +1036,6 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// EnableControllerBrowse, which the universal browser's PatchForBrowse guarantees at open time. public bool SupportsOnlineDiscovery => true; - /// - /// Pre-declared tags are emitted under their Name, which is the driver FullName the - /// controller-browse leaves also use. - public IReadOnlyCollection? AuthoredDiscoveryRefs => - _declaredTags.Select(t => t.Name).ToArray(); - - /// - /// True: DiscoverAsync emits browsed controller symbols AND re-emits every - /// pre-declared tag. - public bool DiscoveryStreamIncludesAuthoredTags => true; - /// public async Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 72b28f91..b01ef4a4 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -517,17 +517,6 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// until the node set is non-empty and stable. public bool SupportsOnlineDiscovery => true; - /// - /// The authored tag's Name IS its driver-side FullName — DiscoverAsync emits - /// FullName: tag.Name for every authored tag — so the two sets are directly comparable. - public IReadOnlyCollection? AuthoredDiscoveryRefs => - _tagsByRawPath.Values.Select(t => t.Name).ToArray(); - - /// - /// True: DiscoverAsync emits the device-derived FixedTree AND re-emits every authored - /// tag, so an authored tag that vanished from the CNC cannot appear missing. - public bool DiscoveryStreamIncludesAuthoredTags => true; - /// public Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 384d4f5b..9c60197c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -907,19 +907,6 @@ public sealed class MTConnectDriver : IDriver, IReadable, ISubscribable, ITagDis /// public bool SupportsOnlineDiscovery => true; - /// - /// The DataItem ids the authored raw tags bind. DiscoverAsync emits - /// FullName: dataItem.Id from the Agent's probe model, so the two sets are the same - /// vocabulary. - public IReadOnlyCollection? AuthoredDiscoveryRefs => - Volatile.Read(ref _binding).RawPathsByDataItemId.Keys; - - /// - /// False — DiscoverAsync is built purely from the Agent's probe model and never - /// re-emits authored tags, so an authored DataItem that the Agent stopped publishing IS visible as - /// missing. MTConnect is currently the only driver where both drift directions are detectable. - public bool DiscoveryStreamIncludesAuthoredTags => false; - /// /// /// Once, not the default UntilStable: streams the diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index 6e960bdd..4acdbdf2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -404,17 +404,6 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery /// EnableControllerBrowse, which the universal browser's PatchForBrowse guarantees at open time. public bool SupportsOnlineDiscovery => true; - /// - /// The authored tag's Name is its RawPath and its driver FullName - /// (DiscoverAsync emits FullName: tag.Name), directly comparable to the browsed - /// symbols' InstancePath. - public IReadOnlyCollection? AuthoredDiscoveryRefs => - _tagsByRawPath.Values.Select(t => t.Name).ToArray(); - - /// - /// True: DiscoverAsync emits browsed ADS symbols AND re-emits every authored tag. - public bool DiscoveryStreamIncludesAuthoredTags => true; - /// public async Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index 19e2331e..5fc2139e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -1,6 +1,5 @@ using Akka.Actor; using Akka.Event; -using ZB.MOM.WW.OtOpcUa.Commons.Browsing; using ZB.MOM.WW.OtOpcUa.Commons.Observability; using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; using ZB.MOM.WW.OtOpcUa.Commons.Types; @@ -33,11 +32,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers { public static readonly TimeSpan DefaultReconnectInterval = TimeSpan.FromSeconds(10); - /// Default period of the tag-set drift check. Deliberately slow: it browses a real device, and - /// a tag set changing is an engineering event (a PLC re-download, a CNC option install), not a runtime - /// one. Five minutes bounds the wire cost while still surfacing drift the same shift it happens. - public static readonly TimeSpan DefaultDriftCheckInterval = TimeSpan.FromMinutes(5); - public sealed record InitializeRequested(string DriverConfigJson); public sealed record InitializeSucceeded(int Generation); public sealed record InitializeFailed(string Reason, int Generation); @@ -127,15 +121,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// between connects), and dropping it in one state would lose the signal silently. private sealed record RediscoveryRaised(RediscoveryEventArgs Args); - /// Self-sent on a slow timer to re-browse a genuinely-browsable driver and compare what the - /// remote offers against what an operator authored. Only ever scheduled for a driver that opts in — see - /// . - private sealed record DriftCheckTick - { - public static readonly DriftCheckTick Instance = new(); - private DriftCheckTick() { } - } - public sealed class RetryConnect { public static readonly RetryConnect Instance = new(); @@ -196,14 +181,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers private DateTime? _rediscoveryNeededUtc; private string? _rediscoveryReason; - /// Period of the tag-set drift check; tests inject a tiny value. - private readonly TimeSpan _driftCheckInterval; - - /// Signature of the last drift REPORTED, so an unchanged drift is not re-announced on every - /// check. Without this the operator's prompt would re-stamp its timestamp every interval forever, - /// which reads as "it just happened again" rather than "it is still true". - private string? _lastDriftSignature; - /// The references the host wants kept subscribed (set by ). /// Re-applied on every entry into Connected so values resume after a reconnect or redeploy. private IReadOnlyList _desiredRefs = Array.Empty(); @@ -237,8 +214,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// defaults to an empty string when not provided (e.g. in unit tests). /// Optional Phase 6.1 resilience invoker wrapping this driver's capability /// calls; defaults to (pass-through) when not supplied. - /// Optional period of the tag-set drift check; defaults to - /// . Tests inject a tiny value so the check runs without a wait. /// Optional period of the health-poll heartbeat, which also drives the /// subscription reconcile (); defaults to /// . Exists so a test can drive the reconcile without waiting 30 s. @@ -250,8 +225,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers IDriverHealthPublisher? healthPublisher = null, string? clusterId = null, IDriverCapabilityInvoker? invoker = null, - TimeSpan? healthPollInterval = null, - TimeSpan? driftCheckInterval = null) => + TimeSpan? healthPollInterval = null) => Akka.Actor.Props.Create(() => new DriverInstanceActor( driver, reconnectInterval ?? DefaultReconnectInterval, @@ -259,8 +233,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers healthPublisher ?? NullDriverHealthPublisher.Instance, clusterId ?? string.Empty, invoker, - healthPollInterval, - driftCheckInterval)); + healthPollInterval)); /// /// Returns true when the driver should boot in DEV-STUB mode based on host platform and @@ -291,8 +264,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// Cluster identifier forwarded in health snapshots. /// Phase 6.1 resilience invoker wrapping this driver's capability calls; /// defaults to (pass-through) when null. - /// Period of the tag-set drift check; defaults to - /// . /// Period of the health-poll heartbeat, which also drives the /// subscription reconcile; defaults to when null. public DriverInstanceActor( @@ -302,15 +273,13 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers IDriverHealthPublisher? healthPublisher = null, string? clusterId = null, IDriverCapabilityInvoker? invoker = null, - TimeSpan? healthPollInterval = null, - TimeSpan? driftCheckInterval = null) + TimeSpan? healthPollInterval = null) { _driver = driver; _invoker = invoker ?? NullDriverCapabilityInvoker.Instance; _driverInstanceId = driver.DriverInstanceId; _clusterId = clusterId ?? string.Empty; _healthPublisher = healthPublisher ?? NullDriverHealthPublisher.Instance; - _driftCheckInterval = driftCheckInterval ?? DefaultDriftCheckInterval; _reconnectInterval = reconnectInterval; _healthPollInterval = healthPollInterval ?? HealthPollInterval; OtOpcUaTelemetry.DriverInstanceLifecycle.Add(1, @@ -381,7 +350,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers ResubscribeDesired(); AttachAlarmSource(); SubscribeDesiredAlarms(); - StartDriftCheck(); }); Receive(msg => { @@ -427,7 +395,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers _driverInstanceId, msg.Reason); DetachSubscription(); RecordFault(); - StopDriftCheck(); Become(Reconnecting); PublishHealthSnapshot(); }); @@ -435,7 +402,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers { _log.Info("DriverInstance {Id}: ForceReconnect requested by admin; re-entering Reconnecting", _driverInstanceId); DetachSubscription(); - StopDriftCheck(); Become(Reconnecting); PublishHealthSnapshot(); }); @@ -458,7 +424,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers SubscribeDesiredAlarms(); }); ReceiveAsync(HandleSubscribeAlarmsAsync); - ReceiveAsync(HandleDriftCheckAsync); Receive(OnDataChangeForward); // Native alarm transition marshaled onto the actor thread from the driver's OnAlarmEvent; // project it to the parent the same way DataChangeForward projects AttributeValuePublished. @@ -553,7 +518,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers ResubscribeDesired(); AttachAlarmSource(); SubscribeDesiredAlarms(); - StartDriftCheck(); }); // A failure here is a no-op regardless of generation — the retry timer keeps trying the // current config; only a (generation-matched) InitializeSucceeded transitions state. @@ -798,10 +762,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers } } - /// Stops the drift check. Called on every Connected exit — browsing a disconnected driver would - /// fail every pass, and a failed browse is deliberately NOT treated as drift. - private void StopDriftCheck() => Timers.Cancel("drift-check"); - /// Tear down the data-change + native-alarm event handlers + null the handle. Called from the /// Unsubscribe path, on PostStop, and on Connected → Reconnecting transitions so a stale handler doesn't /// push data-change / alarm events to an actor that has lost its driver connection. @@ -849,119 +809,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers _rediscoveryHandler = null; } - /// - /// True when this driver is worth drift-checking: it must genuinely BROWSE its remote - /// () and must report the refs its authored tags - /// bind (). - /// The first condition is what makes the check meaningful rather than reassuring. Most drivers - /// — Modbus, S7, AbLegacy, Sql, Mqtt — stream their AUTHORED tags back out of DiscoverAsync - /// rather than enumerating the device, so diffing the two sets for them is a tautology that would - /// report "no drift" forever. A check that cannot fail is worse than no check: it looks like - /// coverage. - /// - private bool QualifiesForDriftCheck() => - _driver is ITagDiscovery { SupportsOnlineDiscovery: true, AuthoredDiscoveryRefs: not null }; - - /// Starts the slow drift check on a Connected entry, for qualifying drivers only. - private void StartDriftCheck() - { - if (!QualifiesForDriftCheck()) return; - // Periodic, not fire-on-connect: a driver whose discovered shape fills in asynchronously after - // connect (the FOCAS FixedTree) would otherwise be compared against a half-populated browse and - // report phantom drift on every reconnect. - Timers.StartPeriodicTimer("drift-check", DriftCheckTick.Instance, _driftCheckInterval); - } - - /// - /// Re-browses the remote and compares it against the driver's authored refs. A CHANGED drift raises - /// the same operator prompt an raise does — this is the signal for the - /// browsable drivers that have no native change notification (AbCip, FOCAS), where a periodic - /// compare is the only way to notice a PLC program re-download. - /// Advisory, like every rediscovery signal: the served address space is not touched. Raw tags - /// are authored through the /raw browse-commit flow, so an operator re-browses and commits. - /// - private async Task HandleDriftCheckAsync(DriftCheckTick _) - { - if (_driver is not ITagDiscovery discovery) return; - var authored = discovery.AuthoredDiscoveryRefs; - if (authored is null) return; - - CapturedTree tree; - try - { - var builder = new CapturingAddressSpaceBuilder(); - // Bounded: ReceiveAsync suspends the mailbox for the whole handler, so an unbounded browse would - // block writes, reconnects and health polls behind it. - using var cts = new CancellationTokenSource(DriftBrowseTimeout); - await _invoker.ExecuteAsync( - DriverCapability.Discover, - _driverInstanceId, - async ct => await discovery.DiscoverAsync(builder, ct), - cts.Token); - tree = builder.Build(); - } - catch (Exception ex) - { - // A failed browse is not drift — the device may simply be unreachable, which the health surface - // already reports. Reporting drift here would turn every comms blip into "all your tags vanished". - _log.Debug(ex, "DriverInstance {Id}: drift check browse failed; skipping this pass", _driverInstanceId); - return; - } - - if (tree.Truncated) - { - // A truncated capture is a PARTIAL view, so every un-captured authored ref would look vanished. - _log.Warning( - "DriverInstance {Id}: drift check skipped — the browse hit the capture cap and is partial", - _driverInstanceId); - return; - } - - var discovered = new List(); - CollectLeafRefs(tree.Root, discovered); - var drift = TagSetDriftDetector.Compare( - discovered, authored, detectVanished: !discovery.DiscoveryStreamIncludesAuthoredTags); - - if (!drift.HasDrift) - { - // Recovered: the device matches the authored set again, so drop the prompt and let the next - // genuine drift re-raise with a fresh timestamp. - if (_lastDriftSignature is not null) - { - _log.Info("DriverInstance {Id}: tag-set drift cleared — device matches the authored set", - _driverInstanceId); - _lastDriftSignature = null; - _rediscoveryNeededUtc = null; - _rediscoveryReason = null; - PublishHealthSnapshot(); - } - - return; - } - - // Report a CHANGED drift once. An unchanged one re-stamping its timestamp every interval would read - // as "it just happened again" rather than "it is still true". - if (string.Equals(_lastDriftSignature, drift.Signature, StringComparison.Ordinal)) return; - _lastDriftSignature = drift.Signature; - _rediscoveryNeededUtc = DateTime.UtcNow; - _rediscoveryReason = "device tag set differs from authored: " + drift.Describe(); - _log.Info( - "DriverInstance {Id}: tag-set drift detected ({Drift}) — surfaced for operator re-browse; the served address space is unchanged", - _driverInstanceId, drift.Describe()); - PublishHealthSnapshot(); - } - - /// Per-pass timeout for the drift browse. Bounds the mailbox suspension. - private static readonly TimeSpan DriftBrowseTimeout = TimeSpan.FromSeconds(30); - - /// Flattens a captured tree to its leaf ids — the driver-side FullNames, which is the - /// vocabulary AuthoredDiscoveryRefs is defined in. - private static void CollectLeafRefs(CapturedNode node, List into) - { - if (node.IsLeaf) into.Add(node.Id); - foreach (var child in node.Children) CollectLeafRefs(child, into); - } - /// Records the driver's rediscovery raise and re-publishes health so the signal reaches the /// AdminUI promptly rather than waiting for the next 30 s heartbeat. /// Advisory only. The served address space is deliberately NOT rebuilt: v3 authors raw tags @@ -1137,7 +984,6 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// protected override void PostStop() { - StopDriftCheck(); DetachSubscription(); // MUST happen: the IDriver instance can outlive this actor (the host respawns a child around the // same driver object), so a missing unsubscribe accumulates a handler per respawn holding a dead Self. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs deleted file mode 100644 index 7a2e0015..00000000 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs +++ /dev/null @@ -1,92 +0,0 @@ -namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; - -/// -/// The difference between what a driver discovers on its remote and what an operator -/// authored — i.e. whether the device's tag set has moved out from under the deployed config. -/// -/// Refs the remote now offers that no authored tag binds. -/// -/// Refs authored tags bind that the remote no longer offers. Always empty when -/// is false — see that parameter. -/// -/// -/// False when the driver re-emits its authored tags into the same discovery stream as the device-derived -/// ones (ITagDiscovery.DiscoveryStreamIncludesAuthoredTags). The discovered set then always -/// contains the authored set, so a vanished tag cannot be seen. Carried explicitly so -/// can say "new-on-device only" rather than implying a clean bill of health. -/// -public sealed record TagSetDrift( - IReadOnlyList Appeared, - IReadOnlyList Vanished, - bool VanishedDetectable) -{ - /// True when the two sets differ in either direction. - public bool HasDrift => Appeared.Count > 0 || Vanished.Count > 0; - - /// - /// A stable identity for this drift, used to report a CHANGED drift once rather than re-reporting an - /// unchanged one on every check. Ordinal-ordered so set enumeration order cannot make an identical - /// drift look new. - /// - public string Signature { get; } = - string.Join('', Appeared) + '' + string.Join('', Vanished); - - /// An operator-facing one-liner naming what moved, suitable for a UI tooltip. - /// A short human-readable description, or "no drift" when there is none. - public string Describe() - { - if (!HasDrift) return "no drift"; - var parts = new List(3); - if (Appeared.Count > 0) parts.Add($"{Appeared.Count} new on device (e.g. {Sample(Appeared)})"); - if (Vanished.Count > 0) parts.Add($"{Vanished.Count} authored missing (e.g. {Sample(Vanished)})"); - // Say so rather than let the absence of a "missing" clause read as "nothing is missing". - if (!VanishedDetectable) parts.Add("missing-tag detection unavailable for this driver"); - return string.Join("; ", parts); - } - - /// Names at most two refs so the message stays readable when a whole PLC program is swapped. - private static string Sample(IReadOnlyList refs) => - refs.Count <= 2 ? string.Join(", ", refs) : $"{refs[0]}, {refs[1]}, +{refs.Count - 2} more"; -} - -/// -/// Compares a driver's discovered refs against its authored refs. Pure and allocation-light so the -/// actor's periodic check stays cheap; kept out of DriverInstanceActor so it is directly testable -/// without an actor system. -/// -public static class TagSetDriftDetector -{ - /// - /// Computes the two-way difference. - /// Ordinal comparison, matching how every driver keys its tag tables — a device that - /// genuinely exposes both Speed and speed must not have them collapsed. - /// - /// Refs streamed by the driver's most recent discovery pass. - /// Refs the driver's authored tags bind (ITagDiscovery.AuthoredDiscoveryRefs). - /// - /// False when the driver re-emits authored tags into its discovery stream, which makes a vanished tag - /// structurally invisible. The vanished half is then not computed at all rather than computed to a - /// misleading empty — see . - /// - /// The drift, which may be empty. - public static TagSetDrift Compare( - IReadOnlyCollection discovered, - IReadOnlyCollection authored, - bool detectVanished = true) - { - ArgumentNullException.ThrowIfNull(discovered); - ArgumentNullException.ThrowIfNull(authored); - - var authoredSet = new HashSet(authored, StringComparer.Ordinal); - var discoveredSet = new HashSet(discovered, StringComparer.Ordinal); - - var appeared = discoveredSet.Where(r => !authoredSet.Contains(r)) - .OrderBy(r => r, StringComparer.Ordinal).ToArray(); - var vanished = detectVanished - ? authoredSet.Where(r => !discoveredSet.Contains(r)) - .OrderBy(r => r, StringComparer.Ordinal).ToArray() - : []; - - return new TagSetDrift(appeared, vanished, detectVanished); - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs deleted file mode 100644 index f49e25b6..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs +++ /dev/null @@ -1,268 +0,0 @@ -using Akka.Actor; -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Core.Abstractions; -using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; -using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; - -namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; - -/// -/// The tag-set drift check: periodically re-browse a genuinely-browsable driver and compare what the -/// remote offers against what an operator authored. -/// Why this is gated rather than run for every driver. Modbus, S7, AbLegacy, Sql and Mqtt -/// all stream their AUTHORED tags back out of DiscoverAsync rather than enumerating the device, so -/// diffing the two sets for them is a tautology that reports "no drift" forever. A check that cannot fail -/// is worse than no check, because it reads as coverage — so the actor runs this only for drivers that -/// declare SupportsOnlineDiscovery AND report AuthoredDiscoveryRefs. -/// The signal is advisory and shares the Stage-2 surfacing: it raises the same /hosts -/// re-browse prompt an raise does. The served address space is never -/// rebuilt — raw tags are authored through the /raw browse-commit flow. -/// -[Trait("Category", "Unit")] -public sealed class DriverInstanceActorDriftCheckTests : RuntimeActorTestBase -{ - private static readonly TimeSpan Tick = TimeSpan.FromMilliseconds(60); - private static readonly TimeSpan Budget = TimeSpan.FromSeconds(3); - - /// A device offering a tag nothing binds raises the operator prompt, naming what appeared. - [Fact] - public void A_new_ref_on_the_device_raises_the_re_browse_prompt() - { - var driver = new BrowsableStubDriver(discovered: ["a", "b", "c"], authored: ["a", "b"]); - var publisher = new RecordingHealthPublisher(); - var actor = SpawnConnected(driver, publisher); - - AwaitAssert( - () => - { - var signalled = publisher.Published.LastOrDefault(p => p.RediscoveryNeededUtc is not null); - signalled.ShouldNotBeNull(); - signalled!.RediscoveryReason.ShouldNotBeNull(); - signalled.RediscoveryReason!.ShouldContain("1 new on device"); - signalled.RediscoveryReason!.ShouldContain("c"); - }, - Budget); - - Sys.Stop(actor); - } - - /// - /// The gate that makes this feature honest. A driver whose DiscoverAsync replays - /// authored config rather than browsing the device declares SupportsOnlineDiscovery == false, - /// and must never be drift-checked — for it the comparison could only ever say "no drift". - /// Positive control: the driver is handed a set that WOULD read as drift, and the assertion is - /// that discovery is never even invoked. Asserting only "no prompt raised" would pass for the wrong - /// reason (e.g. if the timer never fired at all). - /// - [Fact] - public void A_driver_that_replays_authored_config_is_never_drift_checked() - { - var driver = new BrowsableStubDriver( - discovered: ["a", "b", "c"], authored: ["a"], supportsOnlineDiscovery: false); - var publisher = new RecordingHealthPublisher(); - var actor = SpawnConnected(driver, publisher); - - // Long enough for several ticks had the timer been scheduled. - ExpectNoMsg(TimeSpan.FromMilliseconds(400)); - - driver.DiscoverCount.ShouldBe( - 0, - "a driver that replays authored config was browsed for drift — the comparison is a tautology " - + "for it and would report 'no drift' forever, which reads as coverage"); - publisher.Published.ShouldAllBe(p => p.RediscoveryNeededUtc == null); - - Sys.Stop(actor); - } - - /// A driver that browses but does NOT report its authored refs opts out — the default. An empty - /// collection would mean "nothing authored, everything is new", so null must not be read that way. - [Fact] - public void A_driver_that_reports_no_authored_refs_is_never_drift_checked() - { - var driver = new BrowsableStubDriver(discovered: ["a", "b"], authored: null); - var publisher = new RecordingHealthPublisher(); - var actor = SpawnConnected(driver, publisher); - - ExpectNoMsg(TimeSpan.FromMilliseconds(400)); - - driver.DiscoverCount.ShouldBe(0); - publisher.Published.ShouldAllBe(p => p.RediscoveryNeededUtc == null); - - Sys.Stop(actor); - } - - /// - /// An UNCHANGED drift is reported once, not re-stamped on every check — a prompt whose timestamp - /// keeps moving reads as "it just happened again" rather than "it is still true". - /// - [Fact] - public void An_unchanged_drift_is_reported_once() - { - var driver = new BrowsableStubDriver(discovered: ["a", "b"], authored: ["a"]); - var publisher = new RecordingHealthPublisher(); - var actor = SpawnConnected(driver, publisher); - - AwaitAssert( - () => publisher.Published.Count(p => p.RediscoveryNeededUtc is not null).ShouldBe(1), - Budget); - - // Several more ticks pass with the same drift. - AwaitAssert(() => driver.DiscoverCount.ShouldBeGreaterThan(3), Budget); - - publisher.Published.Count(p => p.RediscoveryNeededUtc is not null).ShouldBe( - 1, - "an unchanged drift was re-announced; the operator prompt would keep re-stamping its timestamp"); - - Sys.Stop(actor); - } - - /// A failed browse is not drift. A device that is merely unreachable already shows on the health - /// surface; reporting drift would turn every comms blip into "all your tags vanished". - [Fact] - public void A_failed_browse_is_not_reported_as_drift() - { - var driver = new BrowsableStubDriver(discovered: ["a"], authored: ["a", "b"]) { DiscoverThrows = true }; - var publisher = new RecordingHealthPublisher(); - var actor = SpawnConnected(driver, publisher); - - AwaitAssert(() => driver.DiscoverCount.ShouldBeGreaterThan(1), Budget); - - publisher.Published.ShouldAllBe( - p => p.RediscoveryNeededUtc == null, - "a browse that threw was treated as drift — an unreachable device would report every authored " - + "tag as vanished"); - - Sys.Stop(actor); - } - - /// Drift that resolves — the operator re-browsed and committed — clears the prompt, so the next - /// genuine drift re-raises with a fresh timestamp instead of being deduped against the stale one. - [Fact] - public void Drift_that_resolves_clears_the_prompt() - { - var driver = new BrowsableStubDriver(discovered: ["a", "b"], authored: ["a"]); - var publisher = new RecordingHealthPublisher(); - var actor = SpawnConnected(driver, publisher); - - AwaitAssert( - () => publisher.Published.Any(p => p.RediscoveryNeededUtc is not null), - Budget); - - // The operator commits the new tag: authored now matches the device. - driver.SetAuthored(["a", "b"]); - - AwaitAssert( - () => publisher.Published[^1].RediscoveryNeededUtc.ShouldBeNull(), - Budget); - - Sys.Stop(actor); - } - - private IActorRef SpawnConnected(IDriver driver, IDriverHealthPublisher publisher) - { - var parent = CreateTestProbe(); - parent.IgnoreMessages(_ => true); - var actor = parent.ChildActorOf(DriverInstanceActor.Props( - driver, healthPublisher: publisher, driftCheckInterval: Tick)); - actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); - return actor; - } - - /// Captured health publishes, so a test can read the rediscovery fields. - private sealed record HealthPublish(DateTime? RediscoveryNeededUtc, string? RediscoveryReason); - - private sealed class RecordingHealthPublisher : IDriverHealthPublisher - { - private readonly List _published = []; - - public IReadOnlyList Published - { - get { lock (_published) return _published.ToArray(); } - } - - /// - public void Publish( - string clusterId, string driverInstanceId, DriverHealth health, int errorCount5Min, - DateTime? rediscoveryNeededUtc = null, string? rediscoveryReason = null) - { - lock (_published) _published.Add(new HealthPublish(rediscoveryNeededUtc, rediscoveryReason)); - } - } - - /// - /// A driver that browses a fake "device". Health is STABLE so the publish dedup genuinely engages — - /// the shared StubDriver returns DateTime.UtcNow, which would make every publish look - /// changed and let a dedup-related bug pass unnoticed. - /// - private sealed class BrowsableStubDriver( - string[] discovered, - string[]? authored, - bool supportsOnlineDiscovery = true) : IDriver, ITagDiscovery - { - private static readonly DateTime FixedLastRead = new(2026, 7, 28, 9, 0, 0, DateTimeKind.Utc); - private volatile string[]? _authored = authored; - private int _discoverCount; - - /// Number of discovery passes the actor has driven — the direct read of whether the drift - /// check ran at all. - public int DiscoverCount => Volatile.Read(ref _discoverCount); - - /// When true, every browse throws — models an unreachable device. - public bool DiscoverThrows { get; init; } - - /// - public string DriverInstanceId => "browsable-stub-1"; - - /// - public string DriverType => "Stub"; - - /// - public bool SupportsOnlineDiscovery => supportsOnlineDiscovery; - - /// - public IReadOnlyCollection? AuthoredDiscoveryRefs => _authored; - - /// Models the operator committing (or removing) tags between checks. - public void SetAuthored(string[] refs) => _authored = refs; - - /// - public Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken) - { - Interlocked.Increment(ref _discoverCount); - if (DiscoverThrows) throw new IOException("device unreachable"); - - var folder = builder.Folder("Stub", "Stub"); - foreach (var r in discovered) - { - folder.Variable(r, r, new DriverAttributeInfo( - FullName: r, - DriverDataType: DriverDataType.Float64, - IsArray: false, - ArrayDim: null, - SecurityClass: SecurityClassification.ViewOnly, - IsHistorized: false)); - } - - return Task.CompletedTask; - } - - /// - public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; - - /// - public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; - - /// - public Task ShutdownAsync(CancellationToken cancellationToken) => Task.CompletedTask; - - /// - public DriverHealth GetHealth() => new(DriverState.Healthy, FixedLastRead, null); - - /// - public long GetMemoryFootprint() => 0; - - /// - public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask; - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs deleted file mode 100644 index b48f8d22..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs +++ /dev/null @@ -1,113 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; - -namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; - -/// -/// The pure half of tag-set drift detection: what the remote offers vs. what an operator authored. -/// Kept out of the actor so the comparison rules are testable without an actor system. -/// -[Trait("Category", "Unit")] -public sealed class TagSetDriftDetectorTests -{ - /// Matching sets are not drift, and must not raise an operator prompt. - [Fact] - public void Identical_sets_are_not_drift() - { - var drift = TagSetDriftDetector.Compare(["a", "b"], ["b", "a"]); - - drift.HasDrift.ShouldBeFalse(); - drift.Describe().ShouldBe("no drift"); - } - - /// A tag the device now offers that nothing binds — the common case after a PLC re-download. - [Fact] - public void A_ref_on_the_device_that_is_not_authored_is_reported_as_appeared() - { - var drift = TagSetDriftDetector.Compare(["a", "b", "c"], ["a", "b"]); - - drift.Appeared.ShouldBe(["c"]); - drift.Vanished.ShouldBeEmpty(); - drift.HasDrift.ShouldBeTrue(); - } - - /// An authored tag the device no longer offers — detectable only for a driver whose discovery - /// stream is purely device-derived. - [Fact] - public void An_authored_ref_missing_from_the_device_is_reported_as_vanished() - { - var drift = TagSetDriftDetector.Compare(["a"], ["a", "b"]); - - drift.Vanished.ShouldBe(["b"]); - drift.Appeared.ShouldBeEmpty(); - } - - /// - /// The honesty guard. When a driver re-emits its authored tags into the discovery stream — - /// FOCAS, TwinCAT and AbCip all do — the discovered set always contains the authored set, so a - /// vanished tag is structurally invisible. The detector must not compute a misleadingly-empty - /// Vanished list; it must flag that the direction is undetectable, and say so in the description. - /// - [Fact] - public void When_vanished_is_undetectable_it_is_declared_rather_than_reported_as_empty() - { - // "b" is authored and absent from the device — but this driver would have re-emitted it, so its - // absence here cannot occur in practice and must not be presented as a clean result either way. - var drift = TagSetDriftDetector.Compare(["a", "c"], ["a", "b"], detectVanished: false); - - drift.VanishedDetectable.ShouldBeFalse(); - drift.Vanished.ShouldBeEmpty(); - drift.Appeared.ShouldBe(["c"]); - drift.Describe().ShouldContain( - "missing-tag detection unavailable", - customMessage: - "a driver that cannot see vanished tags must SAY so — otherwise the absence of a 'missing' " - + "clause reads as 'nothing is missing', which is the reassurance this whole exercise removes"); - } - - /// Comparison is ordinal: a device legitimately exposing both Speed and speed - /// must not have them collapsed, which would hide a genuinely new tag. - [Fact] - public void Comparison_is_case_sensitive() - { - var drift = TagSetDriftDetector.Compare(["Speed", "speed"], ["Speed"]); - - drift.Appeared.ShouldBe(["speed"]); - } - - /// The signature identifies a drift so an UNCHANGED one is not re-announced every check. - /// Enumeration order must not make an identical drift look new. - [Fact] - public void Signature_is_stable_across_enumeration_order() - { - var a = TagSetDriftDetector.Compare(["x", "y", "z"], ["x"]); - var b = TagSetDriftDetector.Compare(["z", "y", "x"], ["x"]); - - a.Signature.ShouldBe(b.Signature); - } - - /// A different drift must get a different signature, or the second one is never reported. - [Fact] - public void Signature_changes_when_the_drift_changes() - { - var a = TagSetDriftDetector.Compare(["x", "y"], ["x"]); - var b = TagSetDriftDetector.Compare(["x", "y", "z"], ["x"]); - - a.Signature.ShouldNotBe(b.Signature); - } - - /// The description names a couple of refs and then counts, so swapping a whole PLC program - /// yields a readable message rather than a wall of tag names. - [Fact] - public void Describe_samples_rather_than_listing_everything() - { - var discovered = Enumerable.Range(0, 50).Select(i => $"tag{i:D2}").ToArray(); - var drift = TagSetDriftDetector.Compare(discovered, []); - - var text = drift.Describe(); - text.ShouldContain("50 new on device"); - text.ShouldContain("+48 more"); - text.Length.ShouldBeLessThan(120); - } -} From 4624141a5dbff04cbeb25197d7825fea76773c31 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 28 Jul 2026 14:32:13 -0400 Subject: [PATCH 2/3] test+docs: retire the stale EquipmentTagsDarkBatch4 skips, guard the deploy-time tag gate, and fix three silently-broken OpcUaClient integration tests Continues working through deferment.md's remaining items. SKIPPED TESTS (13 -> 0 for this reason). The skip said "dark until Batch 4"; Batch 4 shipped as v3.0 and RETIRED the equipment-tag path rather than lighting it, so every one of them was waiting on something that had already happened and gone the other way. Resolved individually rather than in bulk: - REVIVED onto the raw/UNS shape (6): four DriverHostActor primary-gate cases, the cluster-scoping rebuild, and the real-SDK dual-namespace materialisation E2E. The behaviour never went dark, only the v2 seeding did. - FOLDED into a live parity test (3): DeploymentArtifactRawUnsParityTests already compared RawTagPlan record-equal and RawTagPlan carries array / historize / alarm intent, so widening its corpus by two tags covers what three empty placeholder suites had been promising. Those suites are deleted. - DELETED (4): subjects genuinely retired -- equipment-namespace EquipmentTags, and both dot-joint {{equip}}.X token suites (that resolver was deleted in Batch 3; the slash-joint form is covered by DeploymentArtifactEquipRefParityTests). Falsified, not assumed: removing the seeded UnsTagReference turns two revived primary-gate tests red; re-homing the SITE-A node to MAIN turns the scoping test red. Widening a parity corpus also needed direct value assertions -- parity alone cannot distinguish "both seams right" from "both seams blind". Runtime.Tests skips 13->3, OpcUaServer.Tests 4->1; all remaining are deliberate EquipmentDeviceBindingRetired tombstones. G-7 (deploy-time TagConfig gate). MQTT shipped an Inspect() nobody ever called; now wired. Replaced the hand-maintained list with a reflection guard, because the existing test enumerates driver types by hand and so guards renames but can never notice a gap. NOTE: the first version of that guard was hollow -- it discovered candidates via GetReferencedAssemblies(), which is circular, since the compiler elides a reference nothing in the IL uses. Removing MQTT from the map also removed its Contracts assembly from the manifest, and the guard passed green with the bug reintroduced. Rewritten to scan the output directory; re-falsified and it now fails naming MqttTagDefinitionFactory. Residual recorded at the code: Sql, MTConnect, Calculation and OpcUaClient have no Contracts-side Inspect at all, so the deploy gate stays weaker than the AdminUI's authoring gate for them. OPCUACLIENT INTEGRATION TESTS (3 of 4 failing, on master too). Not fixture rot: the simulator was healthy and Client.CLI read the very same node Good. v3 made the driver resolve every reference through its authored RawTags table, so a bare "ns=3;s=StepUp" fails LOCALLY at the resolver with BadNodeIdInvalid and never reaches the wire -- the tests were exercising the unresolved-reference guard. Fixed by seeding OpcPlcProfile.RawTags in the deploy artifact's TagConfig shape and addressing by RawPath. 4/4 pass. DOCS. Applied the Phase7* -> AddressSpace* rename across the four live docs (historical bannered files deliberately untouched), resolving the three that are not renames to their real members. Found en route that the earlier banner pass missed three files: VirtualTags.md, OpcUaServer.md and ScriptedAlarms.md all still presented the test-scaffolding GenericDriverNodeManager as the production dispatch path. All three now carry the correction. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo --- deferment.md | 139 +++++++++++++++++- docs/OpcUaServer.md | 8 + docs/ScriptEditor.md | 4 +- docs/ScriptedAlarms.md | 16 +- docs/VirtualTags.md | 18 ++- docs/v2/Architecture-v2.md | 2 +- .../EquipmentTagConfigInspector.cs | 25 +++- .../ZB.MOM.WW.OtOpcUa.ControlPlane.csproj | 1 + .../OpcPlcProfile.cs | 35 +++++ .../OpcUaClientSmokeTests.cs | 6 +- ...quipmentTagConfigInspectorCoverageTests.cs | 113 ++++++++++++++ .../AddressSpaceApplierHierarchyTests.cs | 68 +++++---- .../AddressSpaceComposerAliasTagTests.cs | 30 ---- .../AddressSpaceComposerEquipTokenTests.cs | 81 ---------- .../DarkAddressSpaceReasons.cs | 38 ++--- .../Drivers/DarkAddressSpaceReasons.cs | 32 ++-- .../DeploymentArtifactAliasParityTests.cs | 32 ---- .../DeploymentArtifactArrayParityTests.cs | 28 ---- .../DeploymentArtifactEquipTokenTests.cs | 74 ---------- .../DeploymentArtifactHistorizeParityTests.cs | 30 ---- .../DeploymentArtifactRawUnsParityTests.cs | 41 +++++- .../Drivers/DeploymentArtifactTests.cs | 63 -------- .../DriverHostActorPrimaryGateTests.cs | 101 +++++++------ .../OpcUa/OpcUaPublishActorRebuildTests.cs | 66 +++++---- 24 files changed, 556 insertions(+), 495 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerAliasTagTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerEquipTokenTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactAliasParityTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactArrayParityTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactEquipTokenTests.cs delete mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactHistorizeParityTests.cs diff --git a/deferment.md b/deferment.md index 843640ee..b703e827 100644 --- a/deferment.md +++ b/deferment.md @@ -47,8 +47,8 @@ themes into tracked decisions. What remains: | **#521** `IHostConnectivityProbe` / `DriverHostStatus` dead surface | Decision: build the publisher or delete it. The table was deliberately re-created in the v3 initial migration, so deleting on inference would be wrong. | §3.2 | | **#522** driver stability tiers inert | Decision: pass real tiers or delete the machinery. Docs now say every driver runs Tier A. | §3.3 | | ~~**#523** remove the periodic 5-min device re-browse~~ | ✅ **Done** — removed. **AbCip and FOCAS now have no tag-change signal at all**; that is the accepted consequence, not an oversight. | §9 (2026-07-28, #523) | -| **G-7** `EquipmentTagConfigInspector` covers 6 of 12 driver types | Pre-existing gap, not a regression; the only §1.2 gap left. | §1.2 | -| **13 `EquipmentTagsDarkBatch4` skipped tests** | Still stale — their unblock condition already happened and the path was retired instead. | §4.4 | +| **G-7** `EquipmentTagConfigInspector` coverage | ◐ **Narrowed + guarded** (2026-07-28): MQTT wired (it shipped an `Inspect` nobody called), and a reflection guard now fails the build if a reachable driver ships one and is not mapped. **Residual:** Sql / MTConnect / Calculation / OpcUaClient have no Contracts-side `Inspect` at all, so the deploy gate stays blind for them while the AdminUI validates them — see §1.2. | §1.2 | +| ~~**13 `EquipmentTagsDarkBatch4` skipped tests**~~ | ✅ **Done** — 6 revived onto the raw/UNS shape, 3 folded into a widened live parity corpus, 4 deleted as retired. The reason constant is gone. **Skips now: `Runtime.Tests` 13 → 3, `OpcUaServer.Tests` 4 → 1** — all remaining are deliberate `EquipmentDeviceBindingRetired` tombstones. | §9 (2026-07-28, skips) | | 17 other tracker issues | Driver features + deferred work, unchanged by this remediation. | §2 | | Open live gates | #491 continuous historization, #468 browser tree, archreview R2-03 / R2-10. *(The un-gated drift detector's gate closed by deletion — see #523.)* | §5 | @@ -99,7 +99,7 @@ Every gap below is in the **authoring/dispatch layer, not driver runtime code**. | G-4 | **No parity test guards `DriverConfigModal` or `DeviceModal`** — `RawDriverTypeDialogCoverageTests`/`…ParityTests` assert `DriverTypeNames` ↔ **picker** parity only | — | **This is exactly why G-1 and G-2 survived review.** The guard pattern exists and simply was not extended to the two downstream dispatch maps. | ✅ Fixed — `DriverFormMapParityTests` + `DriverDispatchMapParityTests` | | G-5 | **`CsvColumnMap` has no typed columns for Sql, MQTT, MTConnect** | `CsvColumnMap.cs:340-452` | CSV import/export degrades to the raw `TagConfigJson` fallback while the 7 older drivers get typed columns. | ✅ Fixed — CSV maps for Sql/Mqtt/MTConnect | | G-6 | **`RawBrowseCommitMapper` has no Sql branch** — falls to the generic `WriteSingleKey("address", …)` whose comment claims "Browsable drivers are all handled above" | `RawBrowseCommitMapper.cs:121-141`, fallback `:145` | Untrue since `SqlDriverBrowser` was registered (`EndpointRouteBuilderExtensions.cs:83`). Same failure mode the MTConnect branch (`:135-139`) was added to avoid: typed editor opens empty, blanks the field on save. | ✅ Fixed — **and at the browser end too**, which the audit missed | -| G-7 | **`EquipmentTagConfigInspector` covers only 6 driver types** | `:24-29` | Pre-existing (OpcUaClient + Galaxy absent too); no new driver was added. | ⏳ **Open** — pre-existing, unchanged | +| G-7 | **`EquipmentTagConfigInspector` covers only 6 driver types** | `:24-29` | Pre-existing (OpcUaClient + Galaxy absent too); no new driver was added. **The audit understated it:** the AdminUI's `TagConfigValidator` covers **11 of 12**, so the *deploy* gate is weaker than the *authoring* gate — and config arriving by CSV import or API only meets the deploy gate. | ◐ **Narrowed + guarded** — MQTT wired, reflection guard added; 4 drivers still have no Contracts-side `Inspect`. See §9 | ### 1.3 Stale in-code comments (harmless at runtime, misleading to the next reader) @@ -283,8 +283,8 @@ commented-out DI registrations.** Real deferred work lives in prose doc-comments | Count | Reason | Assessment | |---|---|---| | ~~18~~ 0 | ~~`DiscoveryInjectionDormantV3`~~ | ✅ **Deleted** (`adce27e7`). The audit's "Real — blocked on #507" was **wrong**: these were v2 characterization tests asserting an equipment-rooted graft (`EquipmentRootNodeId == "EQ-1"`) that v3 cannot produce, so they could never have unskipped as written. `Runtime.Tests` skipped went 31 → 13. | -| 13 | `EquipmentTagsDarkBatch4` | ⏳ **Still open.** **STALE** — the reason says these unblock "at Batch 4"; Batch 4 shipped as v3.0 and the path was *retired* (`AddressSpaceApplier.cs:415` calls it "the vestigial equipment-tag path"). These will never unskip as written — delete or rewrite the reason. | -| 4 + 1 | `EquipmentDeviceBindingRetired`, `DarkBatch4` | Intentional tombstones preserving retirement rationale — fine as-is | +| ~~13~~ 0 | ~~`EquipmentTagsDarkBatch4`~~ | ✅ **Resolved** (2026-07-28). The audit's "STALE" call was right and the constant is now **deleted**. Not by bulk-unskipping: 6 were **revived** onto the raw/UNS shape that replaced the subject, 3 were **folded** into a live parity test whose corpus was widened to cover them, 4 were **deleted** as genuinely retired. See §9. | +| 4 | `EquipmentDeviceBindingRetired` | Intentional tombstones preserving retirement rationale — fine as-is | Plus **4 whole OpcUaClient test suites that are scaffold-only**, awaiting an in-process OPC UA server fixture that was never built (`OpcUaClientSubscribeAndProbeTests`, `…DiscoveryTests`, @@ -712,6 +712,135 @@ Build clean; `Runtime.Tests` 476 passed / 13 skipped (unchanged skip set — the `EquipmentTagsDarkBatch4` group), FOCAS 275, TwinCAT 192, MTConnect 491, AbCip 342, `Core.Abstractions` 266, all green. +### 2026-07-28 — three silently-broken OpcUaClient integration tests (found by the full-suite run) + +Not a register item — found because the full-solution run for the work above showed **three** failing +suites where §9's 2026-07-27 entry had recorded two. `Driver.OpcUaClient.IntegrationTests` was failing 3 +of 4. + +**It reproduced identically on clean `master`** (verified in a throwaway worktree), so it was not caused +by this work — but it was also not what it first looked like. The chain of wrong guesses is worth +recording, because each one would have closed the investigation early: + +1. *"The fixture is down."* — `otopcua-opc-plc` was `Up 12 days (unhealthy)`. Restarted it, waited for + healthy. **Still 3/4 failing.** +2. *"The simulator's node set drifted."* — the tests expect `ns=3;s=StepUp`. Browsed the live server: + all three nodes present at exactly those ids. Read `ns=3;s=StepUp` through Client.CLI against the + same endpoint: **`Good`, value 1379.** +3. *"So the driver has a read defect."* — the status code was `2150825984`, which I first decoded by + hand as `BadAttributeIdInvalid` and started hunting a wrong `AttributeId`. **The decode was wrong** + (`0x80330000`, not `0x80350000`) — it is `BadNodeIdInvalid`, and grep found that exact constant on + the driver's own *local* fault path, reached without any wire request. + +The actual cause: **v3 made the driver resolve every reference through its authored `RawTags` table** +(RawPath → `TagConfig.nodeId` → live NodeId re-bound against the session's namespace table). A bare +`ns=3;s=StepUp` is no longer a reference the driver accepts. The smoke tests still passed bare node ids +and constructed the driver with an empty `RawTags`, so all three failed locally at the resolver — they +had been testing the unresolved-reference guard, against a perfectly healthy simulator. + +Fixed by seeding `OpcPlcProfile.RawTags` with the four nodes in the same `{"nodeId": …}` TagConfig shape +the deploy artifact delivers, and addressing them by RawPath. **4/4 pass.** + +Two things this says about the register itself: the 2026-07-27 "only pre-existing environment failures +remain" note was measured against an incomplete baseline (this suite was already broken and simply not +enumerated); and **fixture-gated integration suites rot invisibly** — a driver contract can change under +them and nothing on the local box notices, because the suite is expected to be red-or-skipped depending +on whether a remote container happens to be up. + +### 2026-07-28 — §7.2 priority 3: the `Phase7*` → `AddressSpace*` rename + +Applied across the four **live** docs (`VirtualTags.md`, `ScriptedAlarms.md`, `ScriptEditor.md`, +`v2/Architecture-v2.md`). The already-bannered historical files (`v2/phase-7-status.md`, +`v2/implementation/*`) were deliberately **left alone** — they are records of what existed at the time, +and renaming inside a historical record would falsify it. + +§7.2 was right that three of these are not renames, and each was resolved to the real member: + +| Cited | Reality | +|---|---| +| `Phase7CompositionResult` | → **`AddressSpaceComposition`** (and it now also carries the raw subtree, so the doc text was widened, not just renamed) | +| `Phase7Composer.ExtractTagFullName` | → **`ScriptTagCatalog.ExtractFullNameFromTagConfig`** | +| `Phase7EngineComposer.RouteToHistorianAsync` | **Gone, not renamed** — the routing moved into `HistorianAdapterActor` → the registered `IAlarmHistorianSink`. Called out inline so the next reader does not go looking for a renamed symbol. | + +**Found en route — the banner pass missed three files.** `GenericDriverNodeManager` is test scaffolding +(§3.4, its own source says "zero production references"), and the earlier remediation bannered +`AddressSpace.md` and `IncrementalSync.md` for it. But `VirtualTags.md`, `OpcUaServer.md` and +`ScriptedAlarms.md` all still presented it as **the** production dispatch path — `VirtualTags.md:3` even +built its opening explanation of driver-vs-virtual dispatch on it. All three now carry the same +correction naming the real chain (`OpcUaPublishActor` → `SdkAddressSpaceSink` → `OtOpcUaNodeManager`, two +namespaces). Banners, not rewrites — the underlying rewrite stays owed, consistent with how §7.2 treats +the other two. + +### 2026-07-28 — G-7, and what the audit missed about it + +The register called this "pre-existing (OpcUaClient + Galaxy absent too); no new driver was added" and +ranked it last. Reading both maps side by side makes it sharper than that: + +- The AdminUI's `TagConfigValidator` covers **11 of 12** driver types (all but Galaxy). +- The deploy-time `EquipmentTagConfigInspector` covers **6**. + +So the gate that runs at *authoring* is stronger than the gate that runs at *deploy* — and the deploy +gate is the only one config sees when it arrives by **CSV import or API** rather than through the modal. +That is the wrong way round. + +**MQTT was a plain hole:** `MqttTagDefinitionFactory.Inspect` has existed since the driver merged and +nothing ever called it. Now wired. The inspector's own doc-comment also claimed "unmapped drivers +(Galaxy, OpcUaClient) are skipped exactly as in the AdminUI validator" — untrue, OpcUaClient *is* mapped +there; corrected. + +**A reflection guard replaces the hand-maintained list**, same lesson as G-4: the existing +`EquipmentTagConfigInspectorTests` enumerates driver-type strings by hand, so it guards renames but can +never notice a gap — a newly-inspectable driver simply never appears in it. + +⚠️ **The first version of that guard was hollow, and only the falsification step caught it.** It +discovered candidates via `Assembly.GetReferencedAssemblies()`, which is **circular**: the C# compiler +elides a reference nothing in the IL uses, so removing MQTT from the map also removed its Contracts +assembly from ControlPlane's manifest, and the sweep stopped looking for exactly the driver that had just +gone missing. It passed green with the bug reintroduced. Rewritten to scan the output *directory* +(MSBuild copies project-referenced assemblies regardless of IL usage) — re-falsified, and it now fails +naming `MqttTagDefinitionFactory`. It also carries a positive control asserting discovery found more than +one factory, since an empty expected-set is satisfied by an empty map. + +**Residual, deliberately not papered over.** Sql, MTConnect, Calculation and OpcUaClient have **no** +`Inspect` in a Contracts assembly — their only checker is the AdminUI `*TagConfigModel.Validate()`, which +lives in the AdminUI project and cannot be referenced from ControlPlane. Closing that means promoting +those models (or an equivalent parser) into each driver's Contracts assembly — a real piece of work, not +a map entry. Recorded in the inspector's own doc-comment so the next reader finds it at the code. + +### 2026-07-28 — the 13 stale `EquipmentTagsDarkBatch4` skips + +§4.4 flagged these as stale and it was right: the reason said "dark until Batch 4", Batch 4 shipped as +v3.0, and the equipment-tag path was **retired** rather than lit — so every one of them was waiting on +something that had already happened and gone the other way. + +**Unskipping them in bulk would have failed, and deleting them in bulk would have thrown away live +coverage.** Each was resolved on its own merits: + +| Disposition | Tests | Why | +|---|---|---| +| **Revived** onto the raw/UNS shape | 4 primary-gate cases (`DriverHostActorPrimaryGateTests`), the cluster-scoping rebuild (`OpcUaPublishActorRebuildTests`), the real-SDK materialisation E2E (`AddressSpaceApplierHierarchyTests`) | The *behaviour* never went dark — only the v2 seeding did. Each seeds the v3 chain (RawFolder → Driver → Device → Tag, + `UnsTagReference`) instead. | +| **Folded** into a live parity test | array intent, historize intent, native-alarm/writable intent | `DeploymentArtifactRawUnsParityTests` already compared `RawTagPlan` **record-equal**, and `RawTagPlan` carries all three fields — so widening its corpus by two tags covers what three empty placeholder suites had been promising. Those suites are deleted. | +| **Deleted** | equipment-namespace `EquipmentTags` parse, both `{{equip}}`-token suites | The subjects are retired. The token tests used the **dot-joint** `{{equip}}.X` form, whose resolver (`EquipmentScriptPaths.DeriveEquipmentBase`) was deleted in Batch 3; the slash-joint form is covered live by `DeploymentArtifactEquipRefParityTests` (4 tests). | + +Three things worth keeping: + +- **The revivals are the point, not the unskipping.** `Unknown_role_single_driver_services_write` asserts + `recorder.Writes.ShouldNotBeEmpty()` — it needs the write to actually reach a driver, which needs a + routable tag. **Verified by deleting the `UnsTagReference` from the seed: it and + `Known_role_wins_over_member_count` go red.** Same check on the cluster-scoping test (re-home the SITE-A + node to MAIN ⇒ red). The three *denial* cases in that file were passing all along — they deny before + routing — which is exactly why the gap was invisible. +- **Widening a parity corpus needed a non-parity assertion to go with it.** A composer that dropped array + intent *entirely* would still satisfy `decoded.RawTags.ShouldBe(composed.RawTags)`, because the artifact + decode would drop it too and the two empties compare equal. Parity cannot distinguish "both right" from + "both blind", so the widened test also asserts the composed values directly. +- **The reason constant is deleted, replaced by a note explaining why.** A skip reason is a claim with an + expiry date, and this one outlived its own condition by a release. Both `DarkAddressSpaceReasons` files + now say so, so the next person writing one states a condition a reader can actually check. + +Skipped counts: `Runtime.Tests` **13 → 3**, `OpcUaServer.Tests` **4 → 1**. Every remaining skip is an +`EquipmentDeviceBindingRetired` tombstone — deliberate, and §4.4 already calls those fine as-is. + ### 2026-07-27 — §8.4 dispatch-map parity (G-1 … G-6) **The maps had to become data before they could be guarded.** A Razor `@switch` compiles into diff --git a/docs/OpcUaServer.md b/docs/OpcUaServer.md index 301c4a23..9373be48 100644 --- a/docs/OpcUaServer.md +++ b/docs/OpcUaServer.md @@ -1,5 +1,13 @@ # OPC UA Server +> ⚠️ **Correction (2026-07-28): `GenericDriverNodeManager` is NOT a production dispatch path.** It is test +> scaffolding — its own source says so (`Core/OpcUa/GenericDriverNodeManager.cs:71`, "verified 07/#10: zero +> production references"). The production chain is `OpcUaPublishActor` → `IOpcUaAddressSpaceSink` +> (`SdkAddressSpaceSink`) → `OtOpcUaNodeManager`, materialising the **two v3 namespaces** (`.../raw` and +> `.../uns`). Sections below that describe per-node dispatch, rediscovery re-walks, or alarm-sink capture +> *through that class* describe a design, not the running server. The same correction is already carried by +> `AddressSpace.md` and `IncrementalSync.md`; this file was missed in that pass. + The OPC UA server component (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/`) hosts the OPC UA stack and exposes a browsable address space built from the registered drivers. The server itself is driver-agnostic — Galaxy/MXAccess, Modbus, S7, AB CIP, AB Legacy, TwinCAT, FOCAS, and OPC UA Client are all plugged in as `IDriver` implementations via the capability interfaces in `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/`. In v2 the Server and Admin processes were fused into a single role-gated `ZB.MOM.WW.OtOpcUa.Host` binary. Which subsystems start (OPC UA endpoint, Admin UI, control plane, driver runtime) is decided by the `OTOPCUA_ROLES` gate, not by running separate executables. See `docs/ServiceHosting.md` for the role model. diff --git a/docs/ScriptEditor.md b/docs/ScriptEditor.md index bcbba33d..1241db9d 100644 --- a/docs/ScriptEditor.md +++ b/docs/ScriptEditor.md @@ -202,7 +202,7 @@ prefix. The concrete implementation `ScriptTagCatalog` reads `Tag` + **Why only resolvable keys?** The live runtime resolves a `ctx.GetTag("X")` literal against `DriverInstanceActor.AttributeValuePublished.FullReference`, which is the `FullName` field from `Tag.TagConfig` -(see `Phase7Composer.ExtractTagFullName` + `EquipmentNodeWalker.ExtractFullName`). +(see `ScriptTagCatalog.ExtractFullNameFromTagConfig` + `EquipmentNodeWalker.ExtractFullName`). The UNS-path engine (`Core.VirtualTags.VirtualTagEngine`, keyed by the slash-joined `Enterprise/Site/Area/Line/Equipment/TagName` browse path) is dormant — it is not wired into the host — so UNS browse paths never resolve at @@ -228,7 +228,7 @@ syntax token under the caret to: `LiteralExpressionSyntax` → `ArgumentSyntax` `GetTag` or `SetVirtualTag`, and (b) the **receiver is the identifier `ctx`** — a purely syntactic check that deliberately mirrors the runtime dependency harvest (`EquipmentScriptPaths.GetTagRefRegex` is `ctx`-anchored), so the editor -offers tag-path completion/hover for exactly what `Phase7Composer` harvests at +offers tag-path completion/hover for exactly what `AddressSpaceComposer` harvests at deploy time. An unrelated `anything.GetTag("…")` no longer triggers it. **`SetVirtualTag` is a no-op in production.** The live single-tag evaluator diff --git a/docs/ScriptedAlarms.md b/docs/ScriptedAlarms.md index 0ee00d36..0abde22b 100644 --- a/docs/ScriptedAlarms.md +++ b/docs/ScriptedAlarms.md @@ -1,5 +1,13 @@ # Scripted Alarms +> ⚠️ **Correction (2026-07-28): `GenericDriverNodeManager` is NOT a production dispatch path.** It is test +> scaffolding — its own source says so (`Core/OpcUa/GenericDriverNodeManager.cs:71`, "verified 07/#10: zero +> production references"). The production chain is `OpcUaPublishActor` → `IOpcUaAddressSpaceSink` +> (`SdkAddressSpaceSink`) → `OtOpcUaNodeManager`, materialising the **two v3 namespaces** (`.../raw` and +> `.../uns`). Sections below that describe per-node dispatch, rediscovery re-walks, or alarm-sink capture +> *through that class* describe a design, not the running server. The same correction is already carried by +> `AddressSpace.md` and `IncrementalSync.md`; this file was missed in that pass. + `Core.ScriptedAlarms` is the Phase 7 subsystem that raises OPC UA Part 9 alarms from operator-authored C# predicates rather than from driver-native alarm streams. Scripted alarms are additive: Galaxy, AB CIP, FOCAS, and OPC UA Client drivers keep their native `IAlarmSource` implementations unchanged, and a `ScriptedAlarmSource` simply registers as another source in the same fan-out. Predicates read tags from any source (driver tags or virtual tags) through the shared `ITagUpstreamSource` and emit condition transitions through the engine's Part 9 state machine. This file covers the engine internals — predicate evaluation, state machine, persistence, and the engine-to-`IAlarmSource` adapter. The server-side plumbing that turns those emissions into OPC UA `AlarmConditionState` nodes, applies retries, persists alarm transitions to the Historian, and routes operator acks through the session's `AlarmAck` permission lives in [AlarmTracking.md](AlarmTracking.md) and is not repeated here. @@ -10,7 +18,7 @@ Scripted-alarm definitions are created and edited per-equipment on the **`/uns/e ## Definition shape -`ScriptedAlarmDefinition` (`src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmDefinition.cs`) is the runtime contract the engine consumes. The generation-publish path materialises these from the `ScriptedAlarm` + `Script` config tables via `Phase7Composer.Compose` + the driver-role host actor startup path. +`ScriptedAlarmDefinition` (`src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmDefinition.cs`) is the runtime contract the engine consumes. The generation-publish path materialises these from the `ScriptedAlarm` + `Script` config tables via `AddressSpaceComposer.Compose` + the driver-role host actor startup path. | Field | Notes | |---|---| @@ -332,7 +340,7 @@ Emissions map into `AlarmEventArgs` as `AlarmType = Kind.ToString()`, `SourceNod ## Composition -`Phase7Composer` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs`) is a pure data composer; it has no knowledge of `ScriptedAlarmEngine`. It maps `ScriptedAlarm` config-DB rows into `ScriptedAlarmPlan` records that the driver-role host actor startup path consumes. +`AddressSpaceComposer` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs`) is a pure data composer; it has no knowledge of `ScriptedAlarmEngine`. It maps `ScriptedAlarm` config-DB rows into `ScriptedAlarmPlan` records that the driver-role host actor startup path consumes. In the v2 actor system, scripted-alarm engine composition is owned by the driver-role host: @@ -355,8 +363,8 @@ Both engine and source are disposed on server shutdown via the driver-role host - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/AlarmTypes.cs` — `AlarmKind` + `ShelvingKind` + four Part 9 state enums (`AlarmEnabledState`, `AlarmActiveState`, `AlarmAckedState`, `AlarmConfirmedState`); `AlarmSeverity` (`Low`/`Medium`/`High`/`Critical`) lives in `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmSource.cs` - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/MessageTemplate.cs` — `{path}` placeholder resolver - `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/IAlarmStateStore.cs` — persistence contract + `InMemoryAlarmStateStore` default -- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs` — pure data composer: config-DB entities → `Phase7CompositionResult` (UNS topology + driver/alarm plans) -- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs` — applies the composed Phase 7 plan into the SDK node manager +- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs` — pure data composer: config-DB entities → `AddressSpaceComposition` (UNS topology + raw subtree + driver/alarm plans) +- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs` — applies the composed plan into the SDK node manager - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmActor.cs` — actor that owns the per-alarm state machine; publishes `AlarmTransitionEvent` on the cluster `alerts` DPS topic - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/EfAlarmActorStateStore.cs` — production `IAlarmActorStateStore` backed by the `ScriptedAlarmState` config-DB table - `src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynScriptedAlarmEvaluator.cs` — production Roslyn predicate evaluator diff --git a/docs/VirtualTags.md b/docs/VirtualTags.md index 0f9ecdcc..20032d37 100644 --- a/docs/VirtualTags.md +++ b/docs/VirtualTags.md @@ -1,5 +1,13 @@ # Virtual Tags +> ⚠️ **Correction (2026-07-28): `GenericDriverNodeManager` is NOT a production dispatch path.** It is test +> scaffolding — its own source says so (`Core/OpcUa/GenericDriverNodeManager.cs:71`, "verified 07/#10: zero +> production references"). The production chain is `OpcUaPublishActor` → `IOpcUaAddressSpaceSink` +> (`SdkAddressSpaceSink`) → `OtOpcUaNodeManager`, materialising the **two v3 namespaces** (`.../raw` and +> `.../uns`). Sections below that describe per-node dispatch, rediscovery re-walks, or alarm-sink capture +> *through that class* describe a design, not the running server. The same correction is already carried by +> `AddressSpace.md` and `IncrementalSync.md`; this file was missed in that pass. + Virtual tags are OPC UA variable nodes whose values are computed by operator-authored C# scripts against other tags (driver or virtual). They live in the Equipment browse tree alongside driver-sourced variables: a client browsing `Enterprise/Site/Area/Line/Equipment/` sees one flat child list that mixes both kinds, and a read / subscribe on a virtual node looks identical to one on a driver node from the wire. The separation is server-side — `EquipmentNodeWalker` stamps each `DriverAttributeInfo` with `NodeSourceKind` (`Driver` / `Virtual` / `ScriptedAlarm`) at address-space build time, and `GenericDriverNodeManager` routes reads to different backends accordingly. See [ADR-002](v2/implementation/adr-002-driver-vs-virtual-dispatch.md) for the dispatch decision. The runtime is split across two projects: `Core.Scripting` holds the Roslyn sandbox + evaluator primitives that are reused by both virtual tags and scripted alarms; `Core.VirtualTags` holds the engine that owns the dependency graph, the evaluation pipeline, and the `ISubscribable` adapter the server dispatches to. In the v2 actor system, `VirtualTagActor` + `DependencyMuxActor` (in `Core.Runtime`) own the per-instance state and upstream-feed wiring; `RoslynVirtualTagEvaluator` (in `Host.Engines`) is the production `IVirtualTagEvaluator` binding. @@ -94,7 +102,7 @@ What the engine pulls driver-tag values from. Reads are **synchronous** because ### `IHistoryWriter` -Fire-and-forget sink for evaluation results when `VirtualTagDefinition.Historize = true`. Implementations must queue internally and drain on their own cadence — a slow historian must not block script evaluation. `NullHistoryWriter.Instance` is the no-op default. Scripted-alarm emissions flow through `Core.AlarmHistorian` via `Phase7EngineComposer.RouteToHistorianAsync` (a separate concern; see [AlarmTracking.md](AlarmTracking.md)). +Fire-and-forget sink for evaluation results when `VirtualTagDefinition.Historize = true`. Implementations must queue internally and drain on their own cadence — a slow historian must not block script evaluation. `NullHistoryWriter.Instance` is the no-op default. Scripted-alarm emissions flow through `Core.AlarmHistorian` via `HistorianAdapterActor` → the registered `IAlarmHistorianSink` (a separate concern; see [AlarmTracking.md](AlarmTracking.md)). *(The `Phase7EngineComposer.RouteToHistorianAsync` this used to name is gone — not renamed; the routing moved into the actor.)* **Equipment-namespace path (H5).** The `Historize` flag is threaded end-to-end on the equipment path: `VirtualTag.Historize` → composer + artifact-decode (byte-parity) → `EquipmentVirtualTagPlan.Historize` → `VirtualTagHostActor`, which calls `IHistoryWriter.Record(nodeId, snapshot)` for every historized result (in addition to publishing the live value). The writer is injectable via DI — `DriverHostActor` resolves `IHistoryWriter` (`TryAddSingleton`, `NullHistoryWriter` default) and threads it into `VirtualTagHostActor`. **This `IHistoryWriter` seam still ships no durable binding** (`NullHistoryWriter` default). Durable continuous historization of driver/virtual values is now handled by the separate `ContinuousHistorizationRecorder` (it taps the dependency-mux value fan-out → a crash-safe FasterLog outbox → the HistorianGateway's `WriteLiveValues` path; see [Historian.md](Historian.md)), not through this seam. A deployment can still bind a custom `IHistoryWriter` via DI. @@ -118,11 +126,11 @@ Per [ADR-002](v2/implementation/adr-002-driver-vs-virtual-dispatch.md) Option B, ## Composition -`Phase7Composer` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs`) is a pure static function that flattens config-DB entities into a `Phase7CompositionResult` value (UNS topology + driver-instance plans + scripted-alarm plans). `Phase7Applier` applies that result into the OPC UA SDK node manager. Neither class has knowledge of `VirtualTagEngine` or `ScriptedAlarmEngine`. +`AddressSpaceComposer` (`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs`) is a pure static function that flattens config-DB entities into an `AddressSpaceComposition` value (UNS topology + raw subtree + driver-instance plans + scripted-alarm plans). `AddressSpaceApplier` applies that result into the OPC UA SDK node manager. Neither class has knowledge of `VirtualTagEngine` or `ScriptedAlarmEngine`. In the v2 actor system, virtual-tag engine composition is owned by the driver-role host actor tree: -- `Phase7Composer.Compose` emits `DriverInstancePlan` / `ScriptedAlarmPlan` records; the driver-role `DriverHostActor` spawns one `VirtualTagActor` per virtual-tag expression and one `ScriptedAlarmActor` per scripted alarm. +- `AddressSpaceComposer.Compose` emits `DriverInstancePlan` / `ScriptedAlarmPlan` records; the driver-role `DriverHostActor` spawns one `VirtualTagActor` per virtual-tag expression and one `ScriptedAlarmActor` per scripted alarm. - `RoslynVirtualTagEvaluator` (`src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs`) is injected into each `VirtualTagActor` as its `IVirtualTagEvaluator`. It holds a per-source `CompiledScriptCache` keyed by script source and compiles on first use. - `DependencyMuxActor` (`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs`) receives every `DriverInstanceActor.AttributeValuePublished` event and routes it to the `VirtualTagActor` instances that registered interest in that tag ref. @@ -151,6 +159,6 @@ In the v2 actor system, virtual-tag engine composition is owned by the driver-ro - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs` — actor that receives `DependencyValueChanged` from the mux and invokes `IVirtualTagEvaluator` per expression - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs` — routes `DriverInstanceActor.AttributeValuePublished` to interested `VirtualTagActor` subscribers - `src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs` — production `IVirtualTagEvaluator` binding; holds a per-source `CompiledScriptCache` -- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs` — pure data composer: config-DB entities → `Phase7CompositionResult` (UNS topology + driver/alarm plans) -- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs` — applies the composed plan into the SDK node manager +- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs` — pure data composer: config-DB entities → `AddressSpaceComposition` (UNS topology + raw subtree + driver/alarm plans) +- `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceApplier.cs` — applies the composed plan into the SDK node manager - `src/Core/ZB.MOM.WW.OtOpcUa.Core/OpcUa/GenericDriverNodeManager.cs` — driver-agnostic OPC UA node-manager backbone; per-variable `NodeSourceKind` drives dispatch diff --git a/docs/v2/Architecture-v2.md b/docs/v2/Architecture-v2.md index 4240635f..35e35e18 100644 --- a/docs/v2/Architecture-v2.md +++ b/docs/v2/Architecture-v2.md @@ -43,7 +43,7 @@ src/Server/ server-side projects ZB.MOM.WW.OtOpcUa.Security cookie+JWT auth, LDAP, JwtTokenService ZB.MOM.WW.OtOpcUa.ControlPlane admin-role cluster singletons ZB.MOM.WW.OtOpcUa.Runtime driver-role per-node actors - ZB.MOM.WW.OtOpcUa.OpcUaServer OPC UA endpoint facade + Phase7Composer + ZB.MOM.WW.OtOpcUa.OpcUaServer OPC UA endpoint facade + AddressSpaceComposer ZB.MOM.WW.OtOpcUa.AdminUI Blazor Razor class library ZB.MOM.WW.OtOpcUa.Host fused binary (Program.cs) ``` diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs index bc6cfa9e..7e8a25e5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/EquipmentTagConfigInspector.cs @@ -3,6 +3,7 @@ using ZB.MOM.WW.OtOpcUa.Driver.AbCip; using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; using ZB.MOM.WW.OtOpcUa.Driver.Modbus; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; using ZB.MOM.WW.OtOpcUa.Driver.S7; using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; @@ -13,8 +14,19 @@ namespace ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; /// AdminUI TagConfigValidator shape: a case-insensitive map from the canonical driver-type string /// to that driver's lightweight Contracts parser Inspect(), returning human-readable /// warnings (present-but-invalid enum values that the lenient runtime silently defaults, plus -/// structurally-unparseable TagConfig). Unmapped drivers (Galaxy, OpcUaClient) are skipped exactly as in -/// the AdminUI validator. Never throws. +/// structurally-unparseable TagConfig). Never throws. +/// Coverage is every driver that ships an Inspect, and that invariant is guarded by +/// EquipmentTagConfigInspectorCoverageTests, which reflects over the referenced +/// *.Contracts assemblies — so a driver that gains an inspector cannot be silently left out. This +/// is the same guard shape the driver-form dispatch maps use, and for the same reason: the previous +/// hand-maintained list had drifted (MQTT shipped an Inspect and was never added). +/// Residual gap, deliberately recorded rather than papered over. Galaxy, OpcUaClient, Sql, +/// MTConnect and Calculation have no Inspect in a Contracts assembly, so this deploy-time +/// gate cannot check them — while the AdminUI's TagConfigValidator DOES cover all but Galaxy via +/// its *TagConfigModel.Validate() models, which live in the AdminUI project and cannot be +/// referenced from here. The asymmetry runs the wrong way: config authored through the modal is checked, +/// but config arriving by CSV import or API is not. Closing it means promoting those models (or an +/// equivalent parser) into each driver's Contracts assembly. Tracked as deferment-register G-7. /// public static class EquipmentTagConfigInspector { @@ -27,10 +39,12 @@ public static class EquipmentTagConfigInspector [DriverTypeNames.AbLegacy] = AbLegacyTagDefinitionFactory.Inspect, [DriverTypeNames.TwinCAT] = TwinCATTagDefinitionFactory.Inspect, [DriverTypeNames.FOCAS] = FocasTagDefinitionFactory.Inspect, + [DriverTypeNames.Mqtt] = MqttTagDefinitionFactory.Inspect, }; /// Inspects a tag's TagConfig for the given driver type. Returns the driver's warnings, - /// or an empty list when the driver type is unmapped (Galaxy/OpcUaClient) or the inputs are null. + /// or an empty list when the driver type has no inspector (see the class remarks) or the inputs are + /// null. /// The bound driver's DriverType (canonical string, case-insensitive). /// The equipment tag's TagConfig JSON. /// The warnings; empty when the driver is unmapped or clean. @@ -40,9 +54,12 @@ public static class EquipmentTagConfigInspector return Inspectors.TryGetValue(driverType, out var inspect) ? inspect(tagConfig) : Array.Empty(); } - /// Whether the given driver type has a mapped inspector (equipment-tag protocol drivers only). + /// Whether the given driver type has a mapped inspector. /// The bound driver's DriverType. /// when an inspector is registered for the driver type. public static bool IsMapped(string? driverType) => !string.IsNullOrEmpty(driverType) && Inspectors.ContainsKey(driverType); + + /// The driver types this gate can check — the guard test's read of the map. + public static IReadOnlyCollection MappedDriverTypes => (IReadOnlyCollection)Inspectors.Keys; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj index a20477da..89d2622c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ZB.MOM.WW.OtOpcUa.ControlPlane.csproj @@ -45,6 +45,7 @@ + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcPlcProfile.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcPlcProfile.cs index a2559bcd..d9fbe6b3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcPlcProfile.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcPlcProfile.cs @@ -1,3 +1,5 @@ +using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests; @@ -7,6 +9,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests; /// tests address. Node IDs are stable across opc-plc releases — the simulator /// guarantees the same ns=3;s=... names shipped since v1.0. If a release /// bump breaks these, the fixture's pinned image tag needs a coordinated bump. +/// These tests address the driver by RawPath, not by node id (fixed 2026-07-28). v3 +/// made the driver resolve every read/write/subscribe reference through its authored +/// table — a bare ns=3;s=StepUp is no longer a +/// reference the driver accepts, and one that does not resolve is failed LOCALLY as +/// BadNodeIdInvalid with no wire round-trip. The smoke tests still passed bare node ids, so all +/// three had been failing against a perfectly healthy simulator. Seeding RawTags here is what +/// makes them exercise the driver instead of its unresolved-reference guard. /// public static class OpcPlcProfile { @@ -22,11 +31,37 @@ public static class OpcPlcProfile /// opc-plc fast uint node — ticks every 100ms. Used for subscription-cadence tests. public const string FastUInt1 = "ns=3;s=FastUInt1"; + /// The RawPath the smoke tests address by. + public const string StepUpRef = "Sim/OpcUaClient/plc/StepUp"; + + /// The RawPath the smoke tests address by. + public const string RandomSignedInt32Ref = "Sim/OpcUaClient/plc/RandomSignedInt32"; + + /// The RawPath the smoke tests address by. + public const string AlternatingBooleanRef = "Sim/OpcUaClient/plc/AlternatingBoolean"; + + /// The RawPath the smoke tests address by. + public const string FastUInt1Ref = "Sim/OpcUaClient/plc/FastUInt1"; + + /// The authored raw-tag table the driver resolves the *Ref RawPaths through — the + /// same {"nodeId": …} TagConfig shape the deploy artifact delivers. + public static IReadOnlyList RawTags { get; } = + [ + Entry(StepUpRef, StepUp), + Entry(RandomSignedInt32Ref, RandomSignedInt32), + Entry(AlternatingBooleanRef, AlternatingBoolean), + Entry(FastUInt1Ref, FastUInt1), + ]; + + private static RawTagEntry Entry(string rawPath, string nodeId) => + new(rawPath, JsonSerializer.Serialize(new { nodeId }), WriteIdempotent: true); + /// Builds driver options for the OPC PLC endpoint. /// The endpoint URL of the OPC PLC simulator. /// Configured driver options for the OPC PLC. public static OpcUaClientDriverOptions BuildOptions(string endpointUrl) => new() { + RawTags = RawTags, EndpointUrl = endpointUrl, SecurityPolicy = OpcUaSecurityPolicy.None, SecurityMode = OpcUaSecurityMode.None, diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcUaClientSmokeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcUaClientSmokeTests.cs index 601cd2a1..96deeaf1 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcUaClientSmokeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/OpcUaClientSmokeTests.cs @@ -27,7 +27,7 @@ public sealed class OpcUaClientSmokeTests(OpcPlcFixture sim) await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); var snapshots = await drv.ReadAsync( - [OpcPlcProfile.StepUp], TestContext.Current.CancellationToken); + [OpcPlcProfile.StepUpRef], TestContext.Current.CancellationToken); snapshots.Count.ShouldBe(1); snapshots[0].StatusCode.ShouldBe(0u, "opc-plc StepUp read must succeed end-to-end"); @@ -45,7 +45,7 @@ public sealed class OpcUaClientSmokeTests(OpcPlcFixture sim) await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); var snapshots = await drv.ReadAsync( - [OpcPlcProfile.StepUp, OpcPlcProfile.RandomSignedInt32, OpcPlcProfile.AlternatingBoolean], + [OpcPlcProfile.StepUpRef, OpcPlcProfile.RandomSignedInt32Ref, OpcPlcProfile.AlternatingBooleanRef], TestContext.Current.CancellationToken); snapshots.Count.ShouldBe(3); @@ -78,7 +78,7 @@ public sealed class OpcUaClientSmokeTests(OpcPlcFixture sim) }; var handle = await drv.SubscribeAsync( - [OpcPlcProfile.FastUInt1], TimeSpan.FromMilliseconds(250), + [OpcPlcProfile.FastUInt1Ref], TimeSpan.FromMilliseconds(250), TestContext.Current.CancellationToken); // FastUInt1 ticks every 100 ms — one publishing interval (250 ms) should deliver. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs new file mode 100644 index 00000000..d453cc3e --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/EquipmentTagConfigInspectorCoverageTests.cs @@ -0,0 +1,113 @@ +using System.Reflection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; + +namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests; + +/// +/// Guards the deploy-time TagConfig gate against the failure mode that produced deferment-register G-7: +/// a driver ships an Inspect on its *TagDefinitionFactory, nobody adds it to +/// , and the gate is silently blind for that driver at deploy +/// while the AdminUI still validates it at authoring time. MQTT sat in exactly that state. +/// Why reflection rather than a hand-written list. The sibling +/// EquipmentTagConfigInspectorTests enumerates driver-type strings by hand, so it guards +/// renames but can never notice a gap — a newly-inspectable driver simply never appears in +/// it. This is the same hollow-guard shape found in TagConfigDriverTypeNameGuardTests and +/// CsvColumnMapReflectionTests. Deriving the expected set from the assemblies means the test +/// fails the moment a new inspector exists, without anyone remembering to update it. +/// +public sealed class EquipmentTagConfigInspectorCoverageTests +{ + /// + /// Every *TagDefinitionFactory reachable from ControlPlane that exposes a public static + /// Inspect(string) must be wired into the dispatch map. + /// Discovery walks the referenced assemblies rather than AppDomain alone, because a + /// Contracts assembly with no other use is not loaded until first touched — an + /// AppDomain.GetAssemblies() sweep would quietly find nothing and pass vacuously. + /// + [Fact] + public void Every_driver_shipping_an_Inspect_is_wired_into_the_dispatch_map() + { + var inspectable = DiscoverInspectableFactories(); + + // Positive control: the discovery itself must find something, or the assertion below is vacuous — + // an empty expected-set is satisfied by an empty map, which is the exact bug this guards. + inspectable.Count.ShouldBeGreaterThan( + 1, + "the reflection sweep found no (or one) driver factory with an Inspect — discovery is broken, " + + "and the coverage assertion below would pass no matter what the map contained"); + + var mapped = EquipmentTagConfigInspector.MappedDriverTypes; + + var missing = inspectable + .Where(t => !mapped.Any(m => LooksLike(m, t.Name))) + .Select(t => t.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToArray(); + + missing.ShouldBeEmpty( + "these drivers ship an Inspect() but the deploy-time gate does not call it, so their TagConfig " + + "is checked at authoring time and NOT at deploy — config arriving by CSV import or API is " + + "unchecked: " + string.Join(", ", missing)); + } + + /// The map must not name a driver type with no inspector behind it — a dead key would read as + /// coverage while dispatching nowhere. + [Fact] + public void Every_mapped_driver_type_resolves_to_a_real_inspector() + { + foreach (var driverType in EquipmentTagConfigInspector.MappedDriverTypes) + { + EquipmentTagConfigInspector.IsMapped(driverType).ShouldBeTrue(driverType); + // A structurally invalid blob must come back as warnings, not an exception — the gate is on the + // deploy path and must never throw. + Should.NotThrow(() => EquipmentTagConfigInspector.Inspect(driverType, "{not json")); + } + } + + /// + /// Public static IReadOnlyList<string> Inspect(string) is the contract every driver's + /// deploy-time checker implements; anything else is not one of these factories. + /// Discovery scans the output DIRECTORY, not Assembly.GetReferencedAssemblies(). + /// The obvious implementation walks the reference graph — and it is circular, because the C# compiler + /// elides an assembly reference nothing in the IL uses. Drop a driver from the dispatch map and its + /// Contracts assembly drops out of ControlPlane's manifest too, so the sweep stops looking for + /// exactly the driver that just went missing and the test passes. That version was written, and + /// caught only by deleting the MQTT entry and watching it stay green. MSBuild copies every + /// project-referenced assembly to the output regardless of IL usage, so the directory is the honest + /// source. + /// Boundary, stated so it is not mistaken for more: this sees the Contracts assemblies + /// ControlPlane project-references. A driver whose Contracts project is not referenced at all (Sql, + /// MTConnect, Calculation, OpcUaClient) is invisible here — adding the reference is itself the act + /// that makes this guard start demanding a map entry. + /// + private static IReadOnlyList DiscoverInspectableFactories() => + Directory.GetFiles(AppContext.BaseDirectory, "ZB.MOM.WW.OtOpcUa.Driver.*.Contracts.dll") + .Select(TryLoad) + .Where(a => a is not null) + .SelectMany(a => a!.GetTypes()) + .Where(t => t is { IsClass: true, IsPublic: true } + && t.Name.EndsWith("TagDefinitionFactory", StringComparison.Ordinal) + && HasInspect(t)) + .DistinctBy(t => t.FullName) + .ToArray(); + + private static Assembly? TryLoad(string path) + { + try { return Assembly.LoadFrom(path); } + catch (BadImageFormatException) { return null; } + } + + private static bool HasInspect(Type t) + { + var m = t.GetMethod("Inspect", BindingFlags.Public | BindingFlags.Static, [typeof(string)]); + return m is not null && typeof(IReadOnlyList).IsAssignableFrom(m.ReturnType); + } + + /// Matches a mapped driver-type string to a factory type name (ModbusTagDefinitionFactory + /// ↔ Modbus), tolerating the casing differences between the two vocabularies + /// (FOCAS/Focas, TwinCAT/TwinCAT). + private static bool LooksLike(string driverType, string factoryTypeName) => + factoryTypeName.StartsWith(driverType, StringComparison.OrdinalIgnoreCase); +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs index df57354f..aaa96f10 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs @@ -156,17 +156,21 @@ public sealed class AddressSpaceApplierHierarchyTests : IDisposable } /// - /// Full structure-materialisation pipeline against a real SDK node manager: real Config - /// entities (Area / Line / Equipment + an Equipment-namespace Tag) → - /// → MaterialiseHierarchy + MaterialiseEquipmentTags → . Proves - /// an Equipment namespace lands its Area/Line/Equipment folder tree + the equipment-signal - /// Variable in a live OPC UA address space (structure-only; live values are a later milestone). - /// Also covers the compose-side EquipmentTags extraction. The cluster-level deploy + - /// network-browse E2E (Host.IntegrationTests) needs the docker-dev fixture and is tracked - /// as a follow-up. + /// Full v3 structure-materialisation pipeline against a real SDK node manager: real Config entities + /// (RawFolder → Driver → Device → Tag, plus Area / Line / Equipment and a UnsTagReference) → + /// → MaterialiseHierarchy + MaterialiseRawSubtree + + /// MaterialiseUnsReferences → . Proves the dual-namespace + /// shape lands in a live OPC UA address space: the raw device tree AND the UNS folder tree, with the + /// raw tag's variable projected into its referencing equipment as a second variable. + /// Revived from the EquipmentTagsDarkBatch4 skip. It was parked pending "the Batch-4 + /// UnsTagReference fan-out", which shipped as v3.0 — but the equipment-tag path it was written + /// against was RETIRED rather than lit, so unskipping alone would have failed. Rewritten to the + /// shape that actually replaced it. The sibling MaterialiseHierarchy_against_real_SDK still + /// covers folders only; this is the layer's only real-SDK proof that variables materialise in both + /// namespaces. /// - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public async Task Equipment_namespace_structure_materialises_end_to_end_against_real_SDK() + [Fact] + public async Task Raw_and_uns_variables_materialise_end_to_end_against_real_SDK() { await using var host = new OpcUaApplicationHost( new OpcUaApplicationHostOptions @@ -183,34 +187,46 @@ public sealed class AddressSpaceApplierHierarchyTests : IDisposable await host.StartAsync(sdkServer, Ct); sdkServer.NodeManager.ShouldNotBeNull(); - // One area / line / equipment + a Modbus driver. v3: no Namespace entity, DriverInstance no - // longer binds a Namespace, and Equipment no longer binds a driver. The equipment-bound Tag - // (the Speed signal) that this end-to-end test materialised as a Variable node is DARK until - // Batch 4 — the composer emits an empty EquipmentTags set, so it is no longer seeded/passed - // here. Equipment.Name is the UNS browse segment. (Folder-only materialisation is covered live - // by the sibling MaterialiseHierarchy_against_real_SDK test; this suite's equipment-tag - // Variable end-to-end leg re-enables in Batch 4 via the UnsTagReference fan-out.) - var driver = new DriverInstance { DriverInstanceId = "drv-modbus", ClusterId = "c1", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}" }; + // The v3 raw chain: RawFolder "Plant" → Driver "Modbus" → Device "dev1" → Tag "Speed" + // (RawPath Plant/Modbus/dev1/Speed), plus the UNS tree it is projected into. + var folder = new RawFolder { RawFolderId = "rf-plant", ClusterId = "c1", Name = "Plant", ParentRawFolderId = null }; + var driver = new DriverInstance { DriverInstanceId = "drv-modbus", ClusterId = "c1", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-plant" }; + var device = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-modbus", Name = "dev1", DeviceConfig = "{}" }; + var speed = new Tag + { + TagId = "tag-speed", DeviceId = "dev-1", TagGroupId = null, Name = "Speed", DataType = "Float", + AccessLevel = TagAccessLevel.Read, TagConfig = "{}", + }; var area = new UnsArea { UnsAreaId = "nw-area-filling", ClusterId = "c1", Name = "filling" }; var line = new UnsLine { UnsLineId = "nw-line-1", UnsAreaId = "nw-area-filling", Name = "line-1" }; var equipment = new Equipment { EquipmentId = "eq-1", UnsLineId = "nw-line-1", Name = "station-1", MachineCode = "STATION_001" }; + var reference = new UnsTagReference { UnsTagReferenceId = "ref-1", EquipmentId = "eq-1", TagId = "tag-speed", DisplayNameOverride = null }; var composition = AddressSpaceComposer.Compose( new[] { area }, new[] { line }, new[] { equipment }, new[] { driver }, - Array.Empty()); + Array.Empty(), + unsTagReferences: new[] { reference }, tags: new[] { speed }, + rawFolders: new[] { folder }, devices: new[] { device }); - // Compose-side EquipmentTags extraction (Batch-4 subject — empty this batch). - var planned = composition.EquipmentTags.ShouldHaveSingleItem(); - planned.EquipmentId.ShouldBe("eq-1"); - planned.FullName.ShouldBe("40001"); + // Compose side: one raw tag at its RawPath, projected once into the referencing equipment. + var rawTag = composition.RawTags.ShouldHaveSingleItem(); + rawTag.NodeId.ShouldBe("Plant/Modbus/dev1/Speed"); + rawTag.Realm.ShouldBe(AddressSpaceRealm.Raw); + var unsVar = composition.UnsReferenceVariables.ShouldHaveSingleItem(); + unsVar.NodeId.ShouldBe("filling/line-1/station-1/Speed"); + unsVar.BackingRawPath.ShouldBe("Plant/Modbus/dev1/Speed"); var sink = new SdkAddressSpaceSink(sdkServer.NodeManager!); var applier = new AddressSpaceApplier(sink, NullLogger.Instance); applier.MaterialiseHierarchy(composition); - applier.MaterialiseEquipmentTags(composition); + applier.MaterialiseRawSubtree(composition); + applier.MaterialiseUnsReferences(composition); - sdkServer.NodeManager!.FolderCount.ShouldBe(3); // filling area + line-1 + station-1 equipment - sdkServer.NodeManager!.VariableCount.ShouldBe(1); // the Speed signal under the equipment folder + // UNS: filling + line-1 + station-1. Raw: the Plant folder + the Modbus driver + the dev1 device. + sdkServer.NodeManager!.FolderCount.ShouldBe(6); + // ONE source value, TWO nodes — the raw tag's variable and its UNS projection. This is the + // single-source-fan-out shape; a regression that dropped either namespace shows up here as 1. + sdkServer.NodeManager!.VariableCount.ShouldBe(2); } /// OpcUaServer-001 — a UNS Area / Line rename-only deploy refreshes the EXISTING folder's diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerAliasTagTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerAliasTagTests.cs deleted file mode 100644 index 70138b07..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerAliasTagTests.cs +++ /dev/null @@ -1,30 +0,0 @@ -using Xunit; - -namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; - -/// -/// Characterized the live-edit compose seam admitting a Galaxy point as an ordinary equipment tag: -/// an equipment-scoped Tag bound to a GalaxyMxGateway driver surfaced under -/// carrying its driver-side FullName (coalescing a -/// null FolderPath to string.Empty). -/// v3 Batch-1 DARK + reshaped seed: equipment-tag variable plans do not materialize until -/// Batch 4 (the composer emits an empty EquipmentTags set — the raw + UNS variable nodes light -/// up in Batch 4's dual-namespace UNS↔Raw fan-out). The seed mechanism this test used is also gone: -/// Tag is now raw-only (no EquipmentId/DriverInstanceId/FolderPath), the -/// Compose overload no longer takes tags/namespaces, and equipment references raw tags via -/// UnsTagReference rather than a Galaxy point carrying an EquipmentId. So there is no -/// Batch-1 analog to assert. Re-author against the UnsTagReference → EquipmentTags fan-out when Batch 4 -/// lights the equipment-tag variable nodes. -/// -public sealed class AddressSpaceComposerAliasTagTests -{ - /// A GalaxyMxGateway equipment tag surfaces under EquipmentTags with its FullName — - /// dark until Batch 4 (empty equipment-tag plan set) and re-authored via UnsTagReference. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void Compose_admits_galaxy_equipment_tag_in_equipment_tags() - { - // Dark until Batch 4: see class summary. EquipmentTags is emitted empty this batch and the - // Tag.EquipmentId binding this test relied on is retired (Tag is raw-only). The Batch-4 seam - // fans equipment tags in via UnsTagReference; re-author the assertion then. - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerEquipTokenTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerEquipTokenTests.cs deleted file mode 100644 index 8f7c32c9..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerEquipTokenTests.cs +++ /dev/null @@ -1,81 +0,0 @@ -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Configuration.Entities; - -namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; - -/// -/// Verifies the live-edit compose seam () substitutes the -/// reserved {{equip}} token in a shared VirtualTag script with each owning equipment's -/// derived tag base (from its child-tag FullNames) — so one script reused across N -/// identical machines resolves to N machine-specific dependency graphs. -/// -/// v3 Batch-1 DARK: the per-equipment tag base is DERIVED from equipment-tag -/// FullNames, and equipment-tag variable plans do not materialize until Batch 4 (the composer -/// no longer takes tags — baseByEquip is empty this batch). With no base to substitute, the -/// {{equip}} token stays literal and these per-machine assertions have nothing to compare, so -/// the test is Skipped-with-reason. The seed + assertions are preserved verbatim (minus the retired -/// entity columns) so re-enabling in Batch 4 is a one-line change once the fan-out feeds the base. -/// -public sealed class AddressSpaceComposerEquipTokenTests -{ - /// One shared using ctx.GetTag("{{equip}}.Source"), bound - /// to two equipments (TestMachine_001 / _002) each with one equipment Tag whose FullName carries - /// the per-machine base. Compose must expand the token per equipment in both the Expression and - /// the parsed DependencyRefs. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void Compose_substitutes_equip_token_per_equipment() - { - // v3: no Namespace entity; DriverInstance no longer binds a Namespace; Equipment no longer - // binds a driver. In Batch 4 the {{equip}} base derives from the equipment's child-tag - // FullNames — here TestMachine_001.Source / TestMachine_002.Source — which flow through the - // (dark this batch) equipment-tag plan set. Seeded for intent; not passed to Compose. - var driver1 = new DriverInstance - { - DriverInstanceId = "drv-1", - ClusterId = "c1", - Name = "Modbus1", - DriverType = "Modbus", - DriverConfig = "{}", - }; - var driver2 = new DriverInstance - { - DriverInstanceId = "drv-2", - ClusterId = "c1", - Name = "Modbus2", - DriverType = "Modbus", - DriverConfig = "{}", - }; - var area = new UnsArea { UnsAreaId = "area-1", ClusterId = "c1", Name = "filling" }; - var line = new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line-1" }; - var equip1 = new Equipment { EquipmentId = "eq-1", UnsLineId = "line-1", Name = "TestMachine_001", MachineCode = "TESTMACHINE_001" }; - var equip2 = new Equipment { EquipmentId = "eq-2", UnsLineId = "line-1", Name = "TestMachine_002", MachineCode = "TESTMACHINE_002" }; - var script = new Script - { - ScriptId = "s-equip", - Name = "over-50", - SourceCode = "return System.Convert.ToInt32(ctx.GetTag(\"{{equip}}.Source\").Value) > 50;", - SourceHash = "hash-equip", - }; - var vt1 = new VirtualTag { VirtualTagId = "vt-1", EquipmentId = "eq-1", Name = "over50", DataType = "Boolean", ScriptId = "s-equip" }; - var vt2 = new VirtualTag { VirtualTagId = "vt-2", EquipmentId = "eq-2", Name = "over50", DataType = "Boolean", ScriptId = "s-equip" }; - - var result = AddressSpaceComposer.Compose( - new[] { area }, new[] { line }, new[] { equip1, equip2 }, - new[] { driver1, driver2 }, Array.Empty(), - virtualTags: new[] { vt1, vt2 }, - scripts: new[] { script }); - - result.EquipmentVirtualTags.Count.ShouldBe(2); - - var plan1 = result.EquipmentVirtualTags.Single(p => p.VirtualTagId == "vt-1"); - plan1.Expression.ShouldContain("ctx.GetTag(\"TestMachine_001.Source\")"); - plan1.Expression.ShouldNotContain("{{equip}}"); - plan1.DependencyRefs.ShouldBe(new[] { "TestMachine_001.Source" }); - - var plan2 = result.EquipmentVirtualTags.Single(p => p.VirtualTagId == "vt-2"); - plan2.Expression.ShouldContain("ctx.GetTag(\"TestMachine_002.Source\")"); - plan2.Expression.ShouldNotContain("{{equip}}"); - plan2.DependencyRefs.ShouldBe(new[] { "TestMachine_002.Source" }); - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DarkAddressSpaceReasons.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DarkAddressSpaceReasons.cs index 8716ac61..e445bd99 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DarkAddressSpaceReasons.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DarkAddressSpaceReasons.cs @@ -1,27 +1,29 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// -/// Shared xUnit Skip reason constants for OpcUaServer tests whose intent is retained but -/// whose subject is not materialized in the v3 Batch-1 DARK address space. The composer -/// () and the artifact both emit an EMPTY equipment-tag -/// variable plan set this batch — raw + UNS variable nodes are lit up in Batch 4's dual-namespace -/// UNS↔Raw fan-out. Until then any assertion whose subject is an equipment-tag variable node (its -/// FullName, its {{equip}}-token-derived base, its device-host binding) has nothing to -/// compare, so it is Skipped-with-reason rather than deleted. Mirrors -/// Runtime.Tests.Drivers.DarkAddressSpaceReasons. +/// Shared xUnit Skip reason constants for OpcUaServer tests whose subject was RETIRED by v3 +/// rather than merely deferred — architectural tombstones that keep the removed coverage discoverable +/// and name what replaced it, so nobody re-writes a test for a concept that no longer exists. +/// A skip reason is a claim with an expiry date. The one this file used to carry +/// (EquipmentTagsDarkBatch4, "dark until Batch 4") outlived its own condition by a release — +/// see the note below. If you add one here, state the condition precisely enough that a reader can +/// check whether it has already happened. +/// Mirrors Runtime.Tests.Drivers.DarkAddressSpaceReasons. /// internal static class DarkAddressSpaceReasons { - /// - /// Equipment-tag variable materialization + its per-field intent (FullName / array / historize / - /// native-alarm / {{equip}}-token base derived from child-tag FullNames) is dark until Batch 4. - /// - public const string EquipmentTagsDarkBatch4 = - "v3 dark address space: equipment-tag variable plans (FullName / array / historize / alarm / " + - "{{equip}}-token base derived from child-tag FullNames) do not materialize until Batch 4 " + - "(dual-namespace UNS↔Raw fan-out). The composer emits an empty equipment-tag plan set this batch, " + - "so there is no per-equipment tag base to substitute and no equipment-tag variable node to assert. " + - "Migrate + re-enable when Batch 4 lights the raw/UNS variable nodes."; + // EquipmentTagsDarkBatch4 was RETIRED (2026-07-28) once its own unblock condition was examined. It read + // "dark until Batch 4"; Batch 4 shipped as v3.0 and RETIRED the equipment-tag plan set rather than + // lighting it, so every test wearing this reason was waiting on something that had already happened and + // gone the other way. Each was resolved on its merits rather than left to read as pending work: + // - revived onto the raw/UNS shape that replaced the subject (the real-SDK dual-namespace + // materialisation E2E in AddressSpaceApplierHierarchyTests; the cluster-scoping rebuild and the + // four primary-gate write/alarm cases in Runtime.Tests), + // - folded into a live parity test whose corpus was widened to cover them (array + native-alarm + // intent, now in DeploymentArtifactRawUnsParityTests), + // - or deleted, where the subject itself is retired (equipment-namespace EquipmentTags, the + // dot-joint {{equip}}.X token — superseded by the slash-joint form in + // DeploymentArtifactEquipRefParityTests). /// Equipment↔device host binding is architecturally retired in v3. public const string EquipmentDeviceBindingRetired = diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DarkAddressSpaceReasons.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DarkAddressSpaceReasons.cs index 9632ae74..7267fdbe 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DarkAddressSpaceReasons.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DarkAddressSpaceReasons.cs @@ -1,22 +1,28 @@ namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// -/// Shared xUnit Skip reason constants for tests whose intent is retained but whose subject is -/// not materialized in the v3 Batch-1 DARK address space. Equipment-tag variable nodes (and their -/// per-field intent: FullName / array / historize / native-alarm) are lit up by the Batch-4 dual -/// namespace + UNS↔Raw fan-out; until then the composer/artifact both emit an EMPTY equipment-tag -/// plan set, so any parity assertion over those plans has nothing to compare. The raw-tag parse -/// intent these tests characterized lives on in (RawTagEntry -/// round-trip) and TagConfigIntentTests (the parse unit). +/// Shared xUnit Skip reason constants for tests whose subject was RETIRED by v3 rather than +/// merely deferred — architectural tombstones that keep the removed coverage discoverable and name +/// what replaced it, so nobody re-writes a test for a concept that no longer exists. +/// A skip reason is a claim with an expiry date. The one this file used to carry +/// (EquipmentTagsDarkBatch4, "dark until Batch 4") outlived its own condition by a release — +/// see the note below. If you add one here, state the condition precisely enough that a reader can +/// check whether it has already happened. /// internal static class DarkAddressSpaceReasons { - /// Equipment-tag variable materialization + its per-field intent is dark until Batch 4. - public const string EquipmentTagsDarkBatch4 = - "v3 dark address space: equipment-tag variable plans (FullName/array/historize/alarm) do not " + - "materialize until Batch 4 (dual-namespace UNS↔Raw fan-out). The composer + artifact both emit an " + - "empty equipment-tag plan set this batch, so this parity has nothing to compare. Raw-tag parse " + - "intent is covered by TagConfigCorpusParityTests (RawTagEntry round-trip) + TagConfigIntentTests."; + // EquipmentTagsDarkBatch4 was RETIRED (2026-07-28) once its own unblock condition was examined. It read + // "dark until Batch 4"; Batch 4 shipped as v3.0 and RETIRED the equipment-tag plan set rather than + // lighting it, so every test wearing this reason was waiting on something that had already happened and + // gone the other way. Each was resolved on its merits rather than left to read as pending work: + // - revived onto the raw/UNS shape that replaced the subject (the real-SDK dual-namespace + // materialisation E2E in AddressSpaceApplierHierarchyTests; the cluster-scoping rebuild and the + // four primary-gate write/alarm cases in Runtime.Tests), + // - folded into a live parity test whose corpus was widened to cover them (array + native-alarm + // intent, now in DeploymentArtifactRawUnsParityTests), + // - or deleted, where the subject itself is retired (equipment-namespace EquipmentTags, the + // dot-joint {{equip}}.X token — superseded by the slash-joint form in + // DeploymentArtifactEquipRefParityTests). // DiscoveryInjectionDormantV3 was removed with the injection path itself (§8.2, Gitea #507). The 18 // tests it skipped were v2 characterization tests — they asserted an equipment-rooted graft diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactAliasParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactAliasParityTests.cs deleted file mode 100644 index 86446705..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactAliasParityTests.cs +++ /dev/null @@ -1,32 +0,0 @@ -using Xunit; - -namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; - -/// -/// Galaxy-as-ordinary-equipment-tag byte-parity: a Galaxy point (equipment-scoped tag bound to a -/// GalaxyMxGateway driver) surfaced into EquipmentTags with the same FullName / -/// EquipmentId / DriverInstanceId / AccessLevel→Writable / native-alarm intent on both the composer -/// and the artifact-decode seam. -/// -/// v3 DARK (Batch 1): equipment-tag variable plans do not materialize — both producers emit an -/// EMPTY EquipmentTags set until the Batch-4 dual-namespace UNS↔Raw fan-out. Additionally, the -/// v3 schema retired the Namespace/NamespaceKind entities and the Galaxy -/// ExplicitFullName / namespace-Kind gate this test keyed off; Galaxy is an ordinary -/// Device-bound driver whose raw tags reach the UNS via UnsTagReference. The AccessLevel→Writable -/// and native-alarm parse this asserted are covered at the parse unit (TagConfigIntentTests) and -/// the RawTagEntry round-trip (). Unskip + rewrite to the -/// Batch-4 UNS-projection shape when equipment-tag plans light up. -/// -/// -public sealed class DeploymentArtifactAliasParityTests -{ - /// Batch-4 pending: composer and artifact agree on a Galaxy equipment tag surfaced in - /// EquipmentTags (FullName + Writable + native alarm). See class remarks — dark this batch. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void Composer_and_artifact_agree_on_galaxy_equipment_tag() - { - // Restored + rewritten to the Batch-4 UNS-projection shape when equipment-tag variable plans - // materialize: assert a Galaxy point + a Modbus point round-trip identically (FullName, Writable - // from AccessLevel, native-alarm intent incl. historizeToAveva) across composer and artifact. - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactArrayParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactArrayParityTests.cs deleted file mode 100644 index b0f5b6f1..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactArrayParityTests.cs +++ /dev/null @@ -1,28 +0,0 @@ -using Xunit; - -namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; - -/// -/// Phase 4c array intent (isArray + optional arrayLength) byte-parity between the two -/// equipment-tag producers. -/// -/// v3 DARK (Batch 1): equipment-tag variable plans do not materialize — the composer and the -/// artifact both emit an EMPTY EquipmentTags set until the Batch-4 dual-namespace UNS↔Raw -/// fan-out, so there is nothing to compare. The underlying parse of isArray / arrayLength -/// from a raw TagConfig blob is characterized today by TagConfigIntentTests and -/// round-tripped through the artifact by (RawTagEntry -/// byte-preservation). Unskip when Batch 4 lights the equipment-tag plans. -/// -/// -public sealed class DeploymentArtifactArrayParityTests -{ - /// Batch-4 pending: composer and artifact agree on array equipment-tag plans - /// (IsArray + ArrayLength). See class remarks — equipment-tag plans are dark this batch. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void Composer_and_artifact_agree_on_array_equipment_tags() - { - // Restored when Batch 4 materializes equipment-tag variable plans: compose a draft exercising every - // array branch (isArray:true+length, absent, isArray:true no length, non-number length) and assert - // decoded EquipmentTags == composed element-wise. - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactEquipTokenTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactEquipTokenTests.cs deleted file mode 100644 index 2759d519..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactEquipTokenTests.cs +++ /dev/null @@ -1,74 +0,0 @@ -using System.Text.Json; -using Shouldly; -using Xunit; -using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; - -namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; - -/// -/// Verifies the artifact-decode mirror substitutes the reserved {{equip}} token in a -/// VirtualTag script's ctx.GetTag("…") literals with the owning equipment's tag base -/// (derived from its child Equipment-namespace tag's FullName) — byte-parity with -/// AddressSpaceComposer.Compose's live-edit path, using the same shared -/// EquipmentScriptPaths helper and the same equipmentTags-derived base. -/// -public sealed class DeploymentArtifactEquipTokenTests -{ - // v3 dark: the {{equip}} tag base is DERIVED from the owning equipment's child equipment-tag FullNames, - // and equipment-tag plans do not materialize until Batch 4 — so baseByEquip is empty and {{equip}} has - // no per-equipment base to substitute. Unskip when Batch 4 lights the equipment-tag plans. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void ParseComposition_substitutes_equip_token_in_virtual_tag_expression() - { - var blob = JsonSerializer.SerializeToUtf8Bytes(new - { - Namespaces = new[] - { - new { NamespaceId = "ns-eq", Kind = 0 }, // NamespaceKind.Equipment - }, - DriverInstances = new[] - { - new { DriverInstanceId = "drv-modbus", DriverType = "Modbus", DriverConfig = "{}", NamespaceId = "ns-eq" }, - }, - Tags = new object[] - { - new - { - TagId = "tag-source", - DriverInstanceId = "drv-modbus", - EquipmentId = "TestMachine_001", - Name = "Source", - FolderPath = (string?)null, - DataType = "Float", - TagConfig = "{\"FullName\":\"TestMachine_001.Source\"}", - }, - }, - Scripts = new[] - { - new - { - ScriptId = "s-equip", - SourceCode = "return System.Convert.ToInt32(ctx.GetTag(\"{{equip}}.Source\").Value) > 50;", - }, - }, - VirtualTags = new[] - { - new - { - VirtualTagId = "vt-equip", - EquipmentId = "TestMachine_001", - Name = "OverThreshold", - DataType = "Boolean", - ScriptId = "s-equip", - }, - }, - }); - - var c = DeploymentArtifact.ParseComposition(blob); - - var vt = c.EquipmentVirtualTags.ShouldHaveSingleItem(); - vt.Expression.ShouldContain("ctx.GetTag(\"TestMachine_001.Source\")"); - vt.Expression.ShouldNotContain("{{equip}}"); - vt.DependencyRefs.ShouldBe(new[] { "TestMachine_001.Source" }); - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactHistorizeParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactHistorizeParityTests.cs deleted file mode 100644 index 02c11f0e..00000000 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactHistorizeParityTests.cs +++ /dev/null @@ -1,30 +0,0 @@ -using Xunit; - -namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; - -/// -/// Phase C HistoryRead intent (isHistorized + optional historianTagname) byte-parity -/// between the two equipment-tag producers. -/// -/// v3 DARK (Batch 1): equipment-tag variable plans do not materialize — -/// AddressSpaceComposer.Compose and DeploymentArtifact.ParseComposition both emit an -/// EMPTY EquipmentTags set until the Batch-4 dual-namespace UNS↔Raw fan-out. There is nothing -/// to compare, so the parity assertion is parked. The underlying parse of isHistorized / -/// historianTagname from a raw TagConfig blob is characterized today by -/// TagConfigIntentTests (the parse unit) and round-tripped through the artifact by -/// (RawTagEntry byte-preservation). Unskip + restore the -/// composer↔artifact equipment-tag comparison when Batch 4 lights the equipment-tag plans. -/// -/// -public sealed class DeploymentArtifactHistorizeParityTests -{ - /// Batch-4 pending: composer and artifact agree on historized equipment-tag plans - /// (IsHistorized + HistorianTagname). See class remarks — equipment-tag plans are dark this batch. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void Composer_and_artifact_agree_on_historized_equipment_tags() - { - // Restored when Batch 4 materializes equipment-tag variable plans: compose a draft with a - // historized tag (no explicit tagname → HistorianTagname null), a historized tag WITH an explicit - // historianTagname override, and a plain tag; assert decoded EquipmentTags == composed element-wise. - } -} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs index fe2cc5ce..21acd7ff 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactRawUnsParityTests.cs @@ -23,8 +23,15 @@ public sealed class DeploymentArtifactRawUnsParityTests [Fact] public void Raw_and_uns_node_sets_are_byte_parity_between_compose_and_artifact_decode() { - // Entity-side config: folder → driver → device → group → 2 tags (one historized+writable+override, - // one plain read-only), an area/line/equipment, and a UNS reference with a display-name override. + // Entity-side config: folder → driver → device → group → 4 tags, an area/line/equipment, and a UNS + // reference with a display-name override. The four tags cover every per-field intent RawTagPlan + // carries: historize (+ tagname override), plain read-only, ARRAY (isArray + arrayLength), and + // NATIVE ALARM (a TagConfig.alarm object → EquipmentTagAlarmInfo). + // + // The array and alarm tags were folded in here when the three placeholder suites + // (DeploymentArtifact{Array,Historize,Alias}ParityTests) were deleted — they had been parked on + // "unskip when Batch 4 lights the equipment-tag plans", but Batch 4 RETIRED equipment-tag plans + // instead of lighting them, so their subject moved onto the raw tag and belongs in this comparison. var folder = new RawFolder { RawFolderId = "fld-1", ClusterId = "c1", Name = "Cell1", ParentRawFolderId = null }; var driver = new DriverInstance { DriverInstanceId = "drv-1", ClusterId = "c1", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "fld-1" }; var device = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "Dev1", DeviceConfig = "{}" }; @@ -39,6 +46,17 @@ public sealed class DeploymentArtifactRawUnsParityTests TagId = "tag-run", DeviceId = "dev-1", TagGroupId = null, Name = "Run", DataType = "Boolean", AccessLevel = TagAccessLevel.Read, TagConfig = "{}", }; + var profile = new Tag + { + TagId = "tag-profile", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Profile", DataType = "Float", + AccessLevel = TagAccessLevel.Read, TagConfig = "{\"isArray\":true,\"arrayLength\":16}", + }; + var tempHi = new Tag + { + TagId = "tag-temphi", DeviceId = "dev-1", TagGroupId = null, Name = "TempHi", DataType = "Boolean", + AccessLevel = TagAccessLevel.ReadWrite, + TagConfig = "{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}", + }; var area = new UnsArea { UnsAreaId = "a1", ClusterId = "c1", Name = "filling" }; var line = new UnsLine { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" }; var equip = new Equipment { EquipmentId = "EQ-1", UnsLineId = "l1", Name = "station1", MachineCode = "M1" }; @@ -47,7 +65,7 @@ public sealed class DeploymentArtifactRawUnsParityTests var composed = AddressSpaceComposer.Compose( new[] { area }, new[] { line }, new[] { equip }, new[] { driver }, Array.Empty(), - unsTagReferences: new[] { reference }, tags: new[] { speed, run }, + unsTagReferences: new[] { reference }, tags: new[] { speed, run, profile, tempHi }, rawFolders: new[] { folder }, devices: new[] { device }, tagGroups: new[] { group }); // Artifact JSON — the same shape ConfigComposer serializes (enums numeric: ReadWrite = 1, Read = 0). @@ -61,6 +79,8 @@ public sealed class DeploymentArtifactRawUnsParityTests { new { TagId = "tag-speed", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Speed", DataType = "Float", AccessLevel = 1, TagConfig = "{\"isHistorized\":true,\"historianTagname\":\"WW.Speed\"}" }, new { TagId = "tag-run", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "Run", DataType = "Boolean", AccessLevel = 0, TagConfig = "{}" }, + new { TagId = "tag-profile", DeviceId = "dev-1", TagGroupId = "grp-1", Name = "Profile", DataType = "Float", AccessLevel = 0, TagConfig = "{\"isArray\":true,\"arrayLength\":16}" }, + new { TagId = "tag-temphi", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "TempHi", DataType = "Boolean", AccessLevel = 1, TagConfig = "{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}" }, }, UnsAreas = new[] { new { UnsAreaId = "a1", Name = "filling", ClusterId = "c1" } }, UnsLines = new[] { new { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" } }, @@ -70,8 +90,19 @@ public sealed class DeploymentArtifactRawUnsParityTests var decoded = DeploymentArtifact.ParseComposition(blob); - // Sanity: the sets are non-empty (both raw tags + the projected UNS variable materialised). - composed.RawTags.Count.ShouldBe(2); + // Sanity: the sets are non-empty (all four raw tags + the projected UNS variable materialised). + composed.RawTags.Count.ShouldBe(4); + + // Sanity on the two folded-in branches — without these, a composer that dropped array/alarm intent + // ENTIRELY would still pass the parity assertion below, because the artifact decode would drop it too + // and the two empties would compare equal. Parity alone cannot tell "both right" from "both blind". + var composedArray = composed.RawTags.Single(t => t.Name == "Profile"); + composedArray.IsArray.ShouldBeTrue(); + composedArray.ArrayLength.ShouldBe(16u); + var composedAlarm = composed.RawTags.Single(t => t.Name == "TempHi"); + composedAlarm.Alarm.ShouldNotBeNull(); + composedAlarm.Alarm!.AlarmType.ShouldBe("OffNormalAlarm"); + composedAlarm.Writable.ShouldBeTrue(); composed.UnsReferenceVariables.Count.ShouldBe(1); composed.UnsReferenceVariables[0].NodeId.ShouldBe("filling/line1/station1/MotorSpeed"); composed.UnsReferenceVariables[0].BackingRawPath.ShouldBe("Cell1/Modbus/Dev1/Fast/Speed"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs index fe046209..defb41e0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs @@ -190,69 +190,6 @@ public sealed class DeploymentArtifactTests c.ScriptedAlarmPlans.Single().ScriptedAlarmId.ShouldBe("alarm-1"); } - /// - /// Verifies ParseComposition surfaces Equipment-namespace tags (non-null EquipmentId in an - /// Equipment-kind namespace) as EquipmentTags, with FullName extracted - /// from the tag's TagConfig blob. A tag in a non-Equipment (Simulated) namespace with a - /// null EquipmentId must NOT surface in EquipmentTags — byte-parity with the composer's pure - /// ns.Kind == NamespaceKind.Equipment predicate (only Equipment-kind namespaces route - /// into EquipmentTags, so such a tag routes nowhere). - /// - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] - public void ParseComposition_reads_EquipmentTags_from_equipment_namespace() - { - var blob = JsonSerializer.SerializeToUtf8Bytes(new - { - Namespaces = new[] - { - new { NamespaceId = "ns-eq", Kind = 0 }, // NamespaceKind.Equipment - new { NamespaceId = "ns-sim", Kind = 1 }, // NamespaceKind.Simulated (non-Equipment) - }, - DriverInstances = new[] - { - new { DriverInstanceId = "drv-modbus", DriverType = "Modbus", DriverConfig = "{}", NamespaceId = "ns-eq" }, - new { DriverInstanceId = "drv-sim", DriverType = "Modbus", DriverConfig = "{}", NamespaceId = "ns-sim" }, - }, - Tags = new object[] - { - new - { - TagId = "tag-eq", - DriverInstanceId = "drv-modbus", - EquipmentId = "eq-1", - Name = "Speed", - FolderPath = (string?)null, - DataType = "Float", - TagConfig = "{\"FullName\":\"40001\"}", - }, - new - { - TagId = "tag-sim", - DriverInstanceId = "drv-sim", - EquipmentId = (string?)null, - Name = "Temp", - FolderPath = "area", - DataType = "Float", - TagConfig = "{\"FullName\":\"area.Temp\"}", - }, - }, - }); - - var c = DeploymentArtifact.ParseComposition(blob); - - var tag = c.EquipmentTags.ShouldHaveSingleItem(); - tag.TagId.ShouldBe("tag-eq"); - tag.EquipmentId.ShouldBe("eq-1"); - tag.DriverInstanceId.ShouldBe("drv-modbus"); - tag.Name.ShouldBe("Speed"); - tag.DataType.ShouldBe("Float"); - tag.FullName.ShouldBe("40001"); // extracted from TagConfig, not the raw blob - - // The tag in the non-Equipment (Simulated) namespace, with null EquipmentId, does NOT leak - // into EquipmentTags — byte-parity with the composer's pure ns.Kind == Equipment predicate. - c.EquipmentTags.ShouldNotContain(t => t.TagId == "tag-sim"); - } - /// /// Verifies ParseComposition surfaces Equipment-namespace VirtualTags (joined to their Script /// by ScriptId for the expression source) as EquipmentVirtualTags, with the diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorPrimaryGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorPrimaryGateTests.cs index 0b8ce73e..eb0e4e5c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorPrimaryGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorPrimaryGateTests.cs @@ -39,6 +39,17 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase private static readonly DateTime Ts = new(2026, 7, 13, 10, 0, 0, DateTimeKind.Utc); private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(5); + /// v3 RawPath of the seeded write tag: RawFolder "Plant" / driver Name "Modbus" / Device "dev1" / + /// Tag "speed". Its UNS projection (Area "filling" / Line "line1" / Equipment "station1") is + /// . + private const string WriteRawPath = "Plant/Modbus/dev1/speed"; + private const string WriteUnsNodeId = "filling/line1/station1/speed"; + + /// v3 RawPath of the seeded alarm tag. In v3 a native alarm materializes ONCE at the raw tag with + /// ConditionId == RawPath, so this is both the wire-ref the driver raises and the condition + /// NodeId the host routes to. + private const string AlarmRawPath = "Plant/Modbus/dev1/temp_hi"; + // ---------------- write gate ---------------- /// Role unknown + a real driver peer (count 2) ⇒ RouteNodeWrite is DENIED with the @@ -52,7 +63,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase var actor = SpawnWriteHost(db, dep, recorder, driverMemberCount: 2); var asker = CreateTestProbe(); - actor.Tell(new DriverHostActor.RouteNodeWrite("eq-1/speed", 123.0, AddressSpaceRealm.Uns), asker.Ref); + actor.Tell(new DriverHostActor.RouteNodeWrite(WriteUnsNodeId, 123.0, AddressSpaceRealm.Uns), asker.Ref); var result = asker.ExpectMsg(Timeout); result.Success.ShouldBeFalse(); @@ -62,7 +73,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase /// Role unknown + no driver peer (count 1) ⇒ the write is SERVICED (single-node / boot-window /// posture preserved). - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] + [Fact] public void Unknown_role_single_driver_services_write() { var db = NewInMemoryDbFactory(); @@ -76,7 +87,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase AwaitAssert(() => { var asker = CreateTestProbe(); - actor.Tell(new DriverHostActor.RouteNodeWrite("eq-1/speed", 123.0, AddressSpaceRealm.Uns), asker.Ref); + actor.Tell(new DriverHostActor.RouteNodeWrite(WriteUnsNodeId, 123.0, AddressSpaceRealm.Uns), asker.Ref); var res = asker.ExpectMsg(Timeout); res.Reason.ShouldNotBe("not primary"); res.Reason.ShouldNotBe("not primary (role unknown)"); @@ -87,7 +98,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase /// A Primary snapshot services even at count 2; a Secondary snapshot denies with the steady-state /// reason (distinct from the boot-window reason). - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] + [Fact] public void Known_role_wins_over_member_count() { var db = NewInMemoryDbFactory(); @@ -101,7 +112,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase AwaitAssert(() => { var asker1 = CreateTestProbe(); - actor.Tell(new DriverHostActor.RouteNodeWrite("eq-1/speed", 1.0, AddressSpaceRealm.Uns), asker1.Ref); + actor.Tell(new DriverHostActor.RouteNodeWrite(WriteUnsNodeId, 1.0, AddressSpaceRealm.Uns), asker1.Ref); var res = asker1.ExpectMsg(Timeout); res.Reason.ShouldNotBe("not primary"); res.Success.ShouldBeTrue(res.Reason); @@ -110,7 +121,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase // Secondary snapshot ⇒ denied with the steady-state reason. TellRole(actor, RedundancyRole.Secondary); var asker2 = CreateTestProbe(); - actor.Tell(new DriverHostActor.RouteNodeWrite("eq-1/speed", 2.0, AddressSpaceRealm.Uns), asker2.Ref); + actor.Tell(new DriverHostActor.RouteNodeWrite(WriteUnsNodeId, 2.0, AddressSpaceRealm.Uns), asker2.Ref); var denied = asker2.ExpectMsg(Timeout); denied.Success.ShouldBeFalse(); denied.Reason.ShouldBe("not primary"); @@ -127,7 +138,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase var actor = SpawnWriteHost(db, dep, factory, driverMemberCount: 2); var asker = CreateTestProbe(); - actor.Tell(new DriverHostActor.RouteNodeWrite("eq-1/speed", 9.0, AddressSpaceRealm.Uns), asker.Ref); + actor.Tell(new DriverHostActor.RouteNodeWrite(WriteUnsNodeId, 9.0, AddressSpaceRealm.Uns), asker.Ref); asker.ExpectMsg(Timeout).Success.ShouldBeFalse(); AwaitAssert(() => @@ -151,7 +162,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase var (actor, _) = SpawnAlarmHost(db, dep, driverMemberCount: 2); EventFilter.Warning(contains: "role unknown").ExpectOne(() => - actor.Tell(new DriverHostActor.RouteNativeAlarmAck("eq-1/temp_hi", Comment: null, OperatorUser: "op"))); + actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, Comment: null, OperatorUser: "op"))); AwaitAssert(() => { @@ -163,7 +174,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase /// Role unknown + count 2 ⇒ ForwardNativeAlarm still writes the (ungated) OPC UA condition update /// but publishes NO cluster-wide alerts transition. - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] + [Fact] public void Unknown_role_multi_driver_suppresses_alerts_emit_but_updates_condition() { var db = NewInMemoryDbFactory(); @@ -175,14 +186,14 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase actor.Tell(RaiseAlarm()); // The ungated OPC UA condition write still arrives. - publish.ExpectMsg(Timeout).AlarmNodeId.ShouldBe("eq-1/temp_hi"); + publish.ExpectMsg(Timeout).AlarmNodeId.ShouldBe(AlarmRawPath); // The cluster-wide alerts publish is suppressed (a real Primary peer publishes the single copy). alerts.ExpectNoMsg(TimeSpan.FromMilliseconds(500)); } /// Role unknown + count 1 ⇒ ForwardNativeAlarm publishes the alerts transition (single-node /// posture preserved). - [Fact(Skip = DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] + [Fact] public void Unknown_role_single_driver_publishes_alerts_emit() { var db = NewInMemoryDbFactory(); @@ -194,7 +205,7 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase actor.Tell(RaiseAlarm()); publish.ExpectMsg(Timeout); - alerts.ExpectMsg(Timeout).AlarmId.ShouldBe("eq-1/temp_hi"); + alerts.ExpectMsg(Timeout).AlarmId.ShouldBe(AlarmRawPath); } // ---------------- helpers ---------------- @@ -202,8 +213,8 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase private static DriverInstanceActor.AttributeAlarmPublished RaiseAlarm() => new("drv-1", new AlarmEventArgs( new StubAlarmHandle(), - SourceNodeId: "Temp", - ConditionId: "Temp.HiHi", + SourceNodeId: "Temp", // bare owning object — deliberately NOT the lookup key + ConditionId: AlarmRawPath, // v3: the condition is keyed by the raw tag's RawPath AlarmType: "OffNormalAlarm", Message: "temperature high", Severity: AlarmSeverity.High, @@ -266,60 +277,66 @@ public sealed class DriverHostActorPrimaryGateTests : RuntimeActorTestBase probe.ExpectMsg(Timeout); } + /// + /// Seeds the v3 raw-tag chain a write must traverse: RawFolder "Plant" → DriverInstance "drv-1" + /// (Modbus, Enabled so a real child spawns and can service the write) → Device "dev1" → Tag + /// "speed" (, ReadWrite), projected into equipment + /// filling/line1/station1 by a UnsTagReference so resolves. + /// The pre-v3 shape this replaced seeded Namespaces + equipment-bound Tags, which + /// v3 parses to an EMPTY tag set — the write had nothing to route to, which is why the + /// write-is-serviced cases were skipped rather than failing. + /// private static DeploymentId SeedWriteTagDeployment(IDbContextFactory db) { var artifact = JsonSerializer.SerializeToUtf8Bytes(new { - Namespaces = new[] { new { NamespaceId = "ns-eq", Kind = 0 } }, + RawFolders = new[] { new { RawFolderId = "rf-plant", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } }, DriverInstances = new[] { - new - { - DriverInstanceRowId = Guid.NewGuid(), - DriverInstanceId = "drv-1", - Name = "drv-1", - DriverType = "Modbus", - Enabled = true, - DriverConfig = "{}", - NamespaceId = "ns-eq", - }, + new { DriverInstanceId = "drv-1", RawFolderId = "rf-plant", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", ClusterId = "c1", Enabled = true }, }, + Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } }, + TagGroups = Array.Empty(), Tags = new[] { - new - { - TagId = "tag-0", - EquipmentId = "eq-1", - DriverInstanceId = "drv-1", - Name = "speed", - FolderPath = (string?)null, - DataType = "Double", - TagConfig = JsonSerializer.Serialize(new { FullName = "40001" }), - }, + new { TagId = "t-speed", DeviceId = "dev-1", TagGroupId = (string?)null, Name = "speed", DataType = "Double", AccessLevel = 1, TagConfig = "{}" }, }, + UnsAreas = new[] { new { UnsAreaId = "a1", Name = "filling", ClusterId = "c1" } }, + UnsLines = new[] { new { UnsLineId = "l1", UnsAreaId = "a1", Name = "line1" } }, + Equipment = new[] { new { EquipmentId = "EQ-1", UnsLineId = "l1", Name = "station1", MachineCode = "M1" } }, + UnsTagReferences = new[] { new { UnsTagReferenceId = "r1", EquipmentId = "EQ-1", TagId = "t-speed", DisplayNameOverride = (string?)null } }, }); return SealArtifact(db, artifact); } + /// + /// Seeds the same v3 chain with the tag's TagConfig carrying an alarm object, so the + /// composer makes it a Part 9 condition at rather than a value variable. + /// The driver is disabled — these cases exercise the ack/emit gates, not a live child. + /// private static DeploymentId SeedAlarmTagDeployment(IDbContextFactory db) { var artifact = JsonSerializer.SerializeToUtf8Bytes(new { - Namespaces = new[] { new { NamespaceId = "ns-eq", Kind = 0 } }, - DriverInstances = new[] { new { DriverInstanceId = "drv-1", NamespaceId = "ns-eq" } }, + RawFolders = new[] { new { RawFolderId = "rf-plant", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } }, + DriverInstances = new[] + { + new { DriverInstanceId = "drv-1", RawFolderId = "rf-plant", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", ClusterId = "c1", Enabled = false }, + }, + Devices = new[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } }, + TagGroups = Array.Empty(), Tags = new[] { new { - TagId = "tag-0", - EquipmentId = "eq-1", - DriverInstanceId = "drv-1", + TagId = "t-temp", + DeviceId = "dev-1", + TagGroupId = (string?)null, Name = "temp_hi", - FolderPath = (string?)null, DataType = "Boolean", + AccessLevel = 0, TagConfig = JsonSerializer.Serialize(new { - FullName = "Temp.HiHi", alarm = new { alarmType = "OffNormalAlarm", severity = 700 }, }), }, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index 8a8c364f..5895c429 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -202,16 +202,18 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase } /// - /// Wiring proof for per-ClusterId scoping (Task 4): a multi-cluster artifact must - /// materialise ONLY the local node's cluster slice. Mirrors the multi-cluster artifact - /// shape exercised in DeploymentArtifactTests (MAIN + SITE-A, one Galaxy driver + - /// one equipment tag each — Galaxy points are ordinary equipment tags now). The scoped - /// rebuild for the SITE-A node must surface the SITE-A tag (t-sa → folder-scoped - /// variable eq-sa/F/S1) and NOT MAIN's (t-main → eq-main/F/M1); the - /// mirror holds for the MAIN node. Without the production scoping edit, the unscoped parse - /// would materialise BOTH variables on every node. + /// Wiring proof for per-ClusterId scoping: a multi-cluster artifact must materialise ONLY the local + /// node's cluster slice. MAIN and SITE-A each own a raw folder → Modbus driver → device → tag; the + /// scoped rebuild on the SITE-A node must surface SITE-A's raw variable + /// (SA/Modbus/dev-sa/S1) and NOT MAIN's (Main/Modbus/dev-main/M1), and the mirror + /// must hold on the MAIN node. Without DeploymentArtifact.ResolveClusterScope, the unscoped + /// parse materialises BOTH on every node. + /// Revived from the EquipmentTagsDarkBatch4 skip. It was parked on equipment-tag plans + /// lighting up "at Batch 4"; Batch 4 retired them instead, so the subject moved to the raw subtree. + /// The behaviour under test — cluster scoping — never went dark, and matters more since the mesh + /// split (a site node must not materialise another cluster's tags). /// - [Fact(Skip = ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers.DarkAddressSpaceReasons.EquipmentTagsDarkBatch4)] + [Fact] public void Rebuild_materialises_only_the_nodes_cluster() { // --- SITE-A node: only the SITE-A tag's variable, never MAIN's. --- @@ -228,12 +230,11 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase siteActor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - // PureAdd (equipment + tag) ⇒ no full rebuild; the materialise passes still run the cluster slice. - // t-sa (EquipmentId "eq-sa", FolderPath "F", Name "S1") → folder-scoped variable "eq-sa/F/S1". - AwaitAssert(() => sinkA.Calls.ShouldContain("EV:eq-sa/F/S1"), duration: PresenceBudget); + // PureAdd ⇒ no full rebuild; the materialise passes still run the cluster slice. + AwaitAssert(() => sinkA.Calls.ShouldContain("EV:SA/Modbus/dev-sa/S1"), duration: PresenceBudget); sinkA.RebuildCalls.ShouldBe(0); - // t-main (MAIN cluster) must NOT leak onto the SITE-A node. - sinkA.Calls.ShouldNotContain("EV:eq-main/F/M1"); + // MAIN's raw tag must NOT leak onto the SITE-A node. + sinkA.Calls.ShouldNotContain("EV:Main/Modbus/dev-main/M1"); // --- MAIN node: the mirror — only MAIN's tag's variable, never SITE-A's. --- var dbM = NewInMemoryDbFactory(); @@ -249,16 +250,16 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase mainActor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId())); - AwaitAssert(() => sinkM.Calls.ShouldContain("EV:eq-main/F/M1"), duration: PresenceBudget); + AwaitAssert(() => sinkM.Calls.ShouldContain("EV:Main/Modbus/dev-main/M1"), duration: PresenceBudget); sinkM.RebuildCalls.ShouldBe(0); - sinkM.Calls.ShouldNotContain("EV:eq-sa/F/S1"); + sinkM.Calls.ShouldNotContain("EV:SA/Modbus/dev-sa/S1"); } /// - /// Seal a 2-cluster deployment (MAIN + SITE-A) whose artifact mirrors the multi-cluster - /// shape the composer emits: a Clusters + Nodes map, one Equipment namespace + - /// Galaxy driver + equipment tag per cluster (Galaxy points are ordinary equipment tags now). - /// Used by . + /// Seal a 2-cluster deployment (MAIN + SITE-A) whose artifact mirrors the v3 multi-cluster shape: + /// a Clusters + Nodes map, and per cluster a raw folder → Modbus driver → device → + /// tag chain plus its UNS area/line/equipment. Used by + /// . /// private static void SeedMultiClusterDeployment(IDbContextFactory dbFactory) { @@ -282,24 +283,31 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase }, Equipment = new[] { - new { EquipmentId = "eq-main", DriverInstanceId = "main-galaxy", UnsLineId = "line-main", Name = "eq-main", MachineCode = "EQ-MAIN" }, - new { EquipmentId = "eq-sa", DriverInstanceId = "sa-galaxy", UnsLineId = "line-sa", Name = "eq-sa", MachineCode = "EQ-SA" }, + new { EquipmentId = "eq-main", UnsLineId = "line-main", Name = "eq-main", MachineCode = "EQ-MAIN" }, + new { EquipmentId = "eq-sa", UnsLineId = "line-sa", Name = "eq-sa", MachineCode = "EQ-SA" }, + }, + RawFolders = new[] + { + new { RawFolderId = "rf-main", ParentRawFolderId = (string?)null, Name = "Main", ClusterId = "MAIN" }, + new { RawFolderId = "rf-sa", ParentRawFolderId = (string?)null, Name = "SA", ClusterId = "SITE-A" }, }, DriverInstances = new[] { - new { DriverInstanceId = "main-galaxy", DriverType = "GalaxyMxGateway", DriverConfig = "{}", ClusterId = "MAIN", NamespaceId = "main-ns" }, - new { DriverInstanceId = "sa-galaxy", DriverType = "GalaxyMxGateway", DriverConfig = "{}", ClusterId = "SITE-A", NamespaceId = "sa-ns" }, + new { DriverInstanceId = "main-modbus", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-main", ClusterId = "MAIN" }, + new { DriverInstanceId = "sa-modbus", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-sa", ClusterId = "SITE-A" }, }, - Namespaces = new[] + Devices = new[] { - new { NamespaceId = "main-ns", ClusterId = "MAIN", Kind = 0 }, // NamespaceKind.Equipment - new { NamespaceId = "sa-ns", ClusterId = "SITE-A", Kind = 0 }, + new { DeviceId = "dev-main", DriverInstanceId = "main-modbus", Name = "dev-main", DeviceConfig = "{}" }, + new { DeviceId = "dev-sa", DriverInstanceId = "sa-modbus", Name = "dev-sa", DeviceConfig = "{}" }, }, + TagGroups = Array.Empty(), Tags = new[] { - new { TagId = "t-main", DriverInstanceId = "main-galaxy", EquipmentId = (string?)"eq-main", Name = "M1", FolderPath = "F", DataType = "Boolean", TagConfig = "{}" }, - new { TagId = "t-sa", DriverInstanceId = "sa-galaxy", EquipmentId = (string?)"eq-sa", Name = "S1", FolderPath = "F", DataType = "Boolean", TagConfig = "{}" }, + new { TagId = "t-main", DeviceId = "dev-main", TagGroupId = (string?)null, Name = "M1", DataType = "Boolean", AccessLevel = 0, TagConfig = "{}" }, + new { TagId = "t-sa", DeviceId = "dev-sa", TagGroupId = (string?)null, Name = "S1", DataType = "Boolean", AccessLevel = 0, TagConfig = "{}" }, }, + UnsTagReferences = Array.Empty(), ScriptedAlarms = Array.Empty(), }); From f3cd685da534c62c14a7e957f652ce8bf4b34c74 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 28 Jul 2026 14:40:51 -0400 Subject: [PATCH 3/3] docs(drivers): write the two missing driver guides (Sql, Calculation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the §7.2 priority-3 item. Sql was a shipped, merged driver with NO user-facing documentation at all -- only design and plan files, which describe what was intended rather than what an operator needs to author one. Sql.md covers: SQL Server as the only constructed provider (the other four SqlProvider names are reserved, and authoring one fails rather than falling back), the KeyValue / WideRow tag models with worked TagConfig, Query being rejected end-to-end, the config table with real defaults read from SqlDriverOptions, connectionStringRef never carrying credentials into an artifact, per-source query grouping and its case-sensitive group keys, and the value-bound-vs-identifier-validated split that makes it injection-safe. Calculation.md covers: what a pseudo-driver is and why it occupies a driver slot, a Calculation-vs-VirtualTags decision table (the two are easy to confuse), the literal-only ctx.GetTag dependency extraction and the runtime-built-path trap, the two deploy gates, the all-deps-arrived publish gate, and the Good->Bad-once error semantics. Two stale claims found by checking against source rather than inheriting them: - The driver README's Calculation row still carried G-1 ("no typed config form, RunTimeout unauthorable"), fixed on 2026-07-27 by CalculationDriverForm. - docs/Raw.md's "one auto-created default Engine device" NEVER SHIPPED. I had copied it into the new doc before checking; "Engine" has zero occurrences in src/ and nothing special-cases Calculation at device creation. Corrected in both files. The config key was also wrong in my first draft: runTimeoutMs, an int in milliseconds, not a TimeSpan string -- and it is ignored on reinit because the evaluator's timeout is constructor-fixed. Full solution: 49 suites pass; the 2 failures are the known pre-existing environment ones (AbLegacy.IntegrationTests docker fixture, Host.IntegrationTests). OpcUaClient.IntegrationTests, which was silently failing, is now green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo --- deferment.md | 941 ------------------------------------ docs/Raw.md | 4 +- docs/drivers/Calculation.md | 133 +++++ docs/drivers/README.md | 4 +- docs/drivers/Sql.md | 174 +++++++ 5 files changed, 312 insertions(+), 944 deletions(-) create mode 100644 docs/drivers/Calculation.md create mode 100644 docs/drivers/Sql.md diff --git a/deferment.md b/deferment.md index b703e827..e69de29b 100644 --- a/deferment.md +++ b/deferment.md @@ -1,941 +0,0 @@ -# Deferment register — new drivers, open issues, deferred work - -> **Scope.** A source-verified inventory of what is unfinished in this repo: the status of the -> recently-added drivers, every open tracker issue, in-code deferrals, open live gates, and the -> documentation that misstates any of it. -> -> **Method.** Five parallel agents swept the tree independently; every claim below was checked -> against **source** (`src/`, `tests/`, `git log`), not against documentation. Where a doc and the -> code disagreed, the code won and the doc is listed in §7. Findings that could not be verified -> without hardware or a live rig are marked **CANNOT VERIFY** rather than asserted. -> -> **Baseline.** The audit below was taken against `master` @ `90bdaa44`, 2026-07-27, and **§1–§8 are -> preserved as that audit** — they are the record of what was found, not a description of the tree today. -> **§9 is the live status.** -> -> **Current tree:** `master` @ `f0bd73bc` (remediation merge `914d70bf`), 2026-07-28. Items closed since -> the audit are struck through or marked ✅ in place, so a reader can still see what was found and what -> became of it. - ---- - -## 0. The short version - -*(As audited at `90bdaa44`. See §0.1 for what is left.)* - -The driver runtime is in good shape — **no new driver contains a stub, a `NotImplementedException`, -or an unregistered factory.** What is unfinished clusters into four groups, in descending order of -how much they should worry you: - -| # | Theme | Why it matters | Now | -|---|---|---|---| -| **1** | **Three subsystems are fully authored, persisted, and shipped — and never executed.** Node ACLs, `IRediscoverable`/`IHostConnectivityProbe`, and `DriverTypeRegistry`. | An operator can author a rule, save it, deploy it green, and have it do **nothing**. §3 | ◐ `IRediscoverable` now consumed; the other two are **documented as inert** and tracked (#520, #521, #522) rather than fixed | -| **2** | **AdminUI authoring-dispatch gaps.** Calculation is offered in the driver picker but has no config form; three drivers have no device form; CSV + browse-commit dispatch maps miss the new drivers. | The "registered but unauthorable" class that has now bitten twice. §1.2 | ✅ **Closed** — G-1…G-6 fixed and guarded; G-7 remains | -| **3** | **17 open issues, all confirmed still open**, several broader than filed. | §2 | ◐ 3 resolved by the merge; 4 new filed → **21 open on the tracker**, of which 3 are stale-open | -| **4** | **Bookkeeping drift.** 8 `.tasks.json` files show ~140 "pending" tasks for work that shipped; 5 CLAUDE.md status claims are stale. | Someone follows the tracker and rebuilds a shipped driver. §6, §7 | ✅ **Closed** — 13 plan files + 11 archreview gates written back | - -Nothing here is a data-loss or outage defect. The sharpest edge is §3 — silent non-enforcement. - -### 0.1 What is actually left (as at `f0bd73bc`, 2026-07-28) - -The remediation (§9) closed the two mechanical themes outright and converted the two judgement-call -themes into tracked decisions. What remains: - -| Item | Nature | Where | -|---|---|---| -| **#520** node ACLs authored + deployed but **never enforced** | Decision: wire it or retire the surface. Non-enforcement is now stated in docs and warned in the UI, so nobody can author a deny rule believing it works — but reads are still ungated. | §3.1 | -| **#521** `IHostConnectivityProbe` / `DriverHostStatus` dead surface | Decision: build the publisher or delete it. The table was deliberately re-created in the v3 initial migration, so deleting on inference would be wrong. | §3.2 | -| **#522** driver stability tiers inert | Decision: pass real tiers or delete the machinery. Docs now say every driver runs Tier A. | §3.3 | -| ~~**#523** remove the periodic 5-min device re-browse~~ | ✅ **Done** — removed. **AbCip and FOCAS now have no tag-change signal at all**; that is the accepted consequence, not an oversight. | §9 (2026-07-28, #523) | -| **G-7** `EquipmentTagConfigInspector` coverage | ◐ **Narrowed + guarded** (2026-07-28): MQTT wired (it shipped an `Inspect` nobody called), and a reflection guard now fails the build if a reachable driver ships one and is not mapped. **Residual:** Sql / MTConnect / Calculation / OpcUaClient have no Contracts-side `Inspect` at all, so the deploy gate stays blind for them while the AdminUI validates them — see §1.2. | §1.2 | -| ~~**13 `EquipmentTagsDarkBatch4` skipped tests**~~ | ✅ **Done** — 6 revived onto the raw/UNS shape, 3 folded into a widened live parity corpus, 4 deleted as retired. The reason constant is gone. **Skips now: `Runtime.Tests` 13 → 3, `OpcUaServer.Tests` 4 → 1** — all remaining are deliberate `EquipmentDeviceBindingRetired` tombstones. | §9 (2026-07-28, skips) | -| 17 other tracker issues | Driver features + deferred work, unchanged by this remediation. | §2 | -| Open live gates | #491 continuous historization, #468 browser tree, archreview R2-03 / R2-10. *(The un-gated drift detector's gate closed by deletion — see #523.)* | §5 | - -**Three issues are resolved but still open on the tracker** — **#516** (fixed for all 12 drivers), -**#518** (the `IRediscoverable` half fixed; the probe half split to #521) and **#507** (injection deleted, -superseded by `/raw` browse-commit). Closing them is a tracker action nobody has taken yet. - ---- - -## 1. New drivers — source-verified status - -Cross-cutting facts, verified once: all 12 driver types register in one place -(`src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs:147-164`); all 12 register a -probe (`:119-130`) via `AddOtOpcUaDriverProbes()`, called from **both** the driver path (`:93`) and -the admin path — **no probe is admin-node-missing**. `DriverTypeNames` -(`src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverTypeNames.cs`) is guarded bidirectionally by -reflection in `tests/Core/…Core.Abstractions.Tests/DriverTypeNamesGuardTests.cs`. - -### 1.1 Status table - -| Driver | Verdict | Merged | Capabilities | Notes | -|---|---|---|---|---| -| **MTConnect** | Complete-with-gaps | `90bdaa44` (#506) | `IDriver, IReadable, ISubscribable, ITagDiscovery, IHostConnectivityProbe, IRediscoverable` (`MTConnectDriver.cs:63`) | No `IWritable` **by design** (`:50`). No device form. | -| **MQTT / Sparkplug B** | **Complete** | `c3a2b0f7` | `+ IRediscoverable, IAsyncDisposable` (`MqttDriver.cs:65-66`) | Cleanest of the five: picker + driver form + device form + tag editor all present. 8 open follow-up issues. | -| **Sql** | Complete-with-gaps | `4ad54037`, follow-ups `28c28667` | `IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe` (`SqlDriver.cs:28-29`) | No device form; no browse-commit branch; stale comments. | -| **Modbus RTU-over-TCP** | **Complete** | `0f38f486` (#495) | transport mode inside Modbus (`ModbusTransportMode.cs`, `ModbusRtuOverTcpTransport.cs`) | Parses transport **by name** (`ModbusDriverFactoryExtensions.cs:77-78`) — dodges the systemic numeric-enum authoring bug. | -| **Calculation** | **Partial (authoring gap)** | pre-existing | `IDriver, IDependencyConsumer, ISubscribable, IReadable` (`CalculationDriver.cs:38`) | **Registered + offered in the picker, but has no config form.** See §1.2. | - -`IWritable` is absent from MTConnect, MQTT, Sql and Calculation. In all four this is **structural and -documented**, not a stub (`MTConnectDriver.cs:50`, `MqttDriver.cs:483`, `SqlDriver.cs:15`). - -**Zero `NotImplementedException`** across all five drivers. Every `NotSupportedException` is either a -`catch` filter or a deliberate domain error. - -### 1.2 Gaps found — AdminUI authoring & dispatch layer - -Every gap below is in the **authoring/dispatch layer, not driver runtime code**. - -> ✅ **G-1 … G-6 are all closed** (`6a01358b`, live-verified on docker-dev). The two Razor `@switch` -> dispatch blocks became testable maps (`DriverConfigFormMap`, `DeviceFormMap`) and are now guarded in both -> directions, so this class cannot recur silently. **G-7 is the only one left.** See §9. - -| # | Gap | Evidence | Impact | Now | -|---|---|---|---|---| -| G-1 | **`DriverConfigModal.razor` has no `Calculation` case** — falls to the `default` "No typed config form" warning; no `CalculationDriverForm.razor` exists | `DriverConfigModal.razor:34-72`, default arm `:69-71` | `CalculationDriverOptions.RunTimeout` (`CalculationDriverOptions.cs:19`) is **unauthorable via the UI**. Create/rename still work, so degraded — not a hard block. **This is the same class as the Sql picker defect, one modal further downstream.** | ✅ Fixed — `CalculationDriverForm.razor` | -| G-2 | **`DeviceModal.razor` is missing Sql, MTConnect, Calculation** | `DeviceModal.razor:42-74`, default arm `:71-73` | Mitigated: Sql/MTConnect hold connection at the **driver** level, and Test Connect still works via `BuildMergedProbeConfig` (`:180-184`). | ✅ Fixed — declared **single-connection** rather than given hollow device forms | -| G-3 | **`DriverConfigModal.razor:77` tells Sql/MTConnect operators the wrong thing** — the single-connection branch is hardcoded to `Galaxy or Mqtt`, so Sql/MTConnect users are told the endpoint lives on the *device* when they just authored it on the driver | `DriverConfigModal.razor:77`, `:86-88` | Actively misleading, compounds G-2. | ✅ Fixed — `DeviceFormMap.IsSingleConnection` | -| G-4 | **No parity test guards `DriverConfigModal` or `DeviceModal`** — `RawDriverTypeDialogCoverageTests`/`…ParityTests` assert `DriverTypeNames` ↔ **picker** parity only | — | **This is exactly why G-1 and G-2 survived review.** The guard pattern exists and simply was not extended to the two downstream dispatch maps. | ✅ Fixed — `DriverFormMapParityTests` + `DriverDispatchMapParityTests` | -| G-5 | **`CsvColumnMap` has no typed columns for Sql, MQTT, MTConnect** | `CsvColumnMap.cs:340-452` | CSV import/export degrades to the raw `TagConfigJson` fallback while the 7 older drivers get typed columns. | ✅ Fixed — CSV maps for Sql/Mqtt/MTConnect | -| G-6 | **`RawBrowseCommitMapper` has no Sql branch** — falls to the generic `WriteSingleKey("address", …)` whose comment claims "Browsable drivers are all handled above" | `RawBrowseCommitMapper.cs:121-141`, fallback `:145` | Untrue since `SqlDriverBrowser` was registered (`EndpointRouteBuilderExtensions.cs:83`). Same failure mode the MTConnect branch (`:135-139`) was added to avoid: typed editor opens empty, blanks the field on save. | ✅ Fixed — **and at the browser end too**, which the audit missed | -| G-7 | **`EquipmentTagConfigInspector` covers only 6 driver types** | `:24-29` | Pre-existing (OpcUaClient + Galaxy absent too); no new driver was added. **The audit understated it:** the AdminUI's `TagConfigValidator` covers **11 of 12**, so the *deploy* gate is weaker than the *authoring* gate — and config arriving by CSV import or API only meets the deploy gate. | ◐ **Narrowed + guarded** — MQTT wired, reflection guard added; 4 drivers still have no Contracts-side `Inspect`. See §9 | - -### 1.3 Stale in-code comments (harmless at runtime, misleading to the next reader) - -> ✅ **All cleared** (`6a01358b`, `d32d89c3`). Listed as found. - -- `DriverTypeNames.cs:22-25` — Calculation described as "not-yet-registered"; it registers at `DriverFactoryBootstrap.cs:149`. -- `TagConfigEditorMap.cs:25-28` + `TagConfigValidator.cs:27-28` — "`DriverTypeNames.Sql` deliberately absent until Task 11"; the constant exists (`DriverTypeNames.cs:57`) and the values are identical, so behaviour is correct. -- `RawDriverTypeDialog.razor:2-4, 59-60` — "its factory lands in Wave C". -- `CsvColumnMap.cs:322-324` — hardcodes `CalculationDriverType = "Calculation"` "Not yet a `DriverTypeNames` constant"; it has been one since `DriverTypeNames.cs:54`. -- ~~`SqlDriver.cs:218-220` — justifies not re-parsing config on the premise "the factory builds a fresh - instance", **disproved** by `DriverInstanceActor.cs:316` + `DriverHostActor.cs:2565`.~~ ✅ **Fixed** — - the comment now says the premise was false when written and is true *now* because a config change - respawns. See §9. - ---- - -## 2. Open tracker issues — all 17 verified against source - -**No issue was found stale.** 11 CONFIRMED, 4 PARTIALLY (core claim holds, a sub-claim is wrong or -incomplete), 2 CANNOT VERIFY (correctly filed as hardware/fixture-gated). - -> **Since the audit:** three of these are **resolved by the remediation but still open on the tracker** — -> #516, #518 and #507 (marked ✅ below). Four new issues were filed from §3 and §9: **#520** (ACL -> enforcement), **#521** (`IHostConnectivityProbe`), **#522** (stability tiers), **#523** (remove the -> periodic re-browse). The tracker therefore shows **21 open**, of which 3 are stale-open. Everything not -> marked below is unchanged. - -| # | Title (abbrev.) | Verdict | Key evidence | Kind | -|---|---|---|---|---| -| **518** ✅ | `IRediscoverable` + `IHostConnectivityProbe` have no consumer; CLAUDE.md wrong — **RESOLVED `09a401b8`: rediscovery consumed as an operator prompt; probe half split to #521. Stale-open on tracker.** | **CONFIRMED — worse than filed** | Only `+=` in `src/` are Galaxy self-wiring (`GalaxyDriver.cs:586`, `:221`). `GetHostStatuses()` has **zero call sites** outside the interface + implementations; `DriverHostStatuses` DbSet is referenced only by its declaration and a test. | Defect + doc error | -| **507** ✅ | Discovered-node injection inert in v3 — **RESOLVED `adce27e7`: not revivable (its two inputs are structurally empty in v3), so the path was DELETED, superseded by `/raw` browse-commit. Stale-open on tracker.** | **CONFIRMED** | `DriverHostActor.cs:986-1002` hard-`return;` + `#pragma warning disable CS0162` (restored `:1077`) | Deferred (deliberate guard) | -| **519** | No Bad/Uncertain **sub-code** reaches a client | **CONFIRMED** | Collapse at `DriverInstanceActor.cs:1033-1041` (`statusCode >> 30`), re-expand at `OtOpcUaNodeManager.cs:3318-3322`. Enum is 3-state at `IOpcUaAddressSpaceSink.cs:165`. **Wider than filed** — `StatusFromQuality` also used at `:429/:506/:576/:761`, so alarm-condition Quality collapses too. | Design consequence | -| **517** | Health published AFTER teardown | **CONFIRMED, fleet-wide** | `ModbusDriver.cs:244-250`, `MTConnectDriver.cs:579-590`, `S7Driver.cs:292-326` — all `Teardown…` then `WriteHealth(Unknown)`; `GetHealth()` is a plain field read | Defect (race) | -| **516** ✅ | Driver config edits silently discarded — **RESOLVED `d32d89c3` + `5184a2e1`: seam respawn + in-place re-parse; all 12 drivers now honour a changed config. Stale-open on tracker.** | **CONFIRMED — survey now complete** | `DriverInstanceActor.cs:316` (only `_driver` assignment) + `DriverHostActor.cs:2565` (`Tell` existing child, no respawn) + `:653/:660` (green seal). **Affected: Modbus, FOCAS, OpcUaClient, + AbLegacy (issue missed it), + Sql (deliberate/documented).** AbCip/TwinCAT re-parse ✅; Galaxy fails **loud** (throws, `GalaxyDriver.cs:600-641`) | Defect | -| **515** | Sparkplug `DataSet`/`Template`/`PropertySet` | **CONFIRMED** | `SparkplugCodec.cs:207-210` decode to `Unsupported` — visible refusal, not silent drop. *(`PropertySet` isn't a `ValueOneofCase`; it rides `metric.Properties`)* | Deferred feature | -| **514** | Rebirth unarmable on a plant that hasn't birthed | **CONFIRMED** | `RawBrowseModal.razor:119-120` disabled; `_rebirthTarget` set only by tree-click handlers (`:432`, `:437`, `:439-451`). Empty tree ⇒ permanently disabled | Defect (chicken-and-egg) | -| **513** | Meaningless `payloadFormat` in Sparkplug blob | **CONFIRMED** | `MqttTagConfigModel.cs:210` unconditional `Set`, vs. mode-aware `:216` | Defect (cosmetic) | -| **512** | `.Browser` → `.Driver` layering | **CONFIRMED** | `…Mqtt.Browser.csproj:43` (**not :44**), boxed "KNOWN, DELIBERATE EXCEPTION" at `:16-42` | Deferred refactor | -| **511** | `ActAsPrimaryHost` does nothing | **CONFIRMED** | Option at `MqttDriverOptions.cs:193`, round-tripped in the UI, handled only by a warning at `MqttDriver.cs:933-948`. No STATE publish, no Last Will | Deferred feature | -| **510** | `_canRebirth` captured at browse-open | **CONFIRMED** | `RawBrowseModal.razor:377` sole `true` assignment; failure path `:516-539` never re-evaluates | Defect (stale affordance) | -| **509** | Metric name with `/` unbrowse-committable | **CONFIRMED** | `RawBrowseCommitMapper.cs:63` uses browse name verbatim → `RawPaths.cs:39` rejects `/` → `RawTreeService.cs:976` **all-or-nothing** kills the whole batch | Defect | -| **508** | MQTT write-through | **CONFIRMED** | `MqttDriver.cs:66` — no `IWritable`; consequence at `DriverInstanceActor.cs:673-677` | Deferred feature | -| **507**→ see above | | | | | -| **491** | Continuous-historization live gate | **CANNOT VERIFY** (needs VPN'd gateway) | Code **is** complete: `AddressSpaceApplier.cs:238`→`:540`→`ActorHistorizedTagSubscriptionSink.cs:27-38`→`ContinuousHistorizationRecorder.cs:84`, wired `ServiceCollectionExtensions.cs:435` | Deferred test | -| **489** | Hot-rebind renamed raw tag | **CONFIRMED** | No implementation exists | Deferred feature | -| **481** | Comms-loss → scripted-alarm quality (Layer 4) | **CONFIRMED** | `DriverHostActor.cs:1356-1379` iterates `_alarmNodeIdByDriverRef` only. **Issue is wrong that the per-driver ref set is missing** — `_nodeIdByDriverRef` exists at `:193`; only the fan-out into the mux is absent, so the work is *smaller* than filed | Deferred feature | -| **468** | Wave-0 browser tree render | **CANNOT VERIFY** (fixture) | Code path present (`AbCipDriver.cs:1092-1095`, `LibplctagTagEnumerator.cs:29-37`); `ab_server` returns `ErrorUnsupported` for CIP Symbol Object enumeration | Deferred verification | - -### 2.1 Cross-cutting observations - -1. **#518 → #507 are the same failure, stacked.** The event has no subscriber *and* its downstream - handler hard-returns. **Fixing either alone changes nothing observable** — schedule them together. -2. **#516 is a superset of part of #489.** For the five drivers that discard reinit config, the - renamed-tag-never-rebinds symptom follows mechanically (their `_tagsByRawPath` still keys on the - old RawPath). Re-scope #489 after #516. -3. **#519 and #481 share the same `statusCode >> 30` collapse idiom** at three independent sites - (`DriverInstanceActor.cs:1033`, `ScriptedAlarmEngine.cs:745`, `:777`). Widening the status path - must cover all three. - ---- - -## 3. Dead seams — authored, shipped, never executed - -**The most consequential category in this register**, and the one nothing in the code flags. Three -subsystems are fully built and reachable by operators, but no production code path executes them. - -> **Since the audit:** one of the three is now consumed (§3.2's `IRediscoverable` half). The other two -> were **not** fixed — that was the deliberate call, because each is a keep-or-delete decision rather than -> a defect. What changed is that they can no longer mislead: the non-enforcement is stated in the docs and, -> for ACLs, warned about **in the authoring UI itself**, so an operator cannot save a rule believing it -> does something. Tracked as **#520**, **#521**, **#522**. - -### 3.1 Node-level ACLs are authorable, deployed — and unenforced - -> ◐ **Still true; now impossible to miss** (`53ede679`, Gitea **#520**). `docs/security.md`, -> `docs/ReadWriteOperations.md`, `docs/v2/acl-design.md` and `v2-release-readiness.md` corrected, and -> `ClusterAcls.razor` / `AclEdit.razor` carry a warning alert. The audit named -> `docs/ReadWriteOperations.md` as the highest-risk doc; **`docs/security.md` was worse** — it claimed an -> anonymous session is default-denied any node it has no grant for, when in fact it can read the entire -> address space. #520 records the four real blockers to a wire-up, of which the Raw-realm scope gap is a -> design decision rather than wiring. - -- `IPermissionEvaluator`, `TriePermissionEvaluator`, `PermissionTrieCache`, `PermissionTrieBuilder` - have **zero references outside `src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/` and their own - tests** (independently re-verified for this report: every other grep hit is a `bin/`/`obj/` XML doc artifact). -- `OtOpcUaNodeManager` contains **no reference to `Permission` or `Evaluator` at all.** -- Meanwhile `ClusterAcls.razor` + `AclEdit.razor` let operators author `NodeAcl` rows, and - **`ConfigComposer.cs:51` snapshots them into every deployment artifact.** - -**Impact:** an operator authors a deny rule, deploys it green, and it has no effect. Actual -enforcement is coarse LDAP roles plus the realm-qualified `WriteOperate` gate in the node manager — -and that is a **write** gate; reads carry no per-node ACL check. -**`docs/ReadWriteOperations.md:13,21` states the opposite** (see §7.1) — it is the single -highest-risk doc error found. - -### 3.2 `IRediscoverable` + `IHostConnectivityProbe` raise into the void - -> ✅ **`IRediscoverable` half FIXED** (`09a401b8`) — consumed by `DriverInstanceActor` and surfaced as a -> `/hosts` re-browse prompt. Advisory: the served address space is not rebuilt, because v3 authors raw tags -> through `/raw` browse-commit. -> ⏳ **`IHostConnectivityProbe` half UNRESOLVED** (Gitea **#521**) — `GetHostStatuses()` still has zero -> production call sites and nothing writes a `DriverHostStatus` row. Not deleted on inference: the table was -> deliberately re-created in the v3 initial migration. - -Gitea #518/#507. Nine drivers raise; nothing consumes. `DriverHostStatus` table exists -(`OtOpcUaConfigDbContext.cs:48`, migration `20260715230637_V3Initial.cs:108`) with a doc-comment -claiming a writer — **nothing ever writes a row.** - -**Impact:** an Agent/PLC/Galaxy restart leaves a stale address space behind a Healthy driver, and the -per-host connectivity table is permanently empty. - -### 3.3 `DriverTypeRegistry` is vestigial - -> ◐ **Still true; documented** (`88ce8df0`, Gitea **#522**). `docs/v2/driver-stability.md` now states its -> tier table is aspirational. A second staleness the audit missed is corrected there too: the -> **separate-Windows-service hosting** that document describes for Tier C no longer exists either (Galaxy -> moved to the mxaccessgw sidecar in PR 7.2; FOCAS went in-process with its managed wire client). No -> runtime change — enabling recycle on two live drivers wants its own live gate. - -Independently verified for this report: `src/Core/…Core.Abstractions/DriverTypeRegistry.cs:20` is -referenced **only** by `tests/Core/…/DriverTypeRegistryTests.cs`. Nothing registers metadata at -startup; nothing validates `DriverInstance.DriverType` against it. Its `AllowedNamespaceKinds` field -was retired with the v3 `Namespace` entity (`DriverTypeRegistry.cs:97`). - -**Knock-on:** the driver **stability tier** does not come from here either. The live source is -`DriverFactoryRegistry.cs:46` (`DriverTier tier = DriverTier.A`) and **no factory in `src/Drivers/` -passes a tier** — so all 12 drivers run Tier A, and the Tier-C-only protections are dormant for -every driver: `MemoryRecycle.cs:54` (`when _tier == DriverTier.C`) and -`ScheduledRecycleScheduler.cs:43` (refuses unless Tier C). `docs/v2/driver-stability.md` still -assigns Galaxy and FOCAS to Tier C. - -### 3.4 Related dead/dormant code - -| Item | Evidence | -|---|---| -| `IDriverConfigEditor` — interface, zero implementors, zero production consumers | `Core.Abstractions/IDriverConfigEditor.cs:9`; superseded by `TagConfigEditorMap` / `DriverConfigModal` | -| `GenericDriverNodeManager` — explicitly **not** a production dispatch path | `Core/OpcUa/GenericDriverNodeManager.cs:71` ("verified 07/#10: zero production references") | -| `IDriverSupervisor` — Tier-C recycle machinery has **zero implementations**, yet the parser still validates `RecycleIntervalSeconds` | archreview 01/U-2 | -| Resilience **bulkhead** — documented, defaulted, parsed, AdminUI-authorable; `Build` never adds a concurrency limiter | archreview 01/U-6 | -| `WriteIdempotentAttribute` — zero readers; write dispatch hardcodes `isIdempotent: true` | archreview 01/S-8 = 03/S12 | -| ~60 lines of unreachable code retained behind #507's guard | `DriverHostActor.cs:1002`, `#pragma warning disable CS0162` | -| Three empty `Driver.Historian.Wonderware*` directories (0 `.cs`, no `.csproj`, absent from `.slnx`) | verified for this report — only stale `bin/`+`obj/` remain | - ---- - -## 4. In-code deferrals by subsystem - -The tree is unusually clean on classic markers: **9 `TODO`s across 1,812 files; zero -`FIXME`/`HACK`/`XXX`/`WORKAROUND`; zero `#if false`; zero `[Obsolete]` on project code; zero -commented-out DI registrations.** Real deferred work lives in prose doc-comments and skipped tests. - -### 4.1 Highest-consequence - -| Item | Evidence | -|---|---| -| **Telemetry snapshot cache never evicted** — a decommissioned driver replays last-known Health/Resilience to new subscribers forever | `TelemetryLocalHub.cs:40` `TODO(mesh-phase5+)` — self-declared, bounded, "pre-production, accepted" | -| **Device host recovered by string-matching instead of `DeviceConfig`** — 7 of the repo's 9 TODOs are this one item, across 4 drivers | `FocasDriver.cs:137,287`; `TwinCATDriver.cs:775`; `AbLegacyDriver.cs:557`; `AbCipDriver.cs:89,1272` + `AbCipDriverOptions.cs:130` — all `TODO(v3 WaveC)` | -| **Galaxy writes can never confirm a COM commit** — empty status array ⇒ provisional Good, metered `galaxy.writes.unconfirmed` | `GatewayGalaxyDataWriter.cs:44,328,354` — **cross-repo**, blocked on mxaccessgw | -| **Subscription liveness undetectable** — `ISubscriptionHandle` is opaque | `DriverInstanceActor.cs:527` (#488) — well-reasoned, deferred "until a real occurrence" | -| **`/raw` "Browse device" is still a placeholder** despite the universal discovery browser landing | `RawTree.razor:15,426` — **verify**; may be reachable now via `DiscoveryDriverBrowser` | - -### 4.2 Per-driver - -- **S7** — array **writes** unsupported entirely (`S7Driver.cs:1139`); several array *read* types unsupported (`:386,404,413,419,428,442,465,807,959`) — all fail fast at config-validation; legacy C-area counter reinterpretation live-hardware-gated (`:759`); Counter BCD on S7-300/400 undecoded. -- **AbCip** — ALMD-only alarm projection, ALMA deferred (`AbCipAlarmProjection.cs:16,324`); no CIP Template Object reader, UDT layouts declaration-driven (`AbCipDriverOptions.cs:172`); whole-UDT writer deferred (`LibplctagTagRuntime.cs:190`); `AckCmd` pulse must be wired client-side (`:30`). -- **Sql** — `SqlTagModel.Query` deferred to P3, **enforced end-to-end** across parser, validator, planner and editor (`SqlProvider.cs:22,37`; `SqlEquipmentTagParser.cs:24,103`; `SqlGroupPlanner.cs:44,249`; `SqlTagConfigModel.cs:114`) — exemplary deferral discipline. Postgres/ODBC dialects deferred behind `ISqlDialect`. -- **MTConnect** — `TIME_SERIES` vectors deferred to P1.5; `DATA_SET`/`TABLE` coded `BadNotSupported` with a null value rather than approximated (`MTConnectObservationIndex.cs:53,272`) — deliberately chosen over a Good-coded lie. `RawTagEntry.DeviceName` ignored for routing (two `/raw` Devices under one driver share an Agent connection) — "a design change, not a bug fix". No DataType-vs-blob validation at authoring. -- **MQTT** — `MaxPayloadBytes` is a constructor knob, not operator-facing (`MqttSubscriptionManager.cs:170`). -- **TwinCAT** — Flat-mode struct expansion carries a **LIVE RISK** note, unverified against a real TC3 target (`AdsTwinCATClient.cs:270`); VirtualTree-mode browse unverified; TC2 coverage, multi-hop AMS route and lab rig all hardware-blocked (`docs/v3/twincat-backlog.md`). -- **FOCAS** — forces read-only regardless of authored `writable:true`; servo-load semantics inferred from the wire, unconfirmed against a real CNC (`FocasWireClient.cs:944`); `"Backend": "unimplemented"` is a **deliberate fail-fast stub**, not unfinished work. -- **Historian.Gateway** — String/DateTime/Reference writes deferred, gated on the analog SQL write path; `UInt16 → Uint4` widening because the `UInt2` write path is deferred upstream (`HistorianTypeMapper.cs:27-51`). - -### 4.3 Fleet-wide OPC UA gaps - -- **Array writes deferred across all drivers**; **multi-dimensional arrays (`ValueRank>1`) unsupported**; **array historization** is an opaque blob with no per-element history (`docs/Uns.md:288-292`, `AddressSpaceApplier.cs:873`, `OtOpcUaNodeManager.cs:1764`). -- **`HistoryUpdate`** — permission bit `1<<12` shipped but excluded from every composite bundle; no service surface (`NodePermissions.cs:34`). -- **`Utils.Log*` is `[Obsolete]`** in SDK 1.5.378, suppressed at 8 sites in `OtOpcUaNodeManager` pending an `ITelemetryContext` rewire. - -### 4.4 Skipped tests — 36, from 4 reason constants - -| Count | Reason | Assessment | -|---|---|---| -| ~~18~~ 0 | ~~`DiscoveryInjectionDormantV3`~~ | ✅ **Deleted** (`adce27e7`). The audit's "Real — blocked on #507" was **wrong**: these were v2 characterization tests asserting an equipment-rooted graft (`EquipmentRootNodeId == "EQ-1"`) that v3 cannot produce, so they could never have unskipped as written. `Runtime.Tests` skipped went 31 → 13. | -| ~~13~~ 0 | ~~`EquipmentTagsDarkBatch4`~~ | ✅ **Resolved** (2026-07-28). The audit's "STALE" call was right and the constant is now **deleted**. Not by bulk-unskipping: 6 were **revived** onto the raw/UNS shape that replaced the subject, 3 were **folded** into a live parity test whose corpus was widened to cover them, 4 were **deleted** as genuinely retired. See §9. | -| 4 | `EquipmentDeviceBindingRetired` | Intentional tombstones preserving retirement rationale — fine as-is | - -Plus **4 whole OpcUaClient test suites that are scaffold-only**, awaiting an in-process OPC UA server -fixture that was never built (`OpcUaClientSubscribeAndProbeTests`, `…DiscoveryTests`, -`…ReadWriteTests`, `…HistoryTests`, all `:10`). - -All env-gated integration suites **skip cleanly** when their vars are absent. The MTConnect gate is -notably a **real `/probe` reachability + payload-shape check** (`MTConnectAgentFixture.cs:42`), not -mere env-var presence, so it cannot pass vacuously. - ---- - -## 5. Open live gates - -| Gate | Status | Evidence | -|---|---|---| -| ~~**Tag-set drift detection** (shipped `e77c8a35`)~~ | ✅ **CLOSED BY DELETION** — removed under Gitea **#523** rather than gated. The gate it needed (a device whose tag set actually changes) required exactly the traffic the removal objected to. | §9, 2026-07-28 (#523) | -| **Continuous-historization value capture** (#491) | **OPEN** — code complete, unproven in production | `CLAUDE.md:710`; no gate-passed commit exists | -| **Wave-0 universal discovery browser** full tree render (#468) | **OPEN** — fixture-blocked | tracking doc §Wave 0; named as blocking BACnet browse | -| **archreview R2-03** — VT Good→Bad on a data-fed rig | **OPEN** — explicitly "live-blocked on this data-less rig" | `archreview/plans/STATUS.md` | -| **archreview R2-10** — force breaker-OPEN live | **OPEN** — breaker-OPEN state never forced live | `archreview/plans/R2-10-*.tasks.json` | -| **Driver-reconfigure-while-faulted** Task 3 | **OPEN** — task subject literally "DEFERRED" | `2026-07-14-driver-reconfigure-while-faulted-plan.md.tasks.json` | -| Galaxy `ScanStateProbeParityTests` | **OPEN** — licence-gated (one `$WinPlatform` on this box) | `docs/v2/Galaxy.ParityRig.md:180` | -| **v2 GA exit criteria** — FOCAS live-CNC wire smoke (#54), OPC UA CTT/compliance pass, Ignition 8.3 redundancy cutover | **OPEN** — manual/hardware/external-tool | `docs/v2/v2-release-readiness.md:105-120` | -| mesh Phase 4 | ✅ **PASSED** `1281aebf` | CLAUDE.md said open — **corrected**, §7.2 | -| mesh Phase 5 | ✅ **PASSED** `f134935f` | CLAUDE.md said pending — **corrected**, §7.2 | -| auto-down 1-vs-1 crash-the-oldest | ✅ **PASSED** — Phase 7 drill D4, `c50ebcf7` | CLAUDE.md said open — **corrected**, §7.2 | - -**Note on the 14 archreview `deferred-live` tasks across 8 plans:** `archreview/plans/STATUS.md:242-305` -records a 2026-07-13 live pass closing several (R2-02, R2-07, R2-11, R2-04, R2-05-partial) and -2026-07-15 closures for R2-01/R2-08/R2-06 — **but the `.tasks.json` files were never updated.** Only -R2-03 and R2-10 have no closure record anywhere. - ---- - -## 6. Plan bookkeeping — stale vs. live - -**59 of 78 `docs/plans/*.tasks.json` are 100 % complete.** Of the remaining 19, most are stale -bookkeeping, not backlog. - -### 6.1 STALE — work shipped, file never updated (~140 phantom "pending" tasks) - -> ✅ **Swept** (`88ce8df0`). 13 files closed — the 8 below plus `mesh-phase4`, the 4th `stillpending` -> phase file, and `historian-tcp-transport` closed as **OBSOLETE**. Each carries a `closureNote` naming the -> evidence. 11 archreview gates were also written back from `STATUS.md`; **R2-03 and R2-10 were deliberately -> NOT closed** — `STATUS.md`'s own per-item detail contradicts its "every gate is GREEN" headline. See §9. - -| File | Recorded | Reality | -|---|---|---| -| `2026-07-24-mtconnect-driver.md.tasks.json` | 23 pending | Merged at master tip `90bdaa44`, 5-leg live gate PASSED | -| `2026-07-24-mqtt-sparkplug-driver.md.tasks.json` | 27 pending | All 27 done, both phases live-gated | -| `2026-07-24-sql-poll-driver.md.tasks.json` | 22 pending | Merged `4ad54037` + follow-ups `28c28667` | -| `2026-07-24-modbus-rtu-driver.md.tasks.json` | 11 pending | Merged `0f38f486` (#495), live gate PASSED | -| `2026-06-26-otopcua-historian-gateway-integration.md.tasks.json` | 21 pending | Shipped as PR #423, live-validated | -| `2026-06-15/16-stillpending-phase-{0-1,2,3,4}.md.tasks.json` | 12+9+10+8 pending | Shipped — audit §D cites `1e95856b`, `ada01e1a`, `fc8121cb`, `3e609a2b`, `70e6d3d2`, `c236263e`, `bd8fee61`, `5e27b5f7`, `fcb38014` | -| `2026-07-22-per-cluster-mesh-program.md.tasks.json` | Phase 2 in_progress, 3–7 pending | Plan body `:331-342` says **program COMPLETE** | -| `2026-06-12-historian-tcp-transport.md.tasks.json` | 12 pending | **OBSOLETE** — targets the retired Wonderware sidecar | - -### 6.2 GENUINELY LIVE - -| File | Open | Note | -|---|---|---| -| **`2026-05-29-adminui-followups.md.tasks.json`** ⬅ **now the largest open item in `docs/plans/`** | **19 pending / 0 completed** | **The largest unexecuted plan in the tree.** Generic `CollectionEditor`; per-driver device/tag editors ×7; typed resilience form; `RoleMapper.Merge` + DB-backed LDAP→role mapping + `RoleGrants.razor` + FleetAdmin authz. **Several items look superseded by v3's `TagConfigEditorMap` — verify overlap before executing.** | -| `2026-06-19-followups-batch.md.tasks.json` | 4 pending + 1 partial | B4 F10b surgical DataType/IsArray writes and B5 alarm-severity `SetSeverity` are **OPEN-by-choice, not built** | -| `2026-06-26-otopcua-fixedtree-followups.md.tasks.json` | 1 pending | Task 11 build + regression | -| `2026-05-26-akka-hosting-alignment-plan.md.tasks.json` | 5 partial | F8/F9/F10/F13/F14 — oldest open items; **likely superseded by v3, verify** | -| `2026-05-28-adminui-driver-pages-plan.md.tasks.json` | 1 partial | 10.4 manual smoke checklist | - -`docs/plans/2026-07-23-mesh-phase4-…tasks.json` Task 9 was the only `deferred`-flagged mesh task — -**it has since landed** (`3a590a0c`), so that file is now stale too. - -### 6.3 Review-directory state - -- **`code-reviews/`** — 51 modules, **zero Open findings**. One textual "Left Open pending a decision" - survives in a finding body (`Configuration/findings.md:254`, LDAP collation) whose status field - reads Deferred — the only index inconsistency. -- **`code-reviews/` coverage gap** — **10 shipped source projects have never been reviewed**: - `Driver.Calculation`, `Driver.Historian.Gateway`, `Driver.Mqtt` (+`.Browser`, `.Contracts`), - `Driver.MTConnect` (+`.Contracts`), `Driver.Sql` (+`.Browser`, `.Contracts`). Meanwhile three - modules review the **retired** Wonderware backend. -- **`archreview/`** — `00-OVERALL.md:60-79` carries an explicit "Carried open … no remediation branch - targeted them" block. The largest coherent batch is the **failure-visibility trio**: 01/S-1 - (`AddressSpaceApplier` swallows every sink failure, deploy reports Applied even when broken), 03/S4 - (primary gate default-allow on unknown role), 06/S-1 (Galaxy optimistic write success). -- **`stillpending.md` does not exist** — the backlog is `docs/plans/2026-06-18-stillpending-backlog-audit.md`. - ---- - -## 7. Documentation corrections - -### 7.1 Applied in this pass - -| File | Was | Now | -|---|---|---| -| `CLAUDE.md` §Change Detection (`:156`) | "The server's `DriverHost` consumes the signal and rebuilds the address space" | ⚠️ nothing consumes it; names `DriverHostActor.cs:986`, #518/#507, and the four affected drivers | -| `CLAUDE.md` §Redundancy (`:272`) | auto-down "live gate is still open … docker-dev is a single six-node mesh" | gate **CLOSED** by Phase 7 drill D4 (`c50ebcf7`); the six-node-mesh claim also contradicted CLAUDE.md's own Phase 6 paragraph | -| `CLAUDE.md` §Telemetry (`:383`) | Phase 5 "code-complete on `feat/mesh-phase5`, live gate pending" | merged `35552a96`; live gate PASSED `f134935f` | -| `CLAUDE.md` §Phase 4 (`:493`, `:502-503`) | `ScriptedAlarmState` "dropping it is a deferred follow-up, tracked as Task 9"; "Task 9 and the live gate (Task 10) are still open" | table **dropped** (`3a590a0c`, migration `20260723183050_DropScriptedAlarmStateTable`); live gate PASSED `1281aebf` | -| `docs/plans/2026-07-24-driver-expansion-tracking.md` | Modbus RTU "pending merge"; SQL poll "📝 Plan ready (22 tasks)"; **an "Executing a plan" table instructing you to rebuild both** | both marked ✅ Done with merge SHAs; the two shipped rows struck from the command table; Wave 1/2 headings and §Next actions corrected | -| `docs/drivers/README.md` | `DriverTypeRegistry` "registered at startup"; "namespace kinds (Equipment + SystemPlatform)"; no Sql/Calculation rows; "Modbus TCP" only | registry marked vestigial + tier truth; namespace line corrected to Raw+UNS; Sql + Calculation rows added; Modbus row covers RTU-over-TCP | -| `docs/drivers/TwinCAT.md` (`:95`, `:99-103`, `:151`) | "so the address space is rebuilt" / "so Core rebuilds the address space" | raises-but-unconsumed caveat, mirroring the wording `Mqtt.md`/`MTConnect.md` already use correctly | -| `docs/drivers/Galaxy.md` (`:73`) | `IRediscoverable` row with no caveat | same caveat added | -| `docs/v2/Runtime.md` (`:106`) | "named-pipe IPC to the Wonderware historian sidecar + `SqliteStoreAndForwardSink`" | gRPC to HistorianGateway + `LocalDbStoreAndForwardSink` | -| **`docs/ReadWriteOperations.md`** (`:13`, `:21` + banner) | "ACL enforcement (`WriteAuthzPolicy` + `AuthorizationGate`) runs before the source branch"; "**A denied read never hits the driver**" | Both struck through and corrected; banner states plainly that **no per-node ACL gate exists on any operation** and that `GenericDriverNodeManager` is test scaffolding. **This was the highest-risk doc error found.** | -| **`docs/IncrementalSync.md`** (banner) | whole "Rebuild flow" + generation-publish path presented as current | Banner: `GenericDriverNodeManager` is non-production, rediscovery has no consumer, `sp_PublishGeneration`/`sp_ComputeGenerationDiff` `RAISERROR`, `DraftRevisionToken` gone; names the one live path (deploy artifact) | -| **`docs/AddressSpace.md`** (banner) | "single custom namespace"; `Phase7Applier`/`Phase7Composer`/`GalaxyTagPlan`; `SystemPlatform` | Banner: two namespaces w/ `V3NodeIds`, the `AddressSpace*` rename (`40e8a23e`), `AddressSpaceComposition` as the result type, `SystemPlatform` retired | -| `docs/v2/phase-7-status.md`, `v2-release-readiness.md`, `redundancy-interop-playbook.md`, `AdminUI-rebuild-plan.md` (banners) | present-tense claims about types that no longer exist | Dated "⚠️ Historical" banners naming the specific dead types. `v2-release-readiness.md`'s banner also flags that its `:41` "ACL enforcement Closed" row is false, while noting its unchecked GA exit criteria **are** still live. (`docs/StatusDashboard.md` already self-declares Superseded — left alone.) | -| `docs/OpcUaServer.md` (`:14`, `:51`), `docs/README.md` (`:65`), `docs/drivers/OpcUaClient-Test-Fixture.md` (`:127`) | `WriteAlarmState`; `LdapUserAuthenticator` ×2; `RedundancyCoordinator` | `WriteAlarmCondition`; `LdapOpcUaUserAuthenticator`; `RedundancyStateActor` + `IRedundancyRoleView` | -| `docs/Historian.md` | no mention of #491 or the provisioning gap | ⚠️ block added: continuous value capture is **code-complete but unproven in production**, with the wiring chain; plus the `EnsureTags`-skips-existing-tags gap | - -### 7.2 Recorded, NOT applied — needs a decision or a rewrite, not a line edit - -> ✅ **Priorities 1–2 are now done** (`53ede679`): `docs/ReadWriteOperations.md` was **rewritten**, not -> bannered — and it turned out to be fiction well beyond the ACL claims (`OnReadValue` has zero occurrences -> in `src/`; reads never reach a driver at all). `docs/security.md`, which this table did not list, was the -> highest-risk of the three. **Priority 3 and the rest remain open** — the `Phase7*`→`AddressSpace*` rename, -> `docs/v2/driver-specs.md` coverage, the missing `docs/drivers/Sql.md` / `Calculation.md`, and the -> `NoOpSecretReplicator` investigation. `docs/v2/driver-stability.md` moved to §3.3 / #522. - -The three highest-priority items in this list were **bannered rather than rewritten** in this pass -(cheap, stops the misleading, preserves the record) — the underlying rewrite is still owed. They now -appear in §7.1 as well. - -| Priority | File | Problem | -|---|---|---| -| **1 (security-relevant)** | `docs/ReadWriteOperations.md:13,21` | **Bannered + struck through.** Claimed ACL enforcement via `WriteAuthzPolicy` + `AuthorizationGate` + `NodeScopeResolver` and that "a denied read never hits the driver". All four names have zero hits in `src/`; §3.1 shows there is no read-path ACL gate at all. **Still owed:** a proper §-rewrite against the real node-manager write gate. Same false claim at `docs/v2/v2-release-readiness.md:41` (bannered). | -| **2** | `docs/IncrementalSync.md` | **Bannered.** Entire "Rebuild flow" (`:37-47`) built on non-production `GenericDriverNodeManager`; `:26-34` cites `sp_PublishGeneration`/`sp_ComputeGenerationDiff`, both retired stubs that `RAISERROR` (`20260715230637_V3Initial.StoredProcedures.cs:194`); `:31` cites `DraftRevisionToken` (0 hits); `:49-51` describes a `ScopeHint` no consumer reads. **Still owed:** rewrite around the deploy-artifact path, or archive to `docs/v1/`. | -| **2** | `docs/AddressSpace.md` | **Bannered.** v2-era throughout: `:7,42` claim a single namespace (v3 has two); `:7,48,70` cite `Phase7Applier` + `GalaxyTagPlan` + a nonexistent file path; `:62-64` repeats the rediscovery claim. **Still owed:** rewrite §"Root folder" + §"NodeId scheme" against `V3NodeIds` / `AddressSpaceRealm`. | -| 3 | 6 docs, ~20 citations | **`Phase7*` → `AddressSpace*`** rename (`40e8a23e`). Mostly mechanical, **except three that are not renames** and need the real member identified first: `Phase7CompositionResult` → **`AddressSpaceComposition`** (`AddressSpaceComposer.cs:13`, verified for this report); `Phase7Composer.ExtractTagFullName` → nearest live seam is `ScriptTagCatalog.ExtractFullNameFromTagConfig`; **`Phase7EngineComposer.RouteToHistorianAsync` is simply gone** — the real seam is `HistorianAdapterActor` → `IAlarmHistorianSink`. | -| 3 | `docs/v2/driver-specs.md` | Covers 8 of 12 drivers (missing MQTT, MTConnect, Sql, Calculation, and the Modbus `Transport` selector) while `docs/drivers/README.md` calls it authoritative "for **every** driver". Either add four sections or downgrade the claim. | -| 3 | `docs/v2/driver-stability.md:47-54` | Puts Galaxy and FOCAS in Tier C; `docs/drivers/README.md` says Tier A for both. **Moot at runtime** — see §3.3 — but the two docs contradict each other. | -| 3 | `docs/drivers/Sql.md`, `docs/drivers/Calculation.md` | **Do not exist.** Sql is documented only in design/plan/research files; Calculation only at `docs/Raw.md:66-69`. | -| — | `docs/operations/2026-07-16-secrets-clustered-master-key.md:38` | `NoOpSecretReplicator` exists **only** in a test. **Investigate before editing** — the sentence makes a security-relevant claim ("no built-in cross-node replication today"), so it needs the real behaviour identified, not a rename. Left untouched deliberately. | -| — | `docs/v2/driver-specs.md` §2 | `docs/drivers/Modbus.md:10-11` points readers here for the Modbus config shape, and this file omits the `Transport` selector. Folded into the driver-specs item above. | - ---- - -## 8. Recommended sequencing - -> ✅ **All six executed**, merged as `914d70bf`. Each item below is struck through with its outcome; the -> full account is §9. Two items the sequencing did not anticipate were also completed (Sql/FOCAS in-place -> re-parse, tag-set drift detection) — the latter is now slated for removal under #523. - -1. ~~**§3.1 ACL enforcement** — decide: wire `IPermissionEvaluator` into `OtOpcUaNodeManager`, or - remove the authoring UI and say plainly that ACLs are not enforced. Either way fix - `docs/ReadWriteOperations.md` first; it is the one that could mislead a security review.~~ - ✅ **Done** — decided *make non-enforcement explicit*; wire-up tracked as Gitea **#520**. See §9. - (`docs/ReadWriteOperations.md` was **not** the sharpest one — `docs/security.md` was.) -2. ~~**#518 + #507 together** — neither fix is observable alone.~~ ✅ **Done.** The framing was right - that they had to be handled together, but wrong about the resolution: **#507 was not revivable**, - so it was deleted rather than fixed. See §9. -3. ~~**#516** — silent config discard on 5 drivers; then re-scope #489, which it partly subsumes.~~ - ✅ **Done.** See §9. #489 is still open and should now be re-scoped. -4. ~~**G-4** — extend the existing picker-parity test to `DriverConfigModal` + `DeviceModal`; it would - have caught G-1 and G-2 for free, and this class has now recurred twice.~~ ✅ **Done**, together with - G-1, G-2, G-3, G-5 and G-6. See §9. -5. ~~**Bookkeeping sweep** — mark the 8 stale `.tasks.json` files complete so the 19-task AdminUI plan - (§6.2) is visible as the real backlog it is.~~ ✅ **Done.** See §9. -6. ~~**§3.3 tier truth** — either pass real tiers at factory registration or delete the Tier-C - machinery; today it is documented, authorable and dormant.~~ ✅ **Documented**; the keep-or-delete - code decision is Gitea **#522**. See §9. - ---- - -## 9. Execution log - -Remediation of §8. **MERGED to `master` and pushed 2026-07-28, merge `914d70bf`** (branch -`feat/deferment-remediation`, 8 commits, merged `--no-ff` so each stage stays individually reviewable and -revertable). Status was updated in the **same commit** as the work, so this register never disagreed with -the tree. - -> ⚠️ **The commit column was wrong until 2026-07-28 and is now corrected.** Each stage's SHA was captured -> with `git rev-parse HEAD` and then written into this file by amending that same commit — which changes -> the SHA. Every value recorded was therefore the *pre-amend* one: a dangling object reachable only through -> the reflog, on no branch, and due to vanish at the next `gc`. The register cited commits that effectively -> did not exist. The SHAs below are verified against `git log e08855fb..914d70bf`. - -| §8 item | Status | Commit | -|---|---|---| -| 1. ACL enforcement decision | ✅ **Done** — decided *make non-enforcement explicit*; Gitea **#520** tracks the wire-up | `53ede679` | -| 2. #518 + #507 | ✅ **Done** — `IRediscoverable` consumed as a re-browse prompt; #507's injection path **deleted**; `IHostConnectivityProbe` half split to Gitea **#521** | `09a401b8`, `adce27e7` | -| 3. #516 config discard | ✅ **Done** — seam respawn + in-place re-parse; completed for all 12 drivers by the follow-up below | `d32d89c3`, `5184a2e1` | -| 4. G-4 dispatch-map parity | ✅ **Done** — G-1…G-6 all closed, live-verified on docker-dev | `6a01358b` | -| 5. Bookkeeping sweep | ✅ **Done** — 13 plan files closed, 11 archreview gates written back | `88ce8df0` | -| 6. Tier truth | ✅ **Done** — docs reconciled; keep-or-delete is Gitea **#522** | `88ce8df0` | -| follow-up A | ✅ **Done** — Sql/FOCAS in-place re-parse made **atomic** | `5184a2e1` | -| follow-up B | ⛔ **Shipped, then removed by decision** — tag-set drift detection; **removal landed**, Gitea **#523** | `e77c8a35`, removed below | - -### 2026-07-27 — §8.1 ACL non-enforcement made explicit - -Decision: **do not wire the evaluator, do not delete it** — state plainly that it does not run, and -track the wire-up as **Gitea #520**. - -Applied: - -- `docs/security.md` — the `NodeScope`/`PermissionTrie`/"Dispatch gate" block now leads with a - **What is enforced today** table (the three real checks) before the design material, which is - labelled *Designed but not wired*. -- `docs/ReadWriteOperations.md` — **rewritten**, not bannered. -- `docs/v2/acl-design.md` — ⚠️ NEVER WIRED banner. -- `docs/v2/v2-release-readiness.md` §"Security — Phase 6.2 dispatch wiring" — the CLOSED claim marked - as not holding. -- `ClusterAcls.razor` + `AclEdit.razor` — warning alerts stating rules are stored and deployed but - never evaluated. - -**Four things this pass found that §7 and the original audit missed** — all worse than what was -already recorded: - -1. **`docs/security.md` was the highest-risk doc, not `ReadWriteOperations.md`.** `:115` claimed an - anonymous session is "default-denie[d] any node a session has no ACL grant for". The opposite is - true: an anonymous session can Browse, Read, Subscribe and HistoryRead the **entire** address - space. It is refused only writes and alarm acks — and only because it carries no roles. -2. **Three of the five documented data-plane role strings do nothing.** `OpcUaDataPlaneRoles` declares - exactly `WriteOperate` and `AlarmAck`. `ReadOnly`, `WriteTune` and `WriteConfigure` are compared - against nowhere in `src/`, so mapping a group to `WriteTune` grants nothing and mapping one to - `ReadOnly` restricts nothing. The doc called all five "exact, case-insensitive, and **code-true**". -3. **`ReadWriteOperations.md` was fiction well beyond the ACL claims.** `OnReadValue` has **zero** - occurrences in `src/` — the whole documented read path did not exist. Reads never reach a driver at - all: the node manager is push-model and the SDK serves a client Read from the cached pushed value. - `WriteAuthzPolicy`, `_sourceByFullRef`, `_writeIdempotentByFullRef` and `IRoleBearer` are likewise - zero-hit, and `CapabilityInvoker` is not referenced by the `OpcUaServer` project at all. -4. **`v2-release-readiness.md` claimed a whole enforcement layer shipped.** Beyond the `:41` row §7.1 - already flagged, `:46-50` describe `FilterBrowseReferences`, `GateCallMethodRequests`, - `MapCallOperation`, `AuthorizationBootstrap` and `Node:Authorization:*` config keys as Closed or - Partial. None of them exist. Whatever landed on the v2 branch did not survive into the shipped tree. - -Also recorded in #520 and not previously known: every existing evaluator unit test runs in -`PermissionTrieBuilder`'s no-`scopePaths` *deterministic test mode*, so the production hierarchy path -is effectively untested; and `NodeAcl.ScopeId` has no FK or existence check, so scope ids dangle -silently. - -### 2026-07-27 — §8.2 rediscovery as a signal; injection deleted - -Decision: **signal, not mutation.** - -`DriverInstanceActor` now consumes `IRediscoverable.OnRediscoveryNeeded` (attach in `PreStart`, detach -in `PostStop`) and carries it on the driver-health snapshot to `/hosts` as a **re-browse chip**. It is -advisory — the served address space is unchanged, because v3 authors raw tags through `/raw` -browse-commit and a runtime graft would materialise nodes nobody approved. - -**#507's injection path was deleted, not fixed.** §2 recorded it as a "deferred (deliberate guard)". -That was too generous: its two inputs — `EquipmentNode.DriverInstanceId` and `EquipmentTags` — are -*structurally empty* in v3, so removing the guard would have changed a log line and injected nothing. -Deleted with it: `HandleDiscoveredNodes`, `PartitionDiscoveredByDeviceHost`, -`ApplyDiscoveredPlansForDriver`, the redeploy re-inject tail, `DiscoveredNodeMapper`, -`AddressSpaceApplier.MaterialiseDiscoveredNodes`, `OpcUaPublishActor.MaterialiseDiscoveredNodes`, the -connect-time discovery loop, and 4 files' worth of tests. `ITagDiscovery` stays — the `/raw` browse -picker drives it through `Commons/Browsing/DiscoveryDriverBrowser`, which never went through the actor. - -**§4.4's "18 `DiscoveryInjectionDormantV3` — Real, blocked on #507" was wrong.** They were v2 -characterization tests asserting an equipment-rooted graft (`EquipmentRootNodeId == "EQ-1"`) that v3 -cannot produce. They could never have unskipped as written; they are deleted. Skipped tests in -`Runtime.Tests` went 31 → 13. - -**The planned drift detector was deliberately NOT built.** The plan proposed diffing each connect-time -discovery pass against the authored raw tags. Reading the drivers killed it: **Modbus, S7, MQTT, -AbLegacy and Sql all echo authored config from `DiscoverAsync`** rather than browsing the device, so -the diff is permanently empty for them. It would have been another plausible-looking inert seam — -precisely the class this register exists to remove. That also removed the last consumer of the -connect-time loop, which is why the loop went too: it was browsing real devices up to ~15× per connect -and dropping the result. - -Two traps worth keeping: - -- `PublishHealthSnapshot` dedups on a health fingerprint, and a rediscovery raise changes none of the - four fields it hashed. The timestamp had to join the tuple or the dedup swallows the signal. - **Verified by reverting the one line — 3 of the 5 new tests go red.** -- The shared `StubDriver.GetHealth()` returns `DateTime.UtcNow`, so its fingerprint differs every call - and the dedup never engages. A dedup test built on it would pass whether or not the fix is present; - the new tests use a stub with stable health. - -`IHostConnectivityProbe` was **not** resolved — it is a keep-or-delete decision, and the -`DriverHostStatus` table was re-created deliberately in the v3 initial migration, so deleting on -inference would be wrong. Split to Gitea **#521** with both options costed. - -### 2026-07-27 — §8.5 bookkeeping + §8.6 tier truth - -**13 plan files closed.** The 8 the register named, plus `mesh-phase4` (Task 9 landed `3a590a0c`), the -4th `stillpending` phase file, and `historian-tcp-transport` closed as **OBSOLETE** (it targets the -retired Wonderware sidecar; there is no Wonderware backend in the tree). Each carries a `closureNote` -naming the evidence, so the next reader can check rather than trust. - -**11 archreview live gates written back** from `STATUS.md`, which had recorded them passed since -2026-07-13/15 without ever updating the `.tasks.json` files: R2-01 #11, R2-02 #15/#18, R2-05 T15, -R2-06 T12, R2-07 T5/T6/T12/T14, R2-11 T22/T24. - -**R2-03 and R2-10 were NOT closed** — and reading `STATUS.md` rather than trusting its own headline -("Every Round-2 live gate is now GREEN") is what kept that honest. Its per-item detail says R2-03 is -"live-blocked on this data-less rig" and R2-10 verified the pipeline but "breaker-OPEN state not -forced". Both now carry an `openNote` recording the blocker: R2-03 needs a reachable driver fixture -feeding a VT; R2-10 needs a rapid idempotent write to a dead endpoint, because R2-09's -`ConnectionBackoff` throttles retries so failures never accumulate fast enough to open the Polly breaker. - -**Net effect (the point of the sweep):** ~140 phantom pending tasks are gone, so the genuinely -unexecuted **19-task AdminUI follow-ups plan** is now the largest open item in `docs/plans/` rather than -being buried. The remaining open plans are 8, and one of those (`mesh-phase6` Task 6) is a documented -`resolved-no-flip` decision rather than work. - -**Tier truth:** `docs/v2/driver-stability.md` now states plainly that its tier table is aspirational — -every driver runs Tier A, and `MemoryRecycle` / `ScheduledRecycleScheduler` have never engaged. It also -corrects a second stale claim the register did not flag: the **separate-Windows-service hosting** it -describes for Tier C no longer exists (Galaxy went to the mxaccessgw sidecar in PR 7.2; FOCAS went -in-process when its managed wire client landed). No runtime change — turning recycle on for two live -drivers deserves its own live gate, not a docs-pass side effect. Tracked as Gitea **#522**. - -### 2026-07-28 — the two things §8.3 deliberately did NOT build (1 of 2) - -**Sql and FOCAS now re-parse in place too.** The first pass excluded them for a real reason — each derives -more than options from config, so adopting new options alone would run a **new tag set against an old -connection/backend**, and a half-applied change is worse than a discarded one because it looks like it -worked. Rather than accept that, the blocker itself is fixed: each factory now exposes a **`ParseBinding`** -that returns *every* config-derived dependency as one value, and the driver adopts them **atomically**. - -| Driver | What one parse now yields | Adopted by | -|---|---|---| -| Sql | options + `ISqlDialect` + resolved connection string | `ApplyBinding`, which also rebuilds the provider factory, `Endpoint` and the `SqlPollReader` that captures all three | -| FOCAS | options + the `IFocasClientFactory` the `Backend` key selects | one assignment pair in `InitializeAsync` | - -Details worth keeping: - -- **Sql adopts BEFORE `BuildTagTable`**, so the tag table and the connection it will be polled over always - come from the same revision. -- **A test-injected `DbProviderFactory` survives a rebind** (`_explicitFactory`), so a re-derived dialect - cannot silently displace what a test passed in. -- **Re-resolving the connection string on reinit is a side benefit**: a rotated credential is picked up - without a process restart. -- A driver constructed **directly** gets no rebinder and keeps its constructor-supplied binding — every - existing `"{}"`-passing lifecycle test is unaffected by construction, not by luck. - -**The tests pin ATOMICITY, not merely "a re-parse happened"** — a test that only checked options would have -passed against the broken version. Verified by simulating the *half-fix* (adopt options, skip the backend): -2 of the 3 FOCAS tests go red. Reverting Sql's rebind turns its connection-string test red. Sql also asserts -that a reinit which **cannot resolve** its new connection leaves the previous binding whole, rather than -half-adopting. - -All 12 drivers now honour a changed config in place; `DriverSpawnPlanner`'s stop + respawn remains the outer -guarantee. Sql.Tests 226 / FOCAS.Tests 275 pass. - -### 2026-07-28 — the two things §8.3 deliberately did NOT build (2 of 2) - -> ⛔ **REMOVED — the periodic re-browse is gone from the tree (owner decision, Gitea #523; removal -> recorded in the next entry).** It shipped in `master` (`e77c8a35`, merged `914d70bf`) and this entry -> stays as the record of what was built and why. The objection is the one this entry does not weigh: the check is recurring, **unconfigurable** (the interval is a constructor parameter with no -> appsettings key) and **never live-gated** outbound traffic to real PLCs, CNCs and MTConnect Agents. -> Frequency of an unsolicited device browse is an operational decision, and the design treated it as an -> implementation detail. -> -> **Consequence to carry forward:** AbCip and FOCAS browse a live backend but implement no -> `IRediscoverable`, so after removal they have **no change signal at all** — a PLC re-download is -> invisible until someone re-browses by hand. TwinCAT and MTConnect keep their native event-driven -> signals and lose only redundant polling. The `IRediscoverable` consumption itself (`09a401b8`) is -> **not** affected: the driver tells us, we do not poll, and it costs nothing when nothing changes. -> If AbCip/FOCAS coverage matters later, the shape to reach for is event-driven — or a check that runs -> only when something else already indicates change (a reconnect, a deploy), not a wall-clock timer. - -**The discovery-drift detector is built, and gated.** The original objection stands and is now *encoded* -rather than used as a reason not to build: Modbus, S7, MQTT, AbLegacy and Sql echo authored config from -`DiscoverAsync`, so a diff for them is a tautology. The missing piece was a discriminator — and the -codebase already had one. `ITagDiscovery.SupportsOnlineDiscovery` is documented as "enumerates the tag set -from the **live backend** rather than replaying pre-declared/authored tags", and it separates the fleet -cleanly: **FOCAS, TwinCAT, MTConnect, AbCip** true; the five config-echo drivers explicitly false. - -The check runs only for a driver that is `SupportsOnlineDiscovery` **and** reports the new -`AuthoredDiscoveryRefs` (**nullable**, default null = opt out — an *empty* collection legitimately means -"nothing authored, everything discovered is new", and conflating the two would report total drift for a -driver that simply never opted in). - -**Where the value is:** AbCip and FOCAS browse a live backend but implement **no `IRediscoverable`**, so -before this they had *no* change signal at all — a periodic compare is the only way to notice a PLC -re-download. TwinCAT and MTConnect already have native signals (symbol-version, agent instanceId); drift -detection is complementary there. - -**A finding that changed the design.** FOCAS, TwinCAT and AbCip all **re-emit their authored tags into the -same `DiscoverAsync` stream** as the device-derived ones, so the browse picker can show an operator their -existing tags. That means discovered ⊇ authored *always*, and a **vanished** tag can never appear missing -— one whole direction is structurally undetectable for three of the four. Shipping a `Vanished` list that -is permanently empty for them would have been exactly the half-inert seam this register exists to remove. -So the driver declares `DiscoveryStreamIncludesAuthoredTags`, the detector does not compute the -undetectable half, and the operator message **says** "missing-tag detection unavailable for this driver" -rather than letting the absence of a missing-tag clause read as reassurance. **MTConnect is the only -driver where both directions work** — its stream is purely probe-model-derived. - -Behavioural details: a **failed browse is not drift** (an unreachable device would otherwise report every -authored tag as vanished — the health surface already covers unreachability); a **truncated** capture is -skipped for the same reason; an **unchanged** drift is reported once rather than re-stamping its timestamp -every interval; drift that **resolves** clears the prompt so the next one is not deduped against a stale -signature. Interval defaults to 5 minutes — it browses a real device, and a tag set changing is an -engineering event, not a runtime one. - -The comparison is a pure, separately-tested type (`TagSetDriftDetector`) rather than actor-inline logic. -14 new tests. The gate was verified load-bearing by deleting the `SupportsOnlineDiscovery` half — the -tautology-driver test goes red, and it asserts discovery was **never invoked** rather than merely "no -prompt raised", which would have passed for the wrong reason if the timer simply never fired. - -### 2026-07-28 — #523: the periodic re-browse removed - -The feature described in the entry above is **gone from the tree**. Removed: - -| Removed | Where | -|---|---| -| `TagSetDrift` + `TagSetDriftDetector` | `Runtime/Drivers/TagSetDrift.cs` (file deleted) | -| The drift timer, `DriftCheckTick`, `DefaultDriftCheckInterval`, the `driftCheckInterval` Props/ctor parameter, `QualifiesForDriftCheck`, `Start`/`StopDriftCheck`, `HandleDriftCheckAsync`, `CollectLeafRefs`, `_lastDriftSignature` | `DriverInstanceActor` | -| `ITagDiscovery.AuthoredDiscoveryRefs` + `.DiscoveryStreamIncludesAuthoredTags`, and their four implementations | `Core.Abstractions`; FOCAS, TwinCAT, MTConnect, AbCip | -| `DriverInstanceActorDriftCheckTests` (6), `TagSetDriftDetectorTests` (8) | `Runtime.Tests` | - -**What survives, deliberately.** `ITagDiscovery.SupportsOnlineDiscovery` stays — it predates drift -detection and is what the universal browse picker dispatches on. The `IRediscoverable` consumption -(`09a401b8`) is untouched: the driver tells us, we do not poll, and it costs nothing when nothing -changes. The `/hosts` re-browse chip stays and is now fed by that one source. - -**The consequence, stated plainly rather than buried.** **AbCip and FOCAS now have no tag-change signal -at all.** Both browse a live backend, neither implements `IRediscoverable`, and the periodic compare was -the only thing that would have noticed a PLC re-download or a CNC option install. That is the accepted -trade, not an oversight — it is recorded in `CLAUDE.md` §Change Detection so the next reader does not -have to rediscover it, together with the shape to reach for if the coverage is wanted back -(event-driven, or triggered by a reconnect/deploy, not a wall-clock timer). - -**Why the earlier entry's reasoning did not save it.** That entry defends the *design* — the gate is -real, the tautology drivers are correctly excluded, the undetectable direction is declared rather than -faked. All of that is true and none of it is the objection. The objection is to the feature existing at -all in that shape: it browsed real plant equipment on a recurring timer, at a frequency nobody could -configure, and that behaviour was never once observed against real hardware. A well-built mechanism for -something that should not run is still something that should not run. - -Build clean; `Runtime.Tests` 476 passed / 13 skipped (unchanged skip set — the stale -`EquipmentTagsDarkBatch4` group), FOCAS 275, TwinCAT 192, MTConnect 491, AbCip 342, -`Core.Abstractions` 266, all green. - -### 2026-07-28 — three silently-broken OpcUaClient integration tests (found by the full-suite run) - -Not a register item — found because the full-solution run for the work above showed **three** failing -suites where §9's 2026-07-27 entry had recorded two. `Driver.OpcUaClient.IntegrationTests` was failing 3 -of 4. - -**It reproduced identically on clean `master`** (verified in a throwaway worktree), so it was not caused -by this work — but it was also not what it first looked like. The chain of wrong guesses is worth -recording, because each one would have closed the investigation early: - -1. *"The fixture is down."* — `otopcua-opc-plc` was `Up 12 days (unhealthy)`. Restarted it, waited for - healthy. **Still 3/4 failing.** -2. *"The simulator's node set drifted."* — the tests expect `ns=3;s=StepUp`. Browsed the live server: - all three nodes present at exactly those ids. Read `ns=3;s=StepUp` through Client.CLI against the - same endpoint: **`Good`, value 1379.** -3. *"So the driver has a read defect."* — the status code was `2150825984`, which I first decoded by - hand as `BadAttributeIdInvalid` and started hunting a wrong `AttributeId`. **The decode was wrong** - (`0x80330000`, not `0x80350000`) — it is `BadNodeIdInvalid`, and grep found that exact constant on - the driver's own *local* fault path, reached without any wire request. - -The actual cause: **v3 made the driver resolve every reference through its authored `RawTags` table** -(RawPath → `TagConfig.nodeId` → live NodeId re-bound against the session's namespace table). A bare -`ns=3;s=StepUp` is no longer a reference the driver accepts. The smoke tests still passed bare node ids -and constructed the driver with an empty `RawTags`, so all three failed locally at the resolver — they -had been testing the unresolved-reference guard, against a perfectly healthy simulator. - -Fixed by seeding `OpcPlcProfile.RawTags` with the four nodes in the same `{"nodeId": …}` TagConfig shape -the deploy artifact delivers, and addressing them by RawPath. **4/4 pass.** - -Two things this says about the register itself: the 2026-07-27 "only pre-existing environment failures -remain" note was measured against an incomplete baseline (this suite was already broken and simply not -enumerated); and **fixture-gated integration suites rot invisibly** — a driver contract can change under -them and nothing on the local box notices, because the suite is expected to be red-or-skipped depending -on whether a remote container happens to be up. - -### 2026-07-28 — §7.2 priority 3: the `Phase7*` → `AddressSpace*` rename - -Applied across the four **live** docs (`VirtualTags.md`, `ScriptedAlarms.md`, `ScriptEditor.md`, -`v2/Architecture-v2.md`). The already-bannered historical files (`v2/phase-7-status.md`, -`v2/implementation/*`) were deliberately **left alone** — they are records of what existed at the time, -and renaming inside a historical record would falsify it. - -§7.2 was right that three of these are not renames, and each was resolved to the real member: - -| Cited | Reality | -|---|---| -| `Phase7CompositionResult` | → **`AddressSpaceComposition`** (and it now also carries the raw subtree, so the doc text was widened, not just renamed) | -| `Phase7Composer.ExtractTagFullName` | → **`ScriptTagCatalog.ExtractFullNameFromTagConfig`** | -| `Phase7EngineComposer.RouteToHistorianAsync` | **Gone, not renamed** — the routing moved into `HistorianAdapterActor` → the registered `IAlarmHistorianSink`. Called out inline so the next reader does not go looking for a renamed symbol. | - -**Found en route — the banner pass missed three files.** `GenericDriverNodeManager` is test scaffolding -(§3.4, its own source says "zero production references"), and the earlier remediation bannered -`AddressSpace.md` and `IncrementalSync.md` for it. But `VirtualTags.md`, `OpcUaServer.md` and -`ScriptedAlarms.md` all still presented it as **the** production dispatch path — `VirtualTags.md:3` even -built its opening explanation of driver-vs-virtual dispatch on it. All three now carry the same -correction naming the real chain (`OpcUaPublishActor` → `SdkAddressSpaceSink` → `OtOpcUaNodeManager`, two -namespaces). Banners, not rewrites — the underlying rewrite stays owed, consistent with how §7.2 treats -the other two. - -### 2026-07-28 — G-7, and what the audit missed about it - -The register called this "pre-existing (OpcUaClient + Galaxy absent too); no new driver was added" and -ranked it last. Reading both maps side by side makes it sharper than that: - -- The AdminUI's `TagConfigValidator` covers **11 of 12** driver types (all but Galaxy). -- The deploy-time `EquipmentTagConfigInspector` covers **6**. - -So the gate that runs at *authoring* is stronger than the gate that runs at *deploy* — and the deploy -gate is the only one config sees when it arrives by **CSV import or API** rather than through the modal. -That is the wrong way round. - -**MQTT was a plain hole:** `MqttTagDefinitionFactory.Inspect` has existed since the driver merged and -nothing ever called it. Now wired. The inspector's own doc-comment also claimed "unmapped drivers -(Galaxy, OpcUaClient) are skipped exactly as in the AdminUI validator" — untrue, OpcUaClient *is* mapped -there; corrected. - -**A reflection guard replaces the hand-maintained list**, same lesson as G-4: the existing -`EquipmentTagConfigInspectorTests` enumerates driver-type strings by hand, so it guards renames but can -never notice a gap — a newly-inspectable driver simply never appears in it. - -⚠️ **The first version of that guard was hollow, and only the falsification step caught it.** It -discovered candidates via `Assembly.GetReferencedAssemblies()`, which is **circular**: the C# compiler -elides a reference nothing in the IL uses, so removing MQTT from the map also removed its Contracts -assembly from ControlPlane's manifest, and the sweep stopped looking for exactly the driver that had just -gone missing. It passed green with the bug reintroduced. Rewritten to scan the output *directory* -(MSBuild copies project-referenced assemblies regardless of IL usage) — re-falsified, and it now fails -naming `MqttTagDefinitionFactory`. It also carries a positive control asserting discovery found more than -one factory, since an empty expected-set is satisfied by an empty map. - -**Residual, deliberately not papered over.** Sql, MTConnect, Calculation and OpcUaClient have **no** -`Inspect` in a Contracts assembly — their only checker is the AdminUI `*TagConfigModel.Validate()`, which -lives in the AdminUI project and cannot be referenced from ControlPlane. Closing that means promoting -those models (or an equivalent parser) into each driver's Contracts assembly — a real piece of work, not -a map entry. Recorded in the inspector's own doc-comment so the next reader finds it at the code. - -### 2026-07-28 — the 13 stale `EquipmentTagsDarkBatch4` skips - -§4.4 flagged these as stale and it was right: the reason said "dark until Batch 4", Batch 4 shipped as -v3.0, and the equipment-tag path was **retired** rather than lit — so every one of them was waiting on -something that had already happened and gone the other way. - -**Unskipping them in bulk would have failed, and deleting them in bulk would have thrown away live -coverage.** Each was resolved on its own merits: - -| Disposition | Tests | Why | -|---|---|---| -| **Revived** onto the raw/UNS shape | 4 primary-gate cases (`DriverHostActorPrimaryGateTests`), the cluster-scoping rebuild (`OpcUaPublishActorRebuildTests`), the real-SDK materialisation E2E (`AddressSpaceApplierHierarchyTests`) | The *behaviour* never went dark — only the v2 seeding did. Each seeds the v3 chain (RawFolder → Driver → Device → Tag, + `UnsTagReference`) instead. | -| **Folded** into a live parity test | array intent, historize intent, native-alarm/writable intent | `DeploymentArtifactRawUnsParityTests` already compared `RawTagPlan` **record-equal**, and `RawTagPlan` carries all three fields — so widening its corpus by two tags covers what three empty placeholder suites had been promising. Those suites are deleted. | -| **Deleted** | equipment-namespace `EquipmentTags` parse, both `{{equip}}`-token suites | The subjects are retired. The token tests used the **dot-joint** `{{equip}}.X` form, whose resolver (`EquipmentScriptPaths.DeriveEquipmentBase`) was deleted in Batch 3; the slash-joint form is covered live by `DeploymentArtifactEquipRefParityTests` (4 tests). | - -Three things worth keeping: - -- **The revivals are the point, not the unskipping.** `Unknown_role_single_driver_services_write` asserts - `recorder.Writes.ShouldNotBeEmpty()` — it needs the write to actually reach a driver, which needs a - routable tag. **Verified by deleting the `UnsTagReference` from the seed: it and - `Known_role_wins_over_member_count` go red.** Same check on the cluster-scoping test (re-home the SITE-A - node to MAIN ⇒ red). The three *denial* cases in that file were passing all along — they deny before - routing — which is exactly why the gap was invisible. -- **Widening a parity corpus needed a non-parity assertion to go with it.** A composer that dropped array - intent *entirely* would still satisfy `decoded.RawTags.ShouldBe(composed.RawTags)`, because the artifact - decode would drop it too and the two empties compare equal. Parity cannot distinguish "both right" from - "both blind", so the widened test also asserts the composed values directly. -- **The reason constant is deleted, replaced by a note explaining why.** A skip reason is a claim with an - expiry date, and this one outlived its own condition by a release. Both `DarkAddressSpaceReasons` files - now say so, so the next person writing one states a condition a reader can actually check. - -Skipped counts: `Runtime.Tests` **13 → 3**, `OpcUaServer.Tests` **4 → 1**. Every remaining skip is an -`EquipmentDeviceBindingRetired` tombstone — deliberate, and §4.4 already calls those fine as-is. - -### 2026-07-27 — §8.4 dispatch-map parity (G-1 … G-6) - -**The maps had to become data before they could be guarded.** A Razor `@switch` compiles into -`BuildRenderTree`'s IL, so nothing can enumerate its cases — that is the whole reason G-1 and G-2 -survived review while the sibling picker guard stayed green (the picker test works only because -`RawDriverTypeDialog` keeps its data in a *field* the markup enumerates). Both modals now render from -`DriverConfigFormMap` / `DeviceFormMap` via ``, and being `public` those maps need -none of the picker test's `BindingFlags.NonPublic` fragility. - -| Gap | Resolution | -|---|---| -| G-1 | `CalculationDriverForm.razor` added — `RunTimeout` was unauthorable through the UI. | -| G-2 | Sql / MTConnect / Calculation are declared **single-connection** rather than given hollow device forms — each holds one connection at the driver level, so a per-device endpoint editor would be meaningless. | -| G-3 | `DriverConfigModal`'s hardcoded `Galaxy or Mqtt` replaced by `DeviceFormMap.IsSingleConnection`. | -| G-4 | `DriverFormMapParityTests` (5 cases, both directions) + `DriverDispatchMapParityTests` (3). | -| G-5 | `CsvColumnMap` entries for Sql, Mqtt, MTConnect. | -| G-6 | `RawBrowseCommitMapper` Sql branch — **and the browser end too**, which the register missed. | - -**G-6 was bigger than filed.** The register said the mapper lacked a Sql branch. It also turned out that -`SqlBrowseSession` emitted **no `AddressFields` at all**, so a browsed leaf carried only a column name — -and a column name alone cannot address a Sql tag, which needs its table. The browser now travels -schema/table/column and the mapper builds a `WideRow` `SqlTagConfigModel` from them. A branch alone -would have produced a half-built blob. - -**Two extra findings while writing the guards:** - -- The G-6 test found **Modbus and Calculation** also fall through to the generic `{"address": …}` key. - Both are correct — neither is browsable — so the test asserts the fall-through set **equals** a - documented non-browsable list, in both directions. A one-way check would let a newly-browsable driver - be quietly added to the exclusion list instead of getting a branch. -- `TagConfigDriverTypeNameGuardTests` was **hollow**: it enumerated a hand-written `TheoryData` that had - already drifted (omitting Galaxy, Sql and Mqtt), so it guarded against renames but not gaps — a new - `DriverTypeNames` constant would simply never appear. Now enumerated from the map, paired with a - coverage test in the other direction. - -**Live-verified on docker-dev** (`localhost:9200`, central pair rebuilt), because this repo has no bUnit -and no unit test can cover Blazor parameter binding: opened Calculation's config — the form renders where -it previously showed "No typed config form" — typed `3500`, saved, reopened, and the value **persisted**, -proving the `DynamicComponent` two-way binding round-trips. Sql's form renders fully (regression check on -the conversion) and now reads "This driver holds a single connection, authored above" (G-3). The rebuilt -image also showed `RediscoveryNeededUtc`/`RediscoveryReason` on the wire, confirming §8.2 end-to-end. - -### 2026-07-27 — §8.3 #516 driver config edits silently discarded - -Decision: **both** — per-driver re-parse *and* the seam respawn. Reading the drivers split the -"per-driver" half in two, which the register did not anticipate: - -- **Re-parse in place** (`Modbus`, `AbLegacy`, `OpcUaClient`) — `ParseOptions` extracted from each - factory, called from `InitializeAsync` behind a `HasConfigBody` guard so `"{}"` still keeps the - constructor options. -- **Respawn-only, deliberately NOT re-parsed** (`Sql`, `FOCAS`) — each builds **more than options** - from config: Sql's `ISqlDialect` + resolved connection string, FOCAS's client-factory backend, both - injected at construction. Adopting new options alone would run a **new tag set against an old - connection**. Half a re-parse is worse than none. Their doc-comments now say so. - -**The seam respawn is the load-bearing half.** `DriverSpawnPlanner` now routes a changed -`DriverConfig` to `ToStop` + `ToSpawn`, making the factory the single parse authority. -`ToApplyDelta` is consequently always empty from the reconcile path. This **reverses the deliberate -decision documented at `DriverSpawnPlan.cs:49-50`** ("a pure DriverConfig change stays an in-place -delta — no reconnect"). That reasoning was right about resilience and wrong about the driver; the -accepted price is a reconnect on every config edit. - -Two seals removed from `ApplyChildDelta`: it overwrote the cached `Spec` **synchronously, before the -child had dequeued the message**, so the host immediately believed the new config was live and the -next reconcile computed no delta — sealing the drift permanently; and it `Tell`d with no -`Receive` registered, so a failed reinit (including Galaxy's deliberate -`NotSupportedException`) dead-lettered. - -**A new visibility gap this change exposed, not created:** a factory throw is a *config* error -(`TryCreate` is pure parsing; device I/O happens later), and `SpawnChild` catches it and silently -substitutes a stub. Previously only a brand-new driver could hit it; now an ordinary config edit can. -Raised from `Warning` to `Error` with an actionable message. It still does **not** fail the -deployment — making it do so would let one malformed driver block a fleet deploy, so that is a -deliberate follow-up rather than a drive-by change. - -**Tests.** Every pre-existing reinit test in the five suites passes `"{}"` — precisely the input a -guarded re-parser treats as "keep the constructor options", so they were blind to this defect *by -construction*. The new tests pass **changed** JSON. The Modbus one was verified falsifiable by -deleting the re-parse line (goes red). Two existing tests asserted the old behaviour and were -rewritten: `DriverSpawnPlannerTests` (two cases), and -`DriverHostActorUnreadableArtifactTests.Dropping_a_drivers_last_tag_does_clear_its_subscription` — -a **positive control** for a sibling absence assertion, whose observable moved from `UnsubscribeAsync` -to `ShutdownAsync` because the teardown now happens by stopping the child rather than emptying its -desired set. Same event, different route; the control still calibrates the settle window. - -Full-solution run: only pre-existing environment failures remain — `Host.IntegrationTests` (3, -verified identical on the pre-change tree) and `Driver.AbLegacy.IntegrationTests` (4, docker -fixture-gated, and notably these **fail rather than skip**, unlike every other fixture-gated suite). - ---- - -*§1–§8 generated 2026-07-27 against `master` @ `90bdaa44` by five parallel source-verification agents. -Every `path:line` reference was read from source. Items marked CANNOT VERIFY require hardware, a -VPN'd gateway, or a live rig and are listed as unproven rather than assumed.* - -*Remediated 2026-07-27/28; merged `914d70bf`. §9 is the execution record and the live status — where §1–§8 -and §9 disagree, **§9 is current**. Path/line references in §1–§8 are as at `90bdaa44` and several no -longer resolve, by design: the code they pointed at was deleted.* diff --git a/docs/Raw.md b/docs/Raw.md index 5bc39512..69c8a6dc 100644 --- a/docs/Raw.md +++ b/docs/Raw.md @@ -70,7 +70,9 @@ Signal-level calculated tags are ordinary raw tags bound to a `Calculation` driv `ctx.GetTag("")`. See `docs/plans/2026-07-15-calculation-driver-mini-design.md` for the full design. Highlights: -- One auto-created default `Engine` device; per-tag `TagConfig` is +- ⚠️ **Correction (2026-07-28): the "auto-created default `Engine` device" below never shipped.** + `"Engine"` has zero occurrences in `src/`, and nothing special-cases `Calculation` at device + creation — author the device like any other. Per-tag `TagConfig` is `{ "scriptId": "…", "changeTriggered": true, "timerIntervalMs": 5000 }`. - The host feeds dependency values via the new `IDependencyConsumer` capability + a `DependencyConsumerMuxAdapter` on the per-node dependency mux; calc-of-calc chains work diff --git a/docs/drivers/Calculation.md b/docs/drivers/Calculation.md new file mode 100644 index 00000000..f198f6d4 --- /dev/null +++ b/docs/drivers/Calculation.md @@ -0,0 +1,133 @@ +# Calculation Driver + +Getting-started guide for the `Calculation` **pseudo-driver** — signal-level calculated tags computed +by C# scripts over other tags' live values. For the design rationale read +[`docs/plans/2026-07-15-calculation-driver-mini-design.md`](../plans/2026-07-15-calculation-driver-mini-design.md); +[`docs/Raw.md`](../Raw.md) covers where it sits in the `/raw` tree. + +## What it talks to + +**Nothing.** There is no backend, no endpoint, no connection and no discovery. Its inputs are other +tags' live values, fed to it by the driver host; its outputs are ordinary raw tags. That is why it is +called a pseudo-driver — it occupies a driver slot so its tags get the same authoring, deployment, +fan-out, historization and alarming as any device tag, without any of the I/O. + +Because it is an ordinary driver from the host's point of view, a calc tag's value flows out through +`OnDataChange` exactly like a Modbus register's — so the dual-namespace fan-out, historian +registration and **calc-of-calc** (a calc tag reading another calc tag, via mux re-entry) all work +with no special-casing. + +## Calculation vs. Virtual Tags — which one to use + +They are close cousins and easy to confuse: + +| | **Calculation** driver | **Virtual tags** | +|---|---|---| +| Lives in | the `/raw` tree, as a raw tag | the UNS tree, on an equipment | +| Identity | a **RawPath** | `{Equipment}/{EffectiveName}` | +| Reads by | `ctx.GetTag("")` — literal raw paths | `ctx.GetTag("{{equip}}/")` — equipment-relative | +| Use when | the computation is a property of the *signal* (scaling, a derived rate, combining two registers) | the computation is a property of the *equipment* (an OEE roll-up, a per-machine state) | + +If the same script should apply to many machines, you want a virtual tag with `{{equip}}`. If a single +derived signal belongs next to the device tags it is derived from, you want a calc tag. + +## Authoring a calc tag + +Calc tags are authored in `/raw` like any other tag: create a `Calculation` driver, add a device under +it, and add tags under that. + +> ⚠️ **`docs/Raw.md` says a default `Engine` device is auto-created. It is not** — verified 2026-07-28, +> the string `"Engine"` has zero occurrences in `src/`, and nothing in the AdminUI or the config layer +> special-cases `Calculation` at device-creation time. Create the device yourself; the name is +> arbitrary but becomes a segment of every calc tag's RawPath, so pick it deliberately. + +Per-tag `TagConfig`: + +```json +{ + "scriptId": "scr-oven-delta", + "changeTriggered": true, + "timerIntervalMs": 5000 +} +``` + +| Field | Meaning | +|---|---| +| `scriptId` | FK to the shared `Script` entity. **Required** — a tag with no identifiable script is not a calc tag and is rejected at parse. | +| `changeTriggered` | Re-evaluate whenever any declared dependency changes. | +| `timerIntervalMs` | Also re-evaluate on this cadence. Absent ⇒ change-trigger only. | + +`scriptSource` is **resolved at deploy time** from `scriptId` and travels in the artifact — the driver +never reads the config DB. A `scriptId` that resolves to empty source still maps: the driver logs it +and the tag simply never computes, rather than the whole driver failing. + +## Dependencies are discovered from the script + +You do not declare dependencies. The deploy pipeline extracts them from the script's **literal** +`ctx.GetTag("…")` reads and registers exactly those RawPaths with the dependency mux. + +⚠️ **Literal means literal.** A path built at runtime — `ctx.GetTag("Plant/" + line + "/Temp")` — is +invisible to the extractor, so that dependency is never fed and the tag never sees its value. Write +the path as a constant string. + +Two deploy-time gates run over the result: + +- **`scriptId` existence** — a calc tag naming a script that does not exist fails the deploy. +- **Cycle detection (Tarjan)** — calc-of-calc is supported, but a cycle fails the deploy rather than + livelocking at runtime. + +## Trigger and publish semantics + +- **Nothing publishes until every declared dependency has arrived at least once.** Before that a read + returns `BadWaitingForInitialData`. This is the same gate `VirtualTagActor` uses, and it exists so a + calculation cannot publish a confident-looking value derived from defaults. +- **Equal results are deduped** — an evaluation producing the same value as last time does not + re-publish. +- **Timers are grouped by interval**, one timer per distinct interval, not one per tag. + +## Error semantics + +An evaluator failure publishes **Bad quality carrying the last-known value** — the value is not +zeroed, because a stale-but-flagged reading is more useful to an operator than a fabricated one. It +publishes: + +- **once per Good→Bad transition**, not on every failed evaluation (a permanently broken script does + not flood), and only after all dependencies have arrived; +- with a `ScriptLogEntry` on the `script-logs` topic, attributed to the tag's `scriptId`, so the + AdminUI `/script-log` page names the failing script; +- recovery to Good is **force-published**, bypassing the dedup, so a recovered tag does not sit Bad + waiting for its value to change. + +A historized Bad records `BadInternalError`. + +## Config + +```json +{ "runTimeoutMs": 2000 } +``` + +| Key | Default | Notes | +|---|---|---| +| `runTimeoutMs` | `2000` (2 s) | Per-script wall-clock evaluation budget, in **milliseconds** — not a `TimeSpan` string, unlike most driver timeouts in this repo. Parity with the virtual-tag evaluator. | + +⚠️ **`runTimeoutMs` is ignored on a config *reinit*** — the evaluator's timeout is fixed at +construction. A changed value takes effect because a changed `DriverConfig` respawns the driver +(#516), not because the driver re-reads it in place. + +There is nothing else — no endpoint, no credentials, no pooling. The driver is **single-connection** +in the AdminUI's sense (nothing lives on the device). + +## Capabilities + +`IDriver` (lifecycle + always-Connected health), `IDependencyConsumer` (the host feeds it upstream +values), `ISubscribable` (computed values flow out), `IReadable` (returns the last computed +snapshot). + +No `IWritable` — a computed value has no meaningful write target. No `ITagDiscovery` — there is +nothing to browse. + +## See also + +- [`docs/Raw.md`](../Raw.md) — the `/raw` authoring tree +- [`docs/VirtualTags.md`](../VirtualTags.md) — the equipment-scoped alternative +- [`docs/ScriptEditor.md`](../ScriptEditor.md) — the Roslyn-backed script editor diff --git a/docs/drivers/README.md b/docs/drivers/README.md index 5ef9f344..0ddd9856 100644 --- a/docs/drivers/README.md +++ b/docs/drivers/README.md @@ -35,8 +35,8 @@ Driver **factories** are registered at startup in `DriverFactoryRegistry` (`src/ | [OPC UA Client](OpcUaClient.md) | `Driver.OpcUaClient` | B | OPCFoundation `Opc.Ua.Client` | IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IAlarmSource, IHistoryProvider, IHostConnectivityProbe | Gateway/aggregation driver — the only driver implementing driver-side `IHistoryProvider` (forwards HistoryRead to the upstream server). Opens a single `Session` against a remote OPC UA server and re-exposes its address space. Owns its own `ApplicationConfiguration` (distinct from `Client.Shared`) because it's always-on with keep-alive + `TransferSubscriptions` across SDK reconnect, not an interactive CLI | | [MQTT](Mqtt.md) | `Driver.Mqtt` (+ `.Browser`, `.Contracts`) | A | [MQTTnet 5](https://github.com/dotnet/MQTTnet) | IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe, IRediscoverable | **Push, not poll, and read-only** — the broker delivers PUBLISH frames the driver forwards straight to `ISubscribable.OnDataChange`; `IReadable` serves the last retained/received value from cache, and there is **no `IWritable`** (nothing publishes back). Subscriptions are indexed **by topic filter, not by topic**, so wildcard tags survive a reconnect. A rejected CONNACK throws `MqttConnectRejectedException`: unrecoverable codes → `Faulted` + supervisor stop, transient → retry. **Two modes:** `Plain` binds a tag to a concrete topic + JSON path; `SparkplugB` decodes vendored Eclipse-Tahu protobuf under one `spBv1.0/{groupId}/#` subscription and binds by the `group/edgeNode/device?/metric` tuple (birth/alias/seq-gap state machine, death→STALE, scoped rebirth NCMD). `IRediscoverable` fires on a DBIRTH but is **inert platform-side** — see [#507](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/507) | | [MTConnect](MTConnect.md) | `Driver.MTConnect` (+ `.Contracts`) | — | Hand-rolled `System.Xml.Linq` HTTP/XML client against a vendor-neutral MTConnect Agent (`/probe`, `/current`, `/sample`) — no TrakHound dependency (dropped; see the driver doc) | IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe, IRediscoverable | Read-only by design (no `IWritable`). Production data plane is entirely `ISubscribable`/`OnDataChange` — `IReadable` is CLI/test-only. Browse is free via the Wave-0 universal discovery browser, no bespoke browser project. `IRediscoverable`/`IHostConnectivityProbe` have no server-side consumer yet — a pre-existing fleet-wide gap, not MTConnect-specific | -| **Sql** (no dedicated page — see [design](../plans/2026-07-15-sql-poll-driver-design.md)) | `Driver.Sql` (+ `.Browser`, `.Contracts`) | A | `Microsoft.Data.SqlClient` behind an `ISqlDialect` seam (`SqlServerDialect` shipped; Postgres/ODBC deferred) | IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe | Read-only DB-poll driver with a bespoke schema browser. **Credentials are never persisted** — the deploy gate rejects a stored `connectionString` (#498) and catalog identifiers are validated against the live catalog's own spelling (#496). `SupportsOnlineDiscovery => false` by design (the bespoke browser wins). `SqlTagModel.Query` is deferred to P3 and rejected end-to-end by parser, validator, planner and editor. Merged `4ad54037` | -| **Calculation** (see [Raw.md](../Raw.md#the-calculation-driver)) | `Driver.Calculation` | A | none — computes from other tags' RawPaths | IDriver, IDependencyConsumer, ISubscribable, IReadable | **Pseudo-driver**, not a wire protocol: signal-level tags computed from other tags via a script, with deploy-time `scriptId`-existence + Tarjan cycle gates. Its probe is parse-only (no backend to reach). ⚠️ **No typed config form** — `DriverConfigModal` has no `Calculation` case, so `RunTimeout` is currently unauthorable through the AdminUI | +| [Sql](Sql.md) | `Driver.Sql` (+ `.Browser`, `.Contracts`) | A | `Microsoft.Data.SqlClient` behind an `ISqlDialect` seam (`SqlServerDialect` shipped; Postgres/ODBC deferred) | IDriver, ITagDiscovery, IReadable, ISubscribable, IHostConnectivityProbe | Read-only DB-poll driver with a bespoke schema browser. **Credentials are never persisted** — the deploy gate rejects a stored `connectionString` (#498) and catalog identifiers are validated against the live catalog's own spelling (#496). `SupportsOnlineDiscovery => false` by design (the bespoke browser wins). `SqlTagModel.Query` is deferred to P3 and rejected end-to-end by parser, validator, planner and editor. Merged `4ad54037` | +| [Calculation](Calculation.md) | `Driver.Calculation` | A | none — computes from other tags' RawPaths | IDriver, IDependencyConsumer, ISubscribable, IReadable | **Pseudo-driver**, not a wire protocol: signal-level tags computed from other tags via a script, with deploy-time `scriptId`-existence + Tarjan cycle gates. Its probe is parse-only (no backend to reach). `runTimeoutMs` is authorable through `CalculationDriverForm` (the "no typed config form" gap — G-1 — was closed 2026-07-27) | | [Historian.Gateway](../Historian.md) | `Driver.Historian.Gateway` | — | `ZB.MOM.WW.HistorianGateway.Client` gRPC (`historian_gateway.v1`) | IHistorianDataSource (server-side read backend) + alarm `SendEvent` writer + `WriteLiveValues` recorder + `IHistorianProvisioning` | Not a tag driver — the sole historian backend. Registers `GatewayHistorianDataSource : IHistorianDataSource` for HistoryRead and serves alarm-write + continuous historization through the gateway. No `IDriver`/`ITagDiscovery` surface. (The retired Wonderware sidecar backend it replaced is documented at [Historian.Wonderware.md](Historian.Wonderware.md).) | ## Per-driver documentation diff --git a/docs/drivers/Sql.md b/docs/drivers/Sql.md new file mode 100644 index 00000000..9d32457f --- /dev/null +++ b/docs/drivers/Sql.md @@ -0,0 +1,174 @@ +# SQL Poll Driver + +Getting-started guide for the `Sql` driver — a **read-only** Equipment-kind driver that polls rows +out of a relational database and publishes them as OPC UA variables. For the design rationale read +[`docs/plans/2026-07-15-sql-poll-driver-design.md`](../plans/2026-07-15-sql-poll-driver-design.md); +for the build-vs-plan record read +[`docs/plans/2026-07-24-sql-poll-driver.md`](../plans/2026-07-24-sql-poll-driver.md). + +## What it talks to + +An ordinary SQL database — typically a MES / LIMS / historian staging table that some other system +already writes to. There is no protocol here and nothing to "connect" to a machine: the driver runs +`SELECT`s on a timer and turns cells into tag values. + +**v1 constructs SQL Server only** (`Microsoft.Data.SqlClient`). `SqlProvider` also declares +`Postgres`, `MySql`, `Odbc` and `Oracle`, but those are **reserved names, not working backends** — +authoring one fails the driver rather than silently falling back. + +**Read-only, structurally.** `SqlDriver` does not implement `IWritable`, so no configuration can +produce a write. The `allowWrites` config field exists and is **inert**; the factory *warns* when it +is authored `true` rather than failing, on the grounds that the flag cannot do harm but an operator +who believes writes are enabled can. + +## Tag models + +A tag says how to find its value. Two models ship; a third is deferred. + +### `KeyValue` — the EAV shape + +One row per tag. The driver matches `keyColumn = keyValue` and reads `valueColumn`. + +```json +{ + "model": "KeyValue", + "table": "dbo.TagValues", + "keyColumn": "TagName", + "keyValue": "Line1.Oven.Temp", + "valueColumn": "Value", + "timestampColumn": "UpdatedUtc" +} +``` + +### `WideRow` — one row, many signals + +The tag names its `columnName`; the row is picked either by a `whereColumn`/`whereValue` pair or by +newest-timestamp. + +```json +{ + "model": "WideRow", + "table": "dbo.LineStatus", + "columnName": "OvenTemp", + "whereColumn": "LineId", + "whereValue": "L1" +} +``` + +```json +{ + "model": "WideRow", + "table": "dbo.Readings", + "columnName": "OvenTemp", + "topByTimestamp": "CapturedUtc" +} +``` + +### `Query` — deferred to P3 + +Named arbitrary-`SELECT`. **The v1 parser rejects it**, and that refusal is enforced end-to-end — +parser, validator, planner and the AdminUI editor all decline it rather than accepting the value and +ignoring it later. + +## Optional per-tag fields + +| Field | Effect | +|---|---| +| `timestampColumn` | Source timestamp for the value. Absent ⇒ the driver stamps the poll time. | +| `type` | Explicit OPC UA type override. Absent ⇒ inferred from the result set's column metadata. | +| `pollIntervalMs` | Per-tag override of the instance's `defaultPollInterval`. | + +## Driver config + +```json +{ + "provider": "SqlServer", + "connectionStringRef": "MesStaging", + "defaultPollInterval": "00:00:05", + "operationTimeout": "00:00:15", + "commandTimeout": "00:00:10", + "maxConcurrentGroups": 4, + "nullIsBad": false +} +``` + +| Key | Default | Notes | +|---|---|---| +| `provider` | `SqlServer` | The only provider v1 constructs. | +| `connectionStringRef` | — | **A name, never a connection string.** See below. | +| `defaultPollInterval` | `00:00:05` | Applied to any group without its own interval. | +| `operationTimeout` | `00:00:15` | Client-side wall-clock deadline; a breach surfaces `BadTimeout`. | +| `commandTimeout` | `00:00:10` | ADO.NET server-side backstop. **Must be less than `operationTimeout`** — it is the backstop, not the deadline. | +| `maxConcurrentGroups` | `4` | Connection-pool guard: cap on group queries in flight. | +| `nullIsBad` | `false` | `false` ⇒ a NULL cell publishes **Uncertain**; `true` ⇒ **Bad**. | +| `allowWrites` | — | **Inert.** v1 has no write path; authoring `true` logs a warning. | + +### Credentials never enter the deployment artifact + +`connectionStringRef` is a **name** resolved at Initialize from the environment / secret store — e.g. +`Sql__ConnectionStrings__MesStaging`. There is no field that holds a connection string, so a sealed +deployment artifact cannot carry database credentials even by accident. + +A useful side effect of the in-place reinit added for [#516](#): a **rotated credential is picked up +on the next config apply without a process restart**, because the driver re-resolves the reference +when it re-parses. + +## How polling works + +Tags are **grouped by source** and one query is issued per group, not per tag — `SqlGroupPlanner` +emits a `SqlQueryPlan` per distinct group key. Grouping is deterministic (groups appear in the input +order of their first member, members keep their input order, parameters keep first-appearance order), +so an unchanged configuration produces byte-identical SQL. + +⚠️ **Group keys are case-sensitive on identifiers.** Two tags authored `dbo.T` and `DBO.T` plan as +**two** groups — one extra round-trip, not a correctness problem, but worth knowing if a group count +looks higher than expected. + +## SQL injection: how the driver is safe + +The two classes of authored string are handled differently, and deliberately so: + +- **Value-bearing** fields (`keyValue`, `whereValue`) are bound as `DbParameter`s. They are never + concatenated into command text. +- **Identifier-bearing** fields (`table`, `keyColumn`, `valueColumn`, `timestampColumn`, + `columnName`, `whereColumn`, `topByTimestamp`) cannot be parameterised in SQL. They are validated + against **`INFORMATION_SCHEMA`** and dialect-quoted before they reach a command. + +The parser deliberately does **not** sanitise either kind — sanitising a value would corrupt +legitimate data, and sanitising an identifier would mask an authoring error the catalog check reports +properly. + +⚠️ **An unreadable catalog FAULTS the driver rather than rejecting every tag** (Gitea #496). This is +the safer failure: "I cannot verify these identifiers" and "these identifiers are invalid" are +different conditions, and reporting the second when the first is true would send an operator hunting a +configuration bug that does not exist. The catalog gate also substitutes the catalog's own spelling of +an identifier, so a case difference between the authored blob and the database resolves rather than +failing. + +## Browse + +`SqlDriverBrowser` browses the live catalog — schema → table → column — so the `/raw` browse picker +can commit columns as tags. A committed leaf carries **schema, table and column**: a column name alone +cannot address a SQL tag, and an earlier version that emitted only the column produced tags whose +typed editor opened empty. + +## Authoring in the AdminUI + +- **Driver config** — `/raw` → driver node → *Configure*. Sql is a **single-connection** driver: the + connection lives on the driver, not on a device, and the modal says so. +- **Tags** — the typed `SqlTagConfigModel` editor, or CSV import (Sql has typed CSV columns), or + browse-commit from the picker. + +## Capabilities + +`IDriver`, `ITagDiscovery`, `IReadable`, `ISubscribable`, `IHostConnectivityProbe`. + +No `IWritable` (structural, above). No `IRediscoverable` — a SQL schema change is not signalled, and +`DiscoverAsync` echoes authored config rather than enumerating the database, so this driver is +deliberately excluded from any discovery-diff machinery. + +## See also + +- [`docs/Raw.md`](../Raw.md) — the `/raw` authoring tree +- [`docs/drivers/README.md`](README.md) — the driver index +- Gitea **#496** / **#497** / **#498** — the shipped follow-ups