fix(runtime): #869 review round 3 — key a native-ack revert to the ack that failed

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

@@ -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).

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

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));
     }
 
     /// <summary>
@@ -2437,7 +2438,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
     /// </summary>
     /// <param name="msg">The failure report from the driver child.</param>
     private void HandleAlarmAckFailed(DriverInstanceActor.AlarmAckFailed msg) =>
-        RevertNativeAlarmAck(msg.ConditionNodeId, msg.Reason);
+        RevertNativeAlarmAck(msg.ConditionNodeId, msg.Reason, msg.AckSequence);
 
     /// <summary>
     ///     Re-projects <paramref name="conditionNodeId"/> as unacknowledged when its local acknowledge is still
@@ -2446,12 +2447,15 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS
     /// </summary>
     /// <param name="conditionNodeId">The bare condition NodeId (Raw realm).</param>
     /// <param name="reason">Why the acknowledge did not reach the upstream alarm system.</param>
-    private void RevertNativeAlarmAck(string conditionNodeId, string reason)
+    /// <param name="ackSequence">The failed acknowledge's projector sequence (lmxopcua#869).</param>
+    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;
         }

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 <see cref="AlarmAckFailed"/> so the host can revert
     /// the locally-applied acknowledge (lmxopcua#760). Null ⇒ no report.</param>
+    /// <param name="AckSequence">The host projector's sequence for this acknowledge (lmxopcua#869), echoed on
+    /// <see cref="AlarmAckFailed"/> so only a failure of THIS ack reverts it; 0 ⇒ the ack changed nothing.</param>
     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);
 
     /// <summary>
     ///     Reply to a <see cref="RouteAlarmAck"/> carrying a <see cref="RouteAlarmAck.ConditionNodeId"/> whose
@@ -99,7 +102,8 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers
     /// </summary>
     /// <param name="ConditionNodeId">The OPC UA condition NodeId the client acknowledged.</param>
     /// <param name="Reason">Why the acknowledge did not go through; never contains a secret.</param>
-    public sealed record AlarmAckFailed(string ConditionNodeId, string Reason);
+    /// <param name="AckSequence">The failed acknowledge's <see cref="RouteAlarmAck.AckSequence"/> (lmxopcua#869).</param>
+    public sealed record AlarmAckFailed(string ConditionNodeId, string Reason, long AckSequence = 0);
     public sealed record Subscribe(IReadOnlyList<string> FullReferences, TimeSpan PublishingInterval);
     /// <summary>
     ///     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)

src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/NativeAlarmProjector.cs

@@ -18,9 +18,13 @@ public sealed class NativeAlarmProjector
     /// <summary>
     ///     Conditions a client acknowledged locally (<see cref="MarkAcknowledged"/>) whose acknowledge has not yet
     ///     been confirmed or superseded by a driver transition — the only ones <see cref="TryRevertAcknowledge"/>
-    ///     may un-acknowledge (lmxopcua#760).
+    ///     may un-acknowledge (lmxopcua#760) — keyed to the sequence <see cref="MarkAcknowledged"/> issued, so only a
+    ///     failure of THAT acknowledge reverts it (lmxopcua#869).
     /// </summary>
-    private readonly HashSet<string> _pendingLocalAck = new(StringComparer.Ordinal);
+    private readonly Dictionary<string, long> _pendingLocalAck = new(StringComparer.Ordinal);
+
+    /// <summary>Last sequence issued by <see cref="MarkAcknowledged"/>; 0 is never issued.</summary>
+    private long _ackSequence;
 
     /// <summary>Project an alarm transition onto the full condition snapshot for <paramref name="nodeId"/>.</summary>
     /// <param name="nodeId">The materialised condition node's id (the projection's state key).</param>
@@ -180,12 +184,23 @@ public sealed class NativeAlarmProjector
     ///     stale unacknowledged prior. An untracked condition already defaults to acknowledged.
     /// </summary>
     /// <param name="nodeId">The acknowledged condition node id.</param>
-    public void MarkAcknowledged(string nodeId)
+    /// <returns>
+    ///     The sequence identifying this pending acknowledge — pass it back to
+    ///     <see cref="TryRevertAcknowledge(string, long, out AlarmConditionSnapshot?)"/> with its failure — or 0 when the
+    ///     ack changed nothing (untracked, or already acknowledged), so its failure has nothing to undo.
+    /// </returns>
+    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;
     }
 
     /// <summary>
@@ -198,10 +213,31 @@ public sealed class NativeAlarmProjector
     /// <param name="nodeId">The condition node id whose acknowledge failed.</param>
     /// <param name="snapshot">The unacknowledged snapshot to project; null when nothing was reverted.</param>
     /// <returns>True when the condition was reverted to unacknowledged.</returns>
-    public bool TryRevertAcknowledge(string nodeId, out AlarmConditionSnapshot? snapshot)
+    public bool TryRevertAcknowledge(string nodeId, out AlarmConditionSnapshot? snapshot) =>
+        TryRevert(nodeId, ackSequence: null, out snapshot);
+
+    /// <summary>
+    ///     Like <see cref="TryRevertAcknowledge(string, out AlarmConditionSnapshot?)"/>, but only when the pending
+    ///     acknowledge is the one <see cref="MarkAcknowledged"/> issued <paramref name="ackSequence"/> 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.
+    /// </summary>
+    /// <param name="nodeId">The condition node id whose acknowledge failed.</param>
+    /// <param name="ackSequence">The sequence of the failed acknowledge; 0 never matches.</param>
+    /// <param name="snapshot">The unacknowledged snapshot to project; null when nothing was reverted.</param>
+    /// <returns>True when the condition was reverted to unacknowledged.</returns>
+    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,

tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs

@@ -83,6 +83,25 @@ public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBa
         router.ExpectMsg<DriverInstanceActor.AlarmAckFailed>(Timeout, cancellationToken: TestContext.Current.CancellationToken).ConditionNodeId.ShouldBe("n-bad");
     }
 
+    /// <summary>
+    ///     lmxopcua#869 review — the failure echoes the routed ack's <see cref="DriverInstanceActor.RouteAlarmAck.AckSequence"/>,
+    ///     which is what lets the host revert only the acknowledge that failed (keyed in
+    ///     <c>NativeAlarmProjector.TryRevertAcknowledge(nodeId, ackSequence, …)</c>).
+    /// </summary>
+    [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<DriverInstanceActor.AlarmAckFailed>(Timeout, cancellationToken: TestContext.Current.CancellationToken)
+            .AckSequence.ShouldBe(42L);
+    }
+
     private static DriverInstanceActor.RouteAlarmAck Ack(string conditionNodeId) =>
         new("Tank.Hi", "c", "alice", ConditionNodeId: conditionNodeId);
 

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");
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [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,