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); + } +}