fix(opcua): shelve declaration ids from a fixed set, not an address-space lookup; pin instance-id + state-machine-id shelve calls (#629)

New() ran FindNodeInAddressSpace under Lock, taking other node managers' locks (lock-order
risk); the three AlarmConditionType_ShelvingState_* ids are now a static set. Adds wire tests:
TimedShelve/Unshelve by the condition's own instance method ids (object = condition or its
ShelvingState) succeed and report, and a ShelvedStateMachineType_* id call is pinned as
BadMethodInvalid with a positive control.

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

docs/AlarmTracking.md

@@ -135,9 +135,11 @@ them by type + browse path), and neither were the Part 9 methods — calling the
 its declaration id. ⚠️ The ShelvingState methods needed help here: their SDK declaration id is the
 `ShelvedStateMachineType`'s (`i=2948`…), and they were only ever found by the `AlarmConditionType_ShelvingState_*`
 id (`i=9212`…) because that WAS their NodeId. `New` adopts that original id as the method's
-`MethodDeclarationId` (only when it is a real standard-namespace method node, never an SDK-generated
-number), so OneShotShelve/TimedShelve/Unshelve by the `AlarmConditionType_ShelvingState_*` id keep working.
-Calling them by the `ShelvedStateMachineType_*` id is not supported.
+`MethodDeclarationId` (a fixed set of those three ids — no address-space lookup, which would take other
+node managers' locks under ours), so OneShotShelve/TimedShelve/Unshelve by the
+`AlarmConditionType_ShelvingState_*` id keep working, as do calls by the condition's own instance method
+ids (`{ConditionId}/ShelvingState/TimedShelve`, on the condition or its ShelvingState). Calling them by the
+`ShelvedStateMachineType_*` id returns `BadMethodInvalid` — a pinned trade-off.
 
 - **ACL:** the slash-joined id makes a child's ACL scope its condition's scope plus the browse path, so
   with `Security:Acl:Enabled` a child is governed exactly like its condition (a grant covering the

src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs

@@ -2499,6 +2499,20 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
         _ => new AlarmConditionState(parent),
     };
 
+    /// <summary>
+    ///     Gitea #629 — the <c>AlarmConditionType_ShelvingState_*</c> method instance declarations a client calls
+    ///     the shelve methods by. <see cref="New"/> records them as the methods' <c>MethodDeclarationId</c>
+    ///     (the SDK's own is the <c>ShelvedStateMachineType</c>'s), so calls by these ids keep resolving after
+    ///     the methods move to instance-scoped NodeIds. Calls by the <c>ShelvedStateMachineType_*</c> ids are
+    ///     not supported (pinned by <c>ConditionChildNodeIdTests</c>).
+    /// </summary>
+    private static readonly HashSet<NodeId> ShelvingMethodDeclarationIds =
+    [
+        MethodIds.AlarmConditionType_ShelvingState_TimedShelve,
+        MethodIds.AlarmConditionType_ShelvingState_OneShotShelve,
+        MethodIds.AlarmConditionType_ShelvingState_Unshelve,
+    ];
+
     /// <summary>
     ///     Gitea #629 — the <see cref="INodeIdFactory"/> hook <c>NodeState.Create(…, assignNodeIds: true)</c> calls
     ///     for the node being created and then, top-down, for every child it built from the type's definition.
@@ -2543,15 +2557,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
             // already, over an SDK-generated ns=0 NodeId. The ShelvingState methods do NOT: their declaration id
             // is the ShelvedStateMachineType's (i=2948), and they were found only because their NodeId WAS the
             // AlarmConditionType instance declaration every client uses. Adopt that id as the declaration before
-            // it is replaced — but only when it is a real node of the standard address space, never an
-            // SDK-generated number — or every shelve/unshelve call fails BadMethodInvalid.
-            if (node is MethodState method
-                && method.NodeId is { NamespaceIndex: 0 } declarationId
-                && declarationId != method.MethodDeclarationId
-                && FindNodeInAddressSpace(declarationId) is MethodState)
-            {
-                method.MethodDeclarationId = declarationId;
-            }
+            // it is replaced, or every shelve/unshelve call fails BadMethodInvalid. A fixed set, deliberately: a
+            // lookup in the standard address space would take other node managers' locks while we hold Lock.
+            if (node is MethodState method && ShelvingMethodDeclarationIds.Contains(method.NodeId))
+                method.MethodDeclarationId = method.NodeId;
 
             return new NodeId($"{parentId.Identifier}/{node.BrowseName.Name}", parentId.NamespaceIndex);
         }

tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionChildNodeIdTests.cs

@@ -246,8 +246,96 @@ public sealed class ConditionChildNodeIdTests
         finally { await rig.DisposeAsync(); }
     }
 
+    /// <summary>
+    ///     The shelve methods resolve by the condition's OWN child method ids
+    ///     (<c>{condition}/ShelvingState/TimedShelve</c>, <c>…/Unshelve</c>) — the ids a browse of the condition
+    ///     returns — whether the call names the condition or its ShelvingState as the object, and each accepted
+    ///     call reports its one condition event.
+    /// </summary>
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task Shelve_methods_called_by_their_instance_ids_succeed_and_report(bool objectIsShelvingState)
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var rig = await StartAsync($"CondChildMethodIds{objectIsShelvingState}", encrypted: false, ct);
+        try
+        {
+            rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", AllMethods));
+            rig.MaterialiseScripted();
+            var current = ConditionMethodEventTests.PreState("TimedShelve", native: false) with { Message = "base" };
+            ConditionMethodEventTests.WireSimulatedOwners(rig, native: false, () => current, s => current = s);
+
+            using var session = await rig.ConnectAsync(anonymous: false, ct);
+            var events = new RecordedEvents();
+            var subscription = await RecordedEvents.SubscribeAsync(session, events, ct);
+            var item = RecordedEvents.AddEventItem(subscription, ObjectIds.Server);
+            await subscription.ApplyChangesAsync(ct);
+
+            rig.Project(false, current);
+            await RecordedEvents.WaitUntilAsync(() => events.Has(item, "base"));
+
+            var condition = ConditionNodeId(session, native: false);
+            var ns = condition.NamespaceIndex;
+            var shelvingState = new NodeId($"{ScriptedAlarmId}/ShelvingState", ns);
+            var target = objectIsShelvingState ? shelvingState : condition;
+
+            await session.CallAsync(target, new NodeId($"{ScriptedAlarmId}/ShelvingState/TimedShelve", ns), ct, 60_000d);
+            await rig.DrainProjectionsAsync();
+            await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count == 2);
+            events.ConditionEvents(item, "base")[1][RecordedEvents.ShelvingField].Value
+                .ShouldBe(ObjectIds.ShelvedStateMachineType_TimedShelved);
+
+            await session.CallAsync(target, new NodeId($"{ScriptedAlarmId}/ShelvingState/Unshelve", ns), ct);
+            await rig.DrainProjectionsAsync();
+            await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count == 3);
+            events.ConditionEvents(item, "base")[2][RecordedEvents.ShelvingField].Value
+                .ShouldBe(ObjectIds.ShelvedStateMachineType_Unshelved);
+
+            await subscription.DeleteAsync(true, ct);
+        }
+        finally { await rig.DisposeAsync(); }
+    }
+
+    /// <summary>
+    ///     Pins the #629 trade-off: the shelve methods resolve by the condition's instance ids and by the
+    ///     <c>AlarmConditionType_ShelvingState_*</c> ids every client uses (the positive control), but NOT by
+    ///     the <c>ShelvedStateMachineType_*</c> ids — those were the methods' SDK declarations, replaced by the
+    ///     AlarmConditionType ones when the methods moved off their ns=0 NodeIds. Change this deliberately.
+    /// </summary>
+    [Fact]
+    public async Task A_shelve_call_by_the_state_machine_types_method_id_is_not_supported()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var rig = await StartAsync("CondChildMethodStateMachineId", encrypted: false, ct);
+        try
+        {
+            rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", AllMethods));
+            rig.MaterialiseScripted();
+            var current = ConditionMethodEventTests.PreState("OneShotShelve", native: false);
+            ConditionMethodEventTests.WireSimulatedOwners(rig, native: false, () => current, s => current = s);
+            rig.Project(false, current);
+
+            using var session = await rig.ConnectAsync(anonymous: false, ct);
+            var condition = ConditionNodeId(session, native: false);
+
+            var refused = await Should.ThrowAsync<ServiceResultException>(
+                session.CallAsync(condition, MethodIds.ShelvedStateMachineType_OneShotShelve, ct));
+            refused.StatusCode.ShouldBe(StatusCodes.BadMethodInvalid);
+
+            // Positive control: the AlarmConditionType declaration id resolves on the same condition.
+            await session.CallAsync(condition, MethodIds.AlarmConditionType_ShelvingState_OneShotShelve, ct);
+        }
+        finally { await rig.DisposeAsync(); }
+    }
+
     // ───────────────────────────── helpers ─────────────────────────────
 
+    private const NodePermissions AllMethods =
+        NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe | NodePermissions.AlarmRead
+        | NodePermissions.AlarmAcknowledge | NodePermissions.AlarmConfirm | NodePermissions.AlarmShelve
+        | NodePermissions.MethodCall;
+
     /// <summary>
     ///     Native: active, unacked, "native-distinct". Scripted: active, unacked, "scripted-distinct", timed
     ///     shelved for 60 s — none of which the condition type's instance declarations can hold.