fix(worker): review follow-up — ghost cache entries, ReadBulk dedupe/pumping, fault hygiene

- AddItem/AddItem2/AddBufferedItem drop stale cache entries for a reused handle id
- ReadBulk: dedupe handles within a batch (advise/teardown once, fan out result);
  pump every 16 items in add/advise and teardown phases
- alarm poll stops without recording a fault once StaRuntime.LoopFault is set
- StaRuntime skips the loop-fault handler during a requested shutdown
- StaHung loop fault diagnostic carries type + HRESULT only
- gateway.md documents ReadBulk's shared deadline

src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessSessionTests.cs

@@ -305,6 +305,129 @@ public sealed class MxAccessSessionTests
         }
     }
 
+    /// <summary>
+    ///     Ghost entry: an OnDataChange/OnWriteComplete already queued when
+    ///     RemoveItem ran is dispatched after the eviction and recreates the
+    ///     entries. When MXAccess later reuses that item id, AddItem must drop
+    ///     them so ReadBulk's cached path cannot serve the stale value.
+    /// </summary>
+    [Fact]
+    public void AddItem_ReusingAnEvictedHandleId_DropsGhostCacheEntries()
+    {
+        RecordingMxAccessServer server = new();
+        CompletionCacheSink sink = new();
+        MxAccessValueCache valueCache = new();
+        using MxAccessSession session = MxAccessSession.CreateForTesting(server, sink, valueCache: valueCache);
+        int serverHandle = session.Register("client");
+
+        int first = session.AddItem(serverHandle, "Tag.Old");
+        session.Advise(serverHandle, first);
+        session.RemoveItem(serverHandle, first);
+
+        // Late dispatch of callbacks queued before the RemoveItem.
+        valueCache.Set(serverHandle, first, BuildDataChange(serverHandle, first, 999));
+        RecordCompletion(sink.WriteCompletionCache, serverHandle, first);
+
+        server.NextItemHandleOverride = first;
+        int reused = session.AddItem(serverHandle, "Tag.New");
+        Assert.Equal(first, reused);
+
+        Assert.False(valueCache.TryGet(serverHandle, reused, out _));
+        Assert.Equal(0UL, sink.WriteCompletionCache.CurrentVersion(serverHandle, reused));
+
+        // Once advised, ReadBulk on the new tag does not report the ghost as cached.
+        session.Advise(serverHandle, reused);
+        BulkReadResult result = Assert.Single(session.ReadBulk(serverHandle, new[] { "Tag.New" }, TimeSpan.FromMilliseconds(30), () => { }));
+        Assert.False(result.WasCached);
+        Assert.False(result.WasSuccessful);
+    }
+
+    /// <summary>
+    ///     A handle MXAccess returns while the registry still tracks it is a
+    ///     live item, so its cached value must survive the AddItem.
+    /// </summary>
+    [Fact]
+    public void AddItem_ReturningAStillRegisteredHandle_KeepsItsCachedValue()
+    {
+        RecordingMxAccessServer server = new() { ReuseHandleForSameTag = true };
+        MxAccessValueCache valueCache = new();
+        using MxAccessSession session = MxAccessSession.CreateForTesting(server, new NoopEventSink(), valueCache: valueCache);
+        int serverHandle = session.Register("client");
+        int handle = session.AddItem(serverHandle, "Tag.Live");
+        valueCache.Set(serverHandle, handle, BuildDataChange(serverHandle, handle, 5));
+
+        Assert.Equal(handle, session.AddItem(serverHandle, "Tag.Live"));
+
+        Assert.True(valueCache.TryGet(serverHandle, handle, out _));
+    }
+
+    /// <summary>
+    ///     Duplicate addresses that MXAccess resolves to one handle: the handle
+    ///     is advised, unadvised, and removed exactly once, and every duplicate
+    ///     receives the shared outcome.
+    /// </summary>
+    [Fact]
+    public void ReadBulk_DuplicateTagsResolvingToOneHandle_AdviseAndTearDownOnceAndFanOutResult()
+    {
+        RecordingMxAccessServer server = new() { ReuseHandleForSameTag = true };
+        MxAccessValueCache valueCache = new();
+        using MxAccessSession session = MxAccessSession.CreateForTesting(server, new NoopEventSink(), valueCache: valueCache);
+        int serverHandle = session.Register("client");
+
+        Action pumpStep = () =>
+        {
+            if (server.TryGetAdvisedHandle("Tag.Dup", out int handle) && !valueCache.TryGet(serverHandle, handle, out _))
+            {
+                valueCache.Set(serverHandle, handle, BuildDataChange(serverHandle, handle, 77));
+            }
+        };
+
+        IReadOnlyList<BulkReadResult> results = session.ReadBulk(
+            serverHandle,
+            new[] { "Tag.Dup", "Tag.Other", "Tag.Dup" },
+            TimeSpan.FromMilliseconds(100),
+            pumpStep);
+
+        int dupHandle = server.HandleForTag("Tag.Dup");
+        string advise = "Advise:" + serverHandle + ":" + dupHandle;
+        string unAdvise = "UnAdvise:" + serverHandle + ":" + dupHandle;
+        string remove = "RemoveItem:" + serverHandle + ":" + dupHandle;
+        Assert.True(results[0].WasSuccessful);
+        Assert.True(results[2].WasSuccessful);
+        Assert.Equal(dupHandle, results[0].ItemHandle);
+        Assert.Equal(dupHandle, results[2].ItemHandle);
+        Assert.Equal(77, results[2].Value.Int32Value);
+        Assert.Equal("Tag.Dup", results[2].TagAddress);
+        Assert.False(results[1].WasSuccessful);
+        Assert.Equal("Tag.Other", results[1].TagAddress);
+
+        Assert.Equal(1, server.Operations.Count(op => op == advise));
+        Assert.Equal(1, server.Operations.Count(op => op == unAdvise));
+        Assert.Equal(1, server.Operations.Count(op => op == remove));
+        Assert.Empty(session.HandleRegistry.ItemHandles);
+    }
+
+    /// <summary>
+    ///     Large batches pump during the add/advise and teardown phases too,
+    ///     not only inside the wait, so the STA heartbeat stays fresh.
+    /// </summary>
+    [Fact]
+    public void ReadBulk_LargeBatch_PumpsDuringAddAdviseAndTeardownPhases()
+    {
+        RecordingMxAccessServer server = new();
+        using MxAccessSession session = MxAccessSession.CreateForTesting(server, new NoopEventSink());
+        int serverHandle = session.Register("client");
+        string[] tags = Enumerable.Range(0, 3 * MxAccessSession.ReadBulkPumpInterval).Select(i => "Tag." + i).ToArray();
+        List<int> pumpAtOperation = new();
+
+        session.ReadBulk(serverHandle, tags, TimeSpan.FromMilliseconds(20), () => pumpAtOperation.Add(server.Operations.Count));
+
+        int lastAdvise = server.Operations.FindLastIndex(op => op.StartsWith("Advise:", StringComparison.Ordinal));
+        int firstUnAdvise = server.Operations.FindIndex(op => op.StartsWith("UnAdvise:", StringComparison.Ordinal));
+        Assert.Contains(pumpAtOperation, at => at < lastAdvise);
+        Assert.Contains(pumpAtOperation, at => at > firstUnAdvise);
+    }
+
     private static void RecordCompletion(MxAccessWriteCompletionCache cache, int serverHandle, int itemHandle)
     {
         RepeatedField<MxStatusProxy> statuses = new() { new MxStatusProxy { Category = MxStatusCategory.Ok } };
@@ -368,6 +491,12 @@ public sealed class MxAccessSessionTests
 
         public HashSet<string> FailAdviseFor { get; } = new(StringComparer.Ordinal);
 
+        /// <summary>When set, the next AddItem returns this handle (simulates MXAccess reusing an id).</summary>
+        public int? NextItemHandleOverride { get; set; }
+
+        /// <summary>When true, AddItem of an already-added address returns the existing handle.</summary>
+        public bool ReuseHandleForSameTag { get; set; }
+
         public int HandleForTag(string tag) => handleByTag[tag];
 
         public bool TryGetAdvisedHandle(string tag, out int handle)
@@ -391,7 +520,21 @@ public sealed class MxAccessSessionTests
                 throw new InvalidOperationException($"AddItem failed for {itemDefinition}");
             }
 
-            int handle = nextItemHandle++;
+            int handle;
+            if (NextItemHandleOverride is int forced)
+            {
+                NextItemHandleOverride = null;
+                handle = forced;
+            }
+            else if (ReuseHandleForSameTag && handleByTag.TryGetValue(itemDefinition, out int existing))
+            {
+                handle = existing;
+            }
+            else
+            {
+                handle = nextItemHandle++;
+            }
+
             handleByTag[itemDefinition] = handle;
             tagByHandle[handle] = itemDefinition;
             return handle;

src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessStaSessionTests.cs

@@ -687,7 +687,7 @@ public sealed class MxAccessStaSessionTests
         await session.StartAsync("session-1", workerProcessId: 1);
         Assert.False(eventQueue.IsFaulted);
 
-        pump.ThrowOnNextPump(new InvalidOperationException("The worker STA message pump failed."));
+        pump.ThrowOnNextPump(new InvalidOperationException("secret-tag-name pump failed."));
 
         DateTime deadline = DateTime.UtcNow.AddSeconds(5);
         while (!eventQueue.IsFaulted && DateTime.UtcNow < deadline)
@@ -701,9 +701,54 @@ public sealed class MxAccessStaSessionTests
         Assert.Equal(WorkerFaultCategory.StaHung, fault!.Category);
         Assert.Equal(typeof(InvalidOperationException).FullName, fault.ExceptionType);
         Assert.Contains("STA loop terminated", fault.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.Contains("exceptionType=System.InvalidOperationException", fault.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.Contains("hresult=0x", fault.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.DoesNotContain("secret-tag-name", fault.DiagnosticMessage, StringComparison.Ordinal);
+        Assert.DoesNotContain("secret-tag-name", fault.ProtocolStatus.Message, StringComparison.Ordinal);
         Assert.Equal(ProtocolStatusCode.WorkerUnavailable, fault.ProtocolStatus.Code);
     }
 
+    /// <summary>
+    /// After the STA loop dies, the alarm poll's fast-failed InvokeAsync must
+    /// not record a second, mislabelled alarm-poll fault: once the StaHung
+    /// fault is drained, nothing else is recorded.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task RunAlarmPollLoop_AfterStaLoopFault_DoesNotRecordAlarmPollFault()
+    {
+        ControllableMessagePump pump = new();
+        FakeAlarmCommandHandler handler = new();
+        using StaRuntime runtime = new(new NoopComApartmentInitializer(), pump, TimeSpan.FromMilliseconds(25));
+        MxAccessEventQueue eventQueue = new();
+        using MxAccessStaSession session = new(
+            runtime,
+            new FakeMxAccessComObjectFactory(),
+            new FakeMxAccessEventSink(),
+            eventQueue,
+            (_eq, _affinity, _comFactory) => handler);
+        session.AlarmPollInterval = TimeSpan.FromMilliseconds(10);
+        session.AlarmPollFailureThreshold = 1;
+        session.AlarmPollWarningSink = _ => { };
+
+        await session.StartAsync("session-1", workerProcessId: 1);
+        pump.ThrowOnNextPump(new InvalidOperationException("pump failed"));
+
+        DateTime deadline = DateTime.UtcNow.AddSeconds(5);
+        while (!eventQueue.IsFaulted && DateTime.UtcNow < deadline)
+        {
+            await Task.Delay(10);
+        }
+
+        WorkerFault? first = session.DrainFault();
+        Assert.NotNull(first);
+        Assert.Equal(WorkerFaultCategory.StaHung, first!.Category);
+
+        // Give the poll loop several ticks to (wrongly) record its own fault.
+        await Task.Delay(300);
+        Assert.Null(session.DrainFault());
+    }
+
     /// <summary>
     /// The STA-affinity guard throws when an
     /// IMxAccessAlarmConsumer call is attempted off the thread that created

src/ZB.MOM.WW.MxGateway.Worker.Tests/Sta/StaRuntimeTests.cs

@@ -274,6 +274,27 @@ public sealed class StaRuntimeTests
         Assert.False(invoked);
     }
 
+    /// <summary>
+    ///     A loop that throws while a requested shutdown is in progress is
+    ///     being torn down on purpose: the loop-fault handler must not run.
+    /// </summary>
+    [Fact]
+    public void LoopException_DuringRequestedShutdown_DoesNotInvokeHandler()
+    {
+        ControllableMessagePump pump = new();
+        using StaRuntime runtime = new(new RecordingComApartmentInitializer(), pump, TimeSpan.FromSeconds(30));
+        runtime.Start();
+        bool invoked = false;
+        runtime.RegisterLoopFaultHandler(_ => invoked = true);
+
+        // The loop is parked in the 30 s idle wait; Shutdown wakes it and the
+        // very next pump throws with shutdownRequested already set.
+        pump.ThrowOnNextPump(new InvalidOperationException("pump failed during shutdown"));
+        Assert.True(runtime.Shutdown(TimeSpan.FromSeconds(5)));
+
+        Assert.False(invoked);
+    }
+
     private static StaRuntime CreateRuntime(RecordingComApartmentInitializer initializer)
     {
         return new StaRuntime(

src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs

@@ -242,6 +242,7 @@ public sealed class MxAccessSession : IDisposable
         ThrowIfDisposed();
 
         int itemHandle = mxAccessServer.AddItem(serverHandle, itemDefinition);
+        EvictStaleHandleState(serverHandle, itemHandle);
         handleRegistry.RegisterItemHandle(
             serverHandle,
             itemHandle,
@@ -265,6 +266,7 @@ public sealed class MxAccessSession : IDisposable
         ThrowIfDisposed();
 
         int itemHandle = mxAccessServer.AddItem2(serverHandle, itemDefinition, itemContext);
+        EvictStaleHandleState(serverHandle, itemHandle);
         handleRegistry.RegisterItemHandle(
             serverHandle,
             itemHandle,
@@ -275,6 +277,30 @@ public sealed class MxAccessSession : IDisposable
         return itemHandle;
     }
 
+    /// <summary>
+    ///     Drops cached per-handle state left over from a previous lifetime of
+    ///     a handle id MXAccess just handed out again. RemoveItem and Unregister
+    ///     already evict, but an OnDataChange or OnWriteComplete that was
+    ///     queued on the STA when they ran is dispatched afterwards (the loop
+    ///     pumps between work items) and recreates the entry. Without this, a
+    ///     reused id would let ReadBulk's cached fast path serve that stale
+    ///     value as <c>was_cached = true</c>. A handle the registry still
+    ///     tracks is a live item (MXAccess returned an existing handle), so its
+    ///     current entries are kept.
+    /// </summary>
+    private void EvictStaleHandleState(
+        int serverHandle,
+        int itemHandle)
+    {
+        if (handleRegistry.ContainsItemHandle(serverHandle, itemHandle))
+        {
+            return;
+        }
+
+        valueCache.Remove(serverHandle, itemHandle);
+        writeCompletionCache.Remove(serverHandle, itemHandle);
+    }
+
     /// <summary>Removes an item from an MXAccess server.</summary>
     /// <param name="serverHandle">Handle returned by the worker.</param>
     /// <param name="itemHandle">Handle returned by the worker.</param>
@@ -409,6 +435,7 @@ public sealed class MxAccessSession : IDisposable
         ThrowIfDisposed();
 
         int itemHandle = mxAccessServer.AddBufferedItem(serverHandle, itemDefinition, itemContext);
+        EvictStaleHandleState(serverHandle, itemHandle);
         handleRegistry.RegisterItemHandle(
             serverHandle,
             itemHandle,
@@ -957,7 +984,7 @@ public sealed class MxAccessSession : IDisposable
             }
             finally
             {
-                TearDownSnapshots(serverHandle, snapshots);
+                TearDownSnapshots(serverHandle, snapshots, pumpStep);
             }
         }
 
@@ -970,6 +997,17 @@ public sealed class MxAccessSession : IDisposable
         return ordered;
     }
 
+    /// <summary>
+    ///     Number of snapshot items ReadBulk adds/advises (phase 2) or tears
+    ///     down (phase 4) between pump steps. Those phases make one or two COM
+    ///     calls per tag without waiting, so a large batch would otherwise hold
+    ///     the STA with no pump and no activity mark for its whole length; a
+    ///     pump every <see cref="ReadBulkPumpInterval"/> items keeps inbound
+    ///     callbacks flowing and keeps the STA heartbeat fresh, so a big batch
+    ///     cannot drift toward <c>HeartbeatStuckCeiling</c>.
+    /// </summary>
+    internal const int ReadBulkPumpInterval = 16;
+
     private void ReadSnapshots(
         int serverHandle,
         List<SnapshotRead> snapshots,
@@ -978,16 +1016,36 @@ public sealed class MxAccessSession : IDisposable
         Action pumpStep)
     {
         // Phase 2: reserve our own item handle and advise for every uncached
-        // tag. A per-tag AddItem/Advise failure fails only that tag.
+        // tag. A per-tag AddItem/Advise failure fails only that tag. When
+        // MXAccess hands back a handle an earlier tag in this batch already
+        // advised (a duplicate address), that tag shares the earlier tag's
+        // subscription instead of advising the handle a second time, and gets
+        // the same outcome fanned out to it after the wait.
         List<SnapshotRead> waiting = new(snapshots.Count);
-        foreach (SnapshotRead snapshot in snapshots)
+        Dictionary<int, SnapshotRead> advisedByHandle = new();
+        List<SnapshotRead> sharing = new();
+        for (int position = 0; position < snapshots.Count; position++)
         {
+            SnapshotRead snapshot = snapshots[position];
             try
             {
+                if (position > 0 && position % ReadBulkPumpInterval == 0)
+                {
+                    pumpStep();
+                }
+
                 snapshot.ItemHandle = AddItem(serverHandle, snapshot.TagAddress);
+                if (advisedByHandle.TryGetValue(snapshot.ItemHandle, out SnapshotRead? owner))
+                {
+                    snapshot.SharedWith = owner;
+                    sharing.Add(snapshot);
+                    continue;
+                }
+
                 snapshot.Baseline = valueCache.CurrentVersion(serverHandle, snapshot.ItemHandle);
                 Advise(serverHandle, snapshot.ItemHandle);
                 snapshot.Advised = true;
+                advisedByHandle[snapshot.ItemHandle] = snapshot;
                 waiting.Add(snapshot);
             }
             catch (Exception exception)
@@ -996,11 +1054,32 @@ public sealed class MxAccessSession : IDisposable
             }
         }
 
-        if (waiting.Count == 0)
+        if (waiting.Count > 0)
         {
-            return;
+            WaitForSnapshots(serverHandle, waiting, results, timeout, pumpStep);
         }
 
+        foreach (SnapshotRead duplicate in sharing)
+        {
+            BulkReadResult? ownerResult = results[duplicate.SharedWith!.Index];
+            if (ownerResult is null)
+            {
+                continue;
+            }
+
+            BulkReadResult fanned = ownerResult.Clone();
+            fanned.TagAddress = duplicate.TagAddress;
+            results[duplicate.Index] = fanned;
+        }
+    }
+
+    private void WaitForSnapshots(
+        int serverHandle,
+        List<SnapshotRead> waiting,
+        BulkReadResult?[] results,
+        TimeSpan timeout,
+        Action pumpStep)
+    {
         // Phase 3: one shared deadline. Pump until every advised handle has
         // seen its first OnDataChange since its baseline, or the deadline
         // elapses.
@@ -1054,17 +1133,20 @@ public sealed class MxAccessSession : IDisposable
 
     private void TearDownSnapshots(
         int serverHandle,
-        List<SnapshotRead> snapshots)
+        List<SnapshotRead> snapshots,
+        Action pumpStep)
     {
-        // Phase 4: snapshot teardown — best-effort, never re-throws. A handle
-        // is torn down once even if MXAccess handed the same id to two tags in
-        // the batch.
+        // Phase 4: snapshot teardown — best-effort, never re-throws. Each
+        // distinct handle is unadvised and removed exactly once, even when
+        // MXAccess handed the same id to several tags in the batch.
         HashSet<int> unadvised = new();
         HashSet<int> removed = new();
+        int calls = 0;
         foreach (SnapshotRead snapshot in snapshots)
         {
             if (snapshot.Advised && unadvised.Add(snapshot.ItemHandle))
             {
+                PumpEvery(ref calls, pumpStep);
                 try { UnAdvise(serverHandle, snapshot.ItemHandle); } catch { /* swallow — best effort */ }
             }
         }
@@ -1073,11 +1155,22 @@ public sealed class MxAccessSession : IDisposable
         {
             if (snapshot.ItemHandle != 0 && removed.Add(snapshot.ItemHandle))
             {
+                PumpEvery(ref calls, pumpStep);
                 try { RemoveItem(serverHandle, snapshot.ItemHandle); } catch { /* swallow — best effort */ }
             }
         }
     }
 
+    private static void PumpEvery(ref int calls, Action pumpStep)
+    {
+        if (calls > 0 && calls % ReadBulkPumpInterval == 0)
+        {
+            try { pumpStep(); } catch { /* swallow — teardown is best effort */ }
+        }
+
+        calls++;
+    }
+
     private bool TryGetCachedReadFor(
         int serverHandle,
         string tagAddress,
@@ -1429,6 +1522,12 @@ public sealed class MxAccessSession : IDisposable
 
         /// <summary>Gets or sets a value indicating whether Advise succeeded.</summary>
         public bool Advised { get; set; }
+
+        /// <summary>
+        ///     Gets or sets the earlier snapshot in the same batch whose
+        ///     subscription this one shares (same item handle), or null.
+        /// </summary>
+        public SnapshotRead? SharedWith { get; set; }
     }
 
     private void ThrowIfDisposed()

src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessStaSession.cs

@@ -465,6 +465,15 @@ public sealed class MxAccessStaSession : IWorkerRuntimeSession
                     // observable on the IPC fault path instead of silently
                     // faulting this never-awaited task. The loop then stops —
                     // the subscription is dead.
+                    if (staRuntime.LoopFault is not null)
+                    {
+                        // The STA loop itself died; its handler already recorded
+                        // the StaHung fault. This failure is just the fast-fail
+                        // InvokeAsync returns afterwards — recording it would add
+                        // a second, mislabelled alarm-poll fault.
+                        return;
+                    }
+
                     consecutiveFailures++;
                     if (consecutiveFailures >= failureThreshold)
                     {
@@ -545,8 +554,13 @@ public sealed class MxAccessStaSession : IWorkerRuntimeSession
     /// </summary>
     internal static WorkerFault CreateStaLoopFault(Exception exception)
     {
-        string message =
-            $"Worker STA loop terminated unexpectedly: {exception.GetType().FullName}: {exception.Message}";
+        // Type and HRESULT only, never exception.Message — the same type-only
+        // rule every other worker failure line follows.
+        string message = string.Format(
+            System.Globalization.CultureInfo.InvariantCulture,
+            "Worker STA loop terminated unexpectedly: exceptionType={0} hresult=0x{1:X8}",
+            exception.GetType().FullName,
+            exception.HResult);
         WorkerFault fault = new()
         {
             Category = WorkerFaultCategory.StaHung,

src/ZB.MOM.WW.MxGateway.Worker/Sta/StaRuntime.cs

@@ -140,7 +140,7 @@ public sealed class StaRuntime : IDisposable
         lock (gate)
         {
             loopFaultHandler = handler;
-            alreadyFaulted = loopFault;
+            alreadyFaulted = shutdownRequested ? null : loopFault;
         }
 
         if (alreadyFaulted is not null)
@@ -362,7 +362,11 @@ public sealed class StaRuntime : IDisposable
                 lock (gate)
                 {
                     loopFault = exception;
-                    handler = loopFaultHandler;
+
+                    // A loop that throws while a requested shutdown is already
+                    // in progress is being torn down on purpose; reporting it
+                    // as a worker fault would turn a clean stop into a fault.
+                    handler = shutdownRequested ? null : loopFaultHandler;
                 }
 
                 if (handler is not null)