feat(historian-sidecar): live aahClientManaged alarm-event write path (C.1)

SdkAlarmHistorianWriteBackend.WriteBatchAsync replaces the RetryPlease
placeholder with the real entry point — HistorianAccess.AddStreamedValue
(HistorianEvent, out HistorianAccessError) in aahClientManaged, pinned by
decompiling the installed SDK.

The write path opens its own ReadOnly=false connection: the query-side
HistorianDataSource opens ReadOnly sessions and AddStreamedValue fails on
those with WriteToReadOnlyFile. IHistorianConnectionFactory gains a readOnly
parameter (default true, query path unchanged); BuildConnectionArgs is
extracted as a pure helper. HistorianClusterEndpointPicker is shared for
node failover; connection-class errors abort the batch as RetryPlease and
reset the connection, malformed-input codes map to PermanentFail.

Tests: connection-unavailable batch deferral, ClassifyOutcome error-code
table, BuildConnectionArgs read-vs-write shaping (80 pass, 2 rig-skipped).
Live_* round-trip tests stay Skip-gated for the D.1 rollout smoke.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs/plans/alarms-worker-wiring-plan.md

@@ -212,36 +212,40 @@ x64, which is not bitness-constrained like the worker). C.1 is independently
 unblockable from A.2 if the goal is to wire up the scripted-alarm historian
 path.
 
-**Current state**:
+**Current state (DONE — code)**:
 
-`SdkAlarmHistorianWriteBackend` in `src\MxGateway.Worker\MxAccess\` is a
-placeholder returning `RetryPlease`. The lmxopcua sidecar's `WriteAlarmEvents`
-IPC slot is defined in `Ipc\Contracts.cs` but `Program.cs` constructs
-`HistorianFrameHandler` without an `alarmWriter` (line 57 per the alarms plan).
-The `IAlarmEventWriter` interface exists; only the production implementation
-and the consumer wiring are missing.
+C.1 shipped. `SdkAlarmHistorianWriteBackend.WriteBatchAsync` writes through the
+real SDK entry point — **`HistorianAccess.AddStreamedValue(HistorianEvent, out
+HistorianAccessError)`** in `aahClientManaged` — pinned 2026-05-18 by
+decompiling the installed SDK. `Program.cs` and `Install-Services.ps1` were
+already wired in the PR C.1 scaffolding. Two corrections to the assumptions
+this doc was written under:
 
-**What it needs**:
+- **There is no `ArchestrAAlarmsAndEvents.SDK` writer.** That assembly
+  (`ArchestrAAlarmsAndEvents.SDK.Common.dll`, the only one installed) is a WCF
+  query-proxy base — no `AlarmHistorianWriter` type. The write path is the
+  `aahClientManaged` `HistorianAccess` surface.
+- **The write path needs its own connection.** The query-side
+  `HistorianDataSource` opens `ReadOnly` sessions; `AddStreamedValue` on a
+  read-only session fails with `WriteToReadOnlyFile`.
+  `SdkAlarmHistorianWriteBackend` opens a dedicated `ReadOnly=false` connection
+  and shares only `HistorianClusterEndpointPicker` (not the connection object).
 
-1. New `AahClientManagedAlarmEventWriter.cs` implementing `IAlarmEventWriter`
-   (defined in `Ipc\HistorianFrameHandler.cs`). Calls `aahClientManaged`'s
-   alarm-event write API — same path v1's `GalaxyHistorianWriter` used.
-   Uses `HistorianClusterEndpointPicker` for multi-node routing.
-   Maps `MxStatus` write outcomes to `HistorianWriteOutcome` enum
-   (Ack / PermanentFail / RetryPlease).
+**What it needed** (all done):
 
-2. `Program.cs` — build `AahClientManagedAlarmEventWriter` next to the
-   existing `BuildHistorian()` call; pass it to `HistorianFrameHandler`.
-   Gate behind `OTOPCUA_HISTORIAN_ALARM_WRITE_ENABLED` env var (default `true`
-   when `OTOPCUA_HISTORIAN_ENABLED=true`).
+1. `SdkAlarmHistorianWriteBackend` builds a `HistorianEvent` per
+   `AlarmHistorianEventDto`, calls `AddStreamedValue`, and maps
+   `HistorianAccessError.ErrorValue` codes through
+   `AahClientManagedAlarmEventWriter.MapOutcome` (Ack / PermanentFail /
+   RetryPlease). `HistorianClusterEndpointPicker` drives multi-node failover.
+2. `Program.cs` — `BuildAlarmWriter()` constructs the backend gated behind
+   `OTOPCUA_HISTORIAN_ALARM_WRITE_ENABLED`.
+3. `Install-Services.ps1` — env var present in the install-time block.
 
-3. `Install-Services.ps1` — add the new env var to the install-time block.
-
-**What blocks C.1**: access to the `aahClientManaged` SDK on the dev box
-(confirmed available per `project_aveva_platform_installed.md` — AVEVA
-Historian SDK is present). C.1 can proceed without A.2 since the sidecar's
-`aahClientManaged` is x64 and does not share the worker's x86 bitness
-constraint.
+**What remains for C.1**: only the live-rig write smoke — the `Live_*` tests
+in `SdkAlarmHistorianWriteBackendTests` stay `Skip`-gated until D.1 confirms a
+round-trip against a real AVEVA Historian, including the exact mandatory
+`HistorianEvent` field set.
 
 **Tests to write**:
 

docs/v2/phase-7-status.md

@@ -138,9 +138,9 @@ All three are verified closed in the 2026-04-23 exit-gate audit:
 
 These are real open items, not issues with the plan reconciliation.
 
-### Gap 1 — OPC UA method-call dispatch for scripted alarm Ack/Confirm/Shelve (Stream G / C.6)
+### Gap 1 — OPC UA method-call dispatch for scripted alarm methods (Stream G / C.6) — CLOSED
 
-`DriverNodeManager.MethodCall` does not route OPC UA `Acknowledge` / `Confirm` / `OneShotShelve` / `TimedShelve` / `Unshelve` / `AddComment` method invocations to the `ScriptedAlarmEngine`. Operators can acknowledge scripted alarms through the Admin UI today; OPC UA HMI clients expecting to use Part 9 method nodes directly cannot. Explicit in `phase-7-e2e-smoke.md` §"Known limitations".
+All Part 9 alarm methods now route to the `ScriptedAlarmEngine`. `Acknowledge` / `Confirm` / `AddComment` route via `DriverNodeManager.RouteScriptedAlarmMethodCalls` (task #24 + follow-up); `AddComment` gates at the `AlarmAcknowledge` tier. `OneShotShelve` / `TimedShelve` / `Unshelve` route via the native `AlarmConditionState.OnShelve` / `OnTimedUnshelve` hooks wired in `MarkAsAlarmCondition`, with the per-instance shelve method NodeIds indexed so the Call gate resolves them to `OpcUaOperation.AlarmShelve`.
 
 ### Gap 2 — Admin UI: no `/virtual-tags` tab or form (Stream F.2)
 

looseends.md

@@ -0,0 +1,54 @@
+# Loose ends
+
+State as of 2026-05-18, after the #9–#29 task-list run. Everything on the
+formal task list is shipped except #20; the items below are what genuinely
+remains, plus follow-ups surfaced during the run.
+
+## Open task
+
+- **#20 — D.1 dev-rig rollout smoke.** A full 3-service deployment
+  (gateway + worker + server + Wonderware historian sidecar): deploy the
+  refreshed binaries, run `scripts/install/Refresh-Services.ps1`, exercise
+  alarms end-to-end, and capture the rollout artifact. The code blockers
+  were cleared by #18; the act itself needs the physical AVEVA dev rig and
+  cannot be produced from a dev box. Runbook context in
+  `docs/plans/alarms-worker-wiring-plan.md`.
+
+## Follow-ups surfaced during the run
+
+- **~~C.1 live SDK binding.~~** DONE (code). `SdkAlarmHistorianWriteBackend`
+  (`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/`) now
+  writes via the real entry point `HistorianAccess.AddStreamedValue(HistorianEvent,
+  out error)` in `aahClientManaged`. Two plan corrections found while pinning it:
+  (a) `ArchestrAAlarmsAndEvents.SDK` has no writer — it's a WCF query proxy;
+  (b) writes need their own `ReadOnly=false` connection, not the shared read
+  pool. Remaining: the live-rig write smoke (the `Live_*` tests are still
+  `Skip`-gated) — folds into #20 / D.1.
+
+- **~~#24 Shelve-method routing.~~** DONE. Acknowledge / Confirm already
+  routed; OneShotShelve / TimedShelve / Unshelve now route via the native
+  `AlarmConditionState.OnShelve` / `OnTimedUnshelve` hooks wired in
+  `DriverNodeManager.MarkAsAlarmCondition` (scripted alarms get a shelvable
+  `ShelvedStateMachine` subtree created before `alarm.Create`). The three
+  per-instance shelve method NodeIds are indexed so the Call gate resolves
+  them to `OpcUaOperation.AlarmShelve`. `AddComment` also now routes to the
+  engine (gated at the `AlarmAcknowledge` tier) — `phase-7-status.md` Gap 1
+  is fully closed. Remaining: address-space materialisation of the shelve
+  method nodes is best confirmed by a live OPC UA browse (pairs with the
+  G6 / D.1 rig steps).
+
+- **mxaccessgw alarm epic branch.** The alarm subsystem work (A.2/A.3/A.4
+  + the two production-gap fixes from #18) lives on the mxaccessgw branch
+  `docs/alarm-client-wm-app-finding`. It is NOT merged to mxaccessgw's main.
+  Whether/when to merge the alarm epic to main is an open release decision.
+
+- **#15 operator/lab GA gates.** Two v2 GA gates are manual lab steps, not
+  automatable here: the OPC UA CTT (Compliance Test Tool) pass and the
+  deployment-checklist signoff. Documented in
+  `docs/plans/v2-ga-lab-gates-plan.md`.
+
+## Done — for reference
+
+The 5 Phase 7 gaps discovered mid-run (#24–#28) were all completed and
+merged; no Phase 7 gaps remain open. Add any new follow-ups above as they
+are spun out.

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/IHistorianConnectionFactory.cs

@@ -10,32 +10,24 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
     /// </summary>
     internal interface IHistorianConnectionFactory
     {
-        HistorianAccess CreateAndConnect(HistorianConfiguration config, HistorianConnectionType type);
+        /// <summary>
+        ///     Opens a Historian SDK connection. <paramref name="readOnly"/> defaults to
+        ///     <c>true</c> for the query path; the alarm-event write backend passes
+        ///     <c>false</c> because <c>HistorianAccess.AddStreamedValue</c> fails with
+        ///     <c>WriteToReadOnlyFile</c> on a read-only session.
+        /// </summary>
+        HistorianAccess CreateAndConnect(
+            HistorianConfiguration config, HistorianConnectionType type, bool readOnly = true);
     }
 
     /// <summary>Production implementation — opens real Historian SDK connections.</summary>
     internal sealed class SdkHistorianConnectionFactory : IHistorianConnectionFactory
     {
-        public HistorianAccess CreateAndConnect(HistorianConfiguration config, HistorianConnectionType type)
+        public HistorianAccess CreateAndConnect(
+            HistorianConfiguration config, HistorianConnectionType type, bool readOnly = true)
         {
             var conn = new HistorianAccess();
-
-            var args = new HistorianConnectionArgs
-            {
-                ServerName = config.ServerName,
-                TcpPort = (ushort)config.Port,
-                IntegratedSecurity = config.IntegratedSecurity,
-                UseArchestrAUser = config.IntegratedSecurity,
-                ConnectionType = type,
-                ReadOnly = true,
-                PacketTimeout = (uint)(config.CommandTimeoutSeconds * 1000)
-            };
-
-            if (!config.IntegratedSecurity)
-            {
-                args.UserName = config.UserName ?? string.Empty;
-                args.Password = config.Password ?? string.Empty;
-            }
+            var args = BuildConnectionArgs(config, type, readOnly);
 
             if (!conn.OpenConnection(args, out var error))
             {
@@ -69,5 +61,32 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
             throw new TimeoutException(
                 $"Historian SDK connection to {config.ServerName}:{config.Port} timed out after {config.CommandTimeoutSeconds}s");
         }
+
+        /// <summary>
+        ///     Builds the <see cref="HistorianConnectionArgs"/> for a connection. Pure (no SDK
+        ///     side effects) so the read-only-vs-write argument shaping is unit-testable.
+        /// </summary>
+        internal static HistorianConnectionArgs BuildConnectionArgs(
+            HistorianConfiguration config, HistorianConnectionType type, bool readOnly)
+        {
+            var args = new HistorianConnectionArgs
+            {
+                ServerName = config.ServerName,
+                TcpPort = (ushort)config.Port,
+                IntegratedSecurity = config.IntegratedSecurity,
+                UseArchestrAUser = config.IntegratedSecurity,
+                ConnectionType = type,
+                ReadOnly = readOnly,
+                PacketTimeout = (uint)(config.CommandTimeoutSeconds * 1000)
+            };
+
+            if (!config.IntegratedSecurity)
+            {
+                args.UserName = config.UserName ?? string.Empty;
+                args.Password = config.Password ?? string.Empty;
+            }
+
+            return args;
+        }
     }
 }

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware/Backend/SdkAlarmHistorianWriteBackend.cs

@@ -1,6 +1,8 @@
 using System;
+using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using ArchestrA;
 using Serilog;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Ipc;
 
@@ -8,39 +10,85 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
 {
     /// <summary>
     ///     Production <see cref="IAlarmHistorianWriteBackend"/> backed by AVEVA Historian's
-    ///     <c>aahClientManaged</c> alarm-event write API. The exact SDK entry point is
-    ///     pinned during the live-rig smoke in PR D.1 — until that gate, this backend
-    ///     reports <see cref="AlarmHistorianWriteOutcome.RetryPlease"/> for every
-    ///     event with a structured diagnostic so the lmxopcua-side
-    ///     <c>SqliteStoreAndForwardSink</c> retains the queued events rather than dropping
-    ///     or hard-failing them.
+    ///     <c>aahClientManaged</c> SDK. Each <see cref="AlarmHistorianEventDto"/> is written via
+    ///     <c>HistorianAccess.AddStreamedValue(HistorianEvent, out HistorianAccessError)</c> —
+    ///     the alarm-event write entry point pinned during PR C.1.
     /// </summary>
     /// <remarks>
     ///     <para>
-    ///         Cluster failover reuses <see cref="HistorianClusterEndpointPicker"/> via
-    ///         the shared <see cref="HistorianDataSource"/> connection pool — there is
-    ///         no second connection pool for writes. Wonderware Historian's alarm-event
-    ///         write surface accepts the same <c>HistorianAccess</c> session a read
-    ///         opens, so reusing the picker is parity-preserving with v1's
-    ///         <c>GalaxyHistorianWriter</c>.
+    ///         The write path needs its <b>own</b> connection. The query-side
+    ///         <see cref="HistorianDataSource"/> opens <c>ReadOnly</c> sessions, and
+    ///         <c>AddStreamedValue</c> on a read-only session fails with
+    ///         <c>WriteToReadOnlyFile</c>. This backend therefore opens a dedicated
+    ///         <c>ReadOnly = false</c> connection; it shares
+    ///         <see cref="HistorianClusterEndpointPicker"/> for node selection and failover but
+    ///         not the connection object itself.
     ///     </para>
     ///     <para>
-    ///         Once D.1 confirms the SDK entry point, this class swaps the placeholder
-    ///         body for the real call sequence. The mapping from raw HRESULT /
-    ///         <c>HistorianError</c> codes onto <see cref="AlarmHistorianWriteOutcome"/>
-    ///         is already shared via <see cref="AahClientManagedAlarmEventWriter.MapOutcome"/>
-    ///         so the smoke-pinned change stays minimal.
+    ///         Per-event <c>HistorianAccessError.ErrorValue</c> codes map onto
+    ///         <see cref="AlarmHistorianWriteOutcome"/> via
+    ///         <see cref="AahClientManagedAlarmEventWriter.MapOutcome"/>. A connection-class
+    ///         error aborts the remainder of the batch as
+    ///         <see cref="AlarmHistorianWriteOutcome.RetryPlease"/> and resets the connection so
+    ///         the next drain tick reconnects — possibly to a different cluster node.
+    ///     </para>
+    ///     <para>
+    ///         The exact <c>HistorianEvent</c> field set required by the Historian is confirmed
+    ///         against a live install during the PR D.1 rollout smoke; <see cref="ToHistorianEvent"/>
+    ///         maps the unambiguous fields and carries operator comment / condition id as event
+    ///         properties.
     ///     </para>
     /// </remarks>
-    public sealed class SdkAlarmHistorianWriteBackend : IAlarmHistorianWriteBackend
+    public sealed class SdkAlarmHistorianWriteBackend : IAlarmHistorianWriteBackend, IDisposable
     {
         private static readonly ILogger Log = Serilog.Log.ForContext<SdkAlarmHistorianWriteBackend>();
 
+        // ErrorValue codes that mean the connection/server is the problem (transient) rather
+        // than the event payload. These abort the rest of the batch and trigger a reconnect.
+        private static readonly HashSet<HistorianAccessError.ErrorValue> ConnectionErrors =
+            new HashSet<HistorianAccessError.ErrorValue>
+            {
+                HistorianAccessError.ErrorValue.FailedToConnect,
+                HistorianAccessError.ErrorValue.FailedToCreateSession,
+                HistorianAccessError.ErrorValue.NoReply,
+                HistorianAccessError.ErrorValue.NotReady,
+                HistorianAccessError.ErrorValue.NotInitialized,
+                HistorianAccessError.ErrorValue.Stopping,
+                HistorianAccessError.ErrorValue.Win32Exception,
+                HistorianAccessError.ErrorValue.InvalidResponse,
+            };
+
+        // ErrorValue codes that mean the event itself is malformed — permanent, never retried.
+        private static readonly HashSet<HistorianAccessError.ErrorValue> MalformedErrors =
+            new HashSet<HistorianAccessError.ErrorValue>
+            {
+                HistorianAccessError.ErrorValue.InvalidArgument,
+                HistorianAccessError.ErrorValue.ValidationFailed,
+                HistorianAccessError.ErrorValue.NullPointerArgument,
+                HistorianAccessError.ErrorValue.WriteToReadOnlyFile,
+                HistorianAccessError.ErrorValue.NotImplemented,
+                HistorianAccessError.ErrorValue.NotApplicable,
+            };
+
         private readonly HistorianConfiguration _config;
+        private readonly IHistorianConnectionFactory _factory;
+        private readonly HistorianClusterEndpointPicker _picker;
+        private readonly object _connectionLock = new object();
+        private HistorianAccess? _connection;
+        private string? _activeNode;
+        private bool _disposed;
 
         public SdkAlarmHistorianWriteBackend(HistorianConfiguration config)
+            : this(config, new SdkHistorianConnectionFactory(), null) { }
+
+        internal SdkAlarmHistorianWriteBackend(
+            HistorianConfiguration config,
+            IHistorianConnectionFactory factory,
+            HistorianClusterEndpointPicker? picker = null)
         {
             _config = config ?? throw new ArgumentNullException(nameof(config));
+            _factory = factory ?? throw new ArgumentNullException(nameof(factory));
+            _picker = picker ?? new HistorianClusterEndpointPicker(config);
         }
 
         public Task<AlarmHistorianWriteOutcome[]> WriteBatchAsync(
@@ -52,22 +100,265 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                 return Task.FromResult(new AlarmHistorianWriteOutcome[0]);
             }
 
-            // Placeholder: pin the SDK entry point in PR D.1 against a live AVEVA
-            // Historian. Until then the call returns RetryPlease for every slot so
-            // the lmxopcua-side sink keeps the events queued rather than dropping
-            // them — same effect as the current NullAlarmHistorianSink fallback,
-            // but visible through the structured diagnostic + per-event outcome.
-            Log.Warning(
-                "Alarm historian SDK write path not yet pinned — returning RetryPlease for {Count} event(s) from server {Server}. PR D.1 swaps this for the live aahClientManaged call.",
-                events.Length,
-                _config.ServerName);
-
             var outcomes = new AlarmHistorianWriteOutcome[events.Length];
-            for (var i = 0; i < outcomes.Length; i++)
+
+            HistorianAccess connection;
+            try
             {
-                outcomes[i] = AlarmHistorianWriteOutcome.RetryPlease;
+                connection = EnsureConnected();
             }
+            catch (ObjectDisposedException)
+            {
+                throw;
+            }
+            catch (Exception ex)
+            {
+                // No reachable node — defer the whole batch so the lmxopcua-side SQLite
+                // store-and-forward sink retains the rows for the next drain tick.
+                Log.Warning(ex,
+                    "Alarm historian write connection unavailable — deferring {Count} event(s) as RetryPlease",
+                    events.Length);
+                FillRemaining(outcomes, 0, AlarmHistorianWriteOutcome.RetryPlease);
+                return Task.FromResult(outcomes);
+            }
+
+            for (var i = 0; i < events.Length; i++)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                try
+                {
+                    var historianEvent = ToHistorianEvent(events[i]);
+                    if (connection.AddStreamedValue(historianEvent, out var error))
+                    {
+                        outcomes[i] = AlarmHistorianWriteOutcome.Ack;
+                        continue;
+                    }
+
+                    var code = error?.ErrorCode ?? HistorianAccessError.ErrorValue.Failure;
+                    if (ConnectionErrors.Contains(code))
+                    {
+                        // Connection died mid-batch — drop it and defer this event + the rest.
+                        Log.Warning(
+                            "Alarm historian write hit connection-level error {Code} ({Desc}); resetting connection, deferring {Remaining} event(s)",
+                            code, error?.ErrorDescription, events.Length - i);
+                        HandleConnectionError(error?.ErrorDescription);
+                        FillRemaining(outcomes, i, AlarmHistorianWriteOutcome.RetryPlease);
+                        return Task.FromResult(outcomes);
+                    }
+
+                    outcomes[i] = ClassifyOutcome(code);
+                    Log.Warning(
+                        "Alarm historian write rejected event {EventId}: {Code} ({Desc}) -> {Outcome}",
+                        events[i].EventId, code, error?.ErrorDescription, outcomes[i]);
+                }
+                catch (OperationCanceledException)
+                {
+                    throw;
+                }
+                catch (Exception ex)
+                {
+                    // Transport-level throw (SDK marshalling fault, broken connection) —
+                    // reset and defer this event + the rest.
+                    Log.Warning(ex,
+                        "Alarm historian write threw for event {EventId}; resetting connection, deferring {Remaining} event(s)",
+                        events[i].EventId, events.Length - i);
+                    HandleConnectionError(ex.Message);
+                    FillRemaining(outcomes, i, AlarmHistorianWriteOutcome.RetryPlease);
+                    return Task.FromResult(outcomes);
+                }
+            }
+
             return Task.FromResult(outcomes);
         }
+
+        /// <summary>
+        ///     Maps an <see cref="AlarmHistorianEventDto"/> onto the SDK's
+        ///     <c>HistorianEvent</c>. Operator comment and originating condition id ride as
+        ///     event properties — operator-comment fidelity is the field the value-driven
+        ///     fallback path cannot carry.
+        /// </summary>
+        internal static HistorianEvent ToHistorianEvent(AlarmHistorianEventDto dto)
+        {
+            // The ArchestrA SDK marks these HistorianEvent members obsolete but still honours
+            // them on write; their successors aren't wired in the version we bind against.
+            // Using them is the documented v1 behaviour — mirrors HistorianDataSource.ToDto,
+            // suppressed locally so any other deprecated-surface use still surfaces as an error.
+#pragma warning disable CS0618
+            var historianEvent = new HistorianEvent
+            {
+                IsAlarm = true,
+                Source = dto.SourceName ?? string.Empty,
+                EventType = string.IsNullOrEmpty(dto.AlarmType) ? "Alarm" : dto.AlarmType,
+                EventTime = new DateTime(dto.EventTimeUtcTicks, DateTimeKind.Utc),
+                ReceivedTime = DateTime.UtcNow,
+                Severity = dto.Severity,
+                DisplayText = dto.Message ?? string.Empty,
+            };
+
+            if (Guid.TryParse(dto.EventId, out var id))
+            {
+                historianEvent.Id = id;
+            }
+#pragma warning restore CS0618
+
+            if (!string.IsNullOrEmpty(dto.AckComment))
+            {
+                historianEvent.AddProperty("Comment", dto.AckComment, out _);
+            }
+            if (!string.IsNullOrEmpty(dto.ConditionId))
+            {
+                historianEvent.AddProperty("ConditionId", dto.ConditionId, out _);
+            }
+
+            return historianEvent;
+        }
+
+        /// <summary>
+        ///     Classifies a non-connection-class <c>HistorianAccessError.ErrorValue</c> into an
+        ///     <see cref="AlarmHistorianWriteOutcome"/> by routing it through the shared
+        ///     <see cref="AahClientManagedAlarmEventWriter.MapOutcome"/> mapping. Exposed for
+        ///     unit tests — connection-class codes are handled separately by the batch loop.
+        /// </summary>
+        internal static AlarmHistorianWriteOutcome ClassifyOutcome(HistorianAccessError.ErrorValue code)
+            => AahClientManagedAlarmEventWriter.MapOutcome(
+                (int)code,
+                isCommunicationError: ConnectionErrors.Contains(code),
+                isMalformedInput: MalformedErrors.Contains(code));
+
+        private static void FillRemaining(
+            AlarmHistorianWriteOutcome[] outcomes, int from, AlarmHistorianWriteOutcome value)
+        {
+            for (var i = from; i < outcomes.Length; i++)
+            {
+                outcomes[i] = value;
+            }
+        }
+
+        private HistorianAccess EnsureConnected()
+        {
+            if (_disposed)
+            {
+                throw new ObjectDisposedException(nameof(SdkAlarmHistorianWriteBackend));
+            }
+
+            var existing = Volatile.Read(ref _connection);
+            if (existing != null) return existing;
+
+            var (conn, node) = ConnectToAnyHealthyNode();
+
+            lock (_connectionLock)
+            {
+                if (_disposed)
+                {
+                    SafeClose(conn);
+                    throw new ObjectDisposedException(nameof(SdkAlarmHistorianWriteBackend));
+                }
+
+                if (_connection != null)
+                {
+                    SafeClose(conn);
+                    return _connection;
+                }
+
+                _connection = conn;
+                _activeNode = node;
+                Log.Information("Alarm historian write connection opened to {Server}:{Port}", node, _config.Port);
+                return conn;
+            }
+        }
+
+        private (HistorianAccess Connection, string Node) ConnectToAnyHealthyNode()
+        {
+            var candidates = _picker.GetHealthyNodes();
+            if (candidates.Count == 0)
+            {
+                throw new InvalidOperationException(
+                    _picker.NodeCount == 0
+                        ? "No historian nodes configured"
+                        : $"All {_picker.NodeCount} historian nodes are in cooldown — no healthy endpoints");
+            }
+
+            Exception? lastException = null;
+            foreach (var node in candidates)
+            {
+                try
+                {
+                    var conn = _factory.CreateAndConnect(
+                        CloneConfigWithServerName(node), HistorianConnectionType.Event, readOnly: false);
+                    _picker.MarkHealthy(node);
+                    return (conn, node);
+                }
+                catch (Exception ex)
+                {
+                    _picker.MarkFailed(node, ex.Message);
+                    lastException = ex;
+                    Log.Warning(ex, "Alarm historian node {Node} failed during write-connect; trying next", node);
+                }
+            }
+
+            throw new InvalidOperationException(
+                $"All {candidates.Count} healthy historian candidate(s) failed during write-connect: " +
+                (lastException?.Message ?? "(no detail)"),
+                lastException);
+        }
+
+        private void HandleConnectionError(string? detail)
+        {
+            lock (_connectionLock)
+            {
+                if (_connection == null) return;
+
+                SafeClose(_connection);
+                _connection = null;
+
+                var failedNode = _activeNode;
+                _activeNode = null;
+                if (failedNode != null) _picker.MarkFailed(failedNode, detail ?? "mid-batch failure");
+                Log.Warning("Alarm historian write connection reset (node={Node})", failedNode ?? "(unknown)");
+            }
+        }
+
+        private static void SafeClose(HistorianAccess conn)
+        {
+            try
+            {
+                conn.CloseConnection(out _);
+                conn.Dispose();
+            }
+            catch (Exception ex)
+            {
+                Log.Debug(ex, "Error closing alarm historian write connection");
+            }
+        }
+
+        private HistorianConfiguration CloneConfigWithServerName(string serverName) => new HistorianConfiguration
+        {
+            Enabled = _config.Enabled,
+            ServerName = serverName,
+            ServerNames = _config.ServerNames,
+            FailureCooldownSeconds = _config.FailureCooldownSeconds,
+            IntegratedSecurity = _config.IntegratedSecurity,
+            UserName = _config.UserName,
+            Password = _config.Password,
+            Port = _config.Port,
+            CommandTimeoutSeconds = _config.CommandTimeoutSeconds,
+            MaxValuesPerRead = _config.MaxValuesPerRead,
+            RequestTimeoutSeconds = _config.RequestTimeoutSeconds,
+        };
+
+        public void Dispose()
+        {
+            if (_disposed) return;
+            _disposed = true;
+
+            lock (_connectionLock)
+            {
+                if (_connection != null)
+                {
+                    SafeClose(_connection);
+                    _connection = null;
+                }
+            }
+        }
     }
 }

src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs

@@ -125,6 +125,16 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
     private readonly Dictionary<string, string> _scriptedAlarmIdByConditionNodeId
         = new(StringComparer.OrdinalIgnoreCase);
 
+    // Task #24 follow-up — NodeIds of the OneShotShelve / TimedShelve / Unshelve method
+    // nodes created on scripted-alarm ShelvedStateMachine subtrees. Those methods carry
+    // per-instance NodeIds (not well-known type MethodIds), so the Call gate can't
+    // constant-match them; it consults this set instead to map a shelve invocation to
+    // OpcUaOperation.AlarmShelve. Routing itself is handled by the native
+    // AlarmConditionState.OnShelve hook wired in MarkAsAlarmCondition — no Call-override
+    // interception is needed because the stack dispatches the method to that delegate.
+    // Populated during the address-space build; read-only once clients are served.
+    private readonly HashSet<NodeId> _scriptedAlarmShelveMethodNodeIds = new();
+
     public DriverNodeManager(IServerInternal server, ApplicationConfiguration configuration,
         IDriver driver, CapabilityInvoker invoker, ILogger<DriverNodeManager> logger,
         AuthorizationGate? authzGate = null, NodeScopeResolver? scopeResolver = null,
@@ -621,7 +631,8 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
         IList<CallMethodResult> results,
         IList<ServiceResult> errors)
     {
-        GateCallMethodRequests(methodsToCall, errors, context.UserIdentity, _authzGate, _scopeResolver);
+        GateCallMethodRequests(methodsToCall, errors, context.UserIdentity, _authzGate, _scopeResolver,
+            _scriptedAlarmShelveMethodNodeIds);
 
         // Task #24 — Phase 7 Gap 1: route Part 9 Acknowledge / Confirm calls that target
         // scripted alarm condition nodes directly to the ScriptedAlarmEngine. The engine
@@ -641,8 +652,8 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
     }
 
     /// <summary>
-    ///     Intercepts Part 9 Acknowledge / Confirm <see cref="CallMethodRequest"/> slots that
-    ///     target scripted alarm condition nodes and routes them to the
+    ///     Intercepts Part 9 Acknowledge / Confirm / AddComment <see cref="CallMethodRequest"/>
+    ///     slots that target scripted alarm condition nodes and routes them to the
     ///     <see cref="ScriptedAlarmEngine"/>. Slots that are handled have their
     ///     <paramref name="errors"/> entry set to <see cref="ServiceResult.Good"/> and are
     ///     not touched by the caller's subsequent <c>base.Call</c> invocation.
@@ -652,7 +663,9 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
     ///         The OPC UA Part 9 Acknowledge method signature is:
     ///         InputArguments[0] = EventId (ByteString, ignored — scripted alarms identify by
     ///         ConditionId, not EventId), InputArguments[1] = Comment (LocalizedText).
-    ///         Confirm has the same shape. Missing or null comment is treated as empty string.
+    ///         Confirm and AddComment have the same two-argument shape. For Acknowledge /
+    ///         Confirm a missing comment is treated as empty; AddComment requires non-empty
+    ///         comment text and returns <see cref="StatusCodes.BadInvalidArgument"/> otherwise.
     ///     </para>
     ///     <para>
     ///         User identity is extracted from <paramref name="userIdentity"/>'s
@@ -674,8 +687,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
         ScriptedAlarmEngine engine,
         IReadOnlyDictionary<string, string> conditionIdToAlarmId)
     {
-        var user = userIdentity?.DisplayName;
-        if (string.IsNullOrWhiteSpace(user)) user = "opcua-client";
+        var user = ResolveCallUser(userIdentity);
 
         for (var i = 0; i < methodsToCall.Count; i++)
         {
@@ -684,10 +696,12 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
 
             var request = methodsToCall[i];
 
-            // Only handle the two well-known Part 9 method ids.
+            // Only handle the well-known Part 9 method ids. AddComment is declared on the
+            // base ConditionType; Acknowledge / Confirm on AcknowledgeableConditionType.
             var isAcknowledge = request.MethodId == MethodIds.AcknowledgeableConditionType_Acknowledge;
             var isConfirm = request.MethodId == MethodIds.AcknowledgeableConditionType_Confirm;
-            if (!isAcknowledge && !isConfirm) continue;
+            var isAddComment = request.MethodId == MethodIds.ConditionType_AddComment;
+            if (!isAcknowledge && !isConfirm && !isAddComment) continue;
 
             // ObjectId must be a string identifier so we can look it up in the index.
             if (request.ObjectId.Identifier is not string conditionKey) continue;
@@ -707,9 +721,14 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
                 if (isAcknowledge)
                     engine.AcknowledgeAsync(alarmId, user, comment, CancellationToken.None)
                         .GetAwaiter().GetResult();
-                else
+                else if (isConfirm)
                     engine.ConfirmAsync(alarmId, user, comment, CancellationToken.None)
                         .GetAwaiter().GetResult();
+                else
+                    // AddComment requires comment text — the engine's Part 9 state machine
+                    // rejects null/empty with ArgumentException, surfaced as BadInvalidArgument.
+                    engine.AddCommentAsync(alarmId, user, comment ?? string.Empty, CancellationToken.None)
+                        .GetAwaiter().GetResult();
 
                 // Mark the slot as handled so base.Call skips it. A pre-populated Good
                 // result (not null and not Bad) is the signal the base class uses to
@@ -732,6 +751,114 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
         }
     }
 
+    /// <summary>
+    ///     Resolves the audit identity for an OPC UA method call. Authenticated LDAP
+    ///     sessions populate <see cref="IUserIdentity.DisplayName"/> during
+    ///     <c>OtOpcUaServer.OnImpersonateUser</c>; anonymous sessions fall back to
+    ///     <c>"opcua-client"</c> so every audit entry carries an identity.
+    /// </summary>
+    internal static string ResolveCallUser(IUserIdentity? userIdentity)
+    {
+        var user = userIdentity?.DisplayName;
+        return string.IsNullOrWhiteSpace(user) ? "opcua-client" : user;
+    }
+
+    /// <summary>
+    ///     Task #24 follow-up — native <c>AlarmConditionState.OnShelve</c> handler for a
+    ///     scripted alarm. The OPC UA stack dispatches OneShotShelve / TimedShelve /
+    ///     Unshelve method calls here after validating the Part 9 state transition. The
+    ///     handler advances the <see cref="ScriptedAlarmEngine"/> with the authenticated
+    ///     principal, then mirrors the new shelving state onto the OPC UA node via
+    ///     <c>SetShelvingState</c>. A failed engine call returns a Bad status so the stack
+    ///     leaves the node's <c>ShelvedStateMachine</c> unchanged.
+    /// </summary>
+    internal static ServiceResult RouteScriptedAlarmShelve(
+        ISystemContext context,
+        OpcAlarmConditionState alarm,
+        bool shelving,
+        bool oneShot,
+        double shelvingTime,
+        ScriptedAlarmEngine engine,
+        string alarmId,
+        ILogger? logger)
+    {
+        var user = ResolveCallUser(context?.UserIdentity);
+        var engineResult = InvokeEngineShelve(engine, alarmId, user, shelving, oneShot, shelvingTime, logger);
+        if (ServiceResult.IsBad(engineResult)) return engineResult;
+
+        // Mirror the engine's new state onto the OPC UA ShelvedStateMachine. The stack
+        // expects the OnShelve handler to advance the node — it does not do so itself.
+        alarm?.SetShelvingState(context, shelving, oneShot, shelvingTime);
+        return ServiceResult.Good;
+    }
+
+    /// <summary>
+    ///     Task #24 follow-up — native <c>AlarmConditionState.OnTimedUnshelve</c> handler:
+    ///     the stack's timed-shelve countdown has expired, so unshelve the alarm in the
+    ///     engine and mirror the Unshelved state onto the OPC UA node.
+    /// </summary>
+    internal static ServiceResult RouteScriptedAlarmTimedUnshelve(
+        ISystemContext context,
+        OpcAlarmConditionState alarm,
+        ScriptedAlarmEngine engine,
+        string alarmId,
+        ILogger? logger)
+    {
+        // The expiry is a server-side timer, not an operator action — attribute the
+        // audit entry to the subsystem rather than a user principal.
+        var engineResult = InvokeEngineShelve(
+            engine, alarmId, "timed-unshelve", shelving: false, oneShot: false, shelvingTime: 0, logger);
+        if (ServiceResult.IsBad(engineResult)) return engineResult;
+
+        alarm?.SetShelvingState(context, false, false, 0);
+        return ServiceResult.Good;
+    }
+
+    /// <summary>
+    ///     Dispatches a shelve transition to the <see cref="ScriptedAlarmEngine"/>. Extracted
+    ///     as a pure function (no OPC UA node dependency) so the engine-routing decision —
+    ///     including the <see cref="OpcUaOperation"/>-shaped status mapping — is unit-testable.
+    ///     <paramref name="shelving"/> / <paramref name="oneShot"/> follow the OPC UA
+    ///     <c>OnShelve</c> contract: <c>(false, *)</c> = Unshelve, <c>(true, true)</c> =
+    ///     OneShotShelve, <c>(true, false)</c> = TimedShelve for <paramref name="shelvingTime"/>
+    ///     milliseconds.
+    /// </summary>
+    internal static ServiceResult InvokeEngineShelve(
+        ScriptedAlarmEngine engine,
+        string alarmId,
+        string user,
+        bool shelving,
+        bool oneShot,
+        double shelvingTime,
+        ILogger? logger)
+    {
+        try
+        {
+            if (!shelving)
+                engine.UnshelveAsync(alarmId, user, CancellationToken.None).GetAwaiter().GetResult();
+            else if (oneShot)
+                engine.OneShotShelveAsync(alarmId, user, CancellationToken.None).GetAwaiter().GetResult();
+            else
+                engine.TimedShelveAsync(
+                        alarmId, user, DateTime.UtcNow.AddMilliseconds(shelvingTime), CancellationToken.None)
+                    .GetAwaiter().GetResult();
+            return ServiceResult.Good;
+        }
+        catch (ArgumentException ex)
+        {
+            // Unknown alarmId or an invalid Part 9 transition — surface as BadInvalidArgument
+            // so the OPC UA client sees a meaningful status.
+            logger?.LogInformation(
+                "Scripted-alarm shelve rejected for {AlarmId}: {Message}", alarmId, ex.Message);
+            return new ServiceResult(StatusCodes.BadInvalidArgument, ex.Message, ex.Message);
+        }
+        catch (Exception ex)
+        {
+            logger?.LogError(ex, "Scripted-alarm shelve failed for {AlarmId}", alarmId);
+            return new ServiceResult(StatusCodes.BadInternalError, ex.Message, ex.Message);
+        }
+    }
+
     /// <summary>
     ///     Pure-function gate for a batch of <see cref="CallMethodRequest"/>. Pre-populates
     ///     <paramref name="errors"/> slots with <see cref="StatusCodes.BadUserAccessDenied"/>
@@ -742,7 +869,8 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
         IList<ServiceResult> errors,
         IUserIdentity? userIdentity,
         AuthorizationGate? gate,
-        NodeScopeResolver? scopeResolver)
+        NodeScopeResolver? scopeResolver,
+        IReadOnlySet<NodeId>? shelveMethodIds = null)
     {
         if (gate is null || scopeResolver is null) return;
         if (methodsToCall.Count == 0) return;
@@ -755,7 +883,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
             if (request.ObjectId.Identifier is not string fullRef) continue;
 
             var scope = scopeResolver.Resolve(fullRef);
-            var operation = MapCallOperation(request.MethodId);
+            var operation = MapCallOperation(request.MethodId, shelveMethodIds);
             if (!gate.IsAllowed(userIdentity, operation, scope))
                 errors[i] = new ServiceResult(StatusCodes.BadUserAccessDenied);
         }
@@ -767,20 +895,32 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
     ///     operator-UI grants can distinguish acknowledge/confirm/shelve; everything else
     ///     falls through to generic <see cref="OpcUaOperation.Call"/>.
     /// </summary>
-    internal static OpcUaOperation MapCallOperation(NodeId methodId)
+    /// <param name="methodId">The <see cref="NodeId"/> of the method being invoked.</param>
+    /// <param name="shelveMethodIds">
+    ///     The set of per-instance OneShotShelve / TimedShelve / Unshelve method NodeIds
+    ///     indexed during the address-space build (see
+    ///     <c>_scriptedAlarmShelveMethodNodeIds</c>). Shelve methods carry per-instance
+    ///     NodeIds rather than well-known type NodeIds, so they can't be constant-matched
+    ///     like Acknowledge / Confirm; a membership test against this set is how they
+    ///     resolve to <see cref="OpcUaOperation.AlarmShelve"/>. When <c>null</c> (no
+    ///     scripted alarms) shelve methods fall through to <see cref="OpcUaOperation.Call"/>.
+    /// </param>
+    internal static OpcUaOperation MapCallOperation(NodeId methodId, IReadOnlySet<NodeId>? shelveMethodIds = null)
     {
         // Standard Part 9 method ids on AcknowledgeableConditionType. The stack models these
-        // as ns=0 numeric ids; comparisons are value-based. Shelve is dispatched on the
-        // ShelvedStateMachine instance's methods — those arrive with per-instance NodeIds
-        // rather than well-known type NodeIds, so we can't reliably constant-match them
-        // here. Shelve falls through to OpcUaOperation.Call; the caller can still set a
-        // permissive Call grant for operators who are allowed to shelve alarms, and
-        // finer-grained AlarmShelve gating is a follow-up when the method-invocation path
-        // also carries a "method-role" annotation.
+        // as ns=0 numeric ids; comparisons are value-based.
         if (methodId == MethodIds.AcknowledgeableConditionType_Acknowledge)
             return OpcUaOperation.AlarmAcknowledge;
         if (methodId == MethodIds.AcknowledgeableConditionType_Confirm)
             return OpcUaOperation.AlarmConfirm;
+        // AddComment (ConditionType) has no dedicated operation/permission bit — it gates at
+        // the same operator tier as Acknowledge, the closest existing alarm-action grant.
+        if (methodId == MethodIds.ConditionType_AddComment)
+            return OpcUaOperation.AlarmAcknowledge;
+        // Shelve methods live on each alarm's own ShelvedStateMachine subtree, so they're
+        // matched by NodeId membership rather than a constant comparison.
+        if (methodId is not null && shelveMethodIds is not null && shelveMethodIds.Contains(methodId))
+            return OpcUaOperation.AlarmShelve;
         return OpcUaOperation.Call;
     }
 
@@ -910,6 +1050,31 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
                     BrowseName = new QualifiedName(_variable.BrowseName.Name + "_Condition", _owner.NamespaceIndex),
                     DisplayName = new LocalizedText(info.SourceName),
                 };
+
+                // Task #24 follow-up — scripted alarms expose a shelvable ShelvingState
+                // subtree so OPC UA Part 9 OneShotShelve / TimedShelve / Unshelve method
+                // calls have method nodes to target. The optional ShelvingState is NOT
+                // created by AlarmConditionState.Create; it must be attached *before*
+                // Create so the stack's AlarmConditionState.OnAfterCreate wires each shelve
+                // method's OnCallMethod handler to the ShelvedStateMachine. Non-scripted
+                // alarms (Galaxy etc.) have no engine to route to, so they stay unshelvable.
+                var isScriptedAlarm =
+                    _owner._scriptedAlarmEngine is not null
+                    && _owner._sourceByFullRef.TryGetValue(FullReference, out var conditionVarSource)
+                    && conditionVarSource == NodeSourceKind.ScriptedAlarm;
+                if (isScriptedAlarm)
+                {
+                    alarm.ShelvingState = new ShelvedStateMachineState(alarm);
+                    alarm.ShelvingState.Create(
+                        _owner.SystemContext, null,
+                        new QualifiedName(BrowseNames.ShelvingState),
+                        new LocalizedText(BrowseNames.ShelvingState), false);
+                    // UnshelveTime carries the timed-shelve countdown; it is optional and
+                    // not materialised by ShelvedStateMachineState.Create — create it so
+                    // the stack's timed-unshelve timer has a node to write.
+                    alarm.ShelvingState.UnshelveTime ??= new PropertyState<double>(alarm.ShelvingState);
+                }
+
                 // assignNodeIds=true makes the stack allocate NodeIds for every inherited
                 // AlarmConditionState child (Severity / Message / ActiveState / AckedState /
                 // EnabledState / …). Without this the children keep Foundation (ns=0) type-
@@ -955,13 +1120,16 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
                 // The condition's string identifier is "{FullReference}.Condition"; the engine
                 // addresses alarms by ScriptedAlarmId (= FullReference for scripted alarms,
                 // because EquipmentNodeWalker sets FullName = ScriptedAlarmId on the attr).
-                if (_owner._scriptedAlarmEngine is not null
-                    && _owner._sourceByFullRef.TryGetValue(FullReference, out var varSource)
-                    && varSource == NodeSourceKind.ScriptedAlarm)
+                if (isScriptedAlarm)
                 {
                     var conditionKey = alarm.NodeId.Identifier?.ToString();
                     if (!string.IsNullOrEmpty(conditionKey))
                         _owner._scriptedAlarmIdByConditionNodeId[conditionKey!] = FullReference;
+
+                    // Task #24 follow-up — wire the shelve methods created above to the
+                    // engine and index their NodeIds for the Call gate.
+                    if (alarm.ShelvingState is not null)
+                        WireScriptedAlarmShelving(alarm, FullReference);
                 }
 
                 // PR 2.3 — when the server-level alarm-condition service is wired, register
@@ -1024,6 +1192,47 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
                 AssignSymbolicDescendantIds(child, child.NodeId, namespaceIndex);
             }
         }
+
+        /// <summary>
+        ///     Task #24 follow-up — connects a scripted alarm's <c>ShelvingState</c> subtree to
+        ///     the <see cref="ScriptedAlarmEngine"/>. The stack dispatches OneShotShelve /
+        ///     TimedShelve / Unshelve method calls to the <c>OnShelve</c> delegate and the
+        ///     expiry of a timed shelve to <c>OnTimedUnshelve</c>; both routes advance the
+        ///     engine state machine and mirror the result onto the OPC UA node. The three
+        ///     shelve method NodeIds are indexed so the Call gate can resolve them to
+        ///     <see cref="OpcUaOperation.AlarmShelve"/>.
+        /// </summary>
+        private void WireScriptedAlarmShelving(OpcAlarmConditionState alarm, string alarmId)
+        {
+            var shelving = alarm.ShelvingState!;
+            var engine = _owner._scriptedAlarmEngine!;
+            var logger = _owner._logger;
+
+            // How often the timed-unshelve countdown ticks toward expiry (milliseconds).
+            alarm.UnshelveTimeUpdateRate = 1000;
+
+            alarm.OnShelve = (context, a, isShelving, oneShot, shelvingTime) =>
+                RouteScriptedAlarmShelve(context, a, isShelving, oneShot, shelvingTime, engine, alarmId, logger);
+            alarm.OnTimedUnshelve = (context, a) =>
+                RouteScriptedAlarmTimedUnshelve(context, a, engine, alarmId, logger);
+
+            CollectShelveMethodNodeIds(shelving, _owner._scriptedAlarmShelveMethodNodeIds);
+        }
+
+        /// <summary>
+        ///     Adds the NodeIds of the <c>ShelvedStateMachine</c>'s method children
+        ///     (OneShotShelve / TimedShelve / Unshelve) to <paramref name="sink"/>.
+        ///     <see cref="AssignSymbolicDescendantIds"/> has already given each a stable
+        ///     NodeId in the node manager's namespace by the time this runs.
+        /// </summary>
+        private static void CollectShelveMethodNodeIds(ShelvedStateMachineState shelving, HashSet<NodeId> sink)
+        {
+            var children = new List<BaseInstanceState>();
+            shelving.GetChildren(null!, children);
+            foreach (var child in children)
+                if (child is MethodState method && !NodeId.IsNull(method.NodeId))
+                    sink.Add(method.NodeId);
+        }
     }
 
     private sealed class ConditionSink(DriverNodeManager owner, OpcAlarmConditionState alarm)

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/Backend/SdkAlarmHistorianWriteBackendTests.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using ArchestrA;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend;
@@ -11,42 +12,42 @@ using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Ipc;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
 {
     /// <summary>
-    ///     PR C.1 — pins the <see cref="SdkAlarmHistorianWriteBackend"/> contract:
+    ///     PR C.1 — covers <see cref="SdkAlarmHistorianWriteBackend"/>, the aahClientManaged-bound
+    ///     alarm-event writer. The SDK-touching batch loop itself is exercised by the rig-gated
+    ///     <c>Live_*</c> tests (D.1); the unit tests below pin the parts that are SDK-type-free:
     ///     <list type="bullet">
-    ///       <item><description>
-    ///           Unit: the placeholder backend returns <see cref="AlarmHistorianWriteOutcome.RetryPlease"/>
-    ///           for every slot so the lmxopcua-side store-and-forward sink retains events rather than
-    ///           dropping them while D.1 is unresolved.
-    ///       </description></item>
-    ///       <item><description>
-    ///           Integration (rig-gated): once D.1 pins the live SDK entry point the Skip attribute is
-    ///           removed. The live test writes a synthetic batch to a real AVEVA Historian and asserts
-    ///           the cluster picker rotates from a broken primary to a healthy secondary.
-    ///       </description></item>
+    ///       <item><description>connection-unavailable → whole batch deferred as RetryPlease;</description></item>
+    ///       <item><description><see cref="SdkAlarmHistorianWriteBackend.ClassifyOutcome"/> error-code mapping;</description></item>
+    ///       <item><description><see cref="SdkHistorianConnectionFactory.BuildConnectionArgs"/> read-only-vs-write shaping.</description></item>
     ///     </list>
     /// </summary>
     [Trait("Category", "Unit")]
     public sealed class SdkAlarmHistorianWriteBackendTests
     {
-        // ── Placeholder-mode tests (no rig required) ─────────────────────────
+        // ── Connection-unavailable path (deterministic, no SDK load) ──────────
 
         [Fact]
-        public async Task Placeholder_returns_RetryPlease_for_every_slot_so_queue_is_preserved()
+        public async Task Empty_batch_returns_empty_array()
         {
-            // The SDK call-site in SdkAlarmHistorianWriteBackend is not yet pinned (PR D.1).
-            // Until D.1 swaps in the live call, the backend must return RetryPlease for every
-            // event so the lmxopcua-side SqliteStoreAndForwardSink retains the rows instead of
-            // dropping them — same effect as the NullAlarmHistorianSink fallback, but each
-            // slot is individually addressable for the drain worker.
-            var cfg = new HistorianConfiguration { ServerName = "placeholder-test", Enabled = true };
-            var backend = new SdkAlarmHistorianWriteBackend(cfg);
+            var backend = new SdkAlarmHistorianWriteBackend(
+                Config("any"), new ThrowingConnectionFactory());
 
-            var events = new[]
-            {
-                AlarmEvent("E1"),
-                AlarmEvent("E2"),
-                AlarmEvent("E3"),
-            };
+            var outcomes = await backend.WriteBatchAsync(
+                Array.Empty<AlarmHistorianEventDto>(), CancellationToken.None);
+
+            outcomes.ShouldBeEmpty();
+        }
+
+        [Fact]
+        public async Task Unreachable_node_defers_whole_batch_as_RetryPlease()
+        {
+            // No node can be connected — the backend must defer every event so the
+            // lmxopcua-side SQLite store-and-forward sink retains the rows rather than
+            // dropping them.
+            var backend = new SdkAlarmHistorianWriteBackend(
+                Config("unreachable"), new ThrowingConnectionFactory());
+
+            var events = new[] { AlarmEvent("E1"), AlarmEvent("E2"), AlarmEvent("E3") };
             var outcomes = await backend.WriteBatchAsync(events, CancellationToken.None);
 
             outcomes.Length.ShouldBe(events.Length);
@@ -54,23 +55,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
         }
 
         [Fact]
-        public async Task Placeholder_returns_empty_array_for_empty_batch()
+        public async Task Unreachable_node_large_batch_returns_one_outcome_per_event()
         {
-            var cfg = new HistorianConfiguration { ServerName = "placeholder-test", Enabled = true };
-            var backend = new SdkAlarmHistorianWriteBackend(cfg);
-
-            var outcomes = await backend.WriteBatchAsync(Array.Empty<AlarmHistorianEventDto>(), CancellationToken.None);
-
-            outcomes.ShouldBeEmpty();
-        }
-
-        [Fact]
-        public async Task Placeholder_returns_same_count_as_input_for_large_batch()
-        {
-            // Guards against an off-by-one error in the placeholder array allocation —
-            // WriteBatchAsync must always return exactly as many outcomes as input events.
-            var cfg = new HistorianConfiguration { ServerName = "placeholder-test", Enabled = true };
-            var backend = new SdkAlarmHistorianWriteBackend(cfg);
+            // Guards the outcome-array allocation: WriteBatchAsync must always return exactly
+            // as many outcomes as input events, even on the whole-batch-deferred path.
+            var backend = new SdkAlarmHistorianWriteBackend(
+                Config("unreachable"), new ThrowingConnectionFactory());
 
             var batch = Enumerable.Range(0, 1000).Select(i => AlarmEvent($"E{i}")).ToArray();
             var outcomes = await backend.WriteBatchAsync(batch, CancellationToken.None);
@@ -79,22 +69,91 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
             outcomes.ShouldAllBe(o => o == AlarmHistorianWriteOutcome.RetryPlease);
         }
 
+        [Fact]
+        public async Task Connect_failure_marks_node_failed_in_picker()
+        {
+            // Every connect attempt throws → the picker should record the failure so the
+            // node enters cooldown (cluster-failover plumbing).
+            var cfg = Config("node-a");
+            var picker = new HistorianClusterEndpointPicker(cfg);
+            var backend = new SdkAlarmHistorianWriteBackend(cfg, new ThrowingConnectionFactory(), picker);
+
+            await backend.WriteBatchAsync(new[] { AlarmEvent("E1") }, CancellationToken.None);
+
+            picker.HealthyNodeCount.ShouldBe(0, "the only node failed to connect and is now in cooldown");
+        }
+
+        // ── ClassifyOutcome — error-code → outcome mapping ────────────────────
+
+        [Theory]
+        [InlineData(HistorianAccessError.ErrorValue.Success, AlarmHistorianWriteOutcome.Ack)]
+        [InlineData(HistorianAccessError.ErrorValue.FailedToConnect, AlarmHistorianWriteOutcome.RetryPlease)]
+        [InlineData(HistorianAccessError.ErrorValue.FailedToCreateSession, AlarmHistorianWriteOutcome.RetryPlease)]
+        [InlineData(HistorianAccessError.ErrorValue.NoReply, AlarmHistorianWriteOutcome.RetryPlease)]
+        [InlineData(HistorianAccessError.ErrorValue.NotReady, AlarmHistorianWriteOutcome.RetryPlease)]
+        [InlineData(HistorianAccessError.ErrorValue.Failure, AlarmHistorianWriteOutcome.RetryPlease)]
+        [InlineData(HistorianAccessError.ErrorValue.NoData, AlarmHistorianWriteOutcome.RetryPlease)]
+        [InlineData(HistorianAccessError.ErrorValue.InvalidArgument, AlarmHistorianWriteOutcome.PermanentFail)]
+        [InlineData(HistorianAccessError.ErrorValue.ValidationFailed, AlarmHistorianWriteOutcome.PermanentFail)]
+        [InlineData(HistorianAccessError.ErrorValue.NullPointerArgument, AlarmHistorianWriteOutcome.PermanentFail)]
+        [InlineData(HistorianAccessError.ErrorValue.WriteToReadOnlyFile, AlarmHistorianWriteOutcome.PermanentFail)]
+        [InlineData(HistorianAccessError.ErrorValue.NotImplemented, AlarmHistorianWriteOutcome.PermanentFail)]
+        public void ClassifyOutcome_maps_error_code_to_expected_outcome(
+            HistorianAccessError.ErrorValue code, AlarmHistorianWriteOutcome expected)
+        {
+            SdkAlarmHistorianWriteBackend.ClassifyOutcome(code).ShouldBe(expected);
+        }
+
+        // ── BuildConnectionArgs — read-only vs write shaping ──────────────────
+
+        [Fact]
+        public void BuildConnectionArgs_write_connection_is_not_read_only()
+        {
+            // The alarm-event write path must open ReadOnly=false; AddStreamedValue on a
+            // read-only session fails with WriteToReadOnlyFile.
+            var args = SdkHistorianConnectionFactory.BuildConnectionArgs(
+                Config("h1"), HistorianConnectionType.Event, readOnly: false);
+
+            args.ReadOnly.ShouldBeFalse();
+            args.ConnectionType.ShouldBe(HistorianConnectionType.Event);
+            args.ServerName.ShouldBe("h1");
+        }
+
+        [Fact]
+        public void BuildConnectionArgs_query_connection_is_read_only()
+        {
+            var args = SdkHistorianConnectionFactory.BuildConnectionArgs(
+                Config("h1"), HistorianConnectionType.Process, readOnly: true);
+
+            args.ReadOnly.ShouldBeTrue();
+            args.ConnectionType.ShouldBe(HistorianConnectionType.Process);
+        }
+
+        [Fact]
+        public void BuildConnectionArgs_non_integrated_security_carries_credentials()
+        {
+            var cfg = Config("h1");
+            cfg.IntegratedSecurity = false;
+            cfg.UserName = "histuser";
+            cfg.Password = "histpass";
+
+            var args = SdkHistorianConnectionFactory.BuildConnectionArgs(
+                cfg, HistorianConnectionType.Event, readOnly: false);
+
+            args.IntegratedSecurity.ShouldBeFalse();
+            args.UserName.ShouldBe("histuser");
+            args.Password.ShouldBe("histpass");
+        }
+
         // ── Rig-gated integration tests ───────────────────────────────────────
         //
-        // The tests below need a live AVEVA Historian install and are gated with
-        // Skip="rig-required". Once PR D.1 pins the SDK entry point, remove the
-        // Skip attribute and add them to the integration test run profile.
+        // The entry point (HistorianAccess.AddStreamedValue) is pinned and implemented;
+        // these need a live AVEVA Historian and are un-skipped during the PR D.1 smoke.
 
-        [Fact(Skip = "rig-required: needs a live AVEVA Historian + aahClientManaged SDK — enable in PR D.1")]
+        [Fact(Skip = "rig-required: needs a live AVEVA Historian — un-skip during the PR D.1 rollout smoke")]
         public async Task Live_single_event_roundtrip_returns_Ack()
         {
-            // Spec (PR C.1, Tests): "1 / 100 / 1000 events through a fake aahClientManaged
-            // writer; assert per-row outcome list parallel to input order."
-            //
-            // This slice exercises the *live* SDK path. The fake-backend variant at
-            // AahClientManagedAlarmEventWriterTests covers the same assertion without the rig.
-            var cfg = BuildRigConfig();
-            var backend = new SdkAlarmHistorianWriteBackend(cfg);
+            var backend = new SdkAlarmHistorianWriteBackend(BuildRigConfig());
 
             var outcomes = await backend.WriteBatchAsync(new[] { AlarmEvent("rig-E1") }, CancellationToken.None);
 
@@ -102,19 +161,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
             outcomes[0].ShouldBe(AlarmHistorianWriteOutcome.Ack);
         }
 
-        [Fact(Skip = "rig-required: needs a live AVEVA Historian cluster (two nodes) — enable in PR D.1")]
+        [Fact(Skip = "rig-required: needs a live AVEVA Historian cluster (two nodes) — un-skip during the PR D.1 rollout smoke")]
         public async Task Live_cluster_failover_primary_bad_rotates_to_secondary()
         {
-            // Spec (PR C.1, Tests): "Cluster failover: primary node returns
-            // BadCommunicationError; picker rotates to secondary; assert eventual success."
-            //
-            // Configure the first server name to point at a deliberately unreachable node
-            // and the second to the real Historian; the picker should mark the first node
-            // failed and succeed via the second.
             var cfg = new HistorianConfiguration
             {
                 Enabled = true,
-                ServerNames = new System.Collections.Generic.List<string>
+                ServerNames = new List<string>
                 {
                     "invalid-primary-node-deliberately-unreachable",
                     Environment.GetEnvironmentVariable("OTOPCUA_HISTORIAN_SERVER") ?? "localhost",
@@ -128,18 +181,29 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
 
             var outcomes = await backend.WriteBatchAsync(new[] { AlarmEvent("rig-failover-E1") }, CancellationToken.None);
 
-            // The backend must succeed (Ack) via the secondary even though the primary was bad.
             outcomes.Length.ShouldBe(1);
             outcomes[0].ShouldBe(AlarmHistorianWriteOutcome.Ack);
         }
 
+        // ── helpers ───────────────────────────────────────────────────────────
+
+        private static HistorianConfiguration Config(string server) => new HistorianConfiguration
+        {
+            Enabled = true,
+            ServerName = server,
+            Port = 32568,
+            IntegratedSecurity = true,
+            CommandTimeoutSeconds = 30,
+            FailureCooldownSeconds = 60,
+        };
+
         private static AlarmHistorianEventDto AlarmEvent(string id) => new AlarmHistorianEventDto
         {
             EventId = id,
             SourceName = "TestSource",
             ConditionId = "TestSource.Level.HiHi",
             AlarmType = "AnalogLimitAlarm.HiHi",
-            Message = "C.1 integration test alarm",
+            Message = "C.1 test alarm",
             Severity = 500,
             EventTimeUtcTicks = DateTime.UtcNow.Ticks,
             AckComment = null,
@@ -160,5 +224,16 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
             var raw = Environment.GetEnvironmentVariable(envName);
             return int.TryParse(raw, out var parsed) ? parsed : defaultValue;
         }
+
+        /// <summary>
+        ///     Fake factory whose every connect attempt throws — drives the
+        ///     connection-unavailable path without loading the native SDK.
+        /// </summary>
+        private sealed class ThrowingConnectionFactory : IHistorianConnectionFactory
+        {
+            public HistorianAccess CreateAndConnect(
+                HistorianConfiguration config, HistorianConnectionType type, bool readOnly = true)
+                => throw new InvalidOperationException($"simulated connect failure to {config.ServerName}");
+        }
     }
 }

tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.csproj

@@ -24,4 +24,14 @@
         <ProjectReference Include="..\..\..\src\Drivers\ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware\ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.csproj"/>
     </ItemGroup>
 
+    <ItemGroup>
+        <!-- Wonderware Historian SDK — SdkAlarmHistorianWriteBackendTests pins the
+             error-code (HistorianAccessError.ErrorValue) and connection-arg shaping;
+             a DLL <Reference> doesn't flow transitively through the ProjectReference. -->
+        <Reference Include="aahClientManaged">
+            <HintPath>..\..\..\lib\aahClientManaged.dll</HintPath>
+            <EmbedInteropTypes>false</EmbedInteropTypes>
+        </Reference>
+    </ItemGroup>
+
 </Project>

tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/CallGatingTests.cs

@@ -33,6 +33,14 @@ public sealed class CallGatingTests
             .ShouldBe(OpcUaOperation.AlarmConfirm);
     }
 
+    [Fact]
+    public void MapCallOperation_AddComment_maps_to_AlarmAcknowledge()
+    {
+        // AddComment has no dedicated permission bit; it gates at the Acknowledge tier.
+        DriverNodeManager.MapCallOperation(MethodIds.ConditionType_AddComment)
+            .ShouldBe(OpcUaOperation.AlarmAcknowledge);
+    }
+
     [Fact]
     public void MapCallOperation_generic_method_maps_to_Call()
     {
@@ -41,6 +49,69 @@ public sealed class CallGatingTests
             .ShouldBe(OpcUaOperation.Call);
     }
 
+    [Fact]
+    public void MapCallOperation_shelve_method_in_index_maps_to_AlarmShelve()
+    {
+        // Shelve methods carry per-instance NodeIds; membership in the indexed set
+        // (built during the address-space build) is how they resolve to AlarmShelve.
+        var shelveMethodId = new NodeId("al-1.Condition.ShelvingState.OneShotShelve", 2);
+        var index = new HashSet<NodeId> { shelveMethodId };
+
+        DriverNodeManager.MapCallOperation(shelveMethodId, index)
+            .ShouldBe(OpcUaOperation.AlarmShelve);
+    }
+
+    [Fact]
+    public void MapCallOperation_shelve_method_not_in_index_falls_through_to_Call()
+    {
+        // A shelve-shaped NodeId that wasn't indexed (e.g. no scripted alarms) is
+        // indistinguishable from a generic method node and gates as Call.
+        var shelveMethodId = new NodeId("al-1.Condition.ShelvingState.OneShotShelve", 2);
+
+        DriverNodeManager.MapCallOperation(shelveMethodId, new HashSet<NodeId>())
+            .ShouldBe(OpcUaOperation.Call);
+        DriverNodeManager.MapCallOperation(shelveMethodId, shelveMethodIds: null)
+            .ShouldBe(OpcUaOperation.Call);
+    }
+
+    [Fact]
+    public void Denied_shelve_call_gets_BadUserAccessDenied()
+    {
+        var shelveMethodId = new NodeId("c1/area/line/eq/alarm1.Condition.ShelvingState.OneShotShelve", 2);
+        var calls = new List<CallMethodRequest>
+        {
+            NewCall("c1/area/line/eq/alarm1", shelveMethodId),
+        };
+        var errors = new List<ServiceResult> { (ServiceResult)null! };
+        // Operator has AlarmAcknowledge but NOT AlarmShelve — shelve must be denied.
+        var gate = MakeGate(strict: true, rows: [Row("grp-ops", NodePermissions.AlarmAcknowledge)]);
+
+        DriverNodeManager.GateCallMethodRequests(
+            calls, errors, NewIdentity("alice", "grp-ops"), gate, new NodeScopeResolver("c1"),
+            shelveMethodIds: new HashSet<NodeId> { shelveMethodId });
+
+        ServiceResult.IsBad(errors[0]).ShouldBeTrue();
+        errors[0].StatusCode.ShouldBe((StatusCode)StatusCodes.BadUserAccessDenied);
+    }
+
+    [Fact]
+    public void Allowed_shelve_call_passes_through()
+    {
+        var shelveMethodId = new NodeId("c1/area/line/eq/alarm1.Condition.ShelvingState.OneShotShelve", 2);
+        var calls = new List<CallMethodRequest>
+        {
+            NewCall("c1/area/line/eq/alarm1", shelveMethodId),
+        };
+        var errors = new List<ServiceResult> { (ServiceResult)null! };
+        var gate = MakeGate(strict: true, rows: [Row("grp-eng", NodePermissions.AlarmShelve)]);
+
+        DriverNodeManager.GateCallMethodRequests(
+            calls, errors, NewIdentity("alice", "grp-eng"), gate, new NodeScopeResolver("c1"),
+            shelveMethodIds: new HashSet<NodeId> { shelveMethodId });
+
+        errors[0].ShouldBeNull("AlarmShelve grant allows the shelve call");
+    }
+
     [Fact]
     public void Gate_null_leaves_errors_untouched()
     {

tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/DeferredGateHardeningTests.cs

@@ -21,8 +21,8 @@ namespace ZB.MOM.WW.OtOpcUa.Server.Tests;
 ///       <item>Lax-mode fall-through for all four deferred gates</item>
 ///       <item>Permission-bit isolation — Subscribe-only grant denies Read; HistoryRead-only
 ///             grant denies Read (Phase 6.2 compliance item "HistoryRead uses its own flag")</item>
-///       <item>AlarmShelve intentional fall-through to Call (documents the ShelvedStateMachine
-///             per-instance NodeId limitation noted in the MapCallOperation implementation)</item>
+///       <item>AlarmShelve resolves via the indexed shelve-method NodeId set (Task #24
+///             follow-up); an unindexed shelve-shaped NodeId still falls through to Call</item>
 ///       <item>Complete OpcUaOperation → NodePermissions mapping coverage for deferred ops</item>
 ///     </list>
 /// </summary>
@@ -203,29 +203,38 @@ public sealed class DeferredGateHardeningTests
     }
 
     // ======================================================================
-    // 5. AlarmShelve falls through to Call in MapCallOperation
-    //    Documents the ShelvedStateMachine per-instance NodeId limitation.
+    // 5. AlarmShelve resolution in MapCallOperation (Task #24 follow-up)
+    //    Shelve methods carry per-instance NodeIds, so they resolve to AlarmShelve
+    //    via membership in the indexed shelve-method set rather than a constant match.
     // ======================================================================
 
     [Fact]
-    public void MapCallOperation_AlarmShelve_falls_through_to_Call()
+    public void MapCallOperation_indexed_shelve_method_maps_to_AlarmShelve()
     {
-        // AlarmShelve methods on ShelvedStateMachine arrive with per-instance NodeIds
-        // (not well-known type NodeIds), so they can't be reliably constant-matched.
-        // MapCallOperation returns OpcUaOperation.Call for any unrecognised method NodeId;
-        // operators who can Shelve must therefore have NodePermissions.MethodCall granted.
-        // (This is an intentional design decision documented in the MapCallOperation
-        // implementation remarks — finer-grained AlarmShelve gating is deferred until
-        // the method-invocation path also carries a "method-role" annotation.)
+        // The address-space build indexes each scripted alarm's three ShelvedStateMachine
+        // method NodeIds. A call whose MethodId is in that set gates as AlarmShelve, so
+        // operators can be granted shelve rights independently of generic MethodCall.
+        var shelveMethodId = new NodeId("al-1.Condition.ShelvingState.OneShotShelve", 2);
+        var index = new HashSet<NodeId> { shelveMethodId };
+
+        DriverNodeManager.MapCallOperation(shelveMethodId, index).ShouldBe(OpcUaOperation.AlarmShelve);
+    }
+
+    [Fact]
+    public void MapCallOperation_unindexed_shelve_method_falls_through_to_Call()
+    {
+        // Without the index (e.g. a deployment with no scripted alarms) a shelve-shaped
+        // NodeId is indistinguishable from a generic driver method and gates as Call.
         var shelveMethodId = new NodeId("ShelvedStateMachine.OneShotShelve", namespaceIndex: 0);
         DriverNodeManager.MapCallOperation(shelveMethodId).ShouldBe(OpcUaOperation.Call);
     }
 
     [Fact]
-    public void MethodCall_grant_allows_generic_Call_including_shelve_path()
+    public void MethodCall_grant_allows_generic_Call()
     {
-        // Users with MethodCall permission can invoke shelve methods because the gate
-        // maps AlarmShelve back to Call (see MapCallOperation_AlarmShelve_falls_through_to_Call).
+        // Users with MethodCall permission can invoke generic (non-alarm) driver methods.
+        // Shelve methods now gate as AlarmShelve when indexed (see
+        // MapCallOperation_indexed_shelve_method_maps_to_AlarmShelve).
         var gate = MakeGate(strict: true, rows:
         [
             Row("grp-eng", NodePermissions.MethodCall),

tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/Phase7/ScriptedAlarmMethodRoutingTests.cs

@@ -161,6 +161,18 @@ public sealed class ScriptedAlarmMethodRoutingTests
             },
         };
 
+    private static CallMethodRequest AddCommentRequest(string conditionNodeId, string comment)
+        => new()
+        {
+            ObjectId = new NodeId(conditionNodeId, 2),
+            MethodId = MethodIds.ConditionType_AddComment,
+            InputArguments = new VariantCollection
+            {
+                new Variant(new byte[] { 1, 2, 3 }), // EventId (ByteString) — ignored
+                new Variant(new LocalizedText(comment)),
+            },
+        };
+
     private static CallMethodRequest GenericRequest(string objectNodeId)
         => new()
         {
@@ -357,6 +369,47 @@ public sealed class ScriptedAlarmMethodRoutingTests
         engine.GetState("confirm-alarm")!.LastConfirmUser.ShouldBe("ops-user");
     }
 
+    // ---- AddComment --------------------------------------------------------
+
+    [Fact]
+    public void AddComment_appends_comment_to_engine_state()
+    {
+        using var engine = BuildEngine("al-1");
+
+        var calls = new List<CallMethodRequest> { AddCommentRequest("al-1.Condition", "checked the line") };
+        var results = new List<CallMethodResult> { new CallMethodResult() };
+        var errors = new List<ServiceResult> { null! };
+        var index = Index(("al-1.Condition", "al-1"));
+
+        DriverNodeManager.RouteScriptedAlarmMethodCalls(
+            MakeIdentity("ops-user"), calls, results, errors, engine, index);
+
+        ServiceResult.IsBad(errors[0]).ShouldBeFalse("AddComment handled");
+        results[0].StatusCode.ShouldBe((StatusCode)StatusCodes.Good);
+        var last = engine.GetState("al-1")!.Comments[^1];
+        last.Kind.ShouldBe("AddComment");
+        last.Text.ShouldBe("checked the line");
+        last.User.ShouldBe("ops-user");
+    }
+
+    [Fact]
+    public void AddComment_with_empty_text_returns_BadInvalidArgument()
+    {
+        using var engine = BuildEngine("al-1");
+
+        // The Part 9 state machine rejects an empty comment — surfaced as BadInvalidArgument.
+        var calls = new List<CallMethodRequest> { AddCommentRequest("al-1.Condition", "") };
+        var results = new List<CallMethodResult> { new CallMethodResult() };
+        var errors = new List<ServiceResult> { null! };
+        var index = Index(("al-1.Condition", "al-1"));
+
+        DriverNodeManager.RouteScriptedAlarmMethodCalls(
+            MakeIdentity("ops-user"), calls, results, errors, engine, index);
+
+        ServiceResult.IsBad(errors[0]).ShouldBeTrue();
+        errors[0].StatusCode.ShouldBe((StatusCode)StatusCodes.BadInvalidArgument);
+    }
+
     // ---- Mixed batches -----------------------------------------------------
 
     [Fact]
@@ -410,6 +463,76 @@ public sealed class ScriptedAlarmMethodRoutingTests
         errors[0].StatusCode.ShouldBe((StatusCode)StatusCodes.BadInvalidArgument);
     }
 
+    // ---- Shelve routing (Task #24 follow-up) -------------------------------
+
+    [Fact]
+    public void InvokeEngineShelve_oneshot_shelves_engine_state()
+    {
+        using var engine = BuildEngine("al-1");
+
+        var result = DriverNodeManager.InvokeEngineShelve(
+            engine, "al-1", "ops-user", shelving: true, oneShot: true, shelvingTime: 0, logger: null);
+
+        ServiceResult.IsBad(result).ShouldBeFalse("OneShotShelve succeeds");
+        engine.GetState("al-1")!.Shelving.Kind.ShouldBe(ShelvingKind.OneShot);
+    }
+
+    [Fact]
+    public void InvokeEngineShelve_timed_shelves_engine_state()
+    {
+        using var engine = BuildEngine("al-1");
+
+        // shelvingTime is a Duration in ms — InvokeEngineShelve adds it to UtcNow.
+        var result = DriverNodeManager.InvokeEngineShelve(
+            engine, "al-1", "ops-user", shelving: true, oneShot: false, shelvingTime: 60_000, logger: null);
+
+        ServiceResult.IsBad(result).ShouldBeFalse("TimedShelve succeeds");
+        var state = engine.GetState("al-1")!;
+        state.Shelving.Kind.ShouldBe(ShelvingKind.Timed);
+        state.Shelving.UnshelveAtUtc.ShouldNotBeNull();
+    }
+
+    [Fact]
+    public void InvokeEngineShelve_unshelve_clears_engine_state()
+    {
+        using var engine = BuildEngine("al-1");
+        DriverNodeManager.InvokeEngineShelve(
+            engine, "al-1", "ops-user", shelving: true, oneShot: true, shelvingTime: 0, logger: null);
+        engine.GetState("al-1")!.Shelving.Kind.ShouldBe(ShelvingKind.OneShot);
+
+        var result = DriverNodeManager.InvokeEngineShelve(
+            engine, "al-1", "ops-user", shelving: false, oneShot: false, shelvingTime: 0, logger: null);
+
+        ServiceResult.IsBad(result).ShouldBeFalse("Unshelve succeeds");
+        engine.GetState("al-1")!.Shelving.Kind.ShouldBe(ShelvingKind.Unshelved);
+    }
+
+    [Fact]
+    public void InvokeEngineShelve_timed_with_non_positive_duration_returns_BadInvalidArgument()
+    {
+        using var engine = BuildEngine("al-1");
+
+        // A TimedShelve resolving to an unshelve time at-or-before now is rejected by the
+        // engine's Part 9 state machine (ArgumentOutOfRangeException → BadInvalidArgument).
+        var result = DriverNodeManager.InvokeEngineShelve(
+            engine, "al-1", "ops-user", shelving: true, oneShot: false, shelvingTime: 0, logger: null);
+
+        ServiceResult.IsBad(result).ShouldBeTrue();
+        result.StatusCode.ShouldBe((StatusCode)StatusCodes.BadInvalidArgument);
+    }
+
+    [Fact]
+    public void InvokeEngineShelve_unknown_alarm_returns_BadInvalidArgument()
+    {
+        using var engine = BuildEngine("al-1");
+
+        var result = DriverNodeManager.InvokeEngineShelve(
+            engine, "not-an-alarm", "ops-user", shelving: true, oneShot: true, shelvingTime: 0, logger: null);
+
+        ServiceResult.IsBad(result).ShouldBeTrue("unknown alarm id → error result");
+        result.StatusCode.ShouldBe((StatusCode)StatusCodes.BadInvalidArgument);
+    }
+
     // ---- Phase7ComposedSources helpers -------------------------------------
 
     private static Script ScriptRow(string id, string source) => new()