fix(opcuaclient): derive AlarmTransitionKind from upstream ActiveState/AckedState (#619 review)

NativeAlarmProjector derives Active/Acked ONLY from AlarmEventArgs.Kind, and this driver
never set it - every routed event projected Unspecified, leaving the condition inactive+acked
(only Message/Severity moved). The live-gate expectation 'toggling Active as opc-plc cycles'
could not have passed. Select ActiveState/Id + AckedState/Id (two-element browse paths) and
emit the Kind sequence that moves the projector from the state last told to the host to the
event's (Raise+Acknowledge when a notification carries both).

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe

docs/drivers/OpcUaClient.md

@@ -97,7 +97,10 @@ rather than surfacing as a runtime surprise.
 > `ConditionId`. Author `nodeId` as the upstream **condition** node for a 1:1 mapping (required when one source carries
 > several conditions, e.g. Hi + HiHi); authoring the **source** node folds every condition of that source into one
 > tag. `Acknowledge` is called on the condition node of the last event seen for the RawPath **with that event's
-> EventId** (Part 9 §5.7.3). Index keys are server-stable (`nsu=`), so a namespace reorder across a reconnect keeps
+> EventId** (Part 9 §5.7.3). The event's `ActiveState/Id` + `AckedState/Id` are selected and turned into the
+> `Raise`/`Acknowledge`/`Clear`/`Retrigger` kinds the host projector consumes (without a kind no upstream alarm could ever
+> show active). No `ConditionRefresh` is issued on subscribe, so an upstream alarm already active when the subscription
+> opens appears at its next transition. Index keys are server-stable (`nsu=`), so a namespace reorder across a reconnect keeps
 > matching. docker-dev seeds an opc-plc driver (`MAIN-opcua`); the alarm tag is
 > created only when `@OpcPlcAlarmSourceNodeId` is set in `docker-dev/seed/seed-clusters.sql`.
 

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs

@@ -98,6 +98,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
     /// node, which differs from the authored source node.</summary>
     private readonly System.Collections.Concurrent.ConcurrentDictionary<string, AlarmAckTarget> _alarmConditionByRawPath =
         new(StringComparer.Ordinal);
+
+    /// <summary>#619 review: the (Active, Acked) state last told to the host per reported condition id, from
+    /// which <see cref="AlarmTransitions"/> picks the Kind sequence that moves the host's projector to the
+    /// upstream event's state.</summary>
+    private readonly System.Collections.Concurrent.ConcurrentDictionary<string, (bool Active, bool Acked)> _alarmStateByRawPath =
+        new(StringComparer.Ordinal);
     private long _nextAlarmSubscriptionId;
 
     /// <summary>Occurs when an alarm event is received from the remote OPC UA server.</summary>
@@ -886,6 +892,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
         }
         _alarmSubscriptions.Clear();
         _alarmConditionByRawPath.Clear();
+        _alarmStateByRawPath.Clear();
 
         await ReleaseSessionAsync(cancellationToken).ConfigureAwait(false);
         _namespaceMap = null;
@@ -1628,6 +1635,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
     private const int AlarmFieldSeverity = 4;
     private const int AlarmFieldTime = 5;
     private const int AlarmFieldConditionId = 6;
+    private const int AlarmFieldActiveState = 7;
+    private const int AlarmFieldAckedState = 8;
 
     /// <inheritdoc />
     public async Task<IAlarmSubscriptionHandle> SubscribeAlarmsAsync(
@@ -1655,9 +1664,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
                 && !NamespaceMap.TryResolve(sessionForResolve, raw, out live)) return null;
             return AlarmIndexKey(mapForResolve, live);
         });
-        // Forget ack targets for alarm tags no longer subscribed (removed/renamed RawPaths).
+        // Forget ack targets + last-told states for alarm tags no longer subscribed (removed/renamed RawPaths).
         foreach (var known in _alarmConditionByRawPath.Keys)
             if (!sourceNodeIds.Contains(known, StringComparer.Ordinal)) _alarmConditionByRawPath.TryRemove(known, out _);
+        foreach (var known in _alarmStateByRawPath.Keys)
+            if (!sourceNodeIds.Contains(known, StringComparer.Ordinal)) _alarmStateByRawPath.TryRemove(known, out _);
 
         var subscription = new Subscription(telemetry: null!, new SubscriptionOptions
         {
@@ -1696,6 +1707,20 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
             BrowsePath = [], // empty path = the condition node itself
             AttributeId = Attributes.NodeId,
         });
+        // #619 review: the host derives Active/Acked solely from AlarmEventArgs.Kind, so the condition state
+        // must be selected. TWO-element browse paths — a single "ActiveState/Id" QualifiedName names nothing.
+        filter.SelectClauses.Add(new SimpleAttributeOperand
+        {
+            TypeDefinitionId = ObjectTypeIds.AlarmConditionType,
+            BrowsePath = [new QualifiedName(BrowseNames.ActiveState), new QualifiedName(BrowseNames.Id)],
+            AttributeId = Attributes.Value,
+        });
+        filter.SelectClauses.Add(new SimpleAttributeOperand
+        {
+            TypeDefinitionId = ObjectTypeIds.AcknowledgeableConditionType,
+            BrowsePath = [new QualifiedName(BrowseNames.AckedState), new QualifiedName(BrowseNames.Id)],
+            AttributeId = Attributes.Value,
+        });
 
         await _gate.WaitAsync(cancellationToken).ConfigureAwait(false);
         try
@@ -1874,8 +1899,58 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
     {
         if (args.NotificationValue is not EventFieldList efl) return;
         var e = ProjectEvent(efl.EventFields, sourceFilter, handle,
-            n => AlarmIndexKey(_namespaceMap, n), (raw, target) => _alarmConditionByRawPath[raw] = target);
-        if (e is not null) OnAlarmEvent?.Invoke(this, e);
+            n => AlarmIndexKey(_namespaceMap, n), (raw, target) => _alarmConditionByRawPath[raw] = target,
+            out var state);
+        if (e is null) return;
+        if (state is not { } target)
+        {
+            // Not a condition (no ActiveState/AckedState) — a message/severity-only update, as before.
+            OnAlarmEvent?.Invoke(this, e);
+            return;
+        }
+
+        // Fresh prior = the host projector's own default (inactive, acked).
+        var prior = _alarmStateByRawPath.TryGetValue(e.ConditionId, out var p) ? p : (Active: false, Acked: true);
+        _alarmStateByRawPath[e.ConditionId] = target;
+        foreach (var kind in AlarmTransitions(prior, target))
+            OnAlarmEvent?.Invoke(this, e with { Kind = kind });
+    }
+
+    /// <summary>
+    ///     #619 review — the <see cref="AlarmTransitionKind"/> sequence that moves the host's
+    ///     <c>NativeAlarmProjector</c> (which knows only kinds: Raise ⇒ active+unacked, Acknowledge ⇒ acked,
+    ///     Clear ⇒ inactive keeping acked) from <paramref name="prior"/> to <paramref name="target"/>. A single
+    ///     upstream event can need two (an alarm that went active and was acknowledged between two notifications).
+    ///     No state change ⇒ one <see cref="AlarmTransitionKind.Unspecified"/> (message/severity refresh).
+    /// </summary>
+    /// <param name="prior">The state last told to the host.</param>
+    /// <param name="target">The upstream event's ActiveState/Id + AckedState/Id.</param>
+    /// <returns>The kinds to emit, in order.</returns>
+    internal static IReadOnlyList<AlarmTransitionKind> AlarmTransitions(
+        (bool Active, bool Acked) prior, (bool Active, bool Acked) target)
+    {
+        var kinds = new List<AlarmTransitionKind>(2);
+        if (target.Active && !prior.Active)
+        {
+            kinds.Add(AlarmTransitionKind.Raise);
+            if (target.Acked) kinds.Add(AlarmTransitionKind.Acknowledge);
+        }
+        else if (!target.Active && prior.Active)
+        {
+            kinds.Add(AlarmTransitionKind.Clear);
+            if (target.Acked && !prior.Acked) kinds.Add(AlarmTransitionKind.Acknowledge);
+        }
+        else if (target.Acked && !prior.Acked)
+        {
+            kinds.Add(AlarmTransitionKind.Acknowledge);
+        }
+        else if (target.Active && !target.Acked && prior.Acked)
+        {
+            kinds.Add(AlarmTransitionKind.Retrigger);
+        }
+
+        if (kinds.Count == 0) kinds.Add(AlarmTransitionKind.Unspecified);
+        return kinds;
     }
 
     /// <summary>
@@ -1907,9 +1982,25 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
         IReadOnlyDictionary<string, string> sourceIndex,
         IAlarmSubscriptionHandle handle,
         Func<NodeId, string> keyOf,
-        Action<string, AlarmAckTarget> rememberCondition)
+        Action<string, AlarmAckTarget> rememberCondition) =>
+        ProjectEvent(fields, sourceIndex, handle, keyOf, rememberCondition, out _);
+
+    /// <summary>As the five-argument overload, also returning the event's (ActiveState/Id, AckedState/Id) through
+    /// <c>state</c> when it is an alarm condition (null otherwise).</summary>
+    internal static AlarmEventArgs? ProjectEvent(
+        IReadOnlyList<Variant> fields,
+        IReadOnlyDictionary<string, string> sourceIndex,
+        IAlarmSubscriptionHandle handle,
+        Func<NodeId, string> keyOf,
+        Action<string, AlarmAckTarget> rememberCondition,
+        out (bool Active, bool Acked)? state)
     {
+        state = null;
         if (fields.Count <= AlarmFieldConditionId) return null;
+        if (fields.Count > AlarmFieldAckedState
+            && fields[AlarmFieldActiveState].Value is bool active
+            && fields[AlarmFieldAckedState].Value is bool acked)
+            state = (active, acked);
 
         // Pass-through (no authored refs) keeps the raw serialized ids; an authored index compares
         // server-stable keys (see AlarmIndexKey), matching the CONDITION node first — the only way to tell

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientAlarmTests.cs

@@ -196,6 +196,42 @@ public sealed class OpcUaClientAlarmTests
         target.Value.EventId.ShouldBe(new byte[] { 1 });
     }
 
+    // ---- #619 review: the host's NativeAlarmProjector derives Active/Acked ONLY from AlarmEventArgs.Kind; an
+    // Unspecified kind keeps the prior state (inactive+acked when fresh), so without a Kind no upstream alarm
+    // could ever show active. ----
+
+    /// <summary>ActiveState/Id and AckedState/Id are read from the two trailing select clauses.</summary>
+    [Fact]
+    public void ProjectEvent_reads_the_condition_state()
+    {
+        var index = new Dictionary<string, string> { ["ns=3;s=Alarms.Trip1"] = "Plant/opc/plc/Trip1" };
+        var fields = EventFields("ns=3;s=Alarms.Trip1", "ns=3;s=Alarms.Trip1.Cond");
+        fields.Add(new Variant(true));    // ActiveState/Id
+        fields.Add(new Variant(false));   // AckedState/Id
+
+        OpcUaClientDriver.ProjectEvent(fields, index, new FakeAlarmHandle(), n => n.ToString(), (_, _) => { },
+            out var state);
+
+        state.ShouldBe((true, false));
+    }
+
+    /// <summary>The transition sequence drives a projector from the driver's last-told state to the event's.</summary>
+    [Theory]
+    [InlineData(false, true, true, false, new[] { AlarmTransitionKind.Raise })]
+    [InlineData(false, true, true, true, new[] { AlarmTransitionKind.Raise, AlarmTransitionKind.Acknowledge })]
+    [InlineData(true, false, true, true, new[] { AlarmTransitionKind.Acknowledge })]
+    [InlineData(true, false, false, false, new[] { AlarmTransitionKind.Clear })]
+    [InlineData(true, true, false, true, new[] { AlarmTransitionKind.Clear })]
+    [InlineData(false, false, false, true, new[] { AlarmTransitionKind.Acknowledge })]
+    [InlineData(true, false, false, true, new[] { AlarmTransitionKind.Clear, AlarmTransitionKind.Acknowledge })]
+    [InlineData(true, true, true, false, new[] { AlarmTransitionKind.Retrigger })]
+    [InlineData(true, false, true, false, new[] { AlarmTransitionKind.Unspecified })]
+    public void AlarmTransitions_move_prior_to_target(
+        bool priorActive, bool priorAcked, bool active, bool acked, AlarmTransitionKind[] expected)
+    {
+        OpcUaClientDriver.AlarmTransitions((priorActive, priorAcked), (active, acked)).ShouldBe(expected);
+    }
+
     private sealed class FakeAlarmHandle : IAlarmSubscriptionHandle
     {
         /// <summary>Gets the diagnostic identifier for this alarm handle.</summary>