From 4e492e6550f8f91aa74914652e6b3584ae30995a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 13:03:04 -0500 Subject: [PATCH 1/4] fix(galaxy): confirm unconfirmed native alarm acks against Galaxy's acknowledged transition (#869) mxaccessgw answers AcknowledgeAlarm OK both when the AckMsg write was applied and when it was only sent (no OnWriteComplete; the reply then carries a diagnostic). EvaluateReply treated both as success, so the H6 live gate saw a condition go Acknowledged while Galaxy never applied it. - EvaluateReply returns Applied (no diagnostic) or Unconfirmed (diagnostic) and logs the gateway diagnostic on every branch that carries one. - GalaxyAckConfirmationTracker watches the alarm feed from before the RPC and, for an unconfirmed reply, waits up to AlarmAck.ConfirmTimeoutMs (default 10 s, 0 = old behaviour) for an Acknowledge/ActiveAcknowledged transition; on timeout it refuses (BadTimeout + diagnostic). - New optional IAlarmAcknowledgeConfirmation: DriverInstanceActor awaits it after AcknowledgeAsync, off its mailbox and outside the resilience pipeline; a failure is the existing #760 AlarmAckFailed revert. - AdminUI Galaxy form carries the new knob; docs updated. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- .claude/rules/alarms.md | 5 + docs/AlarmTracking.md | 13 +- docs/drivers/Galaxy.md | 36 +- .../IAlarmAcknowledgeConfirmation.cs | 33 ++ .../GalaxyDriverOptions.cs | 22 + .../GalaxyDriver.cs | 50 ++- .../GalaxyDriverFactoryExtensions.cs | 7 +- .../Runtime/GalaxyAckConfirmationTracker.cs | 249 ++++++++++++ .../Runtime/GatewayGalaxyAlarmAcknowledger.cs | 52 ++- .../Runtime/IGalaxyAlarmAcknowledger.cs | 26 +- .../Drivers/Forms/GalaxyDriverForm.razor | 12 +- .../Drivers/DriverInstanceActor.cs | 57 +++ .../GalaxyDriverAlarmSourceTests.cs | 4 +- .../Runtime/GalaxyAckConfirmationTests.cs | 381 ++++++++++++++++++ .../Runtime/GalaxyAckCredentialTests.cs | 4 +- .../Runtime/GalaxyDriverRawPathKeyingTests.cs | 4 +- .../GatewayGalaxyAlarmAcknowledgerTests.cs | 16 +- .../GalaxyDriverPageFormSerializationTests.cs | 26 ++ ...iverHostActorNativeAlarmAckRoutingTests.cs | 83 +++- 19 files changed, 1046 insertions(+), 34 deletions(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmAcknowledgeConfirmation.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckConfirmationTracker.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs diff --git a/.claude/rules/alarms.md b/.claude/rules/alarms.md index 975043532..a4c527978 100644 --- a/.claude/rules/alarms.md +++ b/.claude/rules/alarms.md @@ -46,6 +46,11 @@ disabled/unspawned/stubbed driver, is decided in the host itself (#765) — and native ack, so it marks the whole `NativeAlarmAckPreflightView` unavailable and every native ack is refused synchronously until `BecomeSteady` (#765). A test that routes an ack and expects it to STAND must start the owning driver (`AckingAlarmDriver` in Runtime.Tests). +**A returned `AcknowledgeAsync` is not proof either (#869):** a driver implementing `IAlarmAcknowledgeConfirmation` +(Galaxy — the gateway's OK also covers "AckMsg write sent, unconfirmed") is asked to confirm AFTER it, off the +`DriverInstanceActor` mailbox and outside the resilience pipeline; a confirmation failure is the same +`AlarmAckFailed` revert. Never await that confirmation inside the ack handler (`ReceiveAsync` suspends the mailbox) +or inside the invoker (Tier-A 5 s timeout + breaker). Behaviours that were wrong or missing and are now fixed — keep them: the SDK's `OnTimedUnshelve` is a **no-op** (the engine's `AutoUnshelve` owns timed-shelve expiry, and `UnshelveTime` reports the real diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index 9502a047b..a30b2bf98 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -483,9 +483,16 @@ from the scripted `AlarmCommandRouter`: before anything is applied — as is every native ack while the driver host is Stale (configuration database unreachable at boot), when no ack can be routed or reverted (#765). Galaxy value writes have a separate correlated - write-outcome path. An awaited - ack RPC does not prove the ack reached AVEVA event history: the H6 live - verification remains [operator-gated](plans/2026-06-15-stillpending-phase-3-opcua-standards-design.md). + write-outcome path. **An OK ack reply is not proof (#869):** the gateway + also answers OK when the `AckMsg` write was only *sent* (its diagnostic + says so). The driver then waits — off the actor mailbox, outside the + resilience pipeline, bounded by `AlarmAck.ConfirmTimeoutMs` (default 10 s) + — for Galaxy's own acknowledged transition on the alarm feed + (`IAlarmAcknowledgeConfirmation`); without one the ack is reverted exactly + as above and the `BadTimeout` verdict plus the gateway diagnostic are + logged. See [`drivers/Galaxy.md` §Ack confirmation](drivers/Galaxy.md#ack-confirmation-869). + The H6 live verification of a confirmed ack reaching AVEVA event history + remains [operator-gated](plans/2026-06-15-stillpending-phase-3-opcua-standards-design.md). Only the **Acknowledge** is routed to the driver. **A native condition supports Acknowledge and AddComment only** (Gitea #587 review): `Confirm`, diff --git a/docs/drivers/Galaxy.md b/docs/drivers/Galaxy.md index 3e17a764f..fb2b4734e 100644 --- a/docs/drivers/Galaxy.md +++ b/docs/drivers/Galaxy.md @@ -85,7 +85,7 @@ Project root files: - **`MxAccess`** — `ClientName` (must be unique per OtOpcUa instance — redundancy pairs enforce uniqueness at install time), `PublishingIntervalMs` (forwarded as `buffered_update_interval_ms` on subscribe), `WriteUserId` for ArchestrA secured-write, `EventPumpChannelCapacity` (default 50_000 — one second of headroom at 50k tags / 1Hz; tune via the `galaxy.events.dropped` metric). - **`Repository`** — `DiscoverPageSize`, `WatchDeployEvents`. - **`Reconnect`** — `InitialBackoffMs`, `MaxBackoffMs`, `ReplayOnSessionLost` (default `true`: after a gateway session is recreated, re-issue `SubscribeBulk` for every tracked subscription; `false` leaves them stranded — Bad + Degraded, see [Reconnect + Replay](#reconnect--replay)). -- **`AlarmAck`** (optional, lmxopcua#749) — `FallbackUser`, `FallbackDomain`, `FallbackPasswordSecretRef`. See [Alarm acknowledge credential](#alarm-acknowledge-credential). +- **`AlarmAck`** (optional, lmxopcua#749) — `FallbackUser`, `FallbackDomain`, `FallbackPasswordSecretRef`. See [Alarm acknowledge credential](#alarm-acknowledge-credential). `ConfirmTimeoutMs` (default `10000`, max `120000`, `0` = do not wait, negative = default; #869) — see [Ack confirmation](#ack-confirmation-869). ### Alarm acknowledge credential @@ -132,6 +132,40 @@ Galaxy password, so `GalaxyAckCredentialResolver` picks the account per acknowle `Retain=true`) and logs the reason. A driver transition projected after the ack wins over a late failure. +### Ack confirmation (#869) + +The gateway answers `AcknowledgeAlarm` **OK with `hresult` 0 in two cases**: the `AckMsg` write's +`OnWriteComplete` reported success (*applied* — empty `diagnostic_message`), or the write was sent and no +completion arrived inside the worker's window (*sent but unconfirmed* — `diagnostic_message` says so, +e.g. `AckMsg write to '…' sent; OnWriteComplete not observed within 5000 ms.`). The H6 live gate +(2026-11-11, VD03) caught the second case: OtOpcUa showed the condition Acknowledged while Galaxy's +`.Acked` stayed False and no ACK row was ever historized. + +- `GatewayGalaxyAlarmAcknowledger.EvaluateReply` returns **Applied** for an OK reply with no diagnostic and + **Unconfirmed** for one with a diagnostic (any unexplained diagnostic counts — the safe direction), and + logs the gateway diagnostic on every branch that carries one. +- `GalaxyAckConfirmationTracker` registers a watch for the alarm **before** the RPC (the alarm feed can + deliver the confirming transition ahead of the unary reply) and, for an unconfirmed reply, waits up to + `AlarmAck.ConfirmTimeoutMs` for an `Acknowledge` / `ActiveAcknowledged` transition of that alarm on the + alarm feed (live or snapshot re-statement; the reference match is case-insensitive). Whoever acked it, + Galaxy now shows it acknowledged — that is the truth the condition must match. +- The wait is the optional `IAlarmAcknowledgeConfirmation` capability. `DriverInstanceActor` calls it after + `AcknowledgeAsync` returned, **off its mailbox** (a detached task) and **outside the resilience pipeline** + (a wait on an upstream event: it must not be cut by the Tier-A 5 s `AlarmAcknowledge` timeout nor count + toward the breaker). The OPC UA method thread and the node-manager `Lock` are never involved — the method + already returned when the fire-and-forget route started. +- **On timeout** the confirmation throws `DriverOperationRefusedException` (`… not confirm … (BadTimeout; + gateway: )`), the driver logs a Warning with the diagnostic, and the actor reports + `AlarmAckFailed`: the host re-projects the condition **unacknowledged** — the same #760 revert path. + **What the OPC UA client sees:** the `Acknowledge` call has already returned `Good` (it cannot be changed + after the fact, exactly as for every other #760 revert); the failure reaches the client as a fresh + condition event with `AckedState = false`, and the `BadTimeout` verdict is recorded in the driver and host + Warning logs. A confirming transition that arrives after the revert simply re-acknowledges the condition. + A confirmation in flight when the driver actor stops is abandoned and reverted the same way. +- `ConfirmTimeoutMs = 0` restores the pre-#869 behaviour (an unconfirmed ack counts as acknowledged, with a + Warning) — an escape hatch only. Why the gateway-side ack did not stick on VD03 is tracked upstream + (mxaccessgw#226). + The comment written to `AckMsg` is suffixed with the OPC UA identity — `" [OPC UA: , key ]"` or `"[OPC UA: anonymous]"` — so the client is not lost when the Historian's Operator is a shared fallback account. The password is never logged: the diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmAcknowledgeConfirmation.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmAcknowledgeConfirmation.cs new file mode 100644 index 000000000..ad80cb07c --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IAlarmAcknowledgeConfirmation.cs @@ -0,0 +1,33 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// Optional companion to for a driver whose upstream can ACCEPT an acknowledge +/// without CONFIRMING it was applied (lmxopcua#869). mxaccessgw answers its AcknowledgeAlarm RPC OK +/// both when the AckMsg write was applied and when it was only sent with no completion observed; the +/// truth arrives later, as the alarm's own acknowledged transition on the alarm stream. +/// +/// +/// +/// The runtime calls after +/// returned normally, with the SAME request instances, OUTSIDE the resilience pipeline (it is a wait on an +/// upstream event, not I/O, so it must neither be cut by the capability timeout nor count toward the +/// breaker) and OFF the driver actor's mailbox (the wait is seconds long). +/// +/// +/// It completes normally when every request is confirmed, or never needed confirming (the upstream +/// reported it applied). It throws — normally a naming why — +/// when an acknowledge is not confirmed within the driver's own bounded window; the runtime then reverts +/// the condition to unacknowledged exactly as it does for an +/// failure (lmxopcua#760). The window is the driver's to own and bound. +/// +/// +public interface IAlarmAcknowledgeConfirmation +{ + /// Waits for the upstream to confirm the acknowledges sent. + /// The request instances previously passed to . + /// Abandons the wait (the caller then treats the acknowledge as unconfirmed). + /// A task that completes when every acknowledge is confirmed or needed no confirmation. + Task ConfirmAcknowledgedAsync( + IReadOnlyList acknowledgements, + CancellationToken cancellationToken); +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs index bca223edd..df25411f8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs @@ -79,6 +79,28 @@ public sealed record GalaxyAlarmAckOptions( string? FallbackDomain = null, string? FallbackPasswordSecretRef = null) { + /// Default . + public const int DefaultConfirmTimeoutMs = 10_000; + + /// Upper bound for . + public const int MaxConfirmTimeoutMs = 120_000; + + /// + /// How long an acknowledge the gateway SENT but could not confirm may wait for Galaxy's own acknowledged + /// transition on the alarm feed before it is treated as failed and the OPC UA condition is reverted to + /// unacknowledged (lmxopcua#869). Default 10 000 ms; 0 disables confirmation (an unconfirmed ack + /// counts as success, with a Warning — the pre-#869 behaviour); a negative value means the default, and + /// anything above 120 000 is clamped to it. + /// + [Display(Name = "Ack confirm timeout (ms)", Description = "Wait for Galaxy to confirm an unconfirmed acknowledge. 0 = do not wait.", GroupName = "Alarms")] + [Range(0, MaxConfirmTimeoutMs)] + public int ConfirmTimeoutMs { get; init; } = DefaultConfirmTimeoutMs; + + /// The confirmation window: negative ⇒ the default, else clamped to . + [System.Text.Json.Serialization.JsonIgnore] + public TimeSpan ConfirmTimeout => TimeSpan.FromMilliseconds( + ConfirmTimeoutMs < 0 ? DefaultConfirmTimeoutMs : Math.Min(ConfirmTimeoutMs, MaxConfirmTimeoutMs)); + /// True when a fallback account is configured. [System.Text.Json.Serialization.JsonIgnore] public bool HasFallback => !string.IsNullOrWhiteSpace(FallbackUser); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs index fd117b4f7..ceaeb7318 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs @@ -38,7 +38,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy; /// /// public sealed class GalaxyDriver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IRediscoverable, IHostConnectivityProbe, IAlarmSource, IAlarmAcknowledgePreflight, ISkippedTagReporter, IDriverDiagnostics, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IRediscoverable, IHostConnectivityProbe, IAlarmSource, IAlarmAcknowledgePreflight, IAlarmAcknowledgeConfirmation, ISkippedTagReporter, IDriverDiagnostics, IDisposable, IAsyncDisposable { // ⚠️ lmxopcua#815 warn-only compatibility (deprecated): this RUNNING driver keeps resolving a pre-#815 // out-of-namespace credential reference (API key on InitializeAsync AND every gateway ReopenAsync, plus the @@ -138,6 +138,15 @@ public sealed class GalaxyDriver // lmxopcua#765: the cached key → has-Galaxy-identity set the synchronous ack pre-check reads (no I/O). // Null = no directory wired, so a keyed session is never pre-refused (the pre-#765 behaviour). private readonly IGalaxyAckIdentityDirectory? _ackIdentityDirectory; + // lmxopcua#869: confirms acks the gateway SENT but could not confirm, against Galaxy's own acknowledged + // transition on the alarm feed (bounded by AlarmAck.ConfirmTimeoutMs). Built in the ctor. + private readonly GalaxyAckConfirmationTracker _ackConfirmations; + + /// The ack confirmation window in force (lmxopcua#869) — tests. + internal TimeSpan AckConfirmationWindow => _ackConfirmations.Window; + + /// Acks still awaiting (or exempt from) confirmation (lmxopcua#869) — tests. + internal int PendingAckConfirmations => _ackConfirmations.PendingCount; private IGalaxyAlarmFeed? _alarmFeed; private readonly Lock _alarmHandlersLock = new(); private readonly Lock _alarmFeedLock = new(); @@ -377,6 +386,8 @@ public sealed class GalaxyDriver _alarmFeed = alarmFeed; _ackCredentialResolver = ackCredentialResolver ?? new GalaxyAckCredentialResolver(_driverInstanceId, _options.AlarmAck, _secretResolver, _logger, legacy: _legacyCredentials); + _ackConfirmations = new GalaxyAckConfirmationTracker( + _options.AlarmAck?.ConfirmTimeout ?? GalaxyAckConfirmationTracker.DefaultWindow, _logger, _driverInstanceId); // Build the v3 RawPath ⇄ attributeRef tables from the authored raw tags. Empty when no tags // are authored (live-browse path), leaving every translation an identity no-op. @@ -2340,11 +2351,40 @@ public sealed class GalaxyDriver _driverInstanceId, alarmFullReference, credential.DisplayName, credential.Source, PrincipalOf(ack), ack.ApiKeyId ?? "none"); - await _alarmAcknowledger.AcknowledgeAsync( - alarmFullReference, comment, credential, cancellationToken).ConfigureAwait(false); + // lmxopcua#869: the gateway's OK can mean "sent but unconfirmed". Watch for Galaxy's own acknowledged + // transition from BEFORE the RPC — the feed can deliver it ahead of the unary reply — and arm the + // watch when the reply is unconfirmed; ConfirmAcknowledgedAsync then awaits it, bounded. + var watch = _ackConfirmations.Begin(ack, alarmFullReference); + var armed = false; + try + { + var outcome = await _alarmAcknowledger.AcknowledgeAsync( + alarmFullReference, comment, credential, cancellationToken).ConfigureAwait(false); + if (!outcome.IsApplied) + { + _ackConfirmations.Arm(watch, outcome.Diagnostic); + armed = true; + } + } + finally + { + if (!armed) _ackConfirmations.Release(watch); + } } } + /// + /// + /// lmxopcua#869: completes at once for an ack the gateway reported applied; for one it only SENT, waits up to + /// AlarmAck.ConfirmTimeoutMs (default 10 s) for Galaxy's acknowledged transition on the alarm feed and + /// throws when it does not come — the host then reverts the + /// condition to unacknowledged. The feed must be running (it is whenever the host holds native conditions, + /// the only way an ack reaches this driver); without it nothing can confirm and the ack reverts. + /// + public Task ConfirmAcknowledgedAsync( + IReadOnlyList acknowledgements, CancellationToken cancellationToken) => + _ackConfirmations.ConfirmAsync(acknowledgements, cancellationToken); + /// /// /// Only cases decidable without I/O, on a driver with no AlarmAck.FallbackUser: an anonymous session @@ -2404,6 +2444,10 @@ public sealed class GalaxyDriver /// private void OnAlarmFeedTransition(object? sender, GalaxyAlarmTransition transition) { + // lmxopcua#869: an acknowledged transition confirms any ack the gateway sent unconfirmed — before the + // subscription gate below, which only governs fan-out to OnAlarmEvent. + _ackConfirmations.Observe(transition); + GalaxyAlarmSubscriptionHandle? handle; int subCount; lock (_alarmHandlersLock) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriverFactoryExtensions.cs index dbe8f9b64..26c523732 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriverFactoryExtensions.cs @@ -114,7 +114,10 @@ public static class GalaxyDriverFactoryExtensions AlarmAck = new GalaxyAlarmAckOptions( FallbackUser: NullIfBlank(dto.AlarmAck?.FallbackUser), FallbackDomain: NullIfBlank(dto.AlarmAck?.FallbackDomain), - FallbackPasswordSecretRef: NullIfBlank(dto.AlarmAck?.FallbackPasswordSecretRef)), + FallbackPasswordSecretRef: NullIfBlank(dto.AlarmAck?.FallbackPasswordSecretRef)) + { + ConfirmTimeoutMs = dto.AlarmAck?.ConfirmTimeoutMs ?? GalaxyAlarmAckOptions.DefaultConfirmTimeoutMs, + }, }; return new GalaxyDriver( @@ -160,6 +163,8 @@ public static class GalaxyDriverFactoryExtensions public string? FallbackDomain { get; init; } /// Gets or sets the fallback password reference (secret:/env:/file:/dev:). public string? FallbackPasswordSecretRef { get; init; } + /// Gets or sets how long an unconfirmed ack waits for Galaxy to confirm it (lmxopcua#869); null = default. + public int? ConfirmTimeoutMs { get; init; } } /// Gateway configuration section. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckConfirmationTracker.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckConfirmationTracker.cs new file mode 100644 index 000000000..d5c16f044 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckConfirmationTracker.cs @@ -0,0 +1,249 @@ +using System.Diagnostics; +using Microsoft.Extensions.Logging; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; + +/// +/// Confirms Galaxy acknowledges the gateway sent but could not confirm (lmxopcua#869) against Galaxy's own +/// acknowledged transition on the session-less alarm feed. +/// +/// +/// +/// Lifecycle of one acknowledge: registers a watch BEFORE the gateway RPC is sent, so a +/// confirming transition that overtakes the RPC reply is not lost (the feed and the unary reply race). The +/// RPC outcome then either releases the watch (: applied, or the RPC failed) or arms it +/// (: sent but unconfirmed — the confirmation window starts). +/// awaits the armed watches of the same request instances, outside the resilience pipeline and off the +/// driver actor's mailbox (see ), and throws a +/// when the window passes without confirmation — the host +/// then reverts the condition to unacknowledged. +/// +/// +/// A confirmation is an or +/// transition (live or snapshot re-statement) for +/// the same alarm reference — whoever acknowledged it, Galaxy now shows it acknowledged, which is the +/// truth the OPC UA condition must match. Galaxy references are case-insensitive, so the match is too. +/// +/// +/// A watch whose never comes (an +/// caller that does not confirm) is pruned once it is well past its window, so the set stays bounded. +/// A zero window disables confirmation: an unconfirmed ack is counted as success with a Warning — the +/// pre-#869 behaviour, kept only as an operator escape hatch. +/// +/// +internal sealed class GalaxyAckConfirmationTracker +{ + /// Default confirmation window — long enough for a gateway alarm-feed round trip, short enough + /// that an operator sees the revert while still looking at the alarm. + internal static readonly TimeSpan DefaultWindow = TimeSpan.FromSeconds(10); + + /// How long past its window an armed watch nobody confirms is kept before it is pruned. + private static readonly TimeSpan AbandonedGrace = TimeSpan.FromMinutes(1); + + private readonly TimeSpan _window; + private readonly ILogger _logger; + private readonly string _driverInstanceId; + private readonly Lock _lock = new(); + private readonly List _watches = []; + + /// Initializes a new instance of the class. + /// How long an unconfirmed ack may wait for Galaxy's transition; zero disables confirmation. + /// Logger for confirmation outcomes. + /// The owning driver, for log context. + public GalaxyAckConfirmationTracker(TimeSpan window, ILogger logger, string driverInstanceId) + { + if (window < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(window), window, "must not be negative"); + _window = window; + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _driverInstanceId = driverInstanceId ?? string.Empty; + } + + /// The confirmation window (zero = disabled). + public TimeSpan Window => _window; + + /// Number of live watches (tests / diagnostics). + internal int PendingCount + { + get { lock (_lock) return _watches.Count; } + } + + /// Registers a watch for before its acknowledge is sent. + /// The request instance the caller will later pass to . + /// The Galaxy alarm reference the ack is sent for. + /// The watch, to be d or ed once the RPC answers. + public Watch Begin(AlarmAcknowledgeRequest request, string alarmFullReference) + { + ArgumentNullException.ThrowIfNull(request); + ArgumentException.ThrowIfNullOrEmpty(alarmFullReference); + var watch = new Watch(request, alarmFullReference); + lock (_lock) + { + PruneAbandoned(); + _watches.Add(watch); + } + return watch; + } + + /// Drops a watch whose acknowledge needs no confirmation (applied) or was never delivered (failed). + /// The watch. + public void Release(Watch watch) + { + lock (_lock) _watches.Remove(watch); + } + + /// + /// Starts the confirmation window for an acknowledge the gateway sent but could not confirm. A watch the feed + /// already confirmed while the RPC was in flight is released at once; with confirmation disabled the ack is + /// counted as success (logged). + /// + /// The watch. + /// The gateway's diagnostic, carried into the timeout report. + public void Arm(Watch watch, string diagnostic) + { + watch.Diagnostic = diagnostic ?? string.Empty; + if (_window == TimeSpan.Zero) + { + Release(watch); + _logger.LogWarning( + "Galaxy driver {DriverInstanceId}: acknowledge of {AlarmRef} was not confirmed by the gateway ({Diagnostic}) " + + "and ack confirmation is disabled (AlarmAck.ConfirmTimeoutMs = 0); counting it as acknowledged.", + _driverInstanceId, watch.AlarmFullReference, watch.Diagnostic); + return; + } + + if (watch.IsConfirmed) + { + Release(watch); + LogConfirmed(watch); + return; + } + + lock (_lock) watch.ArmedTimestamp = Stopwatch.GetTimestamp(); + } + + /// + /// Feed hook: confirms every watch for the transition's alarm when the transition says Galaxy shows it + /// acknowledged. Runs on the feed's dispatch thread — never blocks. + /// + /// The alarm transition. + public void Observe(GalaxyAlarmTransition transition) + { + if (transition.TransitionKind is not (GalaxyAlarmTransitionKind.Acknowledge or GalaxyAlarmTransitionKind.ActiveAcknowledged)) + return; + if (string.IsNullOrEmpty(transition.AlarmFullReference)) return; + + lock (_lock) + { + foreach (var watch in _watches) + { + if (string.Equals(watch.AlarmFullReference, transition.AlarmFullReference, StringComparison.OrdinalIgnoreCase)) + watch.Confirm(); + } + } + } + + /// + /// Awaits confirmation of every armed watch belonging to (matched by instance). + /// Requests with no armed watch (applied, or never sent) need nothing. Throws + /// when any is not confirmed inside the window. + /// + /// The request instances the acknowledge was sent with. + /// Abandons the wait. + /// A task that completes when every armed watch is confirmed. + public async Task ConfirmAsync(IReadOnlyList requests, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(requests); + List mine; + lock (_lock) + { + mine = _watches.Where(w => w.ArmedTimestamp != 0 && requests.Any(r => ReferenceEquals(r, w.Request))).ToList(); + } + if (mine.Count == 0) return; + + var failures = new List(); + try + { + foreach (var watch in mine) + { + var remaining = _window - Stopwatch.GetElapsedTime(watch.ArmedTimestamp); + try + { + if (!watch.IsConfirmed) + { + if (remaining <= TimeSpan.Zero) throw new TimeoutException(); + await watch.Confirmed.WaitAsync(remaining, cancellationToken).ConfigureAwait(false); + } + LogConfirmed(watch); + } + catch (TimeoutException) + { + _logger.LogWarning( + "Galaxy driver {DriverInstanceId}: acknowledge of {AlarmRef} was NOT confirmed — Galaxy reported no " + + "acknowledged transition within {Window} after the gateway sent it unconfirmed (gateway diagnostic: " + + "{Diagnostic}). Treating it as failed (BadTimeout); the OPC UA condition is reverted to unacknowledged.", + _driverInstanceId, watch.AlarmFullReference, _window, watch.Diagnostic); + failures.Add( + $"Galaxy did not confirm the acknowledge of '{watch.AlarmFullReference}' within " + + $"{_window.TotalSeconds:0.#} s (BadTimeout; gateway: {watch.Diagnostic})"); + } + } + } + finally + { + lock (_lock) + { + foreach (var watch in mine) _watches.Remove(watch); + } + } + + if (failures.Count > 0) + throw new DriverOperationRefusedException(string.Join("; ", failures)); + } + + private void LogConfirmed(Watch watch) => + _logger.LogInformation( + "Galaxy driver {DriverInstanceId}: acknowledge of {AlarmRef} confirmed by Galaxy's acknowledged transition " + + "(the gateway had reported it sent but unconfirmed: {Diagnostic}).", + _driverInstanceId, watch.AlarmFullReference, watch.Diagnostic); + + // Caller holds _lock. + private void PruneAbandoned() + { + var limit = _window + AbandonedGrace; + _watches.RemoveAll(w => w.ArmedTimestamp != 0 && Stopwatch.GetElapsedTime(w.ArmedTimestamp) > limit); + } + + /// One acknowledge awaiting (or exempt from) confirmation. + internal sealed class Watch + { + private readonly TaskCompletionSource _confirmed = new(TaskCreationOptions.RunContinuationsAsynchronously); + + internal Watch(AlarmAcknowledgeRequest request, string alarmFullReference) + { + Request = request; + AlarmFullReference = alarmFullReference; + } + + /// The request instance this watch belongs to. + public AlarmAcknowledgeRequest Request { get; } + + /// The Galaxy alarm reference. + public string AlarmFullReference { get; } + + /// The gateway's diagnostic for the unconfirmed send. + public string Diagnostic { get; set; } = string.Empty; + + /// timestamp the window started at; 0 while not armed. + public long ArmedTimestamp { get; set; } + + /// Completes when Galaxy reports the alarm acknowledged. + public Task Confirmed => _confirmed.Task; + + /// True once Galaxy reported the alarm acknowledged. + public bool IsConfirmed => _confirmed.Task.IsCompleted; + + /// Marks the watch confirmed (idempotent). + public void Confirm() => _confirmed.TrySetResult(); + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyAlarmAcknowledger.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyAlarmAcknowledger.cs index 211f11a6c..87553de7d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyAlarmAcknowledger.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GatewayGalaxyAlarmAcknowledger.cs @@ -34,6 +34,18 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; /// gateway answered — so it neither retries nor counts toward the resilience breaker (a mis-keyed /// account must not open the breaker for every correctly-configured caller). /// +/// +/// An OK reply is not proof the ack was applied (lmxopcua#869). The gateway answers OK with +/// hresult 0 both when the AckMsg write completed and when it was only SENT — its worker saw no +/// OnWriteComplete inside its window — and in the second case it sets diagnostic_message +/// ("AckMsg write to '…' sent; OnWriteComplete not observed within N ms."). An applied ack carries an empty +/// diagnostic. So an OK reply with a non-empty diagnostic is returned as +/// and the driver waits for Galaxy's own acknowledged transition +/// (). Any unexplained diagnostic on an OK reply is treated the +/// same way — the safe direction: at worst an applied ack waits for its (arriving) transition. The H6 live +/// gate (2026-11-11) caught an unconfirmed ack being reported Acknowledged while Galaxy never applied it. +/// The diagnostic is logged on every branch that carries one. +/// /// internal sealed class GatewayGalaxyAlarmAcknowledger : IGalaxyAlarmAcknowledger { @@ -53,7 +65,7 @@ internal sealed class GatewayGalaxyAlarmAcknowledger : IGalaxyAlarmAcknowledger } /// - public async Task AcknowledgeAsync( + public async Task AcknowledgeAsync( string alarmFullReference, string comment, GalaxyAckCredential credential, @@ -77,7 +89,7 @@ internal sealed class GatewayGalaxyAlarmAcknowledger : IGalaxyAlarmAcknowledger + "details withheld because they echoed the operator credential."); } - EvaluateReply(reply, alarmFullReference, credential, _logger); + return EvaluateReply(reply, alarmFullReference, credential, _logger); } /// Maps an acknowledge onto the gateway request. Pure — pinned by unit tests. @@ -104,23 +116,32 @@ internal sealed class GatewayGalaxyAlarmAcknowledger : IGalaxyAlarmAcknowledger /// /// Applies the reply semantics: non-OK protocol status throws; a non-zero native code warns and throws a - /// (lmxopcua#760). - /// Every message is scrubbed of the password. + /// (lmxopcua#760); an OK reply is + /// when it carries no diagnostic and + /// when it does (lmxopcua#869). The gateway's diagnostic is + /// logged on every branch that carries one. Every message is scrubbed of the password. /// /// The gateway reply. /// The alarm reference. /// The credential the request carried. /// The logger. - internal static void EvaluateReply( + /// Whether the gateway confirmed the ack was applied. + internal static GalaxyAckOutcome EvaluateReply( AcknowledgeAlarmReply reply, string alarmFullReference, GalaxyAckCredential credential, ILogger logger) { + var diagnostic = Scrub(reply.DiagnosticMessage, credential.Password).Trim(); + // Protocol status — the gateway failed before MXAccess saw the ack. This is a // hard failure: the operator's request was not delivered at all. if (reply.ProtocolStatus is { } proto && proto.Code != ProtocolStatusCode.Ok) { + var protoMessage = Scrub(proto.Message, credential.Password); + logger.LogWarning( + "Galaxy AcknowledgeAlarm for {AlarmRef} as {GalaxyUser} failed at the gateway: {ProtocolCode} {ProtocolMessage} (gateway diagnostic: {Diagnostic})", + alarmFullReference, credential.DisplayName, proto.Code, protoMessage, DiagnosticText(diagnostic)); throw new InvalidOperationException( $"Galaxy AcknowledgeAlarm for '{alarmFullReference}' as '{credential.DisplayName}' failed at the gateway: " - + $"{proto.Code} {Scrub(proto.Message, credential.Password)}"); + + $"{proto.Code} {protoMessage}"); } // hresult is the authoritative native ack return code (0 = success) — the AuthenticateUser HRESULT @@ -129,14 +150,29 @@ internal sealed class GatewayGalaxyAlarmAcknowledger : IGalaxyAlarmAcknowledger if (reply.HasHresult && reply.Hresult != 0) { logger.LogWarning( - "Galaxy AcknowledgeAlarm for {AlarmRef} as {GalaxyUser} returned native ack failure code {Hresult}.", - alarmFullReference, credential.DisplayName, reply.Hresult); + "Galaxy AcknowledgeAlarm for {AlarmRef} as {GalaxyUser} returned native ack failure code {Hresult} (gateway diagnostic: {Diagnostic}).", + alarmFullReference, credential.DisplayName, reply.Hresult, DiagnosticText(diagnostic)); throw new DriverOperationRefusedException( $"MXAccess rejected the Galaxy acknowledge of '{alarmFullReference}' as '{credential.DisplayName}' " + $"(native code 0x{reply.Hresult:X8}); check the account's password and Galaxy acknowledge permission."); } + + // lmxopcua#869: OK + hresult 0 means "applied OR sent-but-unconfirmed"; only the unconfirmed case carries a + // diagnostic. Not a failure yet — the driver waits for Galaxy's own acknowledged transition. + if (diagnostic.Length > 0) + { + logger.LogInformation( + "Galaxy AcknowledgeAlarm for {AlarmRef} as {GalaxyUser} was sent but not confirmed by the gateway " + + "(gateway diagnostic: {Diagnostic}); awaiting Galaxy's acknowledged transition.", + alarmFullReference, credential.DisplayName, diagnostic); + return GalaxyAckOutcome.Unconfirmed(diagnostic); + } + + return GalaxyAckOutcome.Applied; } + private static string DiagnosticText(string diagnostic) => diagnostic.Length > 0 ? diagnostic : "none"; + /// Replaces every occurrence of in . /// Text that may echo the secret. /// The secret; empty means nothing to scrub. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/IGalaxyAlarmAcknowledger.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/IGalaxyAlarmAcknowledger.cs index 2e5272fc5..2cac0d363 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/IGalaxyAlarmAcknowledger.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/IGalaxyAlarmAcknowledger.cs @@ -22,10 +22,32 @@ internal interface IGalaxyAlarmAcknowledger /// Carries a password — implementations must keep it out of logs and exception text. /// /// Cancels the gateway RPC. - /// A task that represents the asynchronous operation. - Task AcknowledgeAsync( + /// + /// Whether the gateway reported the ack APPLIED or only SENT without confirmation (lmxopcua#869). A refusal + /// or failure throws instead. + /// + Task AcknowledgeAsync( string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken); } + +/// +/// What the gateway said about an acknowledge it accepted (lmxopcua#869). mxaccessgw answers OK both when the +/// AckMsg write's OnWriteComplete reported success () and when the write was +/// sent but no completion arrived inside the worker's window () — its contract tells +/// clients to watch the alarm stream for the real acknowledged transition in that case. +/// +/// True when the gateway confirmed the ack write was applied. +/// The gateway's diagnostic message (password-scrubbed); empty when it sent none. +internal sealed record GalaxyAckOutcome(bool IsApplied, string Diagnostic) +{ + /// The gateway confirmed the ack write. + public static GalaxyAckOutcome Applied { get; } = new(true, string.Empty); + + /// The gateway sent the ack write but could not confirm it. + /// The gateway's diagnostic message. + /// The outcome. + public static GalaxyAckOutcome Unconfirmed(string diagnostic) => new(false, diagnostic ?? string.Empty); +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor index 865fd7339..cdfe37893 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor @@ -109,6 +109,11 @@ placeholder="secret:otopcua/galaxy/ack-fallback-password" />
Forms: secret:galaxy/NAME or secret:otopcua/galaxy/NAME (production), env:OTOPCUA_GALAXY_NAME, dev:VALUE. A bare literal, file:, or a reference outside the Galaxy namespace is refused (#815).
+
+ + +
When the gateway sends an ack but cannot confirm it, wait this long for Galaxy's acknowledged transition before reverting the condition to unacknowledged (#869). Default 10000; 0 = do not wait.
+
@@ -270,6 +275,7 @@ public string? AckFallbackUser { get; set; } public string? AckFallbackDomain { get; set; } public string? AckFallbackPasswordSecretRef { get; set; } + public int AckConfirmTimeoutMs { get; set; } = GalaxyAlarmAckOptions.DefaultConfirmTimeoutMs; public static GalaxyFormModel FromRecord(GalaxyDriverOptions r) { @@ -300,6 +306,7 @@ AckFallbackUser = ack.FallbackUser, AckFallbackDomain = ack.FallbackDomain, AckFallbackPasswordSecretRef = ack.FallbackPasswordSecretRef, + AckConfirmTimeoutMs = ack.ConfirmTimeoutMs, }; } @@ -329,7 +336,10 @@ AlarmAck = new GalaxyAlarmAckOptions( FallbackUser: string.IsNullOrWhiteSpace(AckFallbackUser) ? null : AckFallbackUser.Trim(), FallbackDomain: string.IsNullOrWhiteSpace(AckFallbackDomain) ? null : AckFallbackDomain.Trim(), - FallbackPasswordSecretRef: string.IsNullOrWhiteSpace(AckFallbackPasswordSecretRef) ? null : AckFallbackPasswordSecretRef.Trim()), + FallbackPasswordSecretRef: string.IsNullOrWhiteSpace(AckFallbackPasswordSecretRef) ? null : AckFallbackPasswordSecretRef.Trim()) + { + ConfirmTimeoutMs = AckConfirmTimeoutMs, + }, }; } } 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 606ac015e..362d709b4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -365,6 +365,10 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// shutdown rather than let it race the teardown on a restart. private CancellationTokenSource? _initCts; + /// Cancelled by to abandon in-flight native-ack confirmations (lmxopcua#869); + /// never disposed — detached confirmation tasks may still hold its token. + private readonly CancellationTokenSource _ackConfirmCts = new(); + /// /// Timestamps of recent Faulted-state transitions; used to compute the 5-minute error count. /// No lock needed — every read/write site runs inside an Akka message handler, which is @@ -1367,6 +1371,13 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers cts.Token); _log.Info("DriverInstance {Id}: acknowledged native condition {Cond} by {User}", _driverInstanceId, msg.ConditionId, msg.OperatorUser); + + // lmxopcua#869: an upstream may accept an ack without confirming it was applied (Galaxy: the gateway's + // "sent but unconfirmed" OK). The confirmation wait is seconds long, so it runs OFF this mailbox and + // OUTSIDE the resilience pipeline (a wait on an upstream event — it must neither be cut by the + // AlarmAcknowledge capability timeout nor count toward the breaker). Failure reverts like any other. + if (src is IAlarmAcknowledgeConfirmation confirmer) + _ = RunAckConfirmationAsync(confirmer, request, msg, replyTo, _ackConfirmCts.Token, _driverInstanceId, _log); } catch (DriverOperationRefusedException ex) { @@ -1385,6 +1396,48 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers } } + /// + /// Backstop over a driver's own confirmation window (lmxopcua#869): the driver bounds the wait itself; this + /// only guarantees a driver that ignores its window cannot hold a confirmation forever. + /// + internal static readonly TimeSpan AckConfirmationCeiling = TimeSpan.FromMinutes(3); + + /// + /// Awaits off the actor context and, when + /// the upstream does not confirm, reports so the host reverts the condition to + /// unacknowledged — the same report an failure makes. Reads no actor + /// state: everything it needs is captured by the caller on the actor thread. + /// + private static async Task RunAckConfirmationAsync( + IAlarmAcknowledgeConfirmation confirmer, IReadOnlyList request, RouteAlarmAck msg, + IActorRef replyTo, CancellationToken stopToken, string driverInstanceId, ILoggingAdapter log) + { + await Task.Yield(); + try + { + await confirmer.ConfirmAcknowledgedAsync(request, stopToken).WaitAsync(AckConfirmationCeiling, stopToken); + } + catch (DriverOperationRefusedException ex) + { + // The driver's verdict (e.g. Galaxy: no acknowledged transition inside its window) — it names why. + log.Warning("DriverInstance {Id}: native-alarm acknowledge of {Cond} by {User} was not confirmed upstream — {Reason}", + driverInstanceId, msg.ConditionId, msg.OperatorUser, ex.Message); + ReportAckFailed(replyTo, msg, ex.Message); + } + catch (OperationCanceledException) when (stopToken.IsCancellationRequested) + { + log.Info("DriverInstance {Id}: confirmation of the native-alarm acknowledge of {Cond} abandoned — the driver stopped", + driverInstanceId, msg.ConditionId); + ReportAckFailed(replyTo, msg, "the driver stopped before the upstream confirmed the acknowledge"); + } + catch (Exception ex) + { + log.Warning(ex, "DriverInstance {Id}: confirmation of the native-alarm acknowledge of {Cond} failed", + driverInstanceId, msg.ConditionId); + ReportAckFailed(replyTo, msg, $"the upstream acknowledge was not confirmed ({ex.GetType().Name})"); + } + } + /// Tells an when the ack names a condition /// NodeId (lmxopcua#760); a no-op for an ack routed without one. private static void ReportAckFailed(IActorRef replyTo, RouteAlarmAck msg, string reason) @@ -2004,6 +2057,10 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers try { _initCts?.Cancel(); } catch (ObjectDisposedException) { /* the attempt already finished */ } + // In-flight native-ack confirmations (lmxopcua#869) are abandoned, and each reports its ack unconfirmed so + // the host reverts it — this incarnation can no longer observe the upstream's confirmation. + _ackConfirmCts.Cancel(); + // BOUNDED by ONE shared budget across shutdown + dispose. PostStop runs on a dispatcher thread and this // blocks it, so an unbounded wait on a driver whose shutdown hangs against a dead socket / COM proxy pinned // that thread for the whole wait, and a few such stops starved the dispatcher. A shutdown that overruns is diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverAlarmSourceTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverAlarmSourceTests.cs index 54bb2324c..eaa571519 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverAlarmSourceTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverAlarmSourceTests.cs @@ -317,10 +317,10 @@ public sealed class GalaxyDriverAlarmSourceTests /// The resolved Galaxy credential. /// Cancellation token. /// A completed task. - public Task AcknowledgeAsync(string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) + public Task AcknowledgeAsync(string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) { Calls.Add((alarmFullReference, comment, credential.User)); - return Task.CompletedTask; + return Task.FromResult(GalaxyAckOutcome.Applied); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs new file mode 100644 index 000000000..2c0e1c34f --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs @@ -0,0 +1,381 @@ +using Microsoft.Extensions.Logging; +using Shouldly; +using Xunit; +using ZB.MOM.WW.MxGateway.Contracts.Proto; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests.Runtime; + +/// +/// lmxopcua#869 — mxaccessgw answers AcknowledgeAlarm OK both when the AckMsg write was APPLIED and +/// when it was only SENT (no OnWriteComplete inside the worker's window; the reply then carries a +/// diagnostic). The H6 live gate saw an unconfirmed ack reported Acknowledged while Galaxy never applied it. +/// The driver now treats an unconfirmed OK as pending and confirms it against Galaxy's own acknowledged +/// transition on the alarm feed, bounded by AlarmAck.ConfirmTimeoutMs. +/// +/// +/// The "fake gateway" is a scripted run through the production +/// — the gRPC transport seam is internal to the +/// client package, so the reply mapping is the part under test. +/// +public sealed class GalaxyAckConfirmationTests +{ + private const string AlarmRef = "Tank01.Level.HiHi"; + + private const string UnconfirmedDiagnostic = + "AckMsg write to 'Tank01.Level.HiHi.AckMsg' sent; OnWriteComplete not observed within 5000 ms."; + + private static readonly GalaxyAckCredential Alice = + new("alice", "CORP", "alice-pw", GalaxyAckCredentialSource.ApiKey); + + // ------------------------------------------------------------------ reply mapping + + [Fact] + public void EvaluateReply_ok_without_diagnostic_is_applied() + { + var logger = new CapturingLogger(); + + var outcome = GatewayGalaxyAlarmAcknowledger.EvaluateReply(AppliedReply(), AlarmRef, Alice, logger); + + outcome.IsApplied.ShouldBeTrue(); + logger.Entries.ShouldBeEmpty(); + } + + /// The H6 reply: OK + hresult 0 + a "sent; not observed" diagnostic is UNCONFIRMED, and the + /// diagnostic is logged. Before #869 the same reply returned silently as success. + [Fact] + public void EvaluateReply_ok_with_diagnostic_is_unconfirmed_and_logs_the_diagnostic() + { + var logger = new CapturingLogger(); + + var outcome = GatewayGalaxyAlarmAcknowledger.EvaluateReply(UnconfirmedReply(), AlarmRef, Alice, logger); + + outcome.IsApplied.ShouldBeFalse(); + outcome.Diagnostic.ShouldBe(UnconfirmedDiagnostic); + logger.AllText.ShouldContain(UnconfirmedDiagnostic); + } + + [Fact] + public void EvaluateReply_native_refusal_logs_the_diagnostic_and_refuses() + { + var reply = new AcknowledgeAlarmReply + { + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Hresult = 1008, + DiagnosticMessage = "AckMsg write to 'Tank01.Level.HiHi.AckMsg' rejected: category SecurityError, detail 1008.", + }; + var logger = new CapturingLogger(); + + Should.Throw( + () => GatewayGalaxyAlarmAcknowledger.EvaluateReply(reply, AlarmRef, Alice, logger)); + + logger.AllText.ShouldContain("rejected: category SecurityError"); + } + + [Fact] + public void EvaluateReply_scrubs_the_password_from_the_diagnostic() + { + var reply = new AcknowledgeAlarmReply + { + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Hresult = 0, + DiagnosticMessage = "sent as alice/alice-pw; not observed", + }; + var logger = new CapturingLogger(); + + var outcome = GatewayGalaxyAlarmAcknowledger.EvaluateReply(reply, AlarmRef, Alice, logger); + + outcome.Diagnostic.ShouldNotContain("alice-pw"); + logger.AllText.ShouldNotContain("alice-pw"); + } + + // ------------------------------------------------------------------ driver: confirmation + + /// Applied OK → acknowledged: nothing pending, the confirmation completes at once. + [Fact] + public async Task Applied_ack_is_confirmed_at_once() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(AppliedReply()); + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 30_000); + await driver.SubscribeAlarmsAsync([AlarmRef], CancellationToken.None); + var request = Ack(AlarmRef); + + await driver.AcknowledgeAsync(request, CancellationToken.None); + var confirm = driver.ConfirmAcknowledgedAsync(request, CancellationToken.None); + + confirm.IsCompletedSuccessfully.ShouldBeTrue("an applied ack needs no confirmation"); + gateway.Calls.ShouldBe(1); + } + + /// + /// Unconfirmed OK + Galaxy's acknowledged transition on the feed → acknowledged. Positive control: the + /// confirmation is still pending after transitions that must NOT confirm (another alarm's ack, this + /// alarm's raise) — it only completes on the matching one. + /// + [Fact] + public async Task Unconfirmed_ack_is_confirmed_by_the_acknowledged_transition() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(UnconfirmedReply()); + var logger = new CapturingLogger(); + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 120_000, logger); + await driver.SubscribeAlarmsAsync([AlarmRef], CancellationToken.None); + var request = Ack(AlarmRef); + + await driver.AcknowledgeAsync(request, CancellationToken.None); + var confirm = driver.ConfirmAcknowledgedAsync(request, CancellationToken.None); + + feed.Emit(Transition("Other01.Level.Hi", GalaxyAlarmTransitionKind.Acknowledge)); + feed.Emit(Transition(AlarmRef, GalaxyAlarmTransitionKind.Raise)); + feed.Emit(Transition(AlarmRef.ToUpperInvariant(), GalaxyAlarmTransitionKind.Acknowledge)); + + await TestWait.ForAsync(confirm, "the acknowledged transition must confirm the pending ack"); + logger.AllText.ShouldContain(UnconfirmedDiagnostic); + } + + /// + /// The feed and the unary reply race: Galaxy's acknowledged transition can arrive BEFORE the RPC answers + /// "unconfirmed". The watch is registered before the RPC, so that early transition still confirms — with a + /// 1 ms window, a lost transition would time out instead. + /// + [Fact] + public async Task Acknowledged_transition_that_overtakes_the_reply_still_confirms() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(UnconfirmedReply()) + { + DuringCall = () => feed.Emit(Transition(AlarmRef, GalaxyAlarmTransitionKind.Acknowledge)), + }; + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 1); + await driver.SubscribeAlarmsAsync([AlarmRef], CancellationToken.None); + var request = Ack(AlarmRef); + + await driver.AcknowledgeAsync(request, CancellationToken.None); + await driver.ConfirmAcknowledgedAsync(request, CancellationToken.None); + } + + /// + /// Unconfirmed OK + no acknowledged transition → the confirmation fails with a refusal naming the timeout and + /// the gateway diagnostic (the runtime reverts the condition on it), and a Warning carrying the diagnostic is + /// logged. Transitions that must not confirm (another alarm's ack, this alarm's retrigger) are emitted during + /// the wait. Before #869 this ack counted as acknowledged. + /// + [Fact] + public async Task Unconfirmed_ack_without_confirmation_fails_with_the_diagnostic() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(UnconfirmedReply()); + var logger = new CapturingLogger(); + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 300, logger); + await driver.SubscribeAlarmsAsync([AlarmRef], CancellationToken.None); + var request = Ack(AlarmRef); + + await driver.AcknowledgeAsync(request, CancellationToken.None); + var confirm = driver.ConfirmAcknowledgedAsync(request, CancellationToken.None); + feed.Emit(Transition("Other01.Level.Hi", GalaxyAlarmTransitionKind.Acknowledge)); + feed.Emit(Transition(AlarmRef, GalaxyAlarmTransitionKind.Retrigger)); + + var ex = await Should.ThrowAsync(() => confirm); + ex.Message.ShouldContain(AlarmRef); + ex.Message.ShouldContain("BadTimeout"); + ex.Message.ShouldContain(UnconfirmedDiagnostic); + logger.Entries.ShouldContain(e => e.Level == LogLevel.Warning && e.Text.Contains(UnconfirmedDiagnostic)); + } + + /// A refused ack (non-zero native code) throws from AcknowledgeAsync — the existing revert path — + /// and leaves nothing pending to confirm. + [Fact] + public async Task Refused_ack_throws_and_leaves_nothing_pending() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(new AcknowledgeAlarmReply + { + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Hresult = unchecked((int)0x80070005), + }); + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 30_000); + await driver.SubscribeAlarmsAsync([AlarmRef], CancellationToken.None); + var request = Ack(AlarmRef); + + await Should.ThrowAsync( + () => driver.AcknowledgeAsync(request, CancellationToken.None)); + + driver.ConfirmAcknowledgedAsync(request, CancellationToken.None).IsCompletedSuccessfully.ShouldBeTrue(); + driver.PendingAckConfirmations.ShouldBe(0); + } + + /// ConfirmTimeoutMs = 0 is the opt-out: an unconfirmed ack counts as acknowledged, with a Warning. + [Fact] + public async Task Zero_timeout_counts_an_unconfirmed_ack_as_acknowledged_with_a_warning() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(UnconfirmedReply()); + var logger = new CapturingLogger(); + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 0, logger); + var request = Ack(AlarmRef); + + await driver.AcknowledgeAsync(request, CancellationToken.None); + + driver.ConfirmAcknowledgedAsync(request, CancellationToken.None).IsCompletedSuccessfully.ShouldBeTrue(); + logger.Entries.ShouldContain(e => e.Level == LogLevel.Warning && e.Text.Contains("confirmation is disabled")); + driver.PendingAckConfirmations.ShouldBe(0); + } + + /// Each confirmation is for ITS request instance: confirming one leaves no watch behind, and a later + /// ack of the same alarm is confirmed independently. + [Fact] + public async Task Confirmations_release_their_watches() + { + var feed = new FakeAlarmFeed(); + var gateway = new ScriptedGateway(UnconfirmedReply()); + using var driver = NewDriver(feed, gateway, confirmTimeoutMs: 120_000); + await driver.SubscribeAlarmsAsync([AlarmRef], CancellationToken.None); + + for (var i = 0; i < 2; i++) + { + var request = Ack(AlarmRef); + await driver.AcknowledgeAsync(request, CancellationToken.None); + driver.PendingAckConfirmations.ShouldBe(1); + var confirm = driver.ConfirmAcknowledgedAsync(request, CancellationToken.None); + feed.Emit(Transition(AlarmRef, GalaxyAlarmTransitionKind.ActiveAcknowledged)); + await TestWait.ForAsync(confirm, $"ack #{i} must be confirmed"); + driver.PendingAckConfirmations.ShouldBe(0); + } + } + + [Fact] + public void Options_default_to_ten_seconds_and_map_negative_to_the_default() + { + new GalaxyAlarmAckOptions().ConfirmTimeout.ShouldBe(TimeSpan.FromSeconds(10)); + new GalaxyAlarmAckOptions { ConfirmTimeoutMs = -5 }.ConfirmTimeout.ShouldBe(TimeSpan.FromSeconds(10)); + new GalaxyAlarmAckOptions { ConfirmTimeoutMs = 0 }.ConfirmTimeout.ShouldBe(TimeSpan.Zero); + new GalaxyAlarmAckOptions { ConfirmTimeoutMs = 999_999 }.ConfirmTimeout.ShouldBe(TimeSpan.FromMinutes(2)); + } + + [Fact] + public void Factory_binds_ConfirmTimeoutMs_and_defaults_when_absent() + { + const string Json = """ + { "gateway": { "endpoint": "https://gw:5001", "apiKeySecretRef": "dev:k" }, + "mxAccess": { "clientName": "C" }, "alarmAck": { "confirmTimeoutMs": 2500 } } + """; + using var withKnob = GalaxyDriverFactoryExtensions.CreateInstance("g", Json); + withKnob.AckConfirmationWindow.ShouldBe(TimeSpan.FromMilliseconds(2500)); + + using var without = GalaxyDriverFactoryExtensions.CreateInstance( + "g", """{ "gateway": { "endpoint": "https://gw:5001", "apiKeySecretRef": "dev:k" }, "mxAccess": { "clientName": "C" } }"""); + without.AckConfirmationWindow.ShouldBe(TimeSpan.FromSeconds(10)); + } + + // ------------------------------------------------------------------ helpers + + private static AcknowledgeAlarmReply AppliedReply() => new() + { + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Hresult = 0, + }; + + private static AcknowledgeAlarmReply UnconfirmedReply() => new() + { + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + Hresult = 0, + DiagnosticMessage = UnconfirmedDiagnostic, + }; + + private static AlarmAcknowledgeRequest[] Ack(string conditionId) => + [new AlarmAcknowledgeRequest(SourceNodeId: "Tank01", ConditionId: conditionId, Comment: "c")]; + + private static GalaxyDriver NewDriver( + IGalaxyAlarmFeed feed, IGalaxyAlarmAcknowledger acknowledger, int confirmTimeoutMs, CapturingLogger? logger = null) + { + var options = new GalaxyDriverOptions( + new GalaxyGatewayOptions("http://localhost:5000", "literal-api-key"), + new GalaxyMxAccessOptions("AckConfirmTest"), + new GalaxyRepositoryOptions(), + new GalaxyReconnectOptions()) + { + AlarmAck = new GalaxyAlarmAckOptions { ConfirmTimeoutMs = confirmTimeoutMs }, + }; + return new GalaxyDriver( + driverInstanceId: "drv-ack", + options: options, + hierarchySource: null, + alarmAcknowledger: acknowledger, + alarmFeed: feed, + logger: logger is null ? null : new TypedLogger(logger), + ackCredentialResolver: FixedAckCredentialResolver.Fallback()); + } + + private static GalaxyAlarmTransition Transition(string alarmRef, GalaxyAlarmTransitionKind kind) => new( + AlarmFullReference: alarmRef, + SourceObjectReference: "Tank01", + AlarmTypeName: "AnalogLimitAlarm.HiHi", + TransitionKind: kind, + SeverityBucket: AlarmSeverity.High, + OpcUaSeverity: 700, + RawMxAccessSeverity: 500, + OriginalRaiseTimestampUtc: null, + TransitionTimestampUtc: DateTime.UtcNow, + OperatorUser: string.Empty, + OperatorComment: string.Empty, + Category: "Process", + Description: "Tank 01 high-high level"); + + /// + /// A fake gateway: every call answers with , mapped by the production + /// . runs while the RPC is + /// "in flight" (before the reply is evaluated). + /// + private sealed class ScriptedGateway(AcknowledgeAlarmReply reply) : IGalaxyAlarmAcknowledger + { + private int _calls; + + public Action? DuringCall { get; init; } + + public int Calls => Volatile.Read(ref _calls); + + public Task AcknowledgeAsync( + string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) + { + Interlocked.Increment(ref _calls); + DuringCall?.Invoke(); + try + { + return Task.FromResult(GatewayGalaxyAlarmAcknowledger.EvaluateReply( + reply, alarmFullReference, credential, Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance)); + } + catch (Exception ex) + { + return Task.FromException(ex); + } + } + } + + private sealed class FakeAlarmFeed : IGalaxyAlarmFeed + { + public event EventHandler? OnAlarmTransition; + + public void Start() { } + + public void Emit(GalaxyAlarmTransition transition) => OnAlarmTransition?.Invoke(this, transition); + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + + private sealed class TypedLogger(CapturingLogger inner) : ILogger + { + private readonly Lock _gate = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel l, EventId id, TState s, Exception? e, Func f) + { + lock (_gate) inner.Log(l, id, s, e, f); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs index 4dfd7db3a..834b4b161 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs @@ -485,11 +485,11 @@ public sealed class GalaxyAckCredentialTests { public List<(string AlarmRef, string Comment, GalaxyAckCredential Credential)> Calls { get; } = []; - public Task AcknowledgeAsync( + public Task AcknowledgeAsync( string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) { Calls.Add((alarmFullReference, comment, credential)); - return Task.CompletedTask; + return Task.FromResult(GalaxyAckOutcome.Applied); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyDriverRawPathKeyingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyDriverRawPathKeyingTests.cs index 632c07910..23f3061fe 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyDriverRawPathKeyingTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyDriverRawPathKeyingTests.cs @@ -242,10 +242,10 @@ public sealed class GalaxyDriverRawPathKeyingTests { public List<(string AlarmRef, string Comment, string Operator)> Calls { get; } = []; - public Task AcknowledgeAsync(string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) + public Task AcknowledgeAsync(string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) { Calls.Add((alarmFullReference, comment, credential.User)); - return Task.CompletedTask; + return Task.FromResult(GalaxyAckOutcome.Applied); } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyAlarmAcknowledgerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyAlarmAcknowledgerTests.cs index 00f0f217b..6ea81e07e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyAlarmAcknowledgerTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GatewayGalaxyAlarmAcknowledgerTests.cs @@ -34,22 +34,22 @@ public sealed class GatewayGalaxyAlarmAcknowledgerTests /// private sealed class FakeAcknowledger : IGalaxyAlarmAcknowledger { - private readonly Queue> _outcomes = new(); + private readonly Queue>> _outcomes = new(); /// Calls recorded in the order received. public List<(string AlarmRef, string Comment, string Operator)> Calls { get; } = []; /// Enqueues an outcome for the next call (completes normally or throws). - public void Enqueue(Func outcome) => _outcomes.Enqueue(outcome); + public void Enqueue(Func> outcome) => _outcomes.Enqueue(outcome); - public Task AcknowledgeAsync( + public Task AcknowledgeAsync( string alarmFullReference, string comment, GalaxyAckCredential credential, CancellationToken cancellationToken) { Calls.Add((alarmFullReference, comment, credential.User)); - return _outcomes.Count > 0 ? _outcomes.Dequeue()() : Task.CompletedTask; + return _outcomes.Count > 0 ? _outcomes.Dequeue()() : Task.FromResult(GalaxyAckOutcome.Applied); } } @@ -77,9 +77,9 @@ public sealed class GatewayGalaxyAlarmAcknowledgerTests { var fake = new FakeAcknowledger(); // First call: simulates a transient transport blip (e.g. RpcException from gRPC). - fake.Enqueue(() => Task.FromException(new IOException("synthetic transport fault"))); + fake.Enqueue(() => Task.FromException(new IOException("synthetic transport fault"))); // Second call: succeeds — proves no dead-state latch. - fake.Enqueue(() => Task.CompletedTask); + fake.Enqueue(() => Task.FromResult(GalaxyAckOutcome.Applied)); var driver = new GalaxyDriver("g", Opts(), hierarchySource: null, alarmAcknowledger: fake, @@ -109,8 +109,8 @@ public sealed class GatewayGalaxyAlarmAcknowledgerTests public async Task Acknowledge_after_protocol_failure_still_issues_next_call() { var fake = new FakeAcknowledger(); - fake.Enqueue(() => Task.FromException(new InvalidOperationException("gateway overload"))); - fake.Enqueue(() => Task.CompletedTask); + fake.Enqueue(() => Task.FromException(new InvalidOperationException("gateway overload"))); + fake.Enqueue(() => Task.FromResult(GalaxyAckOutcome.Applied)); var driver = new GalaxyDriver("g", Opts(), hierarchySource: null, alarmAcknowledger: fake, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/GalaxyDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/GalaxyDriverPageFormSerializationTests.cs index 68239b43b..46f8d5ed4 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/GalaxyDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/GalaxyDriverPageFormSerializationTests.cs @@ -259,4 +259,30 @@ public sealed class GalaxyDriverPageFormSerializationTests json.ShouldNotContain("hasFallback"); JsonSerializer.Deserialize(json, _pageOpts)!.AlarmAck.ShouldBe(original.AlarmAck); } + + /// + /// lmxopcua#869: the ack-confirmation window must survive FromRecord/ToRecord and JSON, or saving the page + /// would silently reset an operator's setting (e.g. the 0 = do-not-wait opt-out) to the default. + /// + [Fact] + public void FormModel_RoundTrips_AlarmAckConfirmTimeout() + { + var original = new GalaxyDriverOptions( + new GalaxyGatewayOptions("https://gw:5001", "env:K"), + new GalaxyMxAccessOptions("OtOpcUa"), + new GalaxyRepositoryOptions(), + new GalaxyReconnectOptions()) + { + AlarmAck = new GalaxyAlarmAckOptions { ConfirmTimeoutMs = 0 }, + }; + + var back = GalaxyDriverForm.GalaxyFormModel.FromRecord(original).ToRecord(); + back.AlarmAck.ConfirmTimeoutMs.ShouldBe(0); + + var json = JsonSerializer.Serialize(back, _pageOpts); + json.ShouldContain("\"confirmTimeoutMs\":0"); + json.ShouldNotContain("\"confirmTimeout\":"); + JsonSerializer.Deserialize(json, _pageOpts)!.AlarmAck.ConfirmTimeoutMs.ShouldBe(0); + new GalaxyDriverForm.GalaxyFormModel().AckConfirmTimeoutMs.ShouldBe(GalaxyAlarmAckOptions.DefaultConfirmTimeoutMs); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs index 3b123c784..b466ea2b7 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs @@ -217,6 +217,9 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest RaiseAndExpectUnacked(actor, publish); actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "cmt", "alice")); AwaitAssert(() => recorder.Acks.Count.ShouldBe(1), duration: Timeout, cancellationToken: TestContext.Current.CancellationToken); + // lmxopcua#869: the driver is an IAlarmAcknowledgeConfirmation whose confirmation succeeds at once — it was + // asked, and a confirmed ack must not revert. + AwaitAssert(() => recorder.ConfirmCalls.ShouldBe(1), duration: Timeout, cancellationToken: TestContext.Current.CancellationToken); actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", Evt(AlarmTransitionKind.Clear))); var next = NextAlarmUpdate(publish); @@ -224,6 +227,62 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest next.State.Acknowledged.ShouldBeTrue("the successful ack must have survived — no revert"); } + /// + /// lmxopcua#869 — the driver ACCEPTED the ack (AcknowledgeAsync returned normally, e.g. the gateway's "sent but + /// unconfirmed" OK) but its upstream never confirmed it: the confirmation refusal reverts the condition to + /// unacknowledged exactly like an AcknowledgeAsync failure. Before #869 nothing awaited a confirmation, so the + /// condition stayed Acknowledged although Galaxy never applied the ack. + /// + [Fact] + public void An_accepted_but_unconfirmed_ack_reverts_the_condition_to_unacknowledged() + { + var db = NewInMemoryDbFactory(); + var recorder = new RecordingAlarmDriverFactory("GalaxyMxGateway") + { + Confirmation = () => Task.FromException(new DriverOperationRefusedException( + "Galaxy did not confirm the acknowledge of 'Tank.Hi' within 10 s (BadTimeout; gateway: AckMsg write sent; OnWriteComplete not observed)")), + }; + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var publish = this.CreateCleanOutcomePublishProbe(); + var actor = SpawnHostAndApply(db, deploymentId, recorder, publish.Ref); + + RaiseAndExpectUnacked(actor, publish); + actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "cmt", "alice")); + + var reverted = NextAlarmUpdate(publish); + reverted.AlarmNodeId.ShouldBe(AlarmRawPath); + reverted.State.Active.ShouldBeTrue(); + reverted.State.Acknowledged.ShouldBeFalse("an ack the upstream never confirmed must not stand"); + recorder.Acks.Count.ShouldBe(1); + recorder.ConfirmCalls.ShouldBe(1); + } + + /// + /// lmxopcua#869 — the confirmation wait runs OFF the driver actor's mailbox: while one ack's confirmation is + /// still pending (it never completes here), a second ack is still routed to the driver. A confirmation awaited + /// inside the ack handler would suspend the mailbox and the second ack would never arrive. + /// + [Fact] + public void A_pending_confirmation_does_not_block_the_driver_actor() + { + var db = NewInMemoryDbFactory(); + var never = new TaskCompletionSource(); + var recorder = new RecordingAlarmDriverFactory("GalaxyMxGateway") + { + Confirmation = () => never.Task, + }; + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var actor = SpawnHostAndApply(db, deploymentId, recorder); + + actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "first", "alice")); + AwaitAssert(() => recorder.ConfirmCalls.ShouldBe(1), duration: Timeout, cancellationToken: TestContext.Current.CancellationToken); + + actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "second", "alice")); + AwaitAssert(() => recorder.Acks.ShouldContain(a => a.Comment == "second"), duration: Timeout, + cancellationToken: TestContext.Current.CancellationToken); + never.TrySetResult(); + } + /// /// lmxopcua#760 — the host publishes the condition → owning-driver pre-check snapshot the OPC UA node /// manager uses to refuse an acknowledge synchronously: the driver's own verdict for the condition it @@ -452,6 +511,12 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest /// The driver's synchronous pre-check verdict (lmxopcua#760); null ⇒ always proceed. public Func? PreflightRefusal { init => _driver.PreflightRefusal = value; } + /// lmxopcua#869: what returns; null ⇒ confirmed at once. + public Func? Confirmation { init => _driver.Confirmation = value; } + + /// How many confirmations the spawned driver was asked for. + public int ConfirmCalls => _driver.ConfirmCalls; + /// public IDriver? TryCreate(string driverType, string driverInstanceId, string driverConfigJson) { @@ -465,8 +530,24 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest } /// An + that records every acknowledge. - private sealed class RecordingAlarmDriver : IDriver, IAlarmSource, IAlarmAcknowledgePreflight + private sealed class RecordingAlarmDriver : IDriver, IAlarmSource, IAlarmAcknowledgePreflight, IAlarmAcknowledgeConfirmation { + /// lmxopcua#869: the confirmation outcome; null ⇒ confirmed at once. + public Func? Confirmation { get; set; } + + private int _confirmCalls; + + /// How many confirmations were requested. + public int ConfirmCalls => Volatile.Read(ref _confirmCalls); + + /// + public Task ConfirmAcknowledgedAsync( + IReadOnlyList acknowledgements, CancellationToken cancellationToken) + { + Interlocked.Increment(ref _confirmCalls); + return Confirmation?.Invoke() ?? Task.CompletedTask; + } + /// When set, records the request and then throws this. public Exception? AckFault { get; set; } From 77872b1d1c91eee82bc48cbb124dbb778c9dafeb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 13:11:49 -0500 Subject: [PATCH 2/4] =?UTF-8?q?fix(runtime,galaxy):=20#869=20review=20roun?= =?UTF-8?q?d=201=20=E2=80=94=20confirmation=20off=20the=20actor=20schedule?= =?UTF-8?q?r,=20stop/async-fault=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Start the ack confirmation with Task.Run + ConfigureAwait(false) so it never depends on the ReceiveAsync handler's ActorTaskScheduler (the PostStop abandon-and-revert report must run after the actor stopped). - DriverInstanceActorAckConfirmationTests: a confirmation that faults after the handler returned is reported; one pending at actor stop is reported; a confirmed ack reports nothing (positive control). - Deterministic tracker matching test (Raise/Clear/Retrigger/other alarm do not confirm; case-insensitive match does). - AdminUI min/max on the confirm-timeout field; Galaxy.md documents the reconnect, UNACK_RTN-as-Clear and RPC-budget limits. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- docs/drivers/Galaxy.md | 10 ++ .../Drivers/Forms/GalaxyDriverForm.razor | 2 +- .../Drivers/DriverInstanceActor.cs | 14 +- .../Runtime/GalaxyAckConfirmationTests.cs | 33 +++- ...DriverInstanceActorAckConfirmationTests.cs | 152 ++++++++++++++++++ 5 files changed, 204 insertions(+), 7 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs diff --git a/docs/drivers/Galaxy.md b/docs/drivers/Galaxy.md index fb2b4734e..d7f70391b 100644 --- a/docs/drivers/Galaxy.md +++ b/docs/drivers/Galaxy.md @@ -162,6 +162,16 @@ e.g. `AckMsg write to '…' sent; OnWriteComplete not observed within 5000 ms.`) condition event with `AckedState = false`, and the `BadTimeout` verdict is recorded in the driver and host Warning logs. A confirming transition that arrives after the revert simply re-acknowledges the condition. A confirmation in flight when the driver actor stops is abandoned and reverted the same way. +- **Limits to know:** + - A gateway **reconnect** detaches the feed; an ack in flight across it is confirmed only if the new feed's + snapshot re-statement (`ActiveAcknowledged`) arrives inside the window, otherwise it reverts (fails safe). + - Acking an alarm that already **returned to normal** (`UNACK_RTN`) is confirmed by the worker's + `Acknowledge` (`ACK_RTN`) transition; if the gateway reconciles it as a `Clear` instead, nothing confirms + and the correctly-applied ack reverts to unacked. Unverified on hardware — include it in the next live gate. + - The RPC itself still runs under the actor's 5 s ack bound and the Tier-A `AlarmAcknowledge` timeout (5 s). + The gateway worker's write-completion window (`DefaultWriteCompletionTimeout`, 1.5 s) plus `AuthenticateUser` must stay + well under that, or the unconfirmed reply is never seen and the ack reverts as a timeout that also counts + toward the breaker. - `ConfirmTimeoutMs = 0` restores the pre-#869 behaviour (an unconfirmed ack counts as acknowledged, with a Warning) — an escape hatch only. Why the gateway-side ack did not stick on VD03 is tracked upstream (mxaccessgw#226). diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor index cdfe37893..9f41fbd84 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor @@ -111,7 +111,7 @@
- +
When the gateway sends an ack but cannot confirm it, wait this long for Galaxy's acknowledged transition before reverting the condition to unacknowledged (#869). Default 10000; 0 = do not wait.
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 362d709b4..7be56023b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -1376,8 +1376,16 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers // "sent but unconfirmed" OK). The confirmation wait is seconds long, so it runs OFF this mailbox and // OUTSIDE the resilience pipeline (a wait on an upstream event — it must neither be cut by the // AlarmAcknowledge capability timeout nor count toward the breaker). Failure reverts like any other. + // Task.Run, not a bare call: this handler is a ReceiveAsync, so TaskScheduler.Current is the actor's + // scheduler; the confirmation must not depend on this actor's context at all — in particular not after + // PostStop, when it still has to report the abandoned ack (pinned by DriverInstanceActorAckConfirmationTests). if (src is IAlarmAcknowledgeConfirmation confirmer) - _ = RunAckConfirmationAsync(confirmer, request, msg, replyTo, _ackConfirmCts.Token, _driverInstanceId, _log); + { + var stopToken = _ackConfirmCts.Token; + var driverInstanceId = _driverInstanceId; + var log = _log; + _ = Task.Run(() => RunAckConfirmationAsync(confirmer, request, msg, replyTo, stopToken, driverInstanceId, log)); + } } catch (DriverOperationRefusedException ex) { @@ -1412,10 +1420,10 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers IAlarmAcknowledgeConfirmation confirmer, IReadOnlyList request, RouteAlarmAck msg, IActorRef replyTo, CancellationToken stopToken, string driverInstanceId, ILoggingAdapter log) { - await Task.Yield(); try { - await confirmer.ConfirmAcknowledgedAsync(request, stopToken).WaitAsync(AckConfirmationCeiling, stopToken); + await confirmer.ConfirmAcknowledgedAsync(request, stopToken) + .WaitAsync(AckConfirmationCeiling, stopToken).ConfigureAwait(false); } catch (DriverOperationRefusedException ex) { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs index 2c0e1c34f..453f81dca 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs @@ -111,9 +111,9 @@ public sealed class GalaxyAckConfirmationTests } /// - /// Unconfirmed OK + Galaxy's acknowledged transition on the feed → acknowledged. Positive control: the - /// confirmation is still pending after transitions that must NOT confirm (another alarm's ack, this - /// alarm's raise) — it only completes on the matching one. + /// Unconfirmed OK + Galaxy's acknowledged transition on the feed → acknowledged, end to end through the + /// driver's feed hook (non-matching transitions are emitted first; that they do NOT confirm is pinned + /// synchronously by ). /// [Fact] public async Task Unconfirmed_ack_is_confirmed_by_the_acknowledged_transition() @@ -246,6 +246,33 @@ public sealed class GalaxyAckConfirmationTests } } + /// + /// Tracker matching rules, checked synchronously: only an Acknowledge / ActiveAcknowledged transition of the + /// SAME alarm (case-insensitive) confirms. Positive control: the matching transition does flip it. + /// + [Fact] + public void Tracker_confirms_only_on_this_alarms_acknowledged_transition() + { + var tracker = new GalaxyAckConfirmationTracker(TimeSpan.FromSeconds(30), new CapturingLogger(), "drv"); + var watch = tracker.Begin(Ack(AlarmRef)[0], AlarmRef); + tracker.Arm(watch, UnconfirmedDiagnostic); + + foreach (var kind in new[] + { + GalaxyAlarmTransitionKind.Raise, GalaxyAlarmTransitionKind.Clear, + GalaxyAlarmTransitionKind.Retrigger, GalaxyAlarmTransitionKind.Unspecified, + }) + { + tracker.Observe(Transition(AlarmRef, kind)); + } + tracker.Observe(Transition("Other01.Level.Hi", GalaxyAlarmTransitionKind.Acknowledge)); + tracker.Observe(Transition("Other01.Level.Hi", GalaxyAlarmTransitionKind.ActiveAcknowledged)); + watch.IsConfirmed.ShouldBeFalse("no transition so far says THIS alarm is acknowledged"); + + tracker.Observe(Transition(AlarmRef.ToLowerInvariant(), GalaxyAlarmTransitionKind.Acknowledge)); + watch.IsConfirmed.ShouldBeTrue(); + } + [Fact] public void Options_default_to_ten_seconds_and_map_negative_to_the_default() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs new file mode 100644 index 000000000..8ec27749d --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs @@ -0,0 +1,152 @@ +using System.Collections.Concurrent; +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; + +/// +/// lmxopcua#869 — asks an driver to +/// confirm an ack AFTER returned, off its mailbox, and reports +/// to the router when the confirmation fails — including one that +/// fails long after the ack handler returned, and one abandoned because the actor stopped. +/// +public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBase +{ + private static readonly TimeSpan Timeout = PresenceBudget; + + /// A confirmation that faults asynchronously, after the ack handler returned, is reported failed. + [Fact] + public void A_confirmation_that_fails_later_is_reported_as_a_failed_ack() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + var gate = new TaskCompletionSource(); + driver.Confirmation = () => gate.Task; + var router = CreateTestProbe(); + + actor.Tell(Ack("n1"), router.Ref); + AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); + gate.SetException(new DriverOperationRefusedException("not confirmed within 10 s (BadTimeout)")); + + var failed = router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken); + failed.ConditionNodeId.ShouldBe("n1"); + failed.Reason.ShouldContain("BadTimeout"); + } + + /// + /// A confirmation still pending when the actor stops is abandoned AND reported failed, so the host reverts the + /// ack this incarnation can no longer confirm. (A confirmation continuation posted back to the stopped actor + /// would dead-letter and never report.) + /// + [Fact] + public void A_confirmation_pending_when_the_actor_stops_is_reported_as_a_failed_ack() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + driver.Confirmation = () => new TaskCompletionSource().Task; // never completes on its own + var router = CreateTestProbe(); + + actor.Tell(Ack("n-stop"), router.Ref); + AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); + Sys.Stop(actor); + + var failed = router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken); + failed.ConditionNodeId.ShouldBe("n-stop"); + failed.Reason.ShouldContain("stopped"); + } + + /// + /// A confirmation that succeeds reports nothing. Positive control: a later ack whose confirmation fails is + /// reported, and it is the FIRST report the router receives — so none came for the confirmed one. + /// + [Fact] + public void A_confirmed_ack_reports_nothing() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + var router = CreateTestProbe(); + + driver.Confirmation = () => Task.Delay(20); + actor.Tell(Ack("n-ok"), router.Ref); + AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); + + driver.Confirmation = () => Task.FromException(new DriverOperationRefusedException("unconfirmed")); + actor.Tell(Ack("n-bad"), router.Ref); + + router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken).ConditionNodeId.ShouldBe("n-bad"); + } + + private static DriverInstanceActor.RouteAlarmAck Ack(string conditionNodeId) => + new("Tank.Hi", "c", "alice", ConditionNodeId: conditionNodeId); + + /// Spawns the actor as a child of the test actor and waits for its Connected announcement. + private IActorRef SpawnConnected(ConfirmingDriver driver) + { + var actor = ChildActorOf(DriverInstanceActor.Props(driver)); + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + FishForMessage(c => c.Connected, Timeout); + return actor; + } + + private sealed class ConfirmingDriver : IDriver, IAlarmSource, IAlarmAcknowledgeConfirmation + { + private int _ackCalls; + private int _confirmCalls; + private readonly ConcurrentQueue _acks = new(); + + public Func? Confirmation { get; set; } + + public int AckCalls => Volatile.Read(ref _ackCalls); + + public int ConfirmCalls => Volatile.Read(ref _confirmCalls); + + public string DriverInstanceId => "drv-confirm"; + + public string DriverType => "Test"; + + 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, null, null); + + public Task SubscribeAlarmsAsync( + IReadOnlyList sourceNodeIds, CancellationToken cancellationToken) => + Task.FromResult(new Handle()); + + public Task UnsubscribeAlarmsAsync(IAlarmSubscriptionHandle handle, CancellationToken cancellationToken) => + Task.CompletedTask; + + public Task AcknowledgeAsync( + IReadOnlyList acknowledgements, CancellationToken cancellationToken) + { + foreach (var a in acknowledgements) _acks.Enqueue(a); + Interlocked.Increment(ref _ackCalls); + return Task.CompletedTask; + } + + public Task ConfirmAcknowledgedAsync( + IReadOnlyList acknowledgements, CancellationToken cancellationToken) + { + Interlocked.Increment(ref _confirmCalls); + return (Confirmation?.Invoke() ?? Task.CompletedTask).WaitAsync(cancellationToken); + } + + public event EventHandler? OnAlarmEvent + { + add { } + remove { } + } + + private sealed class Handle : IAlarmSubscriptionHandle + { + public string DiagnosticId => "h"; + } + } +} From 5001e4069043d76851cc006c79f4817e70cfee61 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 13:19:52 -0500 Subject: [PATCH 3/4] =?UTF-8?q?fix(runtime):=20#869=20review=20round=202?= =?UTF-8?q?=20=E2=80=94=20a=20carry-through=20transition=20no=20longer=20s?= =?UTF-8?q?ettles=20a=20pending=20local=20ack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NativeAlarmProjector dropped the pending local ack on EVERY driver transition, so an operator ack followed by the alarm returning to normal (a Clear carrying the optimistic ack) inside the confirmation window made a later "not confirmed" report a no-op — the condition stayed inactive+acked while Galaxy showed UNACK_RTN. Only a transition that states the ack (Acknowledge, ActiveAcknowledged, or an un-acking Raise/Retrigger/InactiveUnacknowledged) now settles it; a Clear / same-message Raise / Unspecified update leaves it revertible, and the revert projects inactive+unacked. Tests: projector + host end-to-end (ack, Clear, confirmation fails -> inactive + unacked); tightened the confirmed-ack positive controls; pinned that the confirmation receives the same request instance as AcknowledgeAsync. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- .claude/rules/alarms.md | 2 +- docs/AlarmTracking.md | 5 ++- docs/drivers/Galaxy.md | 4 +-- .../Drivers/NativeAlarmProjector.cs | 17 ++++++--- ...iverHostActorNativeAlarmAckRoutingTests.cs | 35 +++++++++++++++++++ ...DriverInstanceActorAckConfirmationTests.cs | 21 +++++++---- .../Drivers/NativeAlarmProjectorTests.cs | 24 +++++++++++++ 7 files changed, 93 insertions(+), 15 deletions(-) diff --git a/.claude/rules/alarms.md b/.claude/rules/alarms.md index a4c527978..5372086b1 100644 --- a/.claude/rules/alarms.md +++ b/.claude/rules/alarms.md @@ -42,7 +42,7 @@ yet re-listed. Only an identity written straight into the store by the `secret` refused until the next refresh, which the refusal itself requests. (2) anything else (a keyed session the cache cannot vouch for, MXAccess non-zero `hresult`, driver still connecting/reconnecting) comes back as `DriverInstanceActor.AlarmAckFailed` — or, for a disabled/unspawned/stubbed driver, is decided in the host itself (#765) — and `DriverHostActor` re-projects the condition unacknowledged — only while the ack is still pending in -`NativeAlarmProjector` (any driver transition since then wins). A host that came up **Stale** cannot route any +`NativeAlarmProjector` (a driver transition since then that STATES the ack wins; a Clear / same-message Raise only carries it through and leaves it revertible, #869). A host that came up **Stale** cannot route any native ack, so it marks the whole `NativeAlarmAckPreflightView` unavailable and every native ack is refused synchronously until `BecomeSteady` (#765). A test that routes an ack and expects it to STAND must start the owning driver (`AckingAlarmDriver` in Runtime.Tests). diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index a30b2bf98..e353a9551 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -474,7 +474,10 @@ from the scripted `AlarmCommandRouter`: condition event). An ack whose owning driver is not running (disabled, not yet spawned) or is a stub on this node is reverted the same way by the host itself (#765). Only an ack still pending in `NativeAlarmProjector` is - reverted: a driver transition projected since then is authoritative. A + reverted: a driver transition projected since then that states the ack + (Acknowledge, ActiveAcknowledged, or an un-acking Raise/Retrigger/ + InactiveUnacknowledged) is authoritative; one that only carries the ack + through (a Clear, a same-message Raise) leaves it revertible (#869). A refusal the driver can decide without I/O (Galaxy, with no fallback ack account: an anonymous session, or a keyed session whose key the cached `GalaxyAckIdentityDirectory` knows has no ack identity — #765) is instead diff --git a/docs/drivers/Galaxy.md b/docs/drivers/Galaxy.md index d7f70391b..a97a611f0 100644 --- a/docs/drivers/Galaxy.md +++ b/docs/drivers/Galaxy.md @@ -129,8 +129,8 @@ Galaxy password, so `GalaxyAckCredentialResolver` picks the account per acknowle identity is broken (a user with no password secret), or MXAccess rejecting the account (a non-zero native `hresult`, e.g. a wrong password, now thrown as a refusal): the call returned `Good`, then `DriverHostActor` re-projects the condition **unacknowledged** (a fresh condition event, - `Retain=true`) and logs the reason. A driver transition projected after the ack wins over a late - failure. + `Retain=true`) and logs the reason. A driver transition projected after the ack that states the ack + state wins over a late failure; a Clear only carries it through and leaves it revertible (#869). ### Ack confirmation (#869) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs index 47f04db98..0c93ef5da 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs @@ -78,9 +78,6 @@ public sealed class NativeAlarmProjector /// The full Part 9 condition snapshot to write to the node. public AlarmConditionSnapshot Project(string nodeId, AlarmEventArgs e, out IReadOnlyList historyKinds) { - // Any driver transition supersedes a pending local acknowledge: from here the projected state is the - // driver's word, so a late ack failure must not rewrite it. - _pendingLocalAck.Remove(nodeId); var prev = _prior.TryGetValue(nodeId, out var p) ? p : (Active: false, Acked: true, Message: (string?)null, Severity: (ushort)0, StatusCode: OpcUaStatus.Good, Class: (AlarmConditionClass?)null); // A source with a finer scale than the four buckets supplies the exact Part 9 severity. @@ -153,6 +150,15 @@ public sealed class NativeAlarmProjector break; } + // A driver transition that STATES the ack (Acknowledge / ActiveAcknowledged, or an un-acking Raise / + // Retrigger / InactiveUnacknowledged) supersedes a pending local acknowledge: from here the ack state is the + // driver's word, so a late ack failure must not rewrite it. One that merely CARRIES the prior ack through + // (a Clear, a same-message Raise, a message/severity update) says nothing about it — the optimistic local + // ack rode along, so it stays pending and a later failure still reverts it (lmxopcua#869: an operator acks, + // the value returns to normal while the unconfirmed ack is still being confirmed, Galaxy shows UNACK_RTN). + if (acked != prev.Acked || e.Kind is AlarmTransitionKind.Acknowledge or AlarmTransitionKind.ActiveAcknowledged) + _pendingLocalAck.Remove(nodeId); + var statusCode = e.QualityStatusCode ?? OpcUaStatus.Good; // A source that classifies its conditions (#839) states the class on its transitions; one that does not (or a // quality-only annotation) keeps whatever the condition last carried. @@ -185,8 +191,9 @@ public sealed class NativeAlarmProjector /// /// Undo a client acknowledgement the driver refused or failed to deliver upstream (lmxopcua#760), returning /// the unacknowledged snapshot to re-project onto the node. Only an acknowledge still pending since - /// is reverted: once any driver transition for the condition has been - /// projected, that transition is authoritative and a late failure changes nothing. + /// is reverted: once a driver transition that STATES the condition's ack state + /// has been projected, that transition is authoritative and a late failure changes nothing. A transition that + /// only carries the prior ack through (e.g. a Clear) leaves it pending (lmxopcua#869). /// /// The condition node id whose acknowledge failed. /// The unacknowledged snapshot to project; null when nothing was reverted. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs index b466ea2b7..455bf635c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorNativeAlarmAckRoutingTests.cs @@ -225,6 +225,41 @@ public sealed class DriverHostActorNativeAlarmAckRoutingTests : RuntimeActorTest var next = NextAlarmUpdate(publish); next.State.Active.ShouldBeFalse(); next.State.Acknowledged.ShouldBeTrue("the successful ack must have survived — no revert"); + + // A Clear carries the ack through and so leaves a still-pending ack revertible (#869): a wrong revert landing + // after it would be the NEXT update. A carry-through update must therefore still show it acknowledged. + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", Evt(AlarmTransitionKind.Unspecified))); + NextAlarmUpdate(publish).State.Acknowledged.ShouldBeTrue("no late revert may follow a confirmed ack"); + } + + /// + /// lmxopcua#869 — the common sequence: an operator acks, the alarm returns to normal (a Clear, carrying the + /// optimistic ack) while the unconfirmed ack is still being confirmed, then the confirmation fails. The ack + /// must still revert — to inactive + UNacknowledged, Galaxy's UNACK_RTN — not stay hidden behind the Clear. + /// + [Fact] + public void An_unconfirmed_ack_reverts_even_after_a_clear_inside_the_window() + { + var db = NewInMemoryDbFactory(); + var gate = new TaskCompletionSource(); + var recorder = new RecordingAlarmDriverFactory("GalaxyMxGateway") { Confirmation = () => gate.Task }; + var deploymentId = SeedV3AlarmDeployment(db, RevA, Driver: "drv-1", Tag: "temp_hi"); + var publish = this.CreateCleanOutcomePublishProbe(); + var actor = SpawnHostAndApply(db, deploymentId, recorder, publish.Ref); + + RaiseAndExpectUnacked(actor, publish); + actor.Tell(new DriverHostActor.RouteNativeAlarmAck(AlarmRawPath, "cmt", "alice")); + AwaitAssert(() => recorder.ConfirmCalls.ShouldBe(1), duration: Timeout, cancellationToken: TestContext.Current.CancellationToken); + + actor.Tell(new DriverInstanceActor.AttributeAlarmPublished("drv-1", Evt(AlarmTransitionKind.Clear))); + var cleared = NextAlarmUpdate(publish); + cleared.State.Active.ShouldBeFalse(); + cleared.State.Acknowledged.ShouldBeTrue("the optimistic ack rides the Clear"); + + gate.SetException(new DriverOperationRefusedException("Galaxy did not confirm the acknowledge (BadTimeout)")); + var reverted = NextAlarmUpdate(publish); + reverted.State.Active.ShouldBeFalse(); + reverted.State.Acknowledged.ShouldBeFalse("the unconfirmed ack must not survive behind the Clear"); } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs index 8ec27749d..ef5b7f1d8 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs @@ -9,9 +9,9 @@ using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; /// -/// lmxopcua#869 — asks an driver to +/// lmxopcua#869 — asks an driver to /// confirm an ack AFTER returned, off its mailbox, and reports -/// to the router when the confirmation fails — including one that +/// to the router when the confirmation fails — including one that /// fails long after the ack handler returned, and one abandoned because the actor stopped. /// public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBase @@ -35,6 +35,7 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa var failed = router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken); failed.ConditionNodeId.ShouldBe("n1"); failed.Reason.ShouldContain("BadTimeout"); + driver.ForeignConfirmations.ShouldBe(0, "the confirmation must get the SAME request instance the ack was sent with"); } /// @@ -61,7 +62,7 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa /// /// A confirmation that succeeds reports nothing. Positive control: a later ack whose confirmation fails is - /// reported, and it is the FIRST report the router receives — so none came for the confirmed one. + /// reported, and it is the FIRST report the router receives — so none came for the confirmed one. /// [Fact] public void A_confirmed_ack_reports_nothing() @@ -70,7 +71,9 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa var actor = SpawnConnected(driver); var router = CreateTestProbe(); - driver.Confirmation = () => Task.Delay(20); + // Already complete: the actor's confirmation task finishes (and would report) right after the call that + // bumps ConfirmCalls, long before the second ack below makes its actor round trip. + driver.Confirmation = () => Task.CompletedTask; actor.Tell(Ack("n-ok"), router.Ref); AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); @@ -96,7 +99,12 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa { private int _ackCalls; private int _confirmCalls; - private readonly ConcurrentQueue _acks = new(); + private readonly ConcurrentQueue> _ackedLists = new(); + private int _foreignConfirmations; + + /// Confirmations asked for a request list that was NOT the instance passed to AcknowledgeAsync — + /// the contract a driver matching by instance (Galaxy) relies on. + public int ForeignConfirmations => Volatile.Read(ref _foreignConfirmations); public Func? Confirmation { get; set; } @@ -126,7 +134,7 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa public Task AcknowledgeAsync( IReadOnlyList acknowledgements, CancellationToken cancellationToken) { - foreach (var a in acknowledgements) _acks.Enqueue(a); + _ackedLists.Enqueue(acknowledgements); Interlocked.Increment(ref _ackCalls); return Task.CompletedTask; } @@ -134,6 +142,7 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa public Task ConfirmAcknowledgedAsync( IReadOnlyList acknowledgements, CancellationToken cancellationToken) { + if (!_ackedLists.Any(l => ReferenceEquals(l, acknowledgements))) Interlocked.Increment(ref _foreignConfirmations); Interlocked.Increment(ref _confirmCalls); return (Confirmation?.Invoke() ?? Task.CompletedTask).WaitAsync(cancellationToken); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs index edb69b6d7..9e5f135c8 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs @@ -606,6 +606,30 @@ public class NativeAlarmProjectorTests snapshot.ShouldBeNull(); } + /// + /// lmxopcua#869 — a transition that only CARRIES the optimistic ack through (a Clear: the alarm returned to + /// normal while the ack was still being confirmed upstream) does not settle it: a late failure still reverts, + /// to inactive + UNacked (Galaxy's UNACK_RTN). Positive control: an Acknowledge transition does settle it. + /// + [Fact] + public void A_carry_through_transition_keeps_the_local_ack_pending() + { + var sut = new NativeAlarmProjector(); + sut.Project("n1", Evt(AlarmTransitionKind.Raise)); + sut.MarkAcknowledged("n1"); + sut.Project("n1", Evt(AlarmTransitionKind.Clear)).Acknowledged.ShouldBeTrue("the optimistic ack rides along"); + + sut.TryRevertAcknowledge("n1", out var reverted).ShouldBeTrue(); + reverted!.Active.ShouldBeFalse(); + reverted.Acknowledged.ShouldBeFalse(); + + sut.Project("n1", Evt(AlarmTransitionKind.Raise)); + sut.MarkAcknowledged("n1"); + sut.Project("n1", Evt(AlarmTransitionKind.Raise)).Acknowledged.ShouldBeTrue("a same-message Raise carries it too"); + sut.Project("n1", Evt(AlarmTransitionKind.Acknowledge)); + sut.TryRevertAcknowledge("n1", out _).ShouldBeFalse("an Acknowledge transition states the ack — authoritative"); + } + private static AlarmEventArgs Evt( AlarmTransitionKind kind, AlarmSeverity sev = AlarmSeverity.High, From b421c7ca9119ef9d3ee763f7b7dbcd475a463c65 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 13:25:58 -0500 Subject: [PATCH 4/4] =?UTF-8?q?fix(runtime):=20#869=20review=20round=203?= =?UTF-8?q?=20=E2=80=94=20key=20a=20native-ack=20revert=20to=20the=20ack?= =?UTF-8?q?=20that=20failed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pending local ack can outlive its own delivery (sources that send no Acknowledge back; a Clear now only carries it through), so a later failed re-ack — or a stale Galaxy confirmation timing out after the alarm re-occurred and was acked again — could revert an ack it does not own. MarkAcknowledged now issues a sequence (0 when the ack changed nothing), carried on RouteAlarmAck and echoed on AlarmAckFailed; TryRevertAcknowledge reverts only on a matching sequence. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- .claude/rules/alarms.md | 2 +- docs/AlarmTracking.md | 4 +- .../Drivers/DriverHostActor.cs | 18 ++++--- .../Drivers/DriverInstanceActor.cs | 10 ++-- .../Drivers/NativeAlarmProjector.cs | 50 ++++++++++++++++--- ...DriverInstanceActorAckConfirmationTests.cs | 19 +++++++ .../Drivers/NativeAlarmProjectorTests.cs | 26 ++++++++++ 7 files changed, 110 insertions(+), 19 deletions(-) diff --git a/.claude/rules/alarms.md b/.claude/rules/alarms.md index 5372086b1..7d70f2de3 100644 --- a/.claude/rules/alarms.md +++ b/.claude/rules/alarms.md @@ -42,7 +42,7 @@ yet re-listed. Only an identity written straight into the store by the `secret` refused until the next refresh, which the refusal itself requests. (2) anything else (a keyed session the cache cannot vouch for, MXAccess non-zero `hresult`, driver still connecting/reconnecting) comes back as `DriverInstanceActor.AlarmAckFailed` — or, for a disabled/unspawned/stubbed driver, is decided in the host itself (#765) — and `DriverHostActor` re-projects the condition unacknowledged — only while the ack is still pending in -`NativeAlarmProjector` (a driver transition since then that STATES the ack wins; a Clear / same-message Raise only carries it through and leaves it revertible, #869). A host that came up **Stale** cannot route any +`NativeAlarmProjector` (a driver transition since then that STATES the ack wins; a Clear / same-message Raise only carries it through and leaves it revertible, #869; the revert is keyed to that ack's `MarkAcknowledged` sequence, echoed on `RouteAlarmAck`/`AlarmAckFailed`, so another ack's failure never undoes it). A host that came up **Stale** cannot route any native ack, so it marks the whole `NativeAlarmAckPreflightView` unavailable and every native ack is refused synchronously until `BecomeSteady` (#765). A test that routes an ack and expects it to STAND must start the owning driver (`AckingAlarmDriver` in Runtime.Tests). diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index e353a9551..6837ac6e3 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -477,7 +477,9 @@ from the scripted `AlarmCommandRouter`: reverted: a driver transition projected since then that states the ack (Acknowledge, ActiveAcknowledged, or an un-acking Raise/Retrigger/ InactiveUnacknowledged) is authoritative; one that only carries the ack - through (a Clear, a same-message Raise) leaves it revertible (#869). A + through (a Clear, a same-message Raise) leaves it revertible (#869) — and + only by a failure of THAT acknowledge: the revert is keyed to the sequence + `MarkAcknowledged` issued, echoed on `RouteAlarmAck` / `AlarmAckFailed`. A refusal the driver can decide without I/O (Galaxy, with no fallback ack account: an anonymous session, or a keyed session whose key the cached `GalaxyAckIdentityDirectory` knows has no ack identity — #765) is instead diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index 3d58971c9..9a8d4b85a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -2406,7 +2406,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS // projector too, or the next driver Clear projects (Active=false, Acked=prev=false) and un-acknowledges it. // Most sources (MTConnect, FOCAS) send no Acknowledge transition back, and the projector now survives a // deploy rather than resetting to its acked default. - _nativeAlarmProjector.MarkAcknowledged(msg.ConditionNodeId); + var ackSequence = _nativeAlarmProjector.MarkAcknowledged(msg.ConditionNodeId); if (!_children.TryGetValue(target.DriverInstanceId, out var entry) || entry.Stubbed) { @@ -2417,7 +2417,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS var reason = entry is null ? "the owning driver is not running" : "the owning driver is stubbed on this node"; _log.Debug("DriverHost {Node}: driver {Driver} cannot take the alarm ack of {Cond} ({Reason}) — dropped", _localNode, target.DriverInstanceId, msg.ConditionNodeId, reason); - RevertNativeAlarmAck(msg.ConditionNodeId, reason); + RevertNativeAlarmAck(msg.ConditionNodeId, reason, ackSequence); return; } @@ -2426,7 +2426,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS // carried here) and reverts the node (HandleAlarmAckFailed). The driver correlates on ConditionId // (= the authored alarm FullName the inverse map keyed on). entry.Actor.Tell(new DriverInstanceActor.RouteAlarmAck( - target.RawPath, msg.Comment, msg.OperatorUser, msg.ApiKeyId, ConditionNodeId: msg.ConditionNodeId)); + target.RawPath, msg.Comment, msg.OperatorUser, msg.ApiKeyId, ConditionNodeId: msg.ConditionNodeId, + AckSequence: ackSequence)); } /// @@ -2437,7 +2438,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// /// The failure report from the driver child. private void HandleAlarmAckFailed(DriverInstanceActor.AlarmAckFailed msg) => - RevertNativeAlarmAck(msg.ConditionNodeId, msg.Reason); + RevertNativeAlarmAck(msg.ConditionNodeId, msg.Reason, msg.AckSequence); /// /// Re-projects as unacknowledged when its local acknowledge is still @@ -2446,12 +2447,15 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// /// The bare condition NodeId (Raw realm). /// Why the acknowledge did not reach the upstream alarm system. - private void RevertNativeAlarmAck(string conditionNodeId, string reason) + /// The failed acknowledge's projector sequence (lmxopcua#869). + private void RevertNativeAlarmAck(string conditionNodeId, string reason, long ackSequence) { - if (!_nativeAlarmProjector.TryRevertAcknowledge(conditionNodeId, out var snapshot) || snapshot is null) + // Keyed to the failed ack's own sequence (lmxopcua#869): a pending ack from an earlier or later acknowledge of + // the same condition is not this failure's to undo. + if (!_nativeAlarmProjector.TryRevertAcknowledge(conditionNodeId, ackSequence, out var snapshot) || snapshot is null) { _log.Info("DriverHost {Node}: native-alarm acknowledge of {Cond} did not reach the alarm system ({Reason}); " + - "the condition has since been re-stated by its driver, so it is left as projected", + "no acknowledge of THIS request is pending (re-stated by its driver since, superseded by another ack, or it changed nothing), so it is left as projected", _localNode, conditionNodeId, reason); return; } 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 7be56023b..b7baf1949 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -90,8 +90,11 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// acknowledge that does not reach the upstream alarm system (refused, failed, or dropped because the driver /// is not connected) is reported back to the sender as an so the host can revert /// the locally-applied acknowledge (lmxopcua#760). Null ⇒ no report. + /// The host projector's sequence for this acknowledge (lmxopcua#869), echoed on + /// so only a failure of THIS ack reverts it; 0 ⇒ the ack changed nothing. public sealed record RouteAlarmAck( - string ConditionId, string? Comment, string OperatorUser, string? ApiKeyId = null, string? ConditionNodeId = null); + string ConditionId, string? Comment, string OperatorUser, string? ApiKeyId = null, string? ConditionNodeId = null, + long AckSequence = 0); /// /// Reply to a carrying a whose @@ -99,7 +102,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers /// /// The OPC UA condition NodeId the client acknowledged. /// Why the acknowledge did not go through; never contains a secret. - public sealed record AlarmAckFailed(string ConditionNodeId, string Reason); + /// The failed acknowledge's (lmxopcua#869). + public sealed record AlarmAckFailed(string ConditionNodeId, string Reason, long AckSequence = 0); public sealed record Subscribe(IReadOnlyList FullReferences, TimeSpan PublishingInterval); /// /// Sets the set of references this driver should keep subscribed for the lifetime of the @@ -1451,7 +1455,7 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers private static void ReportAckFailed(IActorRef replyTo, RouteAlarmAck msg, string reason) { if (string.IsNullOrEmpty(msg.ConditionNodeId) || replyTo.IsNobody()) return; - replyTo.Tell(new AlarmAckFailed(msg.ConditionNodeId, reason)); + replyTo.Tell(new AlarmAckFailed(msg.ConditionNodeId, reason, msg.AckSequence)); } private async Task HandleSubscribeAsync(Subscribe msg) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs index 0c93ef5da..308c08beb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs @@ -18,9 +18,13 @@ public sealed class NativeAlarmProjector /// /// Conditions a client acknowledged locally () whose acknowledge has not yet /// been confirmed or superseded by a driver transition — the only ones - /// may un-acknowledge (lmxopcua#760). + /// may un-acknowledge (lmxopcua#760) — keyed to the sequence issued, so only a + /// failure of THAT acknowledge reverts it (lmxopcua#869). /// - private readonly HashSet _pendingLocalAck = new(StringComparer.Ordinal); + private readonly Dictionary _pendingLocalAck = new(StringComparer.Ordinal); + + /// Last sequence issued by ; 0 is never issued. + private long _ackSequence; /// Project an alarm transition onto the full condition snapshot for . /// The materialised condition node's id (the projection's state key). @@ -180,12 +184,23 @@ public sealed class NativeAlarmProjector /// stale unacknowledged prior. An untracked condition already defaults to acknowledged. /// /// The acknowledged condition node id. - public void MarkAcknowledged(string nodeId) + /// + /// The sequence identifying this pending acknowledge — pass it back to + /// with its failure — or 0 when the + /// ack changed nothing (untracked, or already acknowledged), so its failure has nothing to undo. + /// + public long MarkAcknowledged(string nodeId) { - if (!_prior.TryGetValue(nodeId, out var p)) return; + if (!_prior.TryGetValue(nodeId, out var p)) return 0; // Only an ack that CHANGED the projected state is pending; re-acking an acked condition has nothing to undo. - if (!p.Acked) _pendingLocalAck.Add(nodeId); + long sequence = 0; + if (!p.Acked) + { + sequence = ++_ackSequence; + _pendingLocalAck[nodeId] = sequence; + } _prior[nodeId] = p with { Acked = true }; + return sequence; } /// @@ -198,10 +213,31 @@ public sealed class NativeAlarmProjector /// The condition node id whose acknowledge failed. /// The unacknowledged snapshot to project; null when nothing was reverted. /// True when the condition was reverted to unacknowledged. - public bool TryRevertAcknowledge(string nodeId, out AlarmConditionSnapshot? snapshot) + public bool TryRevertAcknowledge(string nodeId, out AlarmConditionSnapshot? snapshot) => + TryRevert(nodeId, ackSequence: null, out snapshot); + + /// + /// Like , but only when the pending + /// acknowledge is the one issued for + /// (lmxopcua#869). A pending ack can outlive its own delivery — a source that never sends an Acknowledge + /// transition back (FOCAS, MTConnect), across a Clear that only carried it through — so a failure of a LATER, + /// unrelated acknowledge (a re-ack while the driver is down) or of an EARLIER one (a Galaxy confirmation timing + /// out after the alarm re-occurred and was acked again) must not revert it. + /// + /// The condition node id whose acknowledge failed. + /// The sequence of the failed acknowledge; 0 never matches. + /// The unacknowledged snapshot to project; null when nothing was reverted. + /// True when the condition was reverted to unacknowledged. + public bool TryRevertAcknowledge(string nodeId, long ackSequence, out AlarmConditionSnapshot? snapshot) => + TryRevert(nodeId, ackSequence, out snapshot); + + private bool TryRevert(string nodeId, long? ackSequence, out AlarmConditionSnapshot? snapshot) { snapshot = null; - if (!_pendingLocalAck.Remove(nodeId) || !_prior.TryGetValue(nodeId, out var p)) return false; + if (!_pendingLocalAck.TryGetValue(nodeId, out var pending) + || (ackSequence is { } wanted && wanted != pending) + || !_prior.TryGetValue(nodeId, out var p)) return false; + _pendingLocalAck.Remove(nodeId); _prior[nodeId] = p with { Acked = false }; snapshot = new AlarmConditionSnapshot( Active: p.Active, Acknowledged: false, Confirmed: true, Enabled: true, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs index ef5b7f1d8..b29cb7126 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs @@ -83,6 +83,25 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken).ConditionNodeId.ShouldBe("n-bad"); } + /// + /// lmxopcua#869 review — the failure echoes the routed ack's , + /// which is what lets the host revert only the acknowledge that failed (keyed in + /// NativeAlarmProjector.TryRevertAcknowledge(nodeId, ackSequence, …)). + /// + [Fact] + public void A_failed_confirmation_echoes_the_ack_sequence() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + driver.Confirmation = () => Task.FromException(new DriverOperationRefusedException("unconfirmed")); + var router = CreateTestProbe(); + + actor.Tell(Ack("n-seq") with { AckSequence = 42 }, router.Ref); + + router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken) + .AckSequence.ShouldBe(42L); + } + private static DriverInstanceActor.RouteAlarmAck Ack(string conditionNodeId) => new("Tank.Hi", "c", "alice", ConditionNodeId: conditionNodeId); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs index 9e5f135c8..bcb413d86 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/NativeAlarmProjectorTests.cs @@ -630,6 +630,32 @@ public class NativeAlarmProjectorTests sut.TryRevertAcknowledge("n1", out _).ShouldBeFalse("an Acknowledge transition states the ack — authoritative"); } + /// + /// lmxopcua#869 review — a pending ack can outlive its own delivery (a source that sends no Acknowledge back, + /// across a Clear that only carried it), so a revert is keyed to the failed ack's sequence: a later re-ack that + /// changed nothing (sequence 0) or a stale earlier ack's failure does not undo it. Positive control: the + /// matching sequence does. + /// + [Fact] + public void A_revert_only_undoes_the_acknowledge_that_failed() + { + var sut = new NativeAlarmProjector(); + sut.Project("n1", Evt(AlarmTransitionKind.Raise)); + var first = sut.MarkAcknowledged("n1"); + first.ShouldBeGreaterThan(0); + sut.Project("n1", Evt(AlarmTransitionKind.Clear)); // first ack delivered; nothing settles it + + var reAck = sut.MarkAcknowledged("n1"); // a re-ack of the acked, cleared condition + reAck.ShouldBe(0L, "it changed nothing, so its failure has nothing to undo"); + sut.TryRevertAcknowledge("n1", reAck, out _).ShouldBeFalse("a later re-ack's failure must not revert the first"); + + sut.Project("n1", Evt(AlarmTransitionKind.Raise, msg: "again")); // a new occurrence (un-acks) + var second = sut.MarkAcknowledged("n1"); + sut.TryRevertAcknowledge("n1", first, out _).ShouldBeFalse("a stale earlier ack's failure must not revert the newer one"); + sut.TryRevertAcknowledge("n1", second, out var reverted).ShouldBeTrue(); + reverted!.Acknowledged.ShouldBeFalse(); + } + private static AlarmEventArgs Evt( AlarmTransitionKind kind, AlarmSeverity sev = AlarmSeverity.High,