docs(worker): reviewer follow-up comments and tests from the remediation reviews

The worker-side half of the review tail. Tests and comments only — nothing here
changes worker behavior, and none of it compiles on the macOS tree (net48/x86),
so it was reviewed line by line against the already-windev-validated files.

- MxAccessHandleRegistryTests gains the multi-candidate case behind
  MxAccessSession.TryGetCachedReadFor's fall-through: one tag under two item
  handles, the lower registered-but-unadvised and the higher advised. Asserted
  at the registry rather than the session because the session's read path needs
  a live MXAccess COM instance; what the registry owes the scan is the stable
  ascending candidate order and a per-item-handle (not per-tag) advice index,
  and both are pinned here along with the fall-through contract in prose.
- A single adversarial lifecycle test — register, advise, re-register the same
  item handle under a new tag, unadvise, unregister the server — asserting every
  index agrees after each step. The individual transitions were already covered;
  what was not was that they compose, and a stale entry in any one index
  resurrects a handle MXAccess has already retired.
- StaWaitHelperTests.WaitForSignalOrMessages_PreSignalledHandle_ReturnsImmediately
  drains pending messages first, like the other two wait tests. Without it a
  stale message can end the wait instead of the handle, failing the
  signal-consumed post-condition for an unrelated reason.
- GatewayTesting.md records the two findings from the Task 24 windev gate:
  SecretsStorePathGuardTests.CreateBuilder_AcceptsSecretsStoreOutsideContentRoot_AndCreatesIt
  fails deterministically on Windows on main too (SQLite pooling holds secrets.db
  open across the cleanup's recursive delete; pre-existing, tracked separately),
  and the StaWaitHelper timing tests' flake signature on a loaded box is a
  message wake — the helper working as designed — not a broken wait.

docs/Diagnostics.md

@@ -101,8 +101,12 @@ correlate a log entry to a specific principal.
 A scheme word survives only when it is one of the recognized authorization schemes (`Bearer`,
 `Basic`, `Digest`, `Negotiate`, `NTLM`, `ApiKey`, `Token`). An unrecognized leading word is as likely
 to be credential material as it is to be a scheme, so it is dropped along with the rest. The key id is
-also dropped when it runs longer than 64 characters, which no issued key id does — a long run before
-the first `_` is secret material, not an identifier.
+also dropped when it runs longer than 64 characters — a long run before the first `_` is more likely to
+be secret material than an identifier. Neither key-creation path (`ApiKeyAdminCommandLineParser.IsValidKeyId`,
+`DashboardApiKeyManagementService.ValidateKeyId`) enforces a length, so this is a redaction heuristic
+rather than a guarantee: operators should keep key ids under 64 characters, or the id stops appearing
+in logs and only the `mxgw_[redacted]` shape survives. The direction of the failure is deliberate —
+losing an identifier is cheap, logging a secret is not.
 
 The parse is span-based (no regex, no `Split` allocation): the value is split once at the first space,
 and the key id is read up to the first `_` of the remainder.

docs/GatewayTesting.md

@@ -554,6 +554,23 @@ windev has 36 logical CPUs and `xunit.runner.json` sets `maxParallelThreads: -1`
 suite runs far wider there than on the macOS dev box — that width is what turns these
 real-clock deadlines into failures.
 
+### Two more findings from the 2026-08-15 windev gate
+
+- `SecretsStorePathGuardTests.CreateBuilder_AcceptsSecretsStoreOutsideContentRoot_AndCreatesIt`
+  fails **deterministically on Windows, on `main` as well as on any branch**, so it is not a
+  signal about the change under test. Creating the builder opens `secrets.db`, and
+  `Microsoft.Data.Sqlite`'s connection pool keeps the file handle alive past the test body,
+  so the recursive directory delete in the cleanup hits a still-open file — a sharing
+  violation Windows enforces and Unix does not. Pre-existing and tracked separately; do not
+  chase it as a regression. Subtract it from the expected pass count on Windows.
+- The `StaWaitHelper` timing tests (`WaitForSignalOrMessages_*`) flake on a loaded box with a
+  signature that reads like a broken wait but is not: the helper wakes on *input being
+  present*, so a message posted to the test thread ends the wait early. That is the helper
+  doing exactly what the STA pump needs. The tests drain the queue with
+  `PumpPendingMessages()` first for that reason; a failure here means the box was busy enough
+  to queue a message mid-test, not that the wait stopped honouring its handle or its timeout.
+  Re-run the class on its own before treating it as real, per the load caveat above.
+
 ### The full-suite testhost hang was a zero-buffer named pipe (fixed)
 
 For months a full-suite run on windev reported `855 passed, 0 failed` and then never

docs/Sessions.md

@@ -215,7 +215,9 @@ Splitting the phases moves the TOCTOU re-check earlier, and that is an accepted
 
 A close that throws no longer abandons the rest of the selected set: the sweep attempts every selected session, captures the first failure, and rethrows it once the pass is done so `SessionLeaseMonitorHostedService` still logs the sweep failure as before. Because the closes run concurrently, *which* failure surfaces when several fail in one pass is nondeterministic; the log line is the diagnostic, not the identity of the exception.
 
-`ShutdownAsync` drains sessions with the same bounded fan-out and the same per-session catch → `KillWorkerAsync` fallback, with two rules that keep a stop deadline from turning into leaked workers. First, its body is **exception-total** — nothing escapes it — because `Parallel.ForEachAsync` cancels the token handed to the sibling bodies as soon as one body throws, which would abort in-flight graceful shutdowns *and* make their kill fallback fail instantly on the freshly cancelled token. Second, the drain loop is deliberately **not** bound to the caller's `CancellationToken` and the kill fallback runs on `CancellationToken.None`: a cancelled `ParallelOptions` token stops dispatching the remaining sessions entirely, whereas the sequential drain this replaced let every remaining session fail its graceful close fast and still kill its worker. The token is passed to the graceful close instead, so a host stop deadline turns the drain into a kill sweep rather than into a leak. This matters because nothing reattaches to a leaked worker — a restarted gateway terminates orphans (see [Design Decisions](DesignDecisions.md)).
+`ShutdownAsync` drains sessions with the same bounded fan-out and the same per-session catch → `KillWorkerAsync` fallback, with two rules that keep a stop deadline from turning into leaked workers. First, its body is **exception-total** — nothing escapes it — because `Parallel.ForEachAsync` cancels the token handed to the sibling bodies as soon as one body throws, which would abort in-flight graceful shutdowns *and* make their kill fallback fail instantly on the freshly cancelled token. Second, the drain loop is deliberately **not** bound to the caller's `CancellationToken` and the kill fallback runs on `CancellationToken.None`: a cancelled `ParallelOptions` token stops dispatching the remaining sessions entirely, so the untried tail would be neither closed nor killed. This **fixes a leak the sequential drain also had**, rather than restoring the sequential drain's behavior — there the kill fallback ran on the caller's already-cancelled token, and `KillWorkerAsync`'s entry `ThrowIfCancellationRequested` threw out of the loop on the very first session, producing zero kills. The token is passed to the graceful close only, so a host stop deadline turns the drain into a kill sweep rather than into a leak. This matters because nothing reattaches to a leaked worker — a restarted gateway terminates orphans (see [Design Decisions](DesignDecisions.md)).
+
+The asymmetry with `CloseExpiredLeasesAsync` — whose `ParallelOptions` *is* token-bound — is intentional: the sweep is periodic maintenance, so a pass abandoned on cancellation loses nothing permanently (the next pass re-selects, and `ShutdownAsync` backstops it), whereas the shutdown drain is terminal and must not be abandoned partway.
 
 **Stranded-`Closing` bound.** A sweep pass that is cancelled after selection leaves its unclosed selections in `Closing` with close already started. `IsFaultedReapableCore` requires `state == Faulted`, so a session selected under `FaultedReason` and stranded this way is not re-selected as faulted; it is swept only when its normal lease expires (up to `MxGateway:Sessions:DefaultLeaseSeconds`, default 1800 s), since `IsLeaseExpiredCore` and `IsDetachGraceExpiredCore` are state-agnostic. This bound is documented rather than closed with a re-selection clause: the sweep's only caller cancels on the host's `stoppingToken`, so the very next thing that runs is `ShutdownAsync`, which drains (or kills) the whole registry — and any worker that still survives that is terminated as an orphan on the next gateway start. Adding a "`Closing` and close-started" re-selection clause would also have to distinguish an abandoned close from one that is merely still in flight, which would weaken the single invariant that makes the parallel close phase safe.
 

docs/plans/2026-08-15-perf-review-remediation.md

@@ -617,6 +617,7 @@ Commit anything found: `docs: remediation plan doc sweep`
 | Control-frame completion coupled to event batch drain | Documented, bounded (≤128 frames) behavior of the two-class writer design; revisit only if heartbeat latency shows up in metrics. |
 | Blazor pages' loopback SignalR hop | Works correctly; in-process `WatchSnapshotsAsync` consumption is a dashboard refactor with payoff only at viewer counts the product doesn't target. |
 | Event-path triple async-iterator flattening | LOW-rated; touches the most invariant-dense code in the gateway for two `MoveNextAsync` hops per event. Reconsider after Tasks 3/4 land and if profiling still shows it. |
+| `SessionEventDistributor._subscribers` `ConcurrentDictionary` → plain `Dictionary` (Task 25 / Task 3 review) | Every mutation is already inside `_lifecycleLock`, so the concurrent type buys nothing. Behavior-neutral refactor with no measurable win, proposed after the windev gate was already green — not worth re-running the verification matrix for. Comment cleanups from the same review landed; this swap did not. |
 
 ## Execution notes for the orchestrator
 

docs/plans/2026-08-15-perf-review-remediation.md.tasks.json

@@ -163,7 +163,7 @@
     {
       "id": 24,
       "subject": "Task 24: Phase B verification on windev (psbridge)",
-      "status": "in_progress",
+      "status": "completed",
       "blockedBy": [
         16,
         17,
@@ -178,11 +178,11 @@
     {
       "id": 25,
       "subject": "Task 25: Wrap-up docs sweep + follow-ups",
-      "status": "pending",
+      "status": "in_progress",
       "blockedBy": [
         24
       ]
     }
   ],
-  "lastUpdated": "2026-08-15T18:05:00Z"
+  "lastUpdated": "2026-08-15T18:40:00Z"
 }

src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs

@@ -91,6 +91,11 @@ public sealed class EventsHubViewerRegistry
             return;
         }
 
+        // Detaching the set is safe against a SubscribeSession that arrives after the disconnect
+        // only because SignalR dispatches a connection's hub invocations sequentially by default
+        // (MaximumParallelInvocationsPerClient = 1): OnDisconnectedAsync cannot overlap an
+        // AddViewer for the same connection, so no late add can re-create the entry and leak a
+        // count that nothing will ever release. Raising that option would break this.
         if (!_sessionsByConnection.TryRemove(connectionId, out ConcurrentDictionary<string, byte>? sessions))
         {
             return;

src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs

@@ -101,6 +101,12 @@ public sealed class MxAccessGatewayService(
         try
         {
             requestValidator.ValidateInvoke(request);
+
+            // PERF(followup): this resolve and the sessionManager.InvokeAsync below look the same
+            // session up twice (a dictionary hit each, so measured cost is negligible). Collapsing
+            // them needs a SessionManager overload taking an already-resolved GatewaySession, which
+            // would duplicate InvokeAsync's fault mapping (SessionNotFound / state checks / metrics)
+            // at a second entry point — deliberately not worth it until a profile says otherwise.
             GatewaySession session = ResolveSession(request.SessionId);
             MxCommand command = request.Command;
             BulkConstraintPlan? bulkConstraintPlan = await ApplyConstraintsAsync(
@@ -697,9 +703,13 @@ public sealed class MxAccessGatewayService(
 
             default:
                 // Only the four bulk-write kinds above reach FilterWriteBulkAsync, so this is
-                // unreachable; keep the previous behaviour (the unmodified command) rather than
-                // emitting a payload-less one if that ever stops holding.
-                return command.Clone();
+                // unreachable. It throws rather than falling back to the unmodified command,
+                // because that fallback failed OPEN: a fifth bulk-write kind added upstream
+                // without a case here would silently ship the DENIED entries to the worker while
+                // still reporting them denied to the caller. Failing loud on a kind nobody can
+                // reach today is strictly safer than a constraint bypass nobody would notice.
+                throw new UnreachableException(
+                    $"Command kind {command.Kind} reached bulk-write constraint filtering without a filter case.");
         }
 
         return filtered;

src/ZB.MOM.WW.MxGateway.Server/Security/Audit/AuditDrainService.cs

@@ -246,6 +246,13 @@ public sealed class AuditDrainService(
         finally
         {
             writer.DetachDrain();
+
+            // Detaching alone leaves a racer that already passed the attached check enqueueing into
+            // a channel this loop will never read again — those events would sit in the buffer until
+            // StopAsync's final drain. Completing the writer as well makes that racer's TryWrite
+            // return false, which is the write-through branch, so the event reaches the store now.
+            // TryComplete is idempotent, so StopAsync's own CompleteWriting stays safe either way.
+            writer.CompleteWriting();
         }
     }
 

src/ZB.MOM.WW.MxGateway.Server/Security/Audit/ChannelAuditWriter.cs

@@ -87,8 +87,11 @@ public sealed class ChannelAuditWriter : IAuditWriter
     public void DetachDrain() => Volatile.Write(ref _drainAttached, 0);
 
     /// <summary>
-    /// Enqueues a canonical audit event for the drain to persist. Never blocks, never throws,
-    /// and never touches the store on the caller's thread while a drain is attached.
+    /// Enqueues a canonical audit event for the drain to persist. Never blocks and never throws.
+    /// It also never touches the store on the caller's thread while a drain is attached — with one
+    /// exception: once the channel has been completed (shutdown, or a drain loop that died), the
+    /// enqueue fails and this falls through to the synchronous write, which is what keeps the event
+    /// rather than stranding it in a buffer nobody reads.
     /// </summary>
     /// <param name="auditEvent">The canonical audit event to persist.</param>
     /// <param name="cancellationToken">Token honoured only by the direct write-through path.</param>

src/ZB.MOM.WW.MxGateway.Server/Security/Audit/SqliteCanonicalAuditStore.cs

@@ -31,7 +31,14 @@ namespace ZB.MOM.WW.MxGateway.Server.Security.Audit;
 /// retries.
 /// </para>
 /// </remarks>
-public sealed class SqliteCanonicalAuditStore(AuthSqliteConnectionFactory connectionFactory) : IAuditEventSink
+/// <param name="connectionFactory">Factory for connections to the shared auth database file.</param>
+/// <param name="logger">
+/// Optional logger for row-level read diagnostics. Optional because the store is also constructed
+/// directly by the <c>apikey</c> CLI path and by DI-free tests, which have no logger to hand.
+/// </param>
+public sealed class SqliteCanonicalAuditStore(
+    AuthSqliteConnectionFactory connectionFactory,
+    ILogger<SqliteCanonicalAuditStore>? logger = null) : IAuditEventSink
 {
     private const string CreateTableSql =
         """
@@ -208,7 +215,7 @@ public sealed class SqliteCanonicalAuditStore(AuthSqliteConnectionFactory connec
             events.Add(new AuditEvent
             {
                 EventId = Guid.Parse(reader.GetString(0)),
-                OccurredAtUtc = ParseUtc(reader.GetString(1)),
+                OccurredAtUtc = ParseUtcOrMinValue(reader.GetString(1), reader.GetString(0)),
                 Actor = reader.GetString(2),
                 Action = reader.GetString(3),
                 Outcome = Enum.Parse<AuditOutcome>(reader.GetString(4)),
@@ -239,6 +246,26 @@ public sealed class SqliteCanonicalAuditStore(AuthSqliteConnectionFactory connec
         Volatile.Write(ref _tableEnsured, 1);
     }
 
-    private static DateTimeOffset ParseUtc(string value) =>
-        DateTimeOffset.Parse(value, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind);
+    // Reading is defensive where writing is not: an insert always round-trips "O", but the table is
+    // append-only shared state that an operator (or a future migration) can put an unparseable
+    // timestamp into, and the retention sweep deliberately keeps such a row — SQLite's datetime()
+    // yields NULL for it, so the DELETE's comparison is never true. A throwing Parse here would let
+    // that single row take out the dashboard's whole recent-audit view. MinValue instead sorts the
+    // row to the far past and keeps every other column readable, which is what an operator looking
+    // at the view actually needs.
+    private DateTimeOffset ParseUtcOrMinValue(string value, string eventId)
+    {
+        if (DateTimeOffset.TryParse(value, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out DateTimeOffset parsed))
+        {
+            return parsed;
+        }
+
+        // Debug, not warning: the row is still returned and the timestamp text itself is not logged
+        // (audit rows are not secrets, but the value is attacker-influenceable in the worst case).
+        logger?.LogDebug(
+            "Audit event {EventId} has an unparseable occurred_at_utc; reporting it as DateTimeOffset.MinValue.",
+            eventId);
+
+        return DateTimeOffset.MinValue;
+    }
 }

src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/AuthStoreServiceCollectionExtensions.cs

@@ -97,8 +97,13 @@ public static class AuthStoreServiceCollectionExtensions
                 sp.GetService<TimeProvider>() ?? TimeProvider.System));
         DecorateVerifierWithCache(services, security);
 
+        // GetService, not GetRequiredService, for the same reason the writer registration below
+        // gives: the DI-only unit tests build a bare ServiceCollection with no AddLogging(). The
+        // store's logger is optional and only carries row-level read diagnostics.
         services.AddSingleton(sp =>
-            new SqliteCanonicalAuditStore(sp.GetRequiredService<AuthSqliteConnectionFactory>()));
+            new SqliteCanonicalAuditStore(
+                sp.GetRequiredService<AuthSqliteConnectionFactory>(),
+                sp.GetService<ILogger<SqliteCanonicalAuditStore>>()));
         services.AddSingleton<IAuditEventSink>(sp => sp.GetRequiredService<SqliteCanonicalAuditStore>());
         // Resolve the logger defensively: the production host always registers ILogger<T>, but the
         // DI-only auth/CLI/dashboard unit tests build a bare ServiceCollection without AddLogging().

src/ZB.MOM.WW.MxGateway.Server/Sessions/ISessionManager.cs

@@ -79,7 +79,14 @@ public interface ISessionManager
         CancellationToken cancellationToken);
 
     /// <summary>Shuts down all sessions and the session manager.</summary>
-    /// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
+    /// <param name="cancellationToken">
+    ///     Token that <em>degrades</em> the drain rather than cancelling it. It is passed only to each
+    ///     session's graceful close; the drain loop and the kill fallback are not bound to it, so
+    ///     cancelling turns the drain into a kill sweep instead of abandoning the untried sessions as
+    ///     leaked workers. The call therefore overruns a cancelled token by a bounded amount —
+    ///     roughly <c>ceil(sessionCount / 4)</c> batches of the worker shutdown timeout in the worst
+    ///     case, where 4 is <c>MaxParallelSessionCloses</c>.
+    /// </param>
     /// <returns>A task that represents the asynchronous operation.</returns>
     Task ShutdownAsync(CancellationToken cancellationToken);
 }

src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs

@@ -112,8 +112,11 @@ public sealed class SessionEventDistributor : IAsyncDisposable
     // inside the _lifecycleLock section of every register/unregister; never mutated in
     // place, so the pump can walk the array it captured with no lock and no allocation.
     // Volatile.Write / Volatile.Read ORDER the access — they keep the publishing store from
-    // sinking past the lock release and keep the pump's read from being hoisted out of the
-    // fan-out loop. They do NOT promise freshness, and nothing here needs them to: a reader
+    // sinking past the lock release, and they keep a lock-free reader's load from being hoisted
+    // or cached. The pump is NOT that reader: its single capture point sits inside the
+    // _replayLock section of AppendToReplayBufferAndCaptureSubscribers, so the lock edge already
+    // orders it. The genuinely lock-free reader is SubscriberCount, which loads the field with no
+    // lock at all. They do NOT promise freshness, and nothing here needs them to: a reader
     // may legitimately observe the previous array, which IS the documented "late subscribers
     // see events after they register" window. Where visibility must be guaranteed — the
     // RegisterWithReplay handoff — it comes from the _replayLock edge, not from Volatile.
@@ -700,8 +703,11 @@ public sealed class SessionEventDistributor : IAsyncDisposable
         // every path that completes a channel during fan-out (lease disposal via Unregister,
         // and this method) removes the subscriber from the set BEFORE completing it, so a
         // completed channel implies the subscriber is already gone and RemoveSubscriber
-        // returns false. (CompleteAllSubscribers completes without removing, but only after
-        // the pump has left its loop, so it cannot be observed here.)
+        // returns false. (CompleteAllSubscribers completes without removing, but only after the
+        // pump has left its loop, so it cannot be observed here — except on the DisposeAsync
+        // abandon path: a source factory that ignores cancellation past the 5 s shutdown timeout
+        // leaves the pump fanning while DisposeAsync completes subscribers, so a spurious overflow
+        // report is possible there. It is harmless, because the session is already being disposed.)
         //
         // Bailing out on false is what keeps a normal stream ending mid-traffic from emitting
         // a bogus EventQueueOverflow metric and — under the default single-subscriber FailFast

src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs

@@ -365,11 +365,19 @@ public sealed class SessionManager : ISessionManager
         // rather than adopting them).
         //
         // For the same reason the loop itself is NOT bound to cancellationToken: a cancelled
-        // ParallelOptions token stops dispatching the remaining sessions entirely, whereas the
-        // sequential drain this replaced let every remaining session fail its graceful close fast
-        // and still kill its worker. The token is passed to the graceful close instead, which
-        // preserves that behavior — a host stop deadline turns the drain into a kill sweep rather
-        // than into a leak.
+        // ParallelOptions token stops dispatching the remaining sessions entirely, so a stop
+        // deadline would leave the untried tail neither closed nor killed. Note this FIXES a leak
+        // the sequential drain also had rather than restoring its behavior: there the kill fallback
+        // ran on the caller's cancelled token, and KillWorkerAsync's entry
+        // ThrowIfCancellationRequested threw out of the loop on the first session — zero kills, not
+        // "fail fast and still kill". The token is passed to the graceful close only, and the kill
+        // runs on CancellationToken.None, so a host stop deadline turns the drain into a kill sweep
+        // rather than into a leak.
+        //
+        // The asymmetry with CloseExpiredLeasesAsync (whose ParallelOptions IS token-bound) is
+        // intentional: that sweep is periodic maintenance whose missed sessions are picked up by
+        // the next pass and, ultimately, by this drain. This drain is terminal — nothing runs after
+        // it — so it must not be abandoned partway.
         await Parallel.ForEachAsync(
             _registry.Snapshot(),
             new ParallelOptions { MaxDegreeOfParallelism = MaxParallelSessionCloses },

src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionShutdownHostedService.cs

@@ -16,7 +16,14 @@ public sealed class SessionShutdownHostedService(
         return Task.CompletedTask;
     }
 
-    /// <summary>Shuts down all gateway sessions as the host stops, logging (without throwing) if the host's shutdown timeout cancels the operation first.</summary>
+    /// <summary>Shuts down all gateway sessions as the host stops.</summary>
+    /// <remarks>
+    ///     The catch below is now effectively unreachable: <see cref="ISessionManager.ShutdownAsync"/>
+    ///     no longer aborts on the host's shutdown timeout, it degrades to a kill sweep and logs a
+    ///     per-session warning for each session that failed its graceful close. The clause is kept as
+    ///     a cheap guard against that contract regressing, not as an expected path — the operator
+    ///     signal for a timed-out shutdown is now those per-session warnings.
+    /// </remarks>
     /// <param name="cancellationToken">Token that signals the host's shutdown timeout has elapsed.</param>
     /// <returns>A task that represents the asynchronous operation.</returns>
     public async Task StopAsync(CancellationToken cancellationToken)

src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/GatewayLogRedactorTests.cs

@@ -50,6 +50,33 @@ public sealed class GatewayLogRedactorTests
         Assert.DoesNotContain("super-secret", redacted, StringComparison.Ordinal);
     }
 
+    /// <summary>
+    /// Pins the 64-character key-id boundary in both directions. Nothing validates key-id length at
+    /// creation time (neither <c>ApiKeyAdminCommandLineParser.IsValidKeyId</c> nor
+    /// <c>DashboardApiKeyManagementService.ValidateKeyId</c> caps it), so the cap here is a redaction
+    /// heuristic that a real key id can cross. This test fixes which way it fails when it does: at
+    /// exactly 64 the id is still an identifier and survives; at 65 the whole run is treated as secret
+    /// material and goes. Losing an identifier is the cheap failure; logging a secret is not.
+    /// </summary>
+    /// <param name="keyIdLength">Length of the key id presented before the secret separator.</param>
+    /// <param name="expectsKeyIdPreserved">Whether the key id must survive redaction at that length.</param>
+    [Theory]
+    [InlineData(64, true)]
+    [InlineData(65, false)]
+    public void RedactClientIdentity_KeyIdLengthBoundary_FailsTowardRedaction(
+        int keyIdLength,
+        bool expectsKeyIdPreserved)
+    {
+        string keyId = new('a', keyIdLength);
+
+        string? redacted = GatewayLogRedactor.RedactClientIdentity($"Bearer mxgw_{keyId}_super-secret");
+
+        Assert.Equal(
+            expectsKeyIdPreserved ? $"Bearer mxgw_{keyId}_[redacted]" : "Bearer mxgw_[redacted]",
+            redacted);
+        Assert.DoesNotContain("super-secret", redacted, StringComparison.Ordinal);
+    }
+
     /// <summary>
     /// Verifies that anything not recognized as a gateway API key fails closed: the scheme word
     /// survives only when it looks like an auth scheme, and the credential never does.

src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGrpcMapperTests.cs

@@ -120,6 +120,30 @@ public sealed class MxAccessGrpcMapperTests
         Assert.Equal(ProtocolStatusCode.ProtocolViolation, publicReply.ProtocolStatus.Code);
     }
 
+    /// <summary>
+    ///     Verifies MapCommandReply transfers ownership of the inner MxCommandReply the same way
+    ///     MapEvent does: the returned reference is the instance carried by the WorkerCommandReply,
+    ///     not a clone. The WorkerCommandReply is discarded after mapping and the awaiting Invoke
+    ///     call is its single consumer, so moving the inner reply out is safe and avoids a deep copy
+    ///     of a potentially large bulk-read payload.
+    /// </summary>
+    [Fact]
+    public void MapCommandReply_TransfersOwnershipOfInnerReplyWithoutCloning()
+    {
+        MxCommandReply innerReply = new()
+        {
+            SessionId = "session-1",
+            Kind = MxCommandKind.Register,
+            ProtocolStatus = MxAccessGrpcMapper.Ok(),
+            Register = new RegisterReply { ServerHandle = 50 },
+        };
+        WorkerCommandReply workerReply = new() { Reply = innerReply };
+
+        MxCommandReply mapped = new MxAccessGrpcMapper().MapCommandReply(workerReply);
+
+        Assert.Same(innerReply, mapped);
+    }
+
     /// <summary>
     ///     Verifies MapEvent transfers ownership of the inner MxEvent (GWC-07 / IPC-05): the
     ///     returned reference is the same instance carried by the WorkerEvent, not a clone. The

src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SessionEventDistributorTests.cs

@@ -1066,6 +1066,14 @@ public sealed class SessionEventDistributorTests
     ///     racing the fan-out must never drop, duplicate, or reorder an event for a subscriber
     ///     registered throughout — nor leave the array and the dictionary disagreeing on the
     ///     subscriber count once the churn stops.
+    /// <para>
+    ///     One assumption worth naming: the snapshot is rebuilt from
+    ///     <c>ConcurrentDictionary.Values</c>, whose bucket order happens to keep the long-lived
+    ///     stable subscriber ahead of the churned ones here, so it is written to before a churned
+    ///     subscriber's disposal can interleave. If that ever stops holding the test does not
+    ///     silently pass — it fails as a <see cref="ReadTimeout"/> expiry on the read below, because
+    ///     an event dropped for the stable subscriber never arrives.
+    /// </para>
     /// </summary>
     /// <returns>A task that represents the asynchronous operation.</returns>
     [Fact]

src/ZB.MOM.WW.MxGateway.Tests/Security/Audit/ChannelAuditWriterTests.cs

@@ -219,6 +219,33 @@ public sealed class ChannelAuditWriterTests : IDisposable
             (await ListActionsAsync(factory)).OrderBy(action => action, StringComparer.Ordinal));
     }
 
+    /// <summary>
+    /// The sweep above deliberately preserves rows it cannot date, so the dashboard's recent-audit
+    /// view is guaranteed to meet one eventually. Reading must therefore be defensive: the
+    /// undateable row is reported at <see cref="DateTimeOffset.MinValue"/> with every other column
+    /// intact, rather than one bad row throwing the whole page away.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task SqliteStore_ListRecent_WithUnparseableRow_ReportsMinValueInsteadOfThrowing()
+    {
+        (SqliteCanonicalAuditStore store, AuthSqliteConnectionFactory factory) = CreateStore();
+
+        DateTimeOffset occurred = new(2026, 5, 18, 0, 0, 0, TimeSpan.Zero);
+        await store.InsertAsync(MakeEvent("dateable", occurred), CancellationToken.None);
+        await InsertRawRowAsync(factory, "undateable", "0000-not-a-timestamp");
+
+        IReadOnlyList<AuditEvent> recent = await store.ListRecentAsync(10, CancellationToken.None);
+
+        Assert.Equal(2, recent.Count);
+        Assert.Equal(occurred, recent.Single(auditEvent => auditEvent.Action == "dateable").OccurredAtUtc);
+
+        AuditEvent undateable = recent.Single(auditEvent => auditEvent.Action == "undateable");
+        Assert.Equal(DateTimeOffset.MinValue, undateable.OccurredAtUtc);
+        Assert.Equal("operator01", undateable.Actor);
+        Assert.Equal(AuditOutcome.Denied, undateable.Outcome);
+    }
+
     private static (ChannelAuditWriter Writer, AuditDrainService Drain) CreateWriterAndDrain(
         IAuditEventSink sink,
         SecurityOptions? security = null,
@@ -265,7 +292,8 @@ public sealed class ChannelAuditWriterTests : IDisposable
         await command.ExecuteNonQueryAsync(CancellationToken.None);
     }
 
-    // Reads actions straight from SQL: ListRecentAsync would throw on the unparseable timestamp.
+    // Reads actions straight from SQL so the sweep assertion depends on the DELETE alone, with no
+    // opinion from the store's read path about how an undateable row is surfaced.
     private static async Task<List<string>> ListActionsAsync(AuthSqliteConnectionFactory factory)
     {
         await using SqliteConnection connection = await factory.OpenConnectionAsync(CancellationToken.None);

src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/CachingApiKeyVerifierTests.cs

@@ -281,6 +281,55 @@ public sealed class CachingApiKeyVerifierTests
         Assert.Same(reparsed, MapConstraints(firstJson));
     }
 
+    /// <summary>
+    ///     Stress: the constraint cache's bound is enforced by the inserting thread itself
+    ///     (<c>GetOrAdd</c>, then enqueue, then evict), so concurrent inserters can each land an
+    ///     entry before any of them reaches the eviction step. That overshoot is real but must be
+    ///     transient and proportional to the in-flight inserters — not unbounded growth — and once
+    ///     the churn stops the cache must be back at or under the cap. Hammered on both the
+    ///     converging path (every iteration maps the same blob, so all but one <c>GetOrAdd</c>
+    ///     loses) and the growth path (a distinct blob per iteration, which is what forces
+    ///     eviction).
+    /// </summary>
+    [Fact]
+    public void ToGatewayIdentity_ConcurrentBlobs_OvershootIsTransientAndCacheSettlesUnderCap()
+    {
+        const int cap = GatewayApiKeyIdentityMapper.MaxCachedConstraintBlobs;
+
+        // Overshoot is bounded by how many inserters can sit between their GetOrAdd and their own
+        // EvictIfOverCapacity, so scale the allowance with the available parallelism rather than
+        // pinning a magic number. Generous on purpose: the assertion under test is "bounded", not
+        // "bounded by exactly this".
+        int transientAllowance = (Environment.ProcessorCount * 8) + 64;
+        string sharedJson = ConstraintsJson("Area_StressShared");
+        int peak = 0;
+
+        Parallel.For(0, (cap * 2) + 64, index =>
+        {
+            MapConstraints(sharedJson);
+            MapConstraints(ConstraintsJson($"Area_Stress_{index}"));
+
+            int size = GatewayApiKeyIdentityMapper.CurrentCacheSize;
+            int seen = Volatile.Read(ref peak);
+            while (size > seen && Interlocked.CompareExchange(ref peak, size, seen) != seen)
+            {
+                seen = Volatile.Read(ref peak);
+            }
+        });
+
+        Assert.True(
+            peak <= cap + transientAllowance,
+            $"cache peaked at {peak} entries, past the {cap} cap plus the {transientAllowance} transient allowance");
+
+        // The cache is process-wide static and other test classes in this assembly map identities
+        // too, so poll rather than asserting on the instant the loop returns.
+        Assert.True(
+            SpinWait.SpinUntil(
+                () => GatewayApiKeyIdentityMapper.CurrentCacheSize <= cap,
+                TimeSpan.FromSeconds(5)),
+            $"cache settled at {GatewayApiKeyIdentityMapper.CurrentCacheSize} entries, past the {cap} cap");
+    }
+
     private static ApiKeyConstraints MapConstraints(string constraintsJson) =>
         GatewayApiKeyIdentityMapper.ToGatewayIdentity(new LibApiKeyIdentity(
             KeyId: "operator01",

src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs

@@ -414,6 +414,46 @@ public sealed class ApiKeyFailureLimiterTests
         Assert.True(limiter.IsTracked(arriving));
     }
 
+    /// <summary>
+    /// The <c>PartitionResolution</c> handed back by <c>Check</c> is carried across an await (the
+    /// inner verification) before <c>Reset</c> consumes it, so the partition it names can be evicted
+    /// in between. Applying a stale resolution must be inert — never throw, and never clear a
+    /// partition or aggregate that belongs to somebody else. The failure direction that matters is
+    /// "clears too little", which costs the caller a probe wait; "clears somebody else's block"
+    /// would be a throttle bypass.
+    /// </summary>
+    [Fact]
+    public void Reset_WithStaleResolutionForEvictedPartition_IsInertAndClearsNoOtherBlock()
+    {
+        ManualTimeProvider clock = new(DateTimeOffset.UnixEpoch);
+        ApiKeyFailureLimiter limiter = CreateLimiter(clock, limit: 3, aggregateLimit: 3, maxPartitions: 2);
+        ApiKeyThrottlePartition victim = new("ipv4:10.0.0.1:1", "victim");
+        ApiKeyThrottlePartition other = new("ipv4:10.0.0.2:1", "other");
+        ApiKeyThrottlePartition arriving = new("ipv4:10.0.0.3:1", "arriving");
+
+        RecordFailures(limiter, victim, 3);
+        Assert.Equal(
+            ApiKeyThrottleDecision.ThrottledByPeer,
+            limiter.Check(victim, out ApiKeyFailureLimiter.PartitionResolution stale));
+        Assert.True(stale.IsResolved);
+
+        // Age the victim's window out, then push the map past its cap so eviction takes it —
+        // expired windows are the first eviction preference, so this is deterministic. The
+        // resolution captured above now names a partition that no longer exists.
+        clock.Advance(Window + TimeSpan.FromSeconds(1));
+        RecordFailures(limiter, other, 3);
+        RecordFailures(limiter, arriving, 3);
+        Assert.False(limiter.IsTracked(victim));
+
+        limiter.Reset(victim, stale);
+
+        Assert.True(limiter.IsTracked(other));
+        Assert.True(limiter.IsTracked(arriving));
+        Assert.Equal(2, limiter.TrackedAggregateCount);
+        Assert.Equal(ApiKeyThrottleDecision.ThrottledByPeer, limiter.Check(other));
+        Assert.Equal(ApiKeyThrottleDecision.ThrottledByPeer, limiter.Check(arriving));
+    }
+
     private static void RecordFailures(ApiKeyFailureLimiter limiter, ApiKeyThrottlePartition partition, int count)
     {
         for (int i = 0; i < count; i++)

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

@@ -256,6 +256,93 @@ public sealed class MxAccessHandleRegistryTests
         Assert.True(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain));
     }
 
+    /// <summary>
+    ///     Pins the registry half of <c>MxAccessSession.TryGetCachedReadFor</c>'s fall-through
+    ///     contract. That scan walks <see cref="MxAccessHandleRegistry.GetItemHandlesForDefinition"/>
+    ///     in order and skips any candidate carrying neither a plain nor a supervisory advice,
+    ///     because an added-but-unadvised item will never receive a fresh <c>OnDataChange</c> and so
+    ///     can only serve a stale cache entry. The registry has to make that skip possible: the
+    ///     duplicate registrations of one tag must come back in a stable ascending order, and the
+    ///     advice index must answer per item handle rather than per tag. Asserted here rather than
+    ///     on the session because the session's read path needs a live MXAccess COM instance.
+    /// </summary>
+    [Fact]
+    public void GetItemHandlesForDefinition_MultipleCandidates_AdviceIndexDiscriminatesTheAdvisedOne()
+    {
+        MxAccessHandleRegistry registry = new();
+        registry.RegisterServerHandle(serverHandle: 1, clientName: "client");
+
+        // Same tag under two item handles: 10 is added but never advised, 20 is advised. Registered
+        // out of order so the ascending-order guarantee is doing real work.
+        registry.RegisterItemHandle(1, itemHandle: 20, "Tank1.PV", string.Empty, hasItemContext: false);
+        registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false);
+        registry.RegisterAdviceHandle(1, itemHandle: 20, MxAccessAdviceKind.Plain);
+
+        IReadOnlyList<int> candidates = registry.GetItemHandlesForDefinition(1, "Tank1.PV");
+
+        Assert.Equal(new[] { 10, 20 }, candidates);
+
+        // The unadvised candidate is visited first and skipped; the advised one is the survivor.
+        Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain));
+        Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Supervisory));
+        Assert.True(registry.ContainsAdviceHandle(1, 20, MxAccessAdviceKind.Plain));
+
+        // Supervisory alone qualifies too, so a later advise on 10 makes it the first survivor.
+        registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Supervisory);
+        Assert.True(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Supervisory));
+    }
+
+    /// <summary>
+    ///     One adversarial lifecycle run over every index at once: register, advise, re-register the
+    ///     same item handle under a new tag, unadvise, then tear the server down. Each individual
+    ///     transition is covered above; this pins that they compose — the reverse definition index,
+    ///     the per-item advice index and the per-server removal index must agree after every step,
+    ///     since a stale entry in any one of them resurrects a handle MXAccess has already retired.
+    /// </summary>
+    [Fact]
+    public void RegisterAdviseReregisterUnadviseUnregister_LeavesEveryIndexConsistent()
+    {
+        MxAccessHandleRegistry registry = new();
+        registry.RegisterServerHandle(serverHandle: 1, clientName: "client");
+
+        // Register.
+        registry.RegisterItemHandle(1, itemHandle: 10, "Tank1.PV", string.Empty, hasItemContext: false);
+        Assert.True(registry.ContainsItemHandle(1, 10));
+        Assert.Equal(new[] { 10 }, registry.GetItemHandlesForDefinition(1, "Tank1.PV"));
+        Assert.Empty(registry.AdviceHandles);
+
+        // Advise.
+        registry.RegisterAdviceHandle(1, itemHandle: 10, MxAccessAdviceKind.Plain);
+        Assert.True(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain));
+        Assert.Single(registry.AdviceHandles);
+
+        // Re-register the SAME item handle under a new tag. The advice is keyed on the item handle,
+        // not the tag, so it survives — but the old definition entry must not.
+        registry.RegisterItemHandle(1, itemHandle: 10, "Tank2.PV", string.Empty, hasItemContext: false);
+        Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank1.PV"));
+        Assert.Equal(new[] { 10 }, registry.GetItemHandlesForDefinition(1, "Tank2.PV"));
+        Assert.Single(registry.ItemHandles);
+        Assert.True(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain));
+
+        // Unadvise: the item stays registered and still resolves by its current tag.
+        registry.RemoveAdviceHandles(1, itemHandle: 10);
+        Assert.False(registry.ContainsAdviceHandle(1, 10, MxAccessAdviceKind.Plain));
+        Assert.Empty(registry.AdviceHandles);
+        Assert.True(registry.ContainsItemHandle(1, 10));
+        Assert.Equal(new[] { 10 }, registry.GetItemHandlesForDefinition(1, "Tank2.PV"));
+
+        // Unregister the server: every index drains, including the definition index the re-register
+        // rewrote.
+        registry.UnregisterServerHandle(1);
+        Assert.False(registry.ContainsServerHandle(1));
+        Assert.False(registry.ContainsItemHandle(1, 10));
+        Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank1.PV"));
+        Assert.Empty(registry.GetItemHandlesForDefinition(1, "Tank2.PV"));
+        Assert.Empty(registry.ServerHandles);
+        Assert.Empty(registry.ItemHandles);
+        Assert.Empty(registry.AdviceHandles);
+    }
+
     private static List<TResult> Map<TSource, TResult>(
         IReadOnlyList<TSource> source,
         Func<TSource, TResult> selector)

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

@@ -90,6 +90,11 @@ public sealed class StaWaitHelperTests
     {
         using AutoResetEvent signal = new(initialState: true);
 
+        // Drained for the same reason as the other two waits: a stale message could end this wait
+        // instead of the handle, which would leave the signal un-consumed and fail the
+        // post-condition below for a reason that has nothing to do with pre-signalling.
+        new StaMessagePump().PumpPendingMessages();
+
         Stopwatch elapsed = Stopwatch.StartNew();
         StaWaitHelper.WaitForSignalOrMessages(signal, 30_000);
         elapsed.Stop();