From e77c8a3569820429b25ecbcd336415bba41a9d61 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 28 Jul 2026 00:48:39 -0400 Subject: [PATCH] feat(drivers): tag-set drift detection, gated on SupportsOnlineDiscovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second thing #516/§8.2 deliberately did not build. The original objection stands — Modbus, S7, MQTT, AbLegacy and Sql echo authored config from DiscoverAsync, so diffing discovered against authored is a tautology for them — but it is now ENCODED rather than used as a reason not to build. The discriminator already existed: 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 and AbCip true; the five config-echo drivers explicitly false. The check runs only for a driver that is SupportsOnlineDiscovery AND reports the new AuthoredDiscoveryRefs. That is nullable rather than empty-by-default on purpose: an empty collection legitimately means "nothing authored, everything discovered is new", so conflating the two would report total drift for a driver that 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. A finding that changed the design: FOCAS, TwinCAT and AbCip all re-emit their authored tags into the same DiscoverAsync stream as device-derived ones, so the browse picker can show an operator their existing tags. Discovered is therefore always a superset of authored, 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 the half-inert seam this whole exercise removes. 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. Behaviour: 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; drift that resolves clears the prompt. Interval defaults to 5 minutes — it browses a real device, and a tag set changing is an engineering event, not a runtime one. It reuses the Stage-2 surfacing, raising the same /hosts re-browse prompt, and never rebuilds the served address space. Comparison logic is a pure, separately-tested type rather than actor-inline. 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 never fired. Full suite green except the same three pre-existing fixture-gated integration suites, identical counts. --- CLAUDE.md | 29 +- deferment.md | 41 +++ .../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, 770 insertions(+), 7 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 7ef172fb..5465302f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -188,10 +188,31 @@ line and injected nothing. Gone with it: `HandleDiscoveredNodes`, `PartitionDisc browse picker drives it through `Commons/Browsing/DiscoveryDriverBrowser`, which never went through the actor. -**Deliberately NOT built: a discovered-vs-authored drift detector.** Modbus, S7, MQTT, AbLegacy and Sql -all *echo authored config* from `DiscoverAsync` rather than browsing the device, so such a diff is -permanently empty for them — another plausible-looking inert seam. `IRediscoverable` is the -trustworthy signal because the driver asserts it deliberately. +**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. ⚠️ **`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 aebcf013..c3e619af 100644 --- a/deferment.md +++ b/deferment.md @@ -530,6 +530,47 @@ 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) + +**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-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 cc713a1b..76d2009d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ITagDiscovery.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/ITagDiscovery.cs @@ -38,4 +38,32 @@ 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 05ad2904..60aff54f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -1036,6 +1036,17 @@ 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 b01ef4a4..72b28f91 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -517,6 +517,17 @@ 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 9c60197c..384d4f5b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -907,6 +907,19 @@ 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 4acdbdf2..6e960bdd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -404,6 +404,17 @@ 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 5fc2139e..19e2331e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -1,5 +1,6 @@ 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; @@ -32,6 +33,11 @@ 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); @@ -121,6 +127,15 @@ 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(); @@ -181,6 +196,14 @@ 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(); @@ -214,6 +237,8 @@ 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. @@ -225,7 +250,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers IDriverHealthPublisher? healthPublisher = null, string? clusterId = null, IDriverCapabilityInvoker? invoker = null, - TimeSpan? healthPollInterval = null) => + TimeSpan? healthPollInterval = null, + TimeSpan? driftCheckInterval = null) => Akka.Actor.Props.Create(() => new DriverInstanceActor( driver, reconnectInterval ?? DefaultReconnectInterval, @@ -233,7 +259,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers healthPublisher ?? NullDriverHealthPublisher.Instance, clusterId ?? string.Empty, invoker, - healthPollInterval)); + healthPollInterval, + driftCheckInterval)); /// /// Returns true when the driver should boot in DEV-STUB mode based on host platform and @@ -264,6 +291,8 @@ 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( @@ -273,13 +302,15 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers IDriverHealthPublisher? healthPublisher = null, string? clusterId = null, IDriverCapabilityInvoker? invoker = null, - TimeSpan? healthPollInterval = null) + TimeSpan? healthPollInterval = null, + TimeSpan? driftCheckInterval = 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, @@ -350,6 +381,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers ResubscribeDesired(); AttachAlarmSource(); SubscribeDesiredAlarms(); + StartDriftCheck(); }); Receive(msg => { @@ -395,6 +427,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers _driverInstanceId, msg.Reason); DetachSubscription(); RecordFault(); + StopDriftCheck(); Become(Reconnecting); PublishHealthSnapshot(); }); @@ -402,6 +435,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers { _log.Info("DriverInstance {Id}: ForceReconnect requested by admin; re-entering Reconnecting", _driverInstanceId); DetachSubscription(); + StopDriftCheck(); Become(Reconnecting); PublishHealthSnapshot(); }); @@ -424,6 +458,7 @@ 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. @@ -518,6 +553,7 @@ 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. @@ -762,6 +798,10 @@ 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. @@ -809,6 +849,119 @@ 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 @@ -984,6 +1137,7 @@ 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 new file mode 100644 index 00000000..7a2e0015 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs @@ -0,0 +1,92 @@ +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 new file mode 100644 index 00000000..f49e25b6 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorDriftCheckTests.cs @@ -0,0 +1,268 @@ +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 new file mode 100644 index 00000000..b48f8d22 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/TagSetDriftDetectorTests.cs @@ -0,0 +1,113 @@ +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); + } +}