feat(client): AddComment + comment CLI, drop Galaxy-era alarm fallback, serialize CLI tests (#598); Client.Shared off obsolete sync SDK APIs (#621)

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

docs/Client.CLI.md

@@ -4,7 +4,7 @@
 
 `ZB.MOM.WW.OtOpcUa.Client.CLI` is a cross-platform command-line client for the OtOpcUa OPC UA server. It targets .NET 10 and uses the shared `IOpcUaClientService` from `Client.Shared` for all OPC UA operations. Commands are routed and parsed by [CliFx](https://github.com/Tyrrrz/CliFx).
 
-The CLI is the primary tool for operators and developers to test and interact with the server from a terminal. It supports all core operations: connectivity testing, browsing, reading, writing, subscriptions, alarm monitoring, alarm operator actions (acknowledge / confirm / shelve / enable / disable), history reads, and redundancy queries. Any driver surface exposed by the server (Galaxy, Modbus, S7, AB CIP, AB Legacy, TwinCAT, FOCAS, OPC UA Client) is reachable through these commands — the CLI is driver-agnostic because everything below the OPC UA endpoint is.
+The CLI is the primary tool for operators and developers to test and interact with the server from a terminal. It supports all core operations: connectivity testing, browsing, reading, writing, subscriptions, alarm monitoring, alarm operator actions (acknowledge / confirm / comment / shelve / enable / disable), history reads, and redundancy queries. Any driver surface exposed by the server (Galaxy, Modbus, S7, AB CIP, AB Legacy, TwinCAT, FOCAS, OPC UA Client) is reachable through these commands — the CLI is driver-agnostic because everything below the OPC UA endpoint is.
 
 ## Build and Run
 
@@ -284,6 +284,21 @@ Example output:
 Confirm successful: ns=1;s=TestMachine_001
 ```
 
+### comment
+
+Attaches an operator comment to an alarm condition (OPC UA Part 9 `AddComment`) without changing its acknowledge/confirm state. Works on scripted and native conditions alike. The `--node` and `--event-id` are the same values `ack`/`confirm` take.
+
+```bash
+otopcua-cli comment -u opc.tcp://localhost:4840/OtOpcUa \
+  -n "ns=2;s=Plant/Line1/Pump1/HighTemp" -e 0A1B2C -c "Checked on site"
+```
+
+| Flag | Description |
+|---|---|
+| `-n` / `--node` | Condition node ID of the alarm to comment on (**required**) |
+| `-e` / `--event-id` | Hex-encoded EventId from the alarm notification (**required**) |
+| `-c` / `--comment` | Operator comment to attach (**required**) |
+
 ### shelve
 
 Shelves or unshelves an active alarm condition via the OPC UA Part 9 `ShelvedStateMachine` (`OneShotShelve` / `TimedShelve` / `Unshelve`). Gated on the `AlarmAck` role. `--duration` is given in **seconds** (converted to milliseconds for the OPC UA call) and is required only for `--kind Timed`. The `--node` is the condition node itself (the `SourceNode`/condition id an `alarms` notification reports): `ack`, `confirm`, `shelve`, `enable` and `disable` call the Part 9 method ids directly on that node — no `.Condition` / `.ShelvingState` suffix is appended (those child nodes do not exist on the OtOpcUa server).

src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/Commands/CommentCommand.cs

@@ -0,0 +1,85 @@
+using CliFx.Attributes;
+using CliFx.Exceptions;
+using CliFx.Infrastructure;
+using Opc.Ua;
+using ZB.MOM.WW.OtOpcUa.Client.CLI.Helpers;
+using ZB.MOM.WW.OtOpcUa.Client.Shared;
+
+namespace ZB.MOM.WW.OtOpcUa.Client.CLI.Commands;
+
+[Command("comment", Description = "Add an operator comment to an alarm condition (OPC UA Part 9 AddComment)")]
+public class CommentCommand : CommandBase
+{
+    /// <summary>
+    /// Creates the comment command used to attach an operator comment to an OPC UA condition from the terminal.
+    /// </summary>
+    /// <param name="factory">The factory that creates the shared client service for the command run.</param>
+    public CommentCommand(IOpcUaClientServiceFactory factory) : base(factory)
+    {
+    }
+
+    /// <summary>
+    /// Gets the condition node ID of the alarm event to comment on.
+    /// </summary>
+    [CommandOption("node", 'n', Description = "Condition node ID of the alarm to comment on", IsRequired = true)]
+    public string NodeId { get; init; } = default!;
+
+    /// <summary>
+    /// Gets the event ID (hex-encoded) returned by the server in the alarm notification.
+    /// </summary>
+    [CommandOption("event-id", 'e', Description = "EventId from the alarm notification (hex-encoded byte array)", IsRequired = true)]
+    public string EventId { get; init; } = default!;
+
+    /// <summary>
+    /// Gets the operator comment to attach to the condition.
+    /// </summary>
+    [CommandOption("comment", 'c', Description = "Operator comment to attach", IsRequired = true)]
+    public string Comment { get; init; } = string.Empty;
+
+    /// <inheritdoc />
+    public override async ValueTask ExecuteAsync(IConsole console)
+    {
+        ConfigureLogging();
+
+        try
+        {
+            NodeIdParser.ParseRequired(NodeId);
+        }
+        catch (Exception ex) when (ex is FormatException or ArgumentException)
+        {
+            throw new CommandException($"Invalid --node value: {ex.Message}");
+        }
+
+        byte[] eventId;
+        try
+        {
+            eventId = Convert.FromHexString(EventId);
+        }
+        catch (FormatException ex)
+        {
+            throw new CommandException($"Invalid --event-id value: {ex.Message} (expected hex-encoded bytes, e.g. 0A1B2C)");
+        }
+
+        IOpcUaClientService? service = null;
+        try
+        {
+            var ct = console.RegisterCancellationHandler();
+            (service, _) = await CreateServiceAndConnectAsync(ct);
+
+            var statusCode = await service.AddCommentAsync(NodeId, eventId, Comment, ct);
+
+            if (StatusCode.IsGood(statusCode))
+                await console.Output.WriteLineAsync($"Comment added: {NodeId}");
+            else
+                await console.Output.WriteLineAsync($"Comment failed: {statusCode}");
+        }
+        finally
+        {
+            if (service != null)
+            {
+                await service.DisconnectAsync();
+                service.Dispose();
+            }
+        }
+    }
+}

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/ClientTelemetry.cs

@@ -0,0 +1,39 @@
+using Opc.Ua;
+
+namespace ZB.MOM.WW.OtOpcUa.Client.Shared.Adapters;
+
+/// <summary>
+///     The one <see cref="ITelemetryContext" /> the shared client hands the SDK's non-obsolete (telemetry-taking)
+///     overloads. An empty builder keeps SDK internals off any logging provider; the client logs through Serilog.
+/// </summary>
+internal static class ClientTelemetry
+{
+    private static readonly Lazy<ITelemetryContext> Instance = new(() => DefaultTelemetry.Create(_ => { }));
+
+    /// <summary>Gets the shared telemetry context.</summary>
+    public static ITelemetryContext Context => Instance.Value;
+}
+
+/// <summary>
+///     Bounded sync-over-async for the <c>Dispose</c> paths, which cannot await. Runs on the thread pool so a
+///     caller on a synchronization context (the Avalonia UI thread) cannot deadlock, and waits at most
+///     <see cref="Timeout" /> so a dead server cannot hang teardown.
+/// </summary>
+internal static class DisposeBridge
+{
+    private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(5);
+
+    /// <summary>Runs <paramref name="work" /> to completion (or the timeout), swallowing every failure.</summary>
+    public static void Run(Func<CancellationToken, Task> work)
+    {
+        using var cts = new CancellationTokenSource(Timeout);
+        try
+        {
+            Task.Run(() => work(cts.Token), cts.Token).Wait(Timeout);
+        }
+        catch
+        {
+            // Teardown is best-effort, exactly as the synchronous Close/Delete it replaces.
+        }
+    }
+}

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultApplicationConfigurationFactory.cs

@@ -58,14 +58,14 @@ internal sealed class DefaultApplicationConfigurationFactory : IApplicationConfi
             }
         };
 
-        await config.Validate(ApplicationType.Client);
+        await config.ValidateAsync(ApplicationType.Client, ct);
 
         if (settings.AutoAcceptCertificates)
             config.CertificateValidator.CertificateValidation += (_, e) => e.Accept = true;
 
         if (settings.SecurityMode != SecurityMode.None)
         {
-            var app = new ApplicationInstance
+            var app = new ApplicationInstance(ClientTelemetry.Context)
             {
                 ApplicationName = "OtOpcUaClient",
                 ApplicationType = ApplicationType.Client,

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultEndpointDiscovery.cs

@@ -12,18 +12,14 @@ internal sealed class DefaultEndpointDiscovery : IEndpointDiscovery
     private static readonly ILogger Logger = Log.ForContext<DefaultEndpointDiscovery>();
 
     /// <inheritdoc />
-    public EndpointDescription SelectEndpoint(ApplicationConfiguration config, string endpointUrl,
+    public async Task<EndpointDescription> SelectEndpointAsync(ApplicationConfiguration config, string endpointUrl,
-        MessageSecurityMode requestedMode)
+        MessageSecurityMode requestedMode, CancellationToken ct = default)
     {
-        if (requestedMode == MessageSecurityMode.None)
+        // One path for every mode: discover, filter by mode, rewrite the host. (The None branch used to call the
-        {
+        // obsolete synchronous CoreClientUtils.SelectEndpoint, which does the same filter + rewrite.)
-#pragma warning disable CS0618 // Acceptable for endpoint selection
+        using var client = await DiscoveryClient.CreateAsync(
-            return CoreClientUtils.SelectEndpoint(config, endpointUrl, false);
+            config, new Uri(endpointUrl), DiagnosticsMasks.None, ct).ConfigureAwait(false);
-#pragma warning restore CS0618
+        var allEndpoints = await client.GetEndpointsAsync(null, ct).ConfigureAwait(false);
-        }
-
-        using var client = DiscoveryClient.Create(new Uri(endpointUrl));
-        var allEndpoints = client.GetEndpoints(null);
 
         return EndpointSelector.SelectBest(allEndpoints, endpointUrl, requestedMode);
     }

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionAdapter.cs

@@ -264,7 +264,7 @@ internal sealed class DefaultSessionAdapter : ISessionAdapter
     {
         try
         {
-            if (_session.Connected) _session.Close();
+            if (_session.Connected) DisposeBridge.Run(ct => _session.CloseAsync(ct));
         }
         catch
         {

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionFactory.cs

@@ -23,14 +23,18 @@ internal sealed class DefaultSessionFactory : ISessionFactory
         var endpointConfig = EndpointConfiguration.Create(config);
         var configuredEndpoint = new ConfiguredEndpoint(null, endpoint, endpointConfig);
 
-        var session = await Session.Create(
+        var created = await new Opc.Ua.Client.DefaultSessionFactory(ClientTelemetry.Context).CreateAsync(
             config,
             configuredEndpoint,
             false,
             sessionName,
             sessionTimeoutMs,
             identity,
-            null);
+            null,
+            ct);
+        var session = created as Session
+            ?? throw new InvalidOperationException(
+                $"Session factory returned {created.GetType().Name}, expected {nameof(Session)}.");
 
         Logger.Information("Session created: {SessionName} -> {EndpointUrl}", sessionName, endpoint.EndpointUrl);
         return new DefaultSessionAdapter(session);

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSubscriptionAdapter.cs

@@ -123,7 +123,7 @@ internal sealed class DefaultSubscriptionAdapter : ISubscriptionAdapter
     {
         try
         {
-            _subscription.Delete(true);
+            DisposeBridge.Run(ct => _subscription.DeleteAsync(true, ct));
         }
         catch
         {

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/IEndpointDiscovery.cs

@@ -14,7 +14,8 @@ internal interface IEndpointDiscovery
     /// <param name="config">The OPC UA application configuration.</param>
     /// <param name="endpointUrl">The endpoint URL to discover.</param>
     /// <param name="requestedMode">The requested message security mode.</param>
+    /// <param name="ct">Cancellation token.</param>
     /// <returns>The best-matching endpoint description, with its URL hostname rewritten to match <paramref name="endpointUrl"/> when they differ.</returns>
-    EndpointDescription SelectEndpoint(ApplicationConfiguration config, string endpointUrl,
+    Task<EndpointDescription> SelectEndpointAsync(ApplicationConfiguration config, string endpointUrl,
-        MessageSecurityMode requestedMode);
+        MessageSecurityMode requestedMode, CancellationToken ct = default);
 }

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/IOpcUaClientService.cs

@@ -127,6 +127,16 @@ public interface IOpcUaClientService : IDisposable
     /// </returns>
     Task<StatusCode> ConfirmAlarmAsync(string conditionNodeId, byte[] eventId, string comment, CancellationToken ct = default);
 
+    /// <summary>
+    /// Attaches an operator comment to a condition (Part 9 <c>AddComment</c>) using the event identifier of an alarm notification.
+    /// </summary>
+    /// <param name="conditionNodeId">The condition node the comment is attached to.</param>
+    /// <param name="eventId">The event identifier returned by the OPC UA server for the alarm event.</param>
+    /// <param name="comment">The operator comment.</param>
+    /// <param name="ct">The cancellation token that aborts the request.</param>
+    /// <returns><see cref="StatusCodes.Good"/> on success, or the server's bad <see cref="StatusCode"/> when the call is refused.</returns>
+    Task<StatusCode> AddCommentAsync(string conditionNodeId, byte[] eventId, string comment, CancellationToken ct = default);
+
     /// <summary>
     /// Shelves or unshelves an active alarm condition (OPC UA Part 9 ShelvedStateMachine).
     /// </summary>

src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs

@@ -428,6 +428,35 @@ public sealed class OpcUaClientService : IOpcUaClientService
         return StatusCodes.Good;
     }
 
+    /// <inheritdoc />
+    public async Task<StatusCode> AddCommentAsync(string conditionNodeId, byte[] eventId, string comment,
+        CancellationToken ct = default)
+    {
+        ThrowIfDisposed();
+        ThrowIfNotConnected();
+
+        // AddComment is a ConditionType method, invoked on the condition node itself.
+        var conditionObjId = NodeId.Parse(conditionNodeId);
+
+        try
+        {
+            await _session!.CallMethodAsync(
+                conditionObjId,
+                MethodIds.ConditionType_AddComment,
+                [eventId, new LocalizedText(comment)],
+                ct);
+        }
+        catch (ServiceResultException ex)
+        {
+            Logger.Warning(ex, "Failed to add comment on {ConditionId} (status {Status})",
+                conditionNodeId, ex.StatusCode);
+            return ex.StatusCode;
+        }
+
+        Logger.Debug("Added comment on {ConditionId}", conditionNodeId);
+        return StatusCodes.Good;
+    }
+
     /// <inheritdoc />
     public async Task<StatusCode> ShelveAlarmAsync(string conditionNodeId, ShelveKind kind,
         double shelvingTimeSeconds = 0, CancellationToken ct = default)
@@ -633,7 +662,7 @@ public sealed class OpcUaClientService : IOpcUaClientService
 
         var config = await _configFactory.CreateAsync(effectiveSettings, ct);
         var requestedMode = SecurityModeMapper.ToMessageSecurityMode(settings.SecurityMode);
-        var endpoint = _endpointDiscovery.SelectEndpoint(config, endpointUrl, requestedMode);
+        var endpoint = await _endpointDiscovery.SelectEndpointAsync(config, endpointUrl, requestedMode, ct);
 
         var identity = settings.Username != null
             ? new UserIdentity(settings.Username, Encoding.UTF8.GetBytes(settings.Password ?? ""))
@@ -824,74 +853,8 @@ public sealed class OpcUaClientService : IOpcUaClientService
         var ackedState = ackedField ?? false;
         var activeState = activeField ?? false;
 
-        // Fallback: read InAlarm/Acked from condition node Galaxy attributes
+        // No Galaxy-era supplemental reads (<condition>.InAlarm/.Acked/...): the server publishes the
-        // when the server doesn't populate standard event fields.
+        // standard Part 9 fields. A foreign server that leaves both null reports Active=false/Acked=false.
-        // Must run on a background thread to avoid deadlocking the notification thread.
-        // Capture the session reference now; skip supplemental reads if the session has
-        // been replaced by a concurrent failover before the Task.Run body executes.
-        if (ackedField == null && activeField == null && conditionNodeId != null && _session != null)
-        {
-            var session = _session;
-            var capturedConditionNodeId = conditionNodeId;
-            var capturedMessage = message;
-            _ = Task.Run(async () =>
-            {
-                // If the session was replaced by a failover before we started reading,
-                // skip the supplemental reads to avoid hitting a disposed session.
-                if (!ReferenceEquals(session, _session))
-                    return;
-
-                try
-                {
-                    var inAlarmValue = await session.ReadValueAsync(NodeId.Parse($"{capturedConditionNodeId}.InAlarm"));
-                    if (inAlarmValue.Value is bool inAlarm) activeState = inAlarm;
-
-                    var ackedValue = await session.ReadValueAsync(NodeId.Parse($"{capturedConditionNodeId}.Acked"));
-                    if (ackedValue.Value is bool acked) ackedState = acked;
-
-                    if (time == DateTime.MinValue && activeState)
-                    {
-                        var timeValue = await session.ReadValueAsync(NodeId.Parse($"{capturedConditionNodeId}.TimeAlarmOn"));
-                        if (timeValue.Value is DateTime alarmTime && alarmTime != DateTime.MinValue)
-                            time = alarmTime;
-                    }
-
-                    // Read alarm description to use as message
-                    try
-                    {
-                        var descValue = await session.ReadValueAsync(NodeId.Parse($"{capturedConditionNodeId}.DescAttrName"));
-                        if (descValue.Value is string desc && !string.IsNullOrEmpty(desc))
-                            capturedMessage = desc;
-                    }
-                    catch { /* DescAttrName may not exist */ }
-                }
-                catch (ObjectDisposedException)
-                {
-                    // Session was disposed during supplemental reads (concurrent failover);
-                    // skip the event rather than delivering stale/default states.
-                    Logger.Debug("Supplemental alarm read skipped — session disposed during failover for {ConditionNodeId}", capturedConditionNodeId);
-                    return;
-                }
-                catch
-                {
-                    // Other supplemental read failure; deliver event with defaults
-                }
-
-                // OperatorComment is intentionally left null (default) here.
-                // Two concrete reasons make it unrecoverable in this Galaxy sub-attribute fallback path:
-                //   (a) Galaxy's AckMsg attribute is WRITE-ONLY — there is no readable attribute that
-                //       exposes the last operator comment, so no supplemental read can recover it.
-                //   (b) The OPC UA event SelectClause (CreateAlarmEventFilter) carries no comment field;
-                //       the OPC UA event notification itself never delivers a comment string.
-                // The operator comment is only available via the gateway's native alarm feed
-                // (the Galaxy driver path, GalaxyAlarmTransition.OperatorComment), not this
-                // OpcUaClient sub-attribute fallback.
-                AlarmEvent?.Invoke(this, new AlarmEventArgs(
-                    sourceName, conditionName, severity, capturedMessage, retain, activeState, ackedState, time,
-                    eventId, capturedConditionNodeId));
-            });
-            return;
-        }
 
         AlarmEvent?.Invoke(this, new AlarmEventArgs(
             sourceName, conditionName, severity, message, retain, activeState, ackedState, time,

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/AssemblyInfo.cs

@@ -0,0 +1,6 @@
+using Xunit;
+
+// Every command run reassigns the process-global Serilog Log.Logger (CommandBase.ConfigureLogging), and
+// LoggerLifecycleTests asserts on that global's dispose lifecycle. A collection attribute on that one class
+// cannot serialize it against the ~20 other classes that run commands, so the whole (small) suite runs serially. (#598)
+[assembly: CollectionBehavior(DisableTestParallelization = true)]

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/CommentCommandTests.cs

@@ -0,0 +1,106 @@
+using CliFx.Exceptions;
+using Opc.Ua;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Client.CLI.Commands;
+using ZB.MOM.WW.OtOpcUa.Client.CLI.Tests.Fakes;
+
+namespace ZB.MOM.WW.OtOpcUa.Client.CLI.Tests;
+
+public class CommentCommandTests
+{
+    private static string HexOf(byte[] bytes) => Convert.ToHexString(bytes);
+
+    /// <summary>Verifies that a successful comment prints a success message and passes correct arguments to the service.</summary>
+    [Fact]
+    public async Task Execute_Success_PrintsSuccessMessage()
+    {
+        var fakeService = new FakeOpcUaClientService
+        {
+            AddCommentResult = StatusCodes.Good
+        };
+        var factory = new FakeOpcUaClientServiceFactory(fakeService);
+        var eventId = new byte[] { 0x0A, 0x1B, 0x2C };
+        var command = new CommentCommand(factory)
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=MyAlarm",
+            EventId = HexOf(eventId),
+            Comment = "operator note"
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        await command.ExecuteAsync(console);
+
+        var output = TestConsoleHelper.GetOutput(console);
+        output.ShouldContain("Comment added");
+        fakeService.AddCommentCalls.Count.ShouldBe(1);
+        fakeService.AddCommentCalls[0].ConditionNodeId.ShouldBe("ns=2;s=MyAlarm");
+        fakeService.AddCommentCalls[0].EventId.ShouldBe(eventId);
+        fakeService.AddCommentCalls[0].Comment.ShouldBe("operator note");
+    }
+
+    /// <summary>Verifies that a bad StatusCode from the service prints a failure message.</summary>
+    [Fact]
+    public async Task Execute_BadStatus_PrintsFailureMessage()
+    {
+        var fakeService = new FakeOpcUaClientService
+        {
+            AddCommentResult = StatusCodes.BadNotSupported
+        };
+        var factory = new FakeOpcUaClientServiceFactory(fakeService);
+        var command = new CommentCommand(factory)
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=MyAlarm",
+            EventId = HexOf(new byte[] { 0xFF }),
+            Comment = ""
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        await command.ExecuteAsync(console);
+
+        var output = TestConsoleHelper.GetOutput(console);
+        output.ShouldContain("Comment failed");
+    }
+
+    /// <summary>Verifies that an invalid hex event-id throws CommandException.</summary>
+    [Fact]
+    public async Task Execute_InvalidEventId_ThrowsCommandException()
+    {
+        var fakeService = new FakeOpcUaClientService();
+        var factory = new FakeOpcUaClientServiceFactory(fakeService);
+        var command = new CommentCommand(factory)
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=MyAlarm",
+            EventId = "ZZZ-not-hex",
+            Comment = ""
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        var ex = await Should.ThrowAsync<CommandException>(async () => await command.ExecuteAsync(console));
+        ex.Message.ShouldContain("event-id", Case.Insensitive);
+    }
+
+    /// <summary>Verifies that the command disconnects in the finally block.</summary>
+    [Fact]
+    public async Task Execute_DisconnectsInFinally()
+    {
+        var fakeService = new FakeOpcUaClientService();
+        var factory = new FakeOpcUaClientServiceFactory(fakeService);
+        var command = new CommentCommand(factory)
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=MyAlarm",
+            EventId = HexOf(new byte[] { 0x01 }),
+            Comment = ""
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        await command.ExecuteAsync(console);
+
+        fakeService.DisconnectCalled.ShouldBeTrue();
+        fakeService.DisposeCalled.ShouldBeTrue();
+    }
+}

tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/Fakes/FakeOpcUaClientService.cs

@@ -264,6 +264,20 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
         return Task.FromResult(ConfirmAlarmResult);
     }
 
+    /// <summary>Gets the list of (conditionNodeId, eventId, comment) tuples from AddCommentAsync calls.</summary>
+    public List<(string ConditionNodeId, byte[] EventId, string Comment)> AddCommentCalls { get; } = [];
+
+    /// <summary>Gets or sets the status code returned by AddCommentAsync.</summary>
+    public StatusCode AddCommentResult { get; set; } = StatusCodes.Good;
+
+    /// <inheritdoc />
+    public Task<StatusCode> AddCommentAsync(string conditionNodeId, byte[] eventId, string comment,
+        CancellationToken ct = default)
+    {
+        AddCommentCalls.Add((conditionNodeId, eventId, comment));
+        return Task.FromResult(AddCommentResult);
+    }
+
     /// <summary>Gets the list of (conditionNodeId, kind, durationSeconds) tuples from ShelveAlarmAsync calls.</summary>
     public List<(string ConditionNodeId, ShelveKind Kind, double DurationSeconds)> ShelveAlarmCalls { get; } = [];
 

tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Fakes/FakeEndpointDiscovery.cs

@@ -17,8 +17,8 @@ internal sealed class FakeEndpointDiscovery : IEndpointDiscovery
     /// <param name="endpointUrl">The endpoint URL to select.</param>
     /// <param name="requestedMode">The requested security mode.</param>
     /// <returns>The selected endpoint description.</returns>
-    public EndpointDescription SelectEndpoint(ApplicationConfiguration config, string endpointUrl,
+    public Task<EndpointDescription> SelectEndpointAsync(ApplicationConfiguration config, string endpointUrl,
-        MessageSecurityMode requestedMode)
+        MessageSecurityMode requestedMode, CancellationToken ct = default)
     {
         SelectCallCount++;
         LastEndpointUrl = endpointUrl;
@@ -26,7 +26,7 @@ internal sealed class FakeEndpointDiscovery : IEndpointDiscovery
         if (ThrowOnSelect)
             throw new InvalidOperationException($"No endpoint found for {endpointUrl}");
 
-        return new EndpointDescription
+        return Task.FromResult(new EndpointDescription
         {
             EndpointUrl = endpointUrl,
             SecurityMode = requestedMode,
@@ -38,6 +38,6 @@ internal sealed class FakeEndpointDiscovery : IEndpointDiscovery
                 ApplicationName = "FakeServer",
                 ApplicationUri = "urn:localhost:FakeServer"
             }
-        };
+        });
     }
-}
+}

tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs

@@ -1263,52 +1263,27 @@ public class OpcUaClientServiceTests : IDisposable
         result.Code.ShouldBe(StatusCodes.BadConditionAlreadyDisabled);
     }
 
-    // --- Alarm fallback path (Client.Shared-011) ---
+    // --- Missing standard alarm fields (#598: the Galaxy-era supplemental reads are gone) ---
 
     /// <summary>
-    /// Regression for Client.Shared-011: when standard AckedState/Id and ActiveState/Id
+    /// When the standard AckedState/Id and ActiveState/Id fields arrive null the event is delivered with
-    /// fields are missing (null Value) but the SourceNode (ConditionId) field at index 12
+    /// Active=false / Acked=false and the client issues NO supplemental reads of
-    /// is populated, the client launches the Task.Run fallback that reads
+    /// <c>&lt;condition&gt;.InAlarm</c>/<c>.Acked</c> (those attributes only existed on the retired Galaxy v2 server).
-    /// <c>InAlarm</c>/<c>Acked</c> from the condition node's Galaxy attributes. Verify
-    /// the alarm event is delivered with the values from the supplemental reads.
     /// </summary>
     [Fact]
-    public async Task OnAlarmEvent_MissingAckedActiveButHasConditionNode_FallbackReadsAndRaisesEvent()
+    public async Task OnAlarmEvent_MissingAckedActive_DeliversDefaultsWithoutExtraReads()
     {
         var fakeSub = new FakeSubscriptionAdapter();
-        var session = new FakeSessionAdapter
+        var session = new FakeSessionAdapter { NextSubscription = fakeSub };
-        {
-            NextSubscription = fakeSub,
-            ReadResponseFunc = nodeId =>
-            {
-                var key = nodeId.ToString();
-                if (key.EndsWith(".InAlarm"))
-                    return new DataValue(new Variant(true), StatusCodes.Good);
-                if (key.EndsWith(".Acked"))
-                    return new DataValue(new Variant(false), StatusCodes.Good);
-                if (key.EndsWith(".TimeAlarmOn"))
-                    return new DataValue(new Variant(new DateTime(2026, 1, 1, 12, 0, 0)), StatusCodes.Good);
-                if (key.EndsWith(".DescAttrName"))
-                    return new DataValue(new Variant("Fallback message"), StatusCodes.Good);
-                return new DataValue(StatusCodes.BadNodeIdUnknown);
-            }
-        };
         _sessionFactory.EnqueueSession(session);
         await _service.ConnectAsync(ValidSettings());
 
         AlarmEventArgs? received = null;
-        var raised = new TaskCompletionSource();
+        _service.AlarmEvent += (_, e) => received = e;
-        _service.AlarmEvent += (_, e) =>
-        {
-            received = e;
-            raised.TrySetResult();
-        };
 
         await _service.SubscribeAlarmsAsync();
 
         var handle = fakeSub.ActiveHandles.First();
-        // AckedState/Id (8) and ActiveState/Id (9) are present but Variant.Value is null,
-        // which triggers the supplemental Galaxy-attribute fallback; SourceNode (12) is set.
         var fields = new EventFieldList
         {
             EventFields =
@@ -1321,8 +1296,8 @@ public class OpcUaClientServiceTests : IDisposable
                 new Variant((ushort)400),                  // 5: Severity
                 new Variant("CondName"),                   // 6: ConditionName
                 new Variant(true),                         // 7: Retain
-                Variant.Null,                              // 8: AckedState/Id — missing
+                Variant.Null,                              // 8: AckedState/Id - missing
-                Variant.Null,                              // 9: ActiveState/Id — missing
+                Variant.Null,                              // 9: ActiveState/Id - missing
                 new Variant(true),                         // 10: EnabledState/Id
                 new Variant(false),                        // 11: SuppressedOrShelved
                 new Variant("ns=2;s=ConditionId")          // 12: SourceNode
@@ -1330,90 +1305,14 @@ public class OpcUaClientServiceTests : IDisposable
         };
         fakeSub.SimulateEvent(handle, fields);
 
-        // The fallback runs on a background Task.Run continuation — wait briefly for it.
+        // Delivery is now synchronous (no background fallback), so the event is already here.
-        await Task.WhenAny(raised.Task, Task.Delay(500));
-
         received.ShouldNotBeNull();
-        received!.ActiveState.ShouldBeTrue();  // from InAlarm read
+        received!.ActiveState.ShouldBeFalse();
-        received.AckedState.ShouldBeFalse();   // from Acked read
+        received.AckedState.ShouldBeFalse();
         received.ConditionNodeId.ShouldBe("ns=2;s=ConditionId");
-        received.Message.ShouldBe("Fallback message"); // from DescAttrName read
+        received.Message.ShouldBe("Initial");
-    }
+        received.OperatorComment.ShouldBeNull();
-
+        session.ReadCount.ShouldBe(0);
-    /// <summary>
-    /// Pins the accepted limitation that OperatorComment is always null on the Galaxy
-    /// sub-attribute fallback path. Two concrete reasons make it unrecoverable:
-    /// (a) Galaxy's AckMsg attribute is WRITE-ONLY — no readable attribute exposes the
-    /// last operator comment — and (b) the OPC UA event SelectClause carries no comment
-    /// field. The operator comment is only available via the gateway's native alarm feed
-    /// (GalaxyAlarmTransition.OperatorComment in the Galaxy driver path, not here).
-    /// </summary>
-    [Fact]
-    public async Task OnAlarmEvent_GalaxyFallback_LeavesOperatorCommentNull()
-    {
-        var fakeSub = new FakeSubscriptionAdapter();
-        var session = new FakeSessionAdapter
-        {
-            NextSubscription = fakeSub,
-            ReadResponseFunc = nodeId =>
-            {
-                var key = nodeId.ToString();
-                if (key.EndsWith(".InAlarm"))
-                    return new DataValue(new Variant(true), StatusCodes.Good);
-                if (key.EndsWith(".Acked"))
-                    return new DataValue(new Variant(true), StatusCodes.Good);
-                if (key.EndsWith(".TimeAlarmOn"))
-                    return new DataValue(new Variant(new DateTime(2026, 1, 1, 12, 0, 0)), StatusCodes.Good);
-                if (key.EndsWith(".DescAttrName"))
-                    return new DataValue(new Variant("Ack message"), StatusCodes.Good);
-                return new DataValue(StatusCodes.BadNodeIdUnknown);
-            }
-        };
-        _sessionFactory.EnqueueSession(session);
-        await _service.ConnectAsync(ValidSettings());
-
-        AlarmEventArgs? received = null;
-        var raised = new TaskCompletionSource();
-        _service.AlarmEvent += (_, e) =>
-        {
-            received = e;
-            raised.TrySetResult();
-        };
-
-        await _service.SubscribeAlarmsAsync();
-
-        var handle = fakeSub.ActiveHandles.First();
-        // AckedState/Id (8) and ActiveState/Id (9) are null, triggering the Galaxy
-        // sub-attribute fallback; SourceNode (12) is set so conditionNodeId is populated.
-        var fields = new EventFieldList
-        {
-            EventFields =
-            [
-                new Variant(new byte[] { 4, 5, 6 }),       // 0: EventId
-                new Variant(ObjectTypeIds.AlarmConditionType), // 1: EventType
-                new Variant("Source2"),                    // 2: SourceName
-                new Variant(DateTime.MinValue),            // 3: Time
-                new Variant(new LocalizedText("Ack")),     // 4: Message
-                new Variant((ushort)200),                  // 5: Severity
-                new Variant("CondName2"),                  // 6: ConditionName
-                new Variant(true),                         // 7: Retain
-                Variant.Null,                              // 8: AckedState/Id — missing
-                Variant.Null,                              // 9: ActiveState/Id — missing
-                new Variant(true),                         // 10: EnabledState/Id
-                new Variant(false),                        // 11: SuppressedOrShelved
-                new Variant("ns=2;s=ConditionId2")         // 12: SourceNode
-            ]
-        };
-        fakeSub.SimulateEvent(handle, fields);
-
-        // The fallback runs on a background Task.Run continuation — wait briefly for it.
-        await Task.WhenAny(raised.Task, Task.Delay(500));
-
-        received.ShouldNotBeNull();
-        // OperatorComment must be null: AckMsg is WRITE-ONLY in Galaxy and the OPC UA
-        // event SelectClause carries no comment field — both reasons are unrecoverable
-        // on this client sub-attribute fallback path.
-        received!.OperatorComment.ShouldBeNull();
     }
 
     // --- Failover tests ---

tests/Client/ZB.MOM.WW.OtOpcUa.Client.UI.Tests/Fakes/FakeOpcUaClientService.cs

@@ -282,6 +282,10 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
         return Task.FromResult(ConfirmResult);
     }
 
+    /// <inheritdoc />
+    public Task<StatusCode> AddCommentAsync(string conditionNodeId, byte[] eventId, string comment,
+        CancellationToken ct = default) => Task.FromResult<StatusCode>(StatusCodes.Good);
+
     /// <summary>Gets or sets the status code returned by shelve operations in UI tests.</summary>
     public StatusCode ShelveResult { get; set; } = StatusCodes.Good;
     /// <summary>Gets or sets the exception thrown to simulate alarm shelve failures in the UI.</summary>

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

@@ -18,7 +18,7 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests;
 /// <remarks>
 ///     The condition id is passed exactly as a CLI user passes <c>--node</c>: the condition node's own id.
 ///     Each call must return Good <b>and</b> move the condition — read back from the condition events the
-///     server reports, not from the call result alone. The client exposes no AddComment API, so it is not covered.
+///     server reports, not from the call result alone. AddComment is covered on both the scripted and the native condition (the native one supports Acknowledge and AddComment only).
 /// </remarks>
 public sealed class ClientAlarmMethodTests
 {
@@ -66,6 +66,10 @@ public sealed class ClientAlarmMethodTests
                 conditionId, events.LatestEventId(item, "base"), "confirm", ct)).ShouldBeTrue();
             (await Next(++n))[RecordedEvents.ConfirmedField].Value.ShouldBe(true);
 
+            StatusCode.IsGood(await client.AddCommentAsync(
+                conditionId, events.LatestEventId(item, "base"), "note", ct)).ShouldBeTrue();
+            ((await Next(++n))[RecordedEvents.CommentField].Value as LocalizedText).ShouldNotBeNull().Text.ShouldBe("note");
+
             StatusCode.IsGood(await client.ShelveAlarmAsync(conditionId, ShelveKind.OneShot, 0, ct)).ShouldBeTrue();
             (await Next(++n))[RecordedEvents.ShelvingField].Value.ShouldBe(ObjectIds.ShelvedStateMachineType_OneShotShelved);
 
@@ -120,6 +124,12 @@ public sealed class ClientAlarmMethodTests
             ack[RecordedEvents.ConfirmedField].Value.ShouldBe(true);
             ack[RecordedEvents.ShelvingField].Value.ShouldBe(ObjectIds.ShelvedStateMachineType_Unshelved);
 
+            StatusCode.IsGood(await client.AddCommentAsync(
+                conditionId, events.LatestEventId(item, "base"), "note", ct)).ShouldBeTrue();
+            await rig.DrainProjectionsAsync();
+            await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count >= 3);
+            (events.ConditionEvents(item, "base")[2][RecordedEvents.CommentField].Value as LocalizedText).ShouldNotBeNull().Text.ShouldBe("note");
+
             await subscription.DeleteAsync(true, ct);
         }
         finally { await rig.DisposeAsync(); }