chore(plans): record sweep re-read + GroupToTag summary follow-ups as closed

Targeted re-read of the 203-file fca978d sweep (docs(src): add missing
XML docs and strip tracking-ID comments): a mechanical pre-pass narrowed
1,383 deletions to 68 files / 816 residual prose lines, and a judged
review of every one found 17 collateral deletions across 10 files —
rationale prose deleted alongside resolved markers with no equivalent
surviving anywhere in the tree. Restored (markers stay stripped, per the
sweep's intent):

- SessionManager: the three metrics-accounting invariants (kill-path
  gauge decrement safety, shutdown kill-fallback registry guard vs
  double bookkeeping, SessionClosed-not-SessionRemoved on failed close)
- SessionManagerTests: the matching accounting expectation note and the
  reason-string propagation pins (test summary + FakeWorkerClient.LastKillReason)
- MxAccessGatewayService.AcknowledgeAlarm: the routing remarks (GUID vs
  Provider!Group.Tag vs InvalidRequest; session-less via IGatewayAlarmService)
  — inheritdoc resolves to nothing (proto-generated base is undocumented)
- HubTokenService.Validate: why the hollow-token guard exists
  (non-empty AuthenticationType alone satisfies IsAuthenticated)
- DashboardSessionAdminService: why both broad catches exist (keep raw
  teardown exceptions out of Blazor's error boundary), Close + Kill paths
- WorkerPipeSession.RunAsync: why the factory result throws instead of
  NREing (unambiguous failure; finally-block Dispose can't no-op)
- LmxSubtagAlarmSource: Advise idempotency; Write is always unsecured
  (user id 0), never WriteSecured semantics
- WnWrapAlarmConsumer: the v1-prefix path is what WIN-911-style code uses
- DashboardSnapshotPublisherTests: what the 10ms slack absorbs
  (Task.Delay's coarse Windows timer quantum)
- DashboardBrowseAndAlarmModelTests: why the label text is pinned, not
  just the CSS class

Everything else flagged verified benign: inheritdoc replacements resolve
to equal-or-richer interface docs, or the substance survives relocated.
NonWindows slnx 0W/0E; touched gateway test classes 65/65.

docs/plans/2026-08-18-followups-and-tickets.md

@@ -511,12 +511,26 @@ Notable findings made along the way:
   other's child nodes (MSB4166) — `-m:1` avoids it; ssh can transiently refuse with "Too
   many authentication failures" under agent concurrency (back off and retry).
 
-Follow-ups recorded, not started (deliberately small):
+Follow-ups recorded, not started (deliberately small) — *first two closed 2026-08-18 on
+`feat/sweep-reread-tag-summary`*:
 
 - The `fca978d` sweep re-read (above) — the one genuinely ticket-worthy item.
+  *Closed in `f82dac1`: a mechanical pre-pass narrowed the sweep's 1,383 deletions to
+  68 files / 816 residual prose lines; a judged review of every one found 17 collateral
+  deletions across 10 files (rationale prose with no surviving equivalent — SessionManager
+  metrics invariants, HubTokenService hollow-token guard, AcknowledgeAlarm routing
+  remarks, LmxSubtagAlarmSource unsecured-write/idempotency notes, WnWrapAlarmConsumer's
+  WIN-911 precedent, DashboardSessionAdminService error-boundary catches,
+  WorkerPipeSession factory-null throw, three test-design rationales) — all restored,
+  markers left stripped. Everything else flagged verified benign (`<inheritdoc/>`
+  resolves to equal-or-richer interface docs, or the substance survives relocated).
+  Verified: NonWindows slnx 0W/0E, touched gateway test classes 65/65; windev worker
+  x86 build 0W/0E, worker suite 524/11/0.*
 - `DashboardGroupTagMapping`'s class-level `<summary>` still describes the lookup as
   "full DN first, leading-RDN fallback" — accurate mechanics, but it could point at the
   short-name-keys consequence the inline comment now records.
+  *Closed in `4b6f907`: the summary now states full-DN keys can never match and points
+  at the lookup comment in `MapGroupsToTags`.*
 - wwtools `mxa read` human-readable formatter throws `RuntimeBinderException` on a failed
   read (`ReadCommand.cs:137`); `--llm-json` works. Different repo, noted here so it isn't
   lost.

src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupTagMapping.cs

@@ -6,6 +6,9 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard;
 /// Sibling of <see cref="DashboardGroupRoleMapping"/> and deliberately follows
 /// the same group-matching rules (full DN first, leading-RDN fallback,
 /// case-insensitive) so operators write one kind of group key for both maps.
+/// Because the shared LDAP provider delivers groups already stripped to short
+/// RDN names, full-DN <c>GroupToTag</c> keys can never match — use short group
+/// names as keys (see the lookup comment in <see cref="MapGroupsToTags"/>).
 /// Tags gate dashboard event VISIBILITY only; they are never a data-access
 /// constraint.
 /// </summary>

src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs

@@ -124,6 +124,11 @@ public sealed class DashboardSessionAdminService(
         }
         catch (Exception exception)
         {
+            // Any non-SessionManagerException (e.g. an IOException or
+            // InvalidOperationException from the session DisposeAsync / pipe
+            // teardown path) would otherwise propagate raw into Blazor's error
+            // boundary. Convert it to a friendly failure so the Razor pages see
+            // only DashboardSessionAdminResult.
             _logger.LogWarning(
                 exception,
                 "Dashboard admin {Actor} close failed unexpectedly for session {SessionId}.",
@@ -206,6 +211,12 @@ public sealed class DashboardSessionAdminService(
         }
         catch (Exception exception)
         {
+            // Any non-SessionManagerException (e.g. an IOException from worker
+            // pipe teardown surfacing through session.DisposeAsync, or an
+            // InvalidOperationException from a corrupted worker handle) would
+            // otherwise propagate raw into Blazor's error boundary. Convert it
+            // to a friendly failure so the page renders the ResultMessage
+            // rather than the circuit error page.
             _logger.LogWarning(
                 exception,
                 "Dashboard admin {Actor} kill failed unexpectedly for session {SessionId}.",

src/ZB.MOM.WW.MxGateway.Server/Dashboard/HubTokenService.cs

@@ -115,6 +115,11 @@ public sealed class HubTokenService
                 return null;
             }
 
+            // Reject a token whose payload carries no caller identity. A
+            // null/empty Name AND NameIdentifier would otherwise produce a
+            // principal that satisfies IsAuthenticated and IsInRole checks
+            // without any associated user, because the AuthenticationType
+            // (the HubToken scheme) is non-empty.
             if (string.IsNullOrEmpty(payload.Name) && string.IsNullOrEmpty(payload.NameIdentifier))
             {
                 return null;

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

@@ -185,6 +185,15 @@ public sealed class MxAccessGatewayService(
     }
 
     /// <inheritdoc />
+    /// <remarks>
+    ///     Surfaces the public AcknowledgeAlarm RPC. Acknowledgement is
+    ///     session-less: the gateway routes it through the always-on
+    ///     <see cref="IGatewayAlarmService"/> monitor session. An
+    ///     <c>alarm_full_reference</c> that parses as a canonical GUID forwards
+    ///     to <c>AcknowledgeAlarmCommand</c>; a <c>Provider!Group.Tag</c>
+    ///     reference forwards to <c>AcknowledgeAlarmByNameCommand</c>; anything
+    ///     else returns an <c>InvalidRequest</c> diagnostic in the reply.
+    /// </remarks>
     public override async Task<AcknowledgeAlarmReply> AcknowledgeAlarm(
         AcknowledgeAlarmRequest request,
         ServerCallContext context)

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

@@ -231,6 +231,10 @@ public sealed class SessionManager : ISessionManager
             session.MarkFaulted(exception.Message);
             _metrics.Fault(SessionManagerErrorCode.CloseFailed.ToString());
 
+            // The open-session gauge was incremented in OpenSessionAsync; every
+            // session reaching KillWorkerAsync had SessionOpened recorded. If the
+            // kill path throws, decrement the gauge here so mxgateway.sessions.open
+            // does not leak — mirroring the equivalent guard in OpenSessionAsync.
             _metrics.SessionRemoved();
             await RemoveSessionAsync(session).ConfigureAwait(false);
             throw new SessionManagerException(
@@ -393,6 +397,11 @@ public sealed class SessionManager : ISessionManager
                         "Graceful shutdown failed for session {SessionId}; killing worker.",
                         session.SessionId);
 
+                    // Defensive fallback: CloseSessionCoreAsync's inner
+                    // SessionCloseStartedException catch normally removes the session
+                    // and accounts the close. This outer fallback only fires for
+                    // sessions still in the registry — route through KillWorkerAsync
+                    // so the bookkeeping is identical to the dashboard kill path.
                     if (_registry.TryGet(session.SessionId, out GatewaySession? registeredSession)
                         && registeredSession is not null)
                     {
@@ -443,6 +452,11 @@ public sealed class SessionManager : ISessionManager
             session.MarkFaulted(exception.Message);
             if (!wasClosed)
             {
+                // Account the close as a SessionClosed (decrements the open-session
+                // gauge AND increments the sessions.closed counter), not just
+                // SessionRemoved. The session is being removed from the registry
+                // below; treating this as a half-finished close that only
+                // decremented the gauge would under-count the closed counter.
                 _metrics.SessionClosed();
             }
 

src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardBrowseAndAlarmModelTests.cs

@@ -171,6 +171,9 @@ public sealed class DashboardBrowseAndAlarmModelTests
         Assert.True(model.IsDegraded);
         Assert.Contains("bg-warning", model.BadgeCssClass, StringComparison.Ordinal);
         Assert.Equal("x", model.Reason);
+
+        // Pin the amber label text, not just the CSS class — a label swap
+        // would otherwise pass this test.
         Assert.Equal(DashboardAlarmProviderStatus.DegradedLabel, model.Label);
     }
 

src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotPublisherTests.cs

@@ -57,6 +57,12 @@ public sealed class DashboardSnapshotPublisherTests
             $"Expected at least 2 subscribe calls, got {snapshotService.SubscribeCount}.");
         Assert.True(hubContext.SendCount >= 1);
 
+        // The gap is measured from the moment the first subscribe actually
+        // threw (inside the fake) to the moment the second subscribe began
+        // (also inside the fake). This isolates the publisher's
+        // Task.Delay(reconnectDelay) — no StartAsync / scheduling overhead in
+        // the baseline. The 10ms slack absorbs Task.Delay's coarse Windows
+        // timer quantum (~15ms) when the underlying scheduler wakes early.
         TimeSpan gap = secondSubscribeAt - firstThrowAt;
         Assert.True(gap >= reconnectDelay - TimeSpan.FromMilliseconds(10),
             $"Expected reconnect gap >= {reconnectDelay.TotalMilliseconds}ms; got {gap.TotalMilliseconds}ms.");

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

@@ -686,6 +686,11 @@ public sealed class SessionManagerTests
         Assert.Equal(1, failingWorkerClient.KillCount);
         Assert.Equal(1, failingWorkerClient.DisposeCount);
         GatewayMetricsSnapshot snapshot = metrics.GetSnapshot();
+
+        // A close-that-failed accounts as SessionClosed (counter += 1) rather
+        // than SessionRemoved (gauge -= 1, counter unchanged). The session is
+        // being removed from the registry on this path, so it must show up in
+        // the closed count.
         Assert.Equal(1, snapshot.SessionsClosed);
         Assert.False(snapshot.EventsBySession.ContainsKey(firstSession.SessionId));
         Assert.Equal(1, snapshot.OpenSessions);
@@ -743,6 +748,9 @@ public sealed class SessionManagerTests
 
     /// <summary>
     /// Verifies that killing a worker removes the session from the registry without calling shutdown.
+    /// Also pins the <c>reason</c> argument propagating through
+    /// <c>SessionManager.KillWorkerAsync</c> → <c>session.KillWorker(reason)</c>
+    /// → <c>IWorkerClient.Kill(reason)</c>.
     /// </summary>
     /// <returns>A task that represents the asynchronous operation.</returns>
     [Fact]
@@ -1419,7 +1427,13 @@ public sealed class SessionManagerTests
         /// <summary>Gets the number of times kill was called on the fake worker client.</summary>
         public int KillCount { get; private set; }
 
-        /// <summary>Gets the last reason argument observed by <see cref="Kill"/>.</summary>
+        /// <summary>
+        /// Gets the last reason argument observed by <see cref="Kill"/>. Pins the
+        /// reason-string propagation through <c>SessionManager.KillWorkerAsync</c>
+        /// → <c>session.KillWorker(reason)</c> → <c>IWorkerClient.Kill(reason)</c>;
+        /// without this, the chain could silently drop or substitute the reason
+        /// argument and existing tests would still pass.
+        /// </summary>
         public string? LastKillReason { get; private set; }
 
         /// <summary>Gets the number of times dispose was called on the fake worker client.</summary>

src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs

@@ -142,6 +142,13 @@ public sealed class WorkerPipeSession
     /// <returns>A task that represents the asynchronous operation.</returns>
     public async Task RunAsync(CancellationToken cancellationToken = default)
     {
+        // The factory delegate itself is null-checked in the constructor, but its
+        // return value is not — a factory that returned null would NRE on the
+        // StartAsync lambda below. Throw a diagnostic exception instead so the
+        // failure is unambiguous (and so the finally block's
+        // _runtimeSession?.Dispose() can't silently no-op on a torn
+        // half-initialized session). Mirrors the same pattern
+        // AlarmCommandHandler.Subscribe uses for its consumerFactory().
         _runtimeSession = _runtimeSessionFactory()
             ?? throw new InvalidOperationException(
                 "Worker runtime session factory returned null.");

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

@@ -108,6 +108,10 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource
     public event EventHandler<SubtagValueChange>? ValueChanged;
 
     /// <inheritdoc />
+    /// <remarks>
+    ///     Idempotent per address: an address already advised is skipped
+    ///     rather than re-registered.
+    /// </remarks>
     public void Advise(IReadOnlyCollection<string> itemAddresses)
     {
         if (itemAddresses is null)
@@ -140,6 +144,10 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource
     }
 
     /// <inheritdoc />
+    /// <remarks>
+    ///     Writes with MXAccess user id 0 — always an unsecured Write, never
+    ///     WriteSecured semantics.
+    /// </remarks>
     public void Write(string itemAddress, object? value)
     {
         if (itemAddress is null)

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

@@ -198,7 +198,9 @@ public sealed class WnWrapAlarmConsumer : IMxAccessAlarmConsumer
             // 2026-08-18) this is the only path that lets AlarmAckByName
             // return rc=0 afterwards. The v2 Initialize/Register/Subscribe
             // methods on the class succeed (return 0) but acks against that
-            // consumer state return -55. Note rc=0 means the call was
+            // consumer state return -55. The v1 prefix path is what
+            // WIN-911-style code uses against the same wnwrap library.
+            // Note rc=0 means the call was
             // accepted, not that an acknowledgement was applied — see
             // AcknowledgeByName below and docs/AlarmProbeFindings.md.
             int init = com.IwwAlarmConsumer_InitializeConsumer(DefaultApplicationName);