fix(opcuaclient): alarm index survives namespace reorder; match condition node first; ack with real EventId (#619 review)

The alarm index was keyed on ns=N strings resolved once at subscribe time, while an
in-place SDK reconnect transfers the subscription without re-running it - a remote
namespace reorder silently orphaned every alarm tag. Keys are now nsu= on both sides.
Events match the authored nodeId against the event's ConditionId first, then its
SourceNode, so several conditions on one source can be authored 1:1. Acknowledge
passes the EventId of the event that named the remembered condition (paired, so node
and EventId can never come from different conditions) instead of an empty one.

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

docs/drivers/OpcUaClient.md

@@ -93,9 +93,12 @@ rather than surfacing as a runtime surprise.
 
 > **Native alarms (#619).** An alarm raw tag is `TagConfig` `{ "nodeId": "<upstream SourceNode>", "alarm": {...} }`.
 > The host subscribes with RawPaths and routes on (driver, RawPath), so the driver matches each event's
-> `SourceNode` against the tag's authored `nodeId` and reports the **RawPath** as `ConditionId`; the upstream
-> condition node (last seen per RawPath) is what `Acknowledge` is called on. `nodeId` must therefore be the
-> event's *source*, not the condition node. docker-dev seeds an opc-plc driver (`MAIN-opcua`); the alarm tag is
+> event's `ConditionId` (then its `SourceNode`) against the tag's authored `nodeId` and reports the **RawPath** as
+> `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
+> 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`.
 
 > This driver does **not** implement `IRediscoverable` — there is no

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

@@ -96,7 +96,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
     /// <summary>#619: RawPath → the upstream condition NodeId last seen for it. The host acknowledges by
     /// RawPath (its only identity for the condition) but Part 9 Acknowledge must be called on the CONDITION
     /// node, which differs from the authored source node.</summary>
-    private readonly System.Collections.Concurrent.ConcurrentDictionary<string, string> _alarmConditionByRawPath =
+    private readonly System.Collections.Concurrent.ConcurrentDictionary<string, AlarmAckTarget> _alarmConditionByRawPath =
         new(StringComparer.Ordinal);
     private long _nextAlarmSubscriptionId;
 
@@ -885,6 +885,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
             catch { /* best-effort */ }
         }
         _alarmSubscriptions.Clear();
+        _alarmConditionByRawPath.Clear();
 
         await ReleaseSessionAsync(cancellationToken).ConfigureAwait(false);
         _namespaceMap = null;
@@ -1643,12 +1644,20 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
         // match in O(1) without re-parsing on every event.
         // #619: under v3 the host passes RawPaths, never upstream node ids. Translate each to the upstream
         // source id its tag authored (TagConfig.nodeId) so the per-event match compares like with like.
+        // Keys are server-stable (AlarmIndexKey) so a namespace reorder across an in-place SDK reconnect —
+        // which transfers this subscription without re-running SubscribeAlarmsAsync — cannot orphan them.
         var sessionForResolve = Session;
+        var mapForResolve = _namespaceMap;
         var sourceFilter = BuildAlarmSourceIndex(sourceNodeIds, raw =>
         {
-            if (sessionForResolve is null || !TryResolveTagNode(sessionForResolve, raw, out var live)) return null;
-            return live.ToString();
+            if (sessionForResolve is null) return null;
+            if (!TryResolveTagNode(sessionForResolve, raw, out var live)
+                && !NamespaceMap.TryResolve(sessionForResolve, raw, out live)) return null;
+            return AlarmIndexKey(mapForResolve, live);
         });
+        // Forget ack targets 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 _);
 
         var subscription = new Subscription(telemetry: null!, new SubscriptionOptions
         {
@@ -1778,18 +1787,30 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
             var callRequests = new CallMethodRequestCollection();
             foreach (var ack in acknowledgements)
             {
-                // ConditionId is an upstream condition-node id captured from the alarm event (not a
-                // tag RawPath), so it re-binds directly against the session via the static resolve.
-                // #619: the host acks by RawPath; Acknowledge must hit the upstream CONDITION node last seen for it.
-                var upstreamCondition = _alarmConditionByRawPath.TryGetValue(ack.ConditionId, out var seen)
-                    ? seen : ack.ConditionId;
-                if (!NamespaceMap.TryResolve(session, upstreamCondition, out var conditionId)) continue;
+                // #619: the host acks by RawPath; Acknowledge must hit the upstream CONDITION node.
+                // The remembered target pairs the condition node with the EventId of the event that named it,
+                // so the ack can never mix one condition's node with another's EventId. Nothing seen yet ⇒ the
+                // tag's authored node (correct when the tag was authored on the condition node), else the id
+                // verbatim (pre-v3 callers that pass an upstream condition id directly).
+                NodeId conditionId;
+                byte[] eventId;
+                if (_alarmConditionByRawPath.TryGetValue(ack.ConditionId, out var seen))
+                {
+                    if (!NamespaceMap.TryResolve(session, seen.ConditionRef, out conditionId)) continue;
+                    eventId = seen.EventId;
+                }
+                else
+                {
+                    if (!TryResolveTagNode(session, ack.ConditionId, out conditionId)
+                        && !NamespaceMap.TryResolve(session, ack.ConditionId, out conditionId)) continue;
+                    eventId = Array.Empty<byte>();
+                }
                 callRequests.Add(new CallMethodRequest
                 {
                     ObjectId = conditionId,
                     MethodId = MethodIds.AcknowledgeableConditionType_Acknowledge,
                     InputArguments = [
-                        new Variant(Array.Empty<byte>()),  // EventId — server-side best-effort; empty resolves to 'most recent'
+                        new Variant(eventId),  // Part 9 §5.7.3: the EventId being acknowledged (empty only when none seen)
                         new Variant(new LocalizedText(ack.Comment ?? string.Empty)),
                     ],
                 });
@@ -1853,7 +1874,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
     {
         if (args.NotificationValue is not EventFieldList efl) return;
         var e = ProjectEvent(efl.EventFields, sourceFilter, handle,
-            (raw, cond) => _alarmConditionByRawPath[raw] = cond);
+            n => AlarmIndexKey(_namespaceMap, n), (raw, target) => _alarmConditionByRawPath[raw] = target);
         if (e is not null) OnAlarmEvent?.Invoke(this, e);
     }
 
@@ -1885,22 +1906,39 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
         IReadOnlyList<Variant> fields,
         IReadOnlyDictionary<string, string> sourceIndex,
         IAlarmSubscriptionHandle handle,
-        Action<string, string> rememberCondition)
+        Func<NodeId, string> keyOf,
+        Action<string, AlarmAckTarget> rememberCondition)
     {
         if (fields.Count <= AlarmFieldConditionId) return null;
 
-        var sourceNode = fields[AlarmFieldSourceNode].Value?.ToString() ?? string.Empty;
-        string reported;
-        if (sourceIndex.Count == 0) reported = sourceNode;
-        else if (!sourceIndex.TryGetValue(sourceNode, out reported!)) return null;
+        // 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
+        // several conditions on one source apart — and then the source node.
+        var sourceField = fields[AlarmFieldSourceNode].Value;
+        var conditionField = fields[AlarmFieldConditionId].Value;
+        string reported, upstreamCondition;
+        if (sourceIndex.Count == 0)
+        {
+            reported = sourceField?.ToString() ?? string.Empty;
+            upstreamCondition = conditionField?.ToString() ?? string.Empty;
+        }
+        else
+        {
+            upstreamCondition = conditionField is NodeId cn && !NodeId.IsNull(cn) ? keyOf(cn) : string.Empty;
+            var sourceKey = sourceField is NodeId sn && !NodeId.IsNull(sn) ? keyOf(sn) : string.Empty;
+            if (!(upstreamCondition.Length > 0 && sourceIndex.TryGetValue(upstreamCondition, out reported!))
+                && !(sourceKey.Length > 0 && sourceIndex.TryGetValue(sourceKey, out reported!)))
+                return null;
+        }
 
         var eventType = fields[AlarmFieldEventType].Value?.ToString() ?? "BaseEventType";
         var message = (fields[AlarmFieldMessage].Value as LocalizedText)?.Text ?? string.Empty;
         var severity = fields[AlarmFieldSeverity].Value is ushort sev ? sev : (ushort)0;
         var time = fields[AlarmFieldTime].Value is DateTime t ? t : DateTime.UtcNow;
-        var upstreamCondition = fields[AlarmFieldConditionId].Value?.ToString() ?? string.Empty;
 
-        if (sourceIndex.Count != 0 && upstreamCondition.Length > 0) rememberCondition(reported, upstreamCondition);
+        if (sourceIndex.Count != 0 && upstreamCondition.Length > 0)
+            rememberCondition(reported, new AlarmAckTarget(
+                upstreamCondition, fields[AlarmFieldEventId].Value as byte[] ?? Array.Empty<byte>()));
 
         return new AlarmEventArgs(
             SubscriptionHandle: handle,
@@ -1912,6 +1950,20 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
             SourceTimestampUtc: time);
     }
 
+    /// <summary>#619 — what Acknowledge needs for one RawPath: the upstream condition node (server-stable
+    /// reference) and the EventId of the event that named it.</summary>
+    /// <param name="ConditionRef">Upstream condition NodeId, rendered by <see cref="AlarmIndexKey"/>.</param>
+    /// <param name="EventId">EventId of the most recent event seen for that condition.</param>
+    internal readonly record struct AlarmAckTarget(string ConditionRef, byte[] EventId);
+
+    /// <summary>#619 — the key both sides of the alarm index use: server-stable (<c>nsu=</c>) when a map is
+    /// available, so a remote namespace-table reorder across an in-place reconnect cannot orphan it.</summary>
+    /// <param name="map">The current session's namespace map, or null.</param>
+    /// <param name="nodeId">The NodeId to key.</param>
+    /// <returns>The index key.</returns>
+    internal static string AlarmIndexKey(NamespaceMap? map, NodeId nodeId) =>
+        map?.ToStableReference(nodeId) ?? nodeId.ToString() ?? string.Empty;
+
     /// <summary>
     ///     Map an OPC UA <c>BaseEventType.Severity</c> (1..1000) to our coarse-grained
     ///     <see cref="AlarmSeverity"/> bucket. Thresholds match the OPC UA A&amp;C Part 9

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

@@ -113,7 +113,7 @@ public sealed class OpcUaClientAlarmTests
 
         var e = OpcUaClientDriver.ProjectEvent(
             EventFields("ns=3;s=Alarms.Trip1", "ns=3;s=Alarms.Trip1.Cond"), index, new FakeAlarmHandle(),
-            (raw, cond) => { rememberedRaw = raw; rememberedCond = cond; });
+            n => n.ToString(), (raw, target) => { rememberedRaw = raw; rememberedCond = target.ConditionRef; });
 
         e.ShouldNotBeNull();
         e.ConditionId.ShouldBe("Plant/opc/plc/Trip1");
@@ -128,10 +128,74 @@ public sealed class OpcUaClientAlarmTests
     {
         var index = new Dictionary<string, string> { ["ns=3;s=Alarms.Trip1"] = "Plant/opc/plc/Trip1" };
         OpcUaClientDriver.ProjectEvent(
-            EventFields("ns=3;s=Other", "ns=3;s=Other.Cond"), index, new FakeAlarmHandle(), (_, _) => { })
+            EventFields("ns=3;s=Other", "ns=3;s=Other.Cond"), index, new FakeAlarmHandle(), n => n.ToString(), (_, _) => { })
             .ShouldBeNull();
     }
 
+    /// <summary>
+    ///     A remote namespace-table reorder across an in-place SDK reconnect must not orphan the alarm index:
+    ///     it is built once at subscribe time, while events keep arriving on the (transferred) subscription with
+    ///     the NEW namespace indexes. Keys are therefore server-stable (<c>nsu=</c>) on both sides.
+    /// </summary>
+    [Fact]
+    public void ProjectEvent_matches_across_a_namespace_reorder()
+    {
+        var before = new NamespaceTable(); before.Append("urn:other"); before.Append("urn:plc");   // urn:plc = 2
+        var after = new NamespaceTable(); after.Append("urn:new"); after.Append("urn:other"); after.Append("urn:plc"); // urn:plc = 3
+        var mapBefore = NamespaceMap.FromTable(before);
+        var mapAfter = NamespaceMap.FromTable(after);
+
+        var index = OpcUaClientDriver.BuildAlarmSourceIndex(
+            ["Plant/opc/plc/Trip1"], _ => OpcUaClientDriver.AlarmIndexKey(mapBefore, new NodeId("Alarms.Trip1", 2)));
+
+        var e = OpcUaClientDriver.ProjectEvent(
+            EventFields("ns=3;s=Alarms.Trip1", "ns=3;s=Alarms.Trip1.Cond"), index, new FakeAlarmHandle(),
+            n => OpcUaClientDriver.AlarmIndexKey(mapAfter, n), (_, _) => { });
+
+        e.ShouldNotBeNull();
+        e.ConditionId.ShouldBe("Plant/opc/plc/Trip1");
+    }
+
+    /// <summary>
+    ///     A source with several conditions (Hi + HiHi on one variable) can only be told apart by the CONDITION
+    ///     node, so a tag authored on the condition node matches on the event's ConditionId — 1:1, no collapse.
+    /// </summary>
+    [Fact]
+    public void ProjectEvent_matches_a_tag_authored_on_the_condition_node()
+    {
+        var index = new Dictionary<string, string>
+        {
+            ["ns=3;s=Tank.HiHi"] = "Plant/opc/plc/HiHi",
+            ["ns=3;s=Tank.Hi"] = "Plant/opc/plc/Hi",
+        };
+
+        var e = OpcUaClientDriver.ProjectEvent(
+            EventFields("ns=3;s=Tank", "ns=3;s=Tank.HiHi"), index, new FakeAlarmHandle(), n => n.ToString(), (_, _) => { });
+
+        e.ShouldNotBeNull();
+        e.ConditionId.ShouldBe("Plant/opc/plc/HiHi");
+    }
+
+    /// <summary>
+    ///     The ack target pairs the condition node WITH the EventId of the event that named it, so Acknowledge
+    ///     carries the real EventId (Part 9 §5.7.3) instead of an empty one a strict server rejects
+    ///     <c>BadEventIdUnknown</c>, and the two can never come from different conditions.
+    /// </summary>
+    [Fact]
+    public void ProjectEvent_remembers_the_condition_with_its_EventId()
+    {
+        var index = new Dictionary<string, string> { ["ns=3;s=Alarms.Trip1"] = "Plant/opc/plc/Trip1" };
+        OpcUaClientDriver.AlarmAckTarget? target = null;
+
+        OpcUaClientDriver.ProjectEvent(
+            EventFields("ns=3;s=Alarms.Trip1", "ns=3;s=Alarms.Trip1.Cond"), index, new FakeAlarmHandle(),
+            n => n.ToString(), (_, t) => target = t);
+
+        target.ShouldNotBeNull();
+        target.Value.ConditionRef.ShouldBe("ns=3;s=Alarms.Trip1.Cond");
+        target.Value.EventId.ShouldBe(new byte[] { 1 });
+    }
+
     private sealed class FakeAlarmHandle : IAlarmSubscriptionHandle
     {
         /// <summary>Gets the diagnostic identifier for this alarm handle.</summary>