fix: address final ackmsg-as-operator review findings

- AlarmAckWriter: classify AuthenticateUser and AddItem/Advise/Write failures by HRESULT for any interop exception (ArgumentException for E_INVALIDARG, UnauthorizedAccessException, COMException); server-gone HRESULTs at AuthenticateUser tear down the registration and report WriteRejected. - Bound the ack item cache with a 256-entry LRU (eviction best-effort UnAdvise+RemoveItem). - After an Unconfirmed ack, drop and re-add the item before the next write so a late OnWriteComplete on the old handle is not credited. - Pass StaRuntime.PumpPendingMessages to the ack writer through a pump-aware alarm handler factory so long completion waits refresh the STA watchdog heartbeat. - Rust CLI: --operator is required for acknowledge-alarm. - Docs: full ack status table, credential-oracle trade-off, upgrade note. Claude-Session: https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT
2026-10-19 19:58:24 -04:00
parent 1998ee3e20
commit e7f56622c1
9 changed files with 332 additions and 31 deletions
@@ -84,7 +84,9 @@ public sealed class AlarmAckWriterTests
    public void Acknowledge_Twice_ReusesItemHandle()
    {
        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
-        using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
+        MxAccessWriteCompletionCache cache = new MxAccessWriteCompletionCache();
+        server.OnWrite = (handle, item) => cache.Record(handle, item, Rows(MxStatusCategory.Ok, 0));
+        using AlarmAckWriter writer = new AlarmAckWriter(server, cache, () => { }, TimeSpan.FromSeconds(1), "MxGateway");

        writer.Acknowledge("T.A.AckMsg", "one", Alice());
        writer.Acknowledge("T.A.AckMsg", "two", Alice());
@@ -114,7 +116,9 @@ public sealed class AlarmAckWriterTests
    public void Acknowledge_WhenCachedWriteThrows_RejectsAndReAddsNextTime()
    {
        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
-        using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
+        MxAccessWriteCompletionCache cache = new MxAccessWriteCompletionCache();
+        server.OnWrite = (handle, item) => cache.Record(handle, item, Rows(MxStatusCategory.Ok, 0));
+        using AlarmAckWriter writer = new AlarmAckWriter(server, cache, () => { }, TimeSpan.FromSeconds(1), "MxGateway");
        writer.Acknowledge("T.A.AckMsg", "one", Alice());
        server.WriteException = new COMException("x", unchecked((int)0x80004005));

@@ -141,6 +145,130 @@ public sealed class AlarmAckWriterTests
        Assert.Contains("Unregister", server.Operations);
    }

+    [Fact]
+    public void Acknowledge_WhenAuthenticateUserDisconnected_TearsDownAndRejects()
+    {
+        RecordingAckServer server = new RecordingAckServer
+        {
+            AuthenticateResult = 5,
+            AuthenticateException = new COMException("gone pw-secret", unchecked((int)0x80010108)),
+        };
+        using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
+
+        AlarmAckResult result = writer.Acknowledge("T.A.AckMsg", "one", Alice());
+
+        Assert.Equal(AlarmAckOutcome.WriteRejected, result.Outcome);
+        Assert.Equal(unchecked((int)0x80010108), result.NativeStatus);
+        Assert.Contains("AuthenticateUser", result.Diagnostic, StringComparison.Ordinal);
+        Assert.Contains("0x80010108", result.Diagnostic, StringComparison.Ordinal);
+        Assert.DoesNotContain("pw-secret", result.Diagnostic, StringComparison.Ordinal);
+        Assert.Contains("Unregister", server.Operations);
+        Assert.DoesNotContain(server.Operations, op => op.StartsWith("AddItem", StringComparison.Ordinal));
+    }
+
+    [Fact]
+    public void Acknowledge_WhenAuthenticateUserThrowsArgumentException_ReturnsAuthenticationFailed()
+    {
+        // Live MXAccess raises E_INVALIDARG for a bad user or wrong password, which the
+        // interop surfaces as ArgumentException rather than COMException.
+        RecordingAckServer server = new RecordingAckServer
+        {
+            AuthenticateResult = 5,
+            AuthenticateException = new ArgumentException("Value does not fall within the expected range. pw-secret"),
+        };
+        using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
+
+        AlarmAckResult result = writer.Acknowledge("T.A.AckMsg", "one", Alice());
+
+        Assert.Equal(AlarmAckOutcome.AuthenticationFailed, result.Outcome);
+        Assert.Equal(unchecked((int)0x80070057), result.NativeStatus);
+        Assert.DoesNotContain("pw-secret", result.Diagnostic, StringComparison.Ordinal);
+        Assert.DoesNotContain(server.Operations, op => op.StartsWith("Write", StringComparison.Ordinal));
+        Assert.DoesNotContain("Unregister", server.Operations);
+    }
+
+    [Fact]
+    public void Acknowledge_WhenWriteThrowsArgumentException_RejectsAndDropsItem()
+    {
+        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5, WriteException = new ArgumentException("bad") };
+        using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(10), "MxGateway");
+
+        AlarmAckResult result = writer.Acknowledge("T.A.AckMsg", "one", Alice());
+
+        Assert.Equal(AlarmAckOutcome.WriteRejected, result.Outcome);
+        Assert.Equal(unchecked((int)0x80070057), result.NativeStatus);
+        Assert.Contains("Write", result.Diagnostic, StringComparison.Ordinal);
+        Assert.Equal(new[] { 100 }, server.RemovedItemHandles);
+        Assert.DoesNotContain("Unregister", server.Operations);
+    }
+
+    [Fact]
+    public void Acknowledge_PastCacheCap_EvictsLeastRecentlyUsedItem()
+    {
+        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
+        using AlarmAckWriter writer = new AlarmAckWriter(server, new MxAccessWriteCompletionCache(), () => { }, TimeSpan.FromMilliseconds(1), "MxGateway", maxCachedItems: 2);
+
+        writer.Acknowledge("A.X.AckMsg", "1", Alice()); // handle 100
+        writer.Acknowledge("B.X.AckMsg", "2", Alice()); // handle 101
+        writer.Acknowledge("C.X.AckMsg", "3", Alice()); // handle 102, evicts A (100)
+
+        Assert.Equal(new[] { 100 }, server.RemovedItemHandles);
+        Assert.Equal(3, server.Operations.Count(op => op.StartsWith("AddItem", StringComparison.Ordinal)));
+    }
+
+    [Fact]
+    public void Acknowledge_ReusedItem_IsMostRecentlyUsedForEviction()
+    {
+        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
+        MxAccessWriteCompletionCache cache = new MxAccessWriteCompletionCache();
+        server.OnWrite = (handle, item) => cache.Record(handle, item, Rows(MxStatusCategory.Ok, 0));
+        using AlarmAckWriter writer = new AlarmAckWriter(server, cache, () => { }, TimeSpan.FromSeconds(1), "MxGateway", maxCachedItems: 2);
+
+        writer.Acknowledge("A.X.AckMsg", "1", Alice()); // handle 100
+        writer.Acknowledge("B.X.AckMsg", "2", Alice()); // handle 101
+        writer.Acknowledge("A.X.AckMsg", "3", Alice()); // reuse 100, now most recent
+        writer.Acknowledge("C.X.AckMsg", "4", Alice()); // handle 102, evicts B (101)
+
+        Assert.Equal(new[] { 101 }, server.RemovedItemHandles);
+        Assert.Equal(3, server.Operations.Count(op => op.StartsWith("AddItem", StringComparison.Ordinal)));
+        Assert.Contains("UnAdvise", server.Operations);
+    }
+
+    [Fact]
+    public void Acknowledge_AfterUnconfirmed_UsesFreshHandleSoStaleCompletionIsNotCredited()
+    {
+        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
+        MxAccessWriteCompletionCache cache = new MxAccessWriteCompletionCache();
+        using AlarmAckWriter writer = new AlarmAckWriter(server, cache, () => { }, TimeSpan.FromMilliseconds(50), "MxGateway");
+
+        AlarmAckResult first = writer.Acknowledge("T.A.AckMsg", "one", Alice()); // handle 100, no completion
+        // The first write's late OnWriteComplete (handle 100) lands while the
+        // second write is in flight; the second write's own completion never comes.
+        server.OnWrite = (handle, item) => cache.Record(handle, 100, Rows(MxStatusCategory.Ok, 0));
+        AlarmAckResult second = writer.Acknowledge("T.A.AckMsg", "two", Alice());
+
+        Assert.Equal(AlarmAckOutcome.Unconfirmed, first.Outcome);
+        Assert.Equal(AlarmAckOutcome.Unconfirmed, second.Outcome);
+        Assert.Contains(100, server.RemovedItemHandles);
+        Assert.Equal(2, server.Operations.Count(op => op.StartsWith("AddItem", StringComparison.Ordinal)));
+        Assert.Equal(new[] { 100, 101 }, server.WrittenItemHandles);
+    }
+
+    [Fact]
+    public void Acknowledge_AfterConfirmed_ReusesHandle()
+    {
+        RecordingAckServer server = new RecordingAckServer { AuthenticateResult = 5 };
+        MxAccessWriteCompletionCache cache = new MxAccessWriteCompletionCache();
+        server.OnWrite = (handle, item) => cache.Record(handle, item, Rows(MxStatusCategory.Ok, 0));
+        using AlarmAckWriter writer = new AlarmAckWriter(server, cache, () => { }, TimeSpan.FromSeconds(1), "MxGateway");
+
+        writer.Acknowledge("T.A.AckMsg", "one", Alice());
+        writer.Acknowledge("T.A.AckMsg", "two", Alice());
+
+        Assert.Equal(new[] { 100, 100 }, server.WrittenItemHandles);
+        Assert.Empty(server.RemovedItemHandles);
+    }
+
    [Fact]
    public void Acknowledge_WhenNonOkCompletionHasNoDetail_ReturnsMinusOne()
    {
@@ -180,6 +308,10 @@ internal sealed class RecordingAckServer : IMxAccessServer

    public Action<int, int>? OnWrite { get; set; }

+    public List<int> RemovedItemHandles { get; } = new();
+
+    public List<int> WrittenItemHandles { get; } = new();
+
    public int Register(string clientName)
    {
        Operations.Add("Register:" + clientName);
@@ -196,7 +328,11 @@ internal sealed class RecordingAckServer : IMxAccessServer

    public int AddItem2(int serverHandle, string itemDefinition, string itemContext) => AddItem(serverHandle, itemDefinition);

-    public void RemoveItem(int serverHandle, int itemHandle) => Operations.Add("RemoveItem");
+    public void RemoveItem(int serverHandle, int itemHandle)
+    {
+        Operations.Add("RemoveItem");
+        RemovedItemHandles.Add(itemHandle);
+    }

    public void Advise(int serverHandle, int itemHandle)
    {
@@ -216,6 +352,7 @@ internal sealed class RecordingAckServer : IMxAccessServer
    public void Write(int serverHandle, int itemHandle, object? value, int userId)
    {
        Operations.Add("Write:" + value + ":" + userId);
+        WrittenItemHandles.Add(itemHandle);
        if (WriteException is not null)
        {
            throw WriteException;