Compare commits

...

4 Commits

Author SHA1 Message Date
Joseph Doherty 462850afb7 fix(worker-tests): doc-comment placement + watchdog-window headroom in long-in-flight test
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Failing after 17s
ci / java (push) Successful in 2m38s
ci / portable (push) Successful in 8m34s
Three review findings on 7da52b6:

1. The AssertNotFault doc block landed between the predicate-overload ReadUntilAsync's
   doc block and the helper, so the compiler attached the merged block to the helper and
   ReadUntilAsync lost its docs entirely. Helper and its docs moved above ReadUntilAsync,
   whose docs are back where they belong.

2. The compressed watchdog windows (50 ms grace, 100 ms ceiling) reintroduced the
   load-sensitivity the fix removed, one layer down.
   ReportWatchdogFaultIfNeededAsync measures staleness AFTER the heartbeat frame is
   written and flushed over the real named pipe, so a beat whose pipe I/O outlasts the
   ceiling faults a healthy session no matter how fresh the captured activity was. At
   100 ms that is a plausible stall on a loaded box, and this is the one test asserting
   the watchdog NEVER fires. Widened to a 200 ms grace and a 1 s ceiling — still two
   orders of magnitude under the 75 s production default.

   The inspection loop is now bounded by a 2 s window (twice the ceiling, so a fake whose
   activity stopped advancing still accumulates past it and faults) with a 30-frame floor,
   rather than a fixed 30 frames that no longer outran the wider ceiling. The floor keeps
   a window that saw almost no beats from passing as a clean one. Two seconds stays well
   inside FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in
   flight for the whole window.

3. AssertNotFault renamed AssertNotWorkerFault, matching the WorkerFault body case it
   tests.

Scenario intent unchanged: long in-flight command, pump refreshing, zero fault frames,
reply delivered. Test-only.
2026-08-18 07:01:16 -04:00
Joseph Doherty d1ae43d0d3 docs(probe): ack-leg third attempt — unavailable by configuration, GUID leg still assumed 2026-08-18 06:56:58 -04:00
Joseph Doherty f57a6ae5ff fix(dashboard): alarms feed doc row + monitor-contract comment + mid-truncation attach test
The In-process page feeds table still described the alarms page's subscription as
provider status only, contradicting the three passages updated in 7b6dfba.

The loop's catch-all comment (and the cadence bullet that repeated it) claimed the
monitor completes a subscriber's stream on restart. It does not: ClearCache pushes
snapshot_status(false) through the still-open channel, and a subscriber is only
completed-with-error on a failed TryWrite, or removed by its own disposal.

The SnapshotStatus arm's priming claim had no test behind it — every push test
attached to an untruncated feed and pushed the edge itself. ScriptedAlarmFeed now
replays an optional open sequence, and a new test attaches to a feed already
primed provider_status then snapshot_status(truncated) and asserts the banner
comes up with no edge pushed after render.
2026-08-18 06:49:20 -04:00
Joseph Doherty d3ac52758c docs(reviews): re-verify doc-only Server-0xx resolutions; re-fix regressions
Audited every Server-0xx finding whose Resolution described a
documentation-only or comment-only change, and spot-checked the doc
sub-claims of otherwise test-backed resolutions. 20 entries annotated
in place (append-only; no historical resolution text rewritten).

Two corrections had not survived and are re-applied:

Server-040: the MapGroupsToRoles lookup-precedence comment moved intact
into DashboardGroupRoleMapping (792e3f9) and was then deleted wholesale
by fca978d, a sweep meant only to strip (Server-NNN) tracking markers.
That also removed a later, substantive paragraph recording that the
shared ZB.MOM.WW.Auth.Ldap provider pre-strips groups to short RDN
names, so a full-DN GroupToRole key is unsupported. Both paragraphs
restored, minus the tracking IDs.

Server-009: the WAL / busy_timeout note vanished when the Storage
section of docs/Authentication.md was rewritten to delegate
connection-factory detail to ZB.MOM.WW.Auth.ApiKeys. The behavior is
still live in the library (confirmed against 0.2.1), so the fix is
prose-only.

Server-011/014/022/023 are annotated as moot rather than regressed:
the IAlarmRpcDispatcher trio was deleted in dc9c0c9 and no stale
'not yet wired' / 'PR A.6/A.7' prose survives in Server source.
Server-038's documented v1 ACL gap was later closed by
IDashboardSessionAcl, so its remarks are current.

Comment/doc-only; no logic changes.
2026-08-18 06:48:45 -04:00
8 changed files with 289 additions and 54 deletions
+20 -20
View File
@@ -259,7 +259,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Add a startup hosted service that finds and kills stale worker processes (by executable path / a well-known argument or environment marker) before the server accepts sessions, or update the design docs if reattachment/cleanup is deliberately deferred.
**Resolution:** Resolved 2026-05-18. Confirmed against source: no code path enumerated or killed leftover workers. Added `IRunningProcessInspector` / `SystemRunningProcessInspector` (a testable seam over `Process.GetProcessesByName`/`Kill`), `OrphanWorkerTerminator` (kills processes matched by the configured worker executable path, or by image name when the x64 gateway cannot introspect the x86 worker's `MainModule`, skipping the current process and tolerating per-process kill failures), and `OrphanWorkerCleanupHostedService` (best-effort `IHostedService`). The hosted service is registered in `AddWorkerProcessLauncher` ahead of `AddGatewaySessions` so cleanup runs before the server accepts sessions. `gateway.md` updated to describe the implemented behavior. Regression tests: `OrphanWorkerTerminatorTests` (`KillsWorkerProcessesMatchingConfiguredExecutablePath`, `KillsImageNameMatchWhenExecutablePathUnreadable`, `DoesNotKillUnrelatedProcessSharingImageName`, `DoesNotKillCurrentProcess`, `ContinuesWhenOneKillThrows`).
**Resolution:** Resolved 2026-05-18. Confirmed against source: no code path enumerated or killed leftover workers. Added `IRunningProcessInspector` / `SystemRunningProcessInspector` (a testable seam over `Process.GetProcessesByName`/`Kill`), `OrphanWorkerTerminator` (kills processes matched by the configured worker executable path, or by image name when the x64 gateway cannot introspect the x86 worker's `MainModule`, skipping the current process and tolerating per-process kill failures), and `OrphanWorkerCleanupHostedService` (best-effort `IHostedService`). The hosted service is registered in `AddWorkerProcessLauncher` ahead of `AddGatewaySessions` so cleanup runs before the server accepts sessions. `gateway.md` updated to describe the implemented behavior. Regression tests: `OrphanWorkerTerminatorTests` (`KillsWorkerProcessesMatchingConfiguredExecutablePath`, `KillsImageNameMatchWhenExecutablePathUnreadable`, `DoesNotKillUnrelatedProcessSharingImageName`, `DoesNotKillCurrentProcess`, `ContinuesWhenOneKillThrows`). Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; the `gateway.md` orphan-cleanup prose (`OrphanWorkerCleanupHostedService` running `OrphanWorkerTerminator` once on startup, before the server accepts sessions) is still there.
### Server-003
@@ -364,7 +364,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Set `Pooling`, a non-zero `DefaultTimeout`/`busy_timeout`, and enable WAL (`PRAGMA journal_mode=WAL`) once at startup so concurrent readers/writers degrade gracefully.
**Resolution:** Resolved 2026-05-18. Confirmed against source: the connection string set only `DataSource` and `Mode`. `AuthSqliteConnectionFactory.CreateConnection` now also sets `Pooling = true` and a non-zero `DefaultTimeout`. A new `OpenConnectionAsync(CancellationToken)` opens the connection and applies `PRAGMA journal_mode=WAL` and `PRAGMA busy_timeout` (5 s); WAL is a persistent database-level setting so re-applying it per connection is a cheap no-op, while `busy_timeout` is per-connection state. All nine auth-store call sites (`SqliteApiKeyAdminStore`, `SqliteApiKeyAuditStore`, `SqliteApiKeyStore`, `SqliteAuthStoreMigrator`) were switched from `CreateConnection()` + `OpenAsync()` to `OpenConnectionAsync()`. `docs/Authentication.md` updated to describe the WAL/busy-timeout behavior. Regression test: `SqliteAuthStoreTests.OpenConnectionAsync_EnablesWalJournalModeAndBusyTimeout`.
**Resolution:** Resolved 2026-05-18. Confirmed against source: the connection string set only `DataSource` and `Mode`. `AuthSqliteConnectionFactory.CreateConnection` now also sets `Pooling = true` and a non-zero `DefaultTimeout`. A new `OpenConnectionAsync(CancellationToken)` opens the connection and applies `PRAGMA journal_mode=WAL` and `PRAGMA busy_timeout` (5 s); WAL is a persistent database-level setting so re-applying it per connection is a cheap no-op, while `busy_timeout` is per-connection state. All nine auth-store call sites (`SqliteApiKeyAdminStore`, `SqliteApiKeyAuditStore`, `SqliteApiKeyStore`, `SqliteAuthStoreMigrator`) were switched from `CreateConnection()` + `OpenAsync()` to `OpenConnectionAsync()`. `docs/Authentication.md` updated to describe the WAL/busy-timeout behavior. Regression test: `SqliteAuthStoreTests.OpenConnectionAsync_EnablesWalJournalModeAndBusyTimeout`. Regressed or never applied; re-fixed 2026-08-18 in `docs/Authentication.md` (feat/followups-tickets) — the doc sub-claim did not survive: the Storage section was later rewritten to delegate schema/connection-factory detail to `ZB.MOM.WW.Auth.ApiKeys` and the WAL/busy-timeout paragraph went with it, leaving no prose for the behavior anywhere. The behavior itself is intact (the library's `AuthSqliteConnectionFactory.OpenConnectionAsync` still issues both pragmas — confirmed against the 0.2.1 package), so the fix is prose-only: a paragraph restating the pooled-connection + `journal_mode=WAL` + 5 s `busy_timeout` contract and why it is load-bearing for the per-request last-used stamp and the per-denial audit append.
### Server-010
@@ -379,7 +379,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Either hide/disable the Rotate action for revoked keys in `ApiKeysPage.razor`, require an explicit confirmation, or have `RotateAsync` preserve `revoked_utc` and add a separate explicit "reactivate" operation.
**Resolution:** Resolved 2026-05-18. Confirmed against source: `ApiKeysPage.razor` rendered the Rotate button unconditionally while Revoke was already gated on `key.RevokedUtc is null`. Took the lowest-risk recommended option — the dashboard now renders the Rotate (and Revoke) actions only for keys whose status is `Active`; a revoked key shows a "No actions" placeholder, so an operator cannot un-revoke a deliberately disabled key as a side effect of a rotation. `RotateAsync`'s store-level behavior is unchanged (rotation by `key_id` still clears `revoked_utc`, which the CLI relies on); `docs/Authentication.md` updated to document both the store behavior and the dashboard restriction. No automated test added: the change is pure conditional Razor rendering and the test project has no bUnit component-rendering harness; the underlying `DashboardApiKeyManagementService` is already unit-tested.
**Resolution:** Resolved 2026-05-18. Confirmed against source: `ApiKeysPage.razor` rendered the Rotate button unconditionally while Revoke was already gated on `key.RevokedUtc is null`. Took the lowest-risk recommended option — the dashboard now renders the Rotate (and Revoke) actions only for keys whose status is `Active`; a revoked key shows a "No actions" placeholder, so an operator cannot un-revoke a deliberately disabled key as a side effect of a rotation. `RotateAsync`'s store-level behavior is unchanged (rotation by `key_id` still clears `revoked_utc`, which the CLI relies on); `docs/Authentication.md` updated to document both the store behavior and the dashboard restriction. No automated test added: the change is pure conditional Razor rendering and the test project has no bUnit component-rendering harness; the underlying `DashboardApiKeyManagementService` is already unit-tested. Re-verified present 2026-08-18 (feat/followups-tickets) — `ApiKeysPage.razor` still gates both Rotate and Revoke on `key.RevokedUtc is null`, with the "Rotate clears revoked_utc" comment intact.
### Server-011
@@ -394,7 +394,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Add the needed `using` directives, drop the inline fully-qualified names, and convert to a primary constructor for consistency.
**Resolution:** Resolved 2026-05-18. Confirmed against source. Converted `WorkerAlarmRpcDispatcher` to a primary constructor with the standard `?? throw new ArgumentNullException(...)` field-initializer guard; dropped the inline `System.Guid` / `System.ArgumentNullException` qualifications (using implicit `using System;`); removed redundant `using System.Collections.Generic;` / `System.Threading` / `System.Threading.Tasks;` directives (covered by `ImplicitUsings`); replaced the two `if (... is null) throw new System.ArgumentNullException(...)` checks with `ArgumentNullException.ThrowIfNull`. The stale class-level `<summary>`/`<remarks>` ("Replaces NotWiredAlarmRpcDispatcher once ... wired in", "partially wired", "returns an Unimplemented diagnostic") were corrected to describe the actual GUID-vs-`Provider!Group.Tag` handling — overlapping with Server-014. No behavior change, so no new test; existing `WorkerAlarmRpcDispatcherTests` continue to pass and the project builds warning-free under `TreatWarningsAsErrors`.
**Resolution:** Resolved 2026-05-18. Confirmed against source. Converted `WorkerAlarmRpcDispatcher` to a primary constructor with the standard `?? throw new ArgumentNullException(...)` field-initializer guard; dropped the inline `System.Guid` / `System.ArgumentNullException` qualifications (using implicit `using System;`); removed redundant `using System.Collections.Generic;` / `System.Threading` / `System.Threading.Tasks;` directives (covered by `ImplicitUsings`); replaced the two `if (... is null) throw new System.ArgumentNullException(...)` checks with `ArgumentNullException.ThrowIfNull`. The stale class-level `<summary>`/`<remarks>` ("Replaces NotWiredAlarmRpcDispatcher once ... wired in", "partially wired", "returns an Unimplemented diagnostic") were corrected to describe the actual GUID-vs-`Provider!Group.Tag` handling — overlapping with Server-014. No behavior change, so no new test; existing `WorkerAlarmRpcDispatcherTests` continue to pass and the project builds warning-free under `TreatWarningsAsErrors`. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but no longer verifiable in place — `WorkerAlarmRpcDispatcher.cs` was **deleted** (not renamed) by the `dc9c0c9` project-rename/alarm-rework commit, which replaced the whole `IAlarmRpcDispatcher` trio with `GatewayAlarmMonitor` / `IGatewayAlarmService`. A repo-wide grep for the prose this finding removed ("not yet wired", "PR A.6/A.7", "worker-pending", "dev-rig") returns nothing in Server source, so the correction is moot rather than lost. Nothing to re-fix.
### Server-012
@@ -409,7 +409,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Update CLAUDE.md's scope list and the `apikey` example to the canonical `*:*` scope strings, per CLAUDE.md's own rule that docs change with the code.
**Resolution:** Resolved 2026-05-18. Confirmed against `GatewayScopes` (`session:open`, `session:close`, `invoke:read`, `invoke:write`, `invoke:secure`, `events:read`, `metadata:read`, `admin`). CLAUDE.md's Build/Test/Run `apikey create` example and the Authentication-section scope list were both updated to the canonical `*:*` strings. (Note: since finding Server-004 was resolved, the old example would now be actively rejected at create time rather than silently creating an unusable key, making the doc correction load-bearing.) Pure documentation change; no test.
**Resolution:** Resolved 2026-05-18. Confirmed against `GatewayScopes` (`session:open`, `session:close`, `invoke:read`, `invoke:write`, `invoke:secure`, `events:read`, `metadata:read`, `admin`). CLAUDE.md's Build/Test/Run `apikey create` example and the Authentication-section scope list were both updated to the canonical `*:*` strings. (Note: since finding Server-004 was resolved, the old example would now be actively rejected at create time rather than silently creating an unusable key, making the doc correction load-bearing.) Pure documentation change; no test. Regression re-fixed 2026-08-18 on feat/followup-closeout (a5f843c, f2a422b); scope lists re-verified present. (Both CLAUDE.md sites confirmed on feat/followups-tickets: the Build/Test/Run `apikey create-key` example carries the full canonical `session:open,session:close,invoke:read,invoke:write,invoke:secure,events:read,metadata:read,admin` set, and the Authentication-section list names the same eight strings.)
### Server-013
@@ -439,7 +439,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Update the `AcknowledgeAlarm`/`QueryActiveAlarms` remarks to reflect that `WorkerAlarmRpcDispatcher` is the wired default, and describe its actual GUID-vs-`Provider!Group.Tag` handling.
**Resolution:** Resolved 2026-05-18. Confirmed against source: `SessionServiceCollectionExtensions` registers `WorkerAlarmRpcDispatcher` as `IAlarmRpcDispatcher`, so the "not yet wired" / "empty stream until PR A.2" / "PR A.6/A.7 follow-up" prose in the `AcknowledgeAlarm` and `QueryActiveAlarms` `<remarks>` and inline comments was stale. Rewrote both `<remarks>` blocks and both inline comments to state that DI binds the production `WorkerAlarmRpcDispatcher`, that it routes over the worker pipe IPC, and that `AcknowledgeAlarm` handles a canonical-GUID reference (→ `AcknowledgeAlarmCommand`) and a `Provider!Group.Tag` reference (→ `AcknowledgeAlarmByNameCommand`), with `NotWiredAlarmRpcDispatcher` being only the null fallback. The matching stale `WorkerAlarmRpcDispatcher` class-level XML doc was corrected as part of Server-011. Pure documentation/comment change; no test.
**Resolution:** Resolved 2026-05-18. Confirmed against source: `SessionServiceCollectionExtensions` registers `WorkerAlarmRpcDispatcher` as `IAlarmRpcDispatcher`, so the "not yet wired" / "empty stream until PR A.2" / "PR A.6/A.7 follow-up" prose in the `AcknowledgeAlarm` and `QueryActiveAlarms` `<remarks>` and inline comments was stale. Rewrote both `<remarks>` blocks and both inline comments to state that DI binds the production `WorkerAlarmRpcDispatcher`, that it routes over the worker pipe IPC, and that `AcknowledgeAlarm` handles a canonical-GUID reference (→ `AcknowledgeAlarmCommand`) and a `Provider!Group.Tag` reference (→ `AcknowledgeAlarmByNameCommand`), with `NotWiredAlarmRpcDispatcher` being only the null fallback. The matching stale `WorkerAlarmRpcDispatcher` class-level XML doc was corrected as part of Server-011. Pure documentation/comment change; no test. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but superseded — `MxAccessGatewayService.AcknowledgeAlarm` / `QueryActiveAlarms` still exist but were rewritten onto `IGatewayAlarmService` (the `IAlarmRpcDispatcher` seam is gone), and both now carry a bare `<inheritdoc />` rather than the rewritten `<remarks>`. The stale prose this finding removed is absent repo-wide, so there is nothing to re-fix; the rewritten remarks describing a now-deleted dispatcher would be wrong if restored.
### Server-015
@@ -454,7 +454,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Make `CloseAsync` mutate `_state` through the existing `TransitionTo(...)` helper (or acquire `_syncRoot` around the reads/writes) so all `_state` access uses the same lock. Either extend `TransitionTo` to accept the `Closing` and `Closed` transitions (it already handles `Faulted`/`Closed` precedence) or refactor `CloseAsync` to call a private `TrySetClosing()` / `MarkClosed()` that locks `_syncRoot`. Add a regression test that forces a `TransitionTo(Ready)` after `CloseAsync` has set `Closing` and asserts the session does not flip back to `Ready`.
**Resolution:** 2026-05-20 — Unified the close path on `_syncRoot`. `GatewaySession.CloseAsync` (`src/MxGateway.Server/Sessions/GatewaySession.cs`) now mutates `_state` only through two private `_syncRoot`-locked helpers — `TryBeginClose` (writes `Closing`, returns the prior `_closeStarted`) and `MarkClosed` (writes `Closed`) — so every `_state` read/write in the session uses the same lock; `_closeLock` keeps its role of serializing concurrent close attempts. `TransitionTo` was tightened to refuse a transition out of `Closing` to anything other than `Closed`/`Faulted` so a late lifecycle callback cannot walk a closing session back to `Ready`. `docs/Sessions.md` updated to describe the unified lock discipline and the extended terminal precedence. Regression tests in `src/MxGateway.Tests/Gateway/Sessions/GatewaySessionTests.cs`: `TransitionTo_AfterCloseStarted_DoesNotOverwriteClosing` (the named scenario — `BlockingShutdownWorkerClient` parks the close inside `worker.ShutdownAsync` so the test can call `TransitionTo(Ready)` between the `Closing` and `Closed` writes and assert the state stays `Closing`) and `MarkFaulted_AfterCloseCompletes_DoesNotResurrectSession`.
**Resolution:** 2026-05-20 — Unified the close path on `_syncRoot`. `GatewaySession.CloseAsync` (`src/MxGateway.Server/Sessions/GatewaySession.cs`) now mutates `_state` only through two private `_syncRoot`-locked helpers — `TryBeginClose` (writes `Closing`, returns the prior `_closeStarted`) and `MarkClosed` (writes `Closed`) — so every `_state` read/write in the session uses the same lock; `_closeLock` keeps its role of serializing concurrent close attempts. `TransitionTo` was tightened to refuse a transition out of `Closing` to anything other than `Closed`/`Faulted` so a late lifecycle callback cannot walk a closing session back to `Ready`. `docs/Sessions.md` updated to describe the unified lock discipline and the extended terminal precedence. Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; `docs/Sessions.md` still states that both close-related writes go through `_syncRoot` via `TryBeginClose`/`MarkClosed` and that `_closeLock` only serializes concurrent close attempts. Regression tests in `src/MxGateway.Tests/Gateway/Sessions/GatewaySessionTests.cs`: `TransitionTo_AfterCloseStarted_DoesNotOverwriteClosing` (the named scenario — `BlockingShutdownWorkerClient` parks the close inside `worker.ShutdownAsync` so the test can call `TransitionTo(Ready)` between the `Closing` and `Closed` writes and assert the state stays `Closing`) and `MarkFaulted_AfterCloseCompletes_DoesNotResurrectSession`.
### Server-016
@@ -484,7 +484,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Add explicit arms to `ResolveRequiredScope`: map `AcknowledgeAlarmRequest` to `GatewayScopes.InvokeWrite` (parity with other write actions; ack changes alarm state) and `QueryActiveAlarmsRequest` to `GatewayScopes.MetadataRead` or `GatewayScopes.InvokeRead`. Update `docs/Authorization.md` to list both. Extend `GatewayGrpcScopeResolverTests` with the new mappings and an assertion that every request type defined by `mxaccess_gateway.proto` is named in the resolver (the test can enumerate the assembly's request types so a future RPC cannot quietly add itself only via the admin fallback).
**Resolution:** 2026-05-20 — Added explicit `AcknowledgeAlarmRequest => GatewayScopes.InvokeWrite` and `QueryActiveAlarmsRequest => GatewayScopes.EventsRead` arms to `GatewayGrpcScopeResolver.ResolveRequiredScope` (`src/MxGateway.Server/Security/Authorization/GatewayGrpcScopeResolver.cs:21-22`). `InvokeWrite` matches the existing `MxCommandKind.Write*` mapping because ack mutates alarm state; `EventsRead` matches `StreamEventsRequest` and `MxCommandKind.DrainEvents` because querying active alarms reads the same alarm/event surface. Extended `GatewayGrpcScopeResolverTests` with two new `InlineData` rows covering both request types (`src/MxGateway.Tests/Security/Authorization/GatewayGrpcScopeResolverTests.cs:16-17`) and added four interceptor-level cases in `GatewayGrpcAuthorizationInterceptorTests` (`UnaryServerHandler_AcknowledgeAlarmMissingScope_ReturnsPermissionDenied`, `UnaryServerHandler_AcknowledgeAlarmWithScope_RunsHandler`, `ServerStreamingServerHandler_QueryActiveAlarmsMissingScope_ReturnsPermissionDenied`, `ServerStreamingServerHandler_QueryActiveAlarmsWithScope_RunsHandler`) proving each new RPC denies callers lacking the chosen scope and runs the handler when the scope is held. Updated `docs/Authorization.md` (resolver snippet and Scope Catalog table) to list both RPCs against their scopes. `dotnet test ... --filter FullyQualifiedName~GatewayGrpcAuthorizationInterceptorTests` → 14 passed, 0 failed; resolver tests 28 passed, 0 failed.
**Resolution:** 2026-05-20 — Added explicit `AcknowledgeAlarmRequest => GatewayScopes.InvokeWrite` and `QueryActiveAlarmsRequest => GatewayScopes.EventsRead` arms to `GatewayGrpcScopeResolver.ResolveRequiredScope` (`src/MxGateway.Server/Security/Authorization/GatewayGrpcScopeResolver.cs:21-22`). `InvokeWrite` matches the existing `MxCommandKind.Write*` mapping because ack mutates alarm state; `EventsRead` matches `StreamEventsRequest` and `MxCommandKind.DrainEvents` because querying active alarms reads the same alarm/event surface. Extended `GatewayGrpcScopeResolverTests` with two new `InlineData` rows covering both request types (`src/MxGateway.Tests/Security/Authorization/GatewayGrpcScopeResolverTests.cs:16-17`) and added four interceptor-level cases in `GatewayGrpcAuthorizationInterceptorTests` (`UnaryServerHandler_AcknowledgeAlarmMissingScope_ReturnsPermissionDenied`, `UnaryServerHandler_AcknowledgeAlarmWithScope_RunsHandler`, `ServerStreamingServerHandler_QueryActiveAlarmsMissingScope_ReturnsPermissionDenied`, `ServerStreamingServerHandler_QueryActiveAlarmsWithScope_RunsHandler`) proving each new RPC denies callers lacking the chosen scope and runs the handler when the scope is held. Updated `docs/Authorization.md` (resolver snippet and Scope Catalog table) to list both RPCs against their scopes. Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; the snippet still shows `AcknowledgeAlarmRequest => GatewayScopes.InvokeWrite` / `QueryActiveAlarmsRequest => GatewayScopes.EventsRead` and both RPCs still appear in the Scope Catalog rows. `dotnet test ... --filter FullyQualifiedName~GatewayGrpcAuthorizationInterceptorTests` → 14 passed, 0 failed; resolver tests 28 passed, 0 failed.
### Server-018
@@ -559,7 +559,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Rewrite the `IAlarmRpcDispatcher` `<remarks>` block to match the language now used on `WorkerAlarmRpcDispatcher` and on the gRPC service: DI binds `WorkerAlarmRpcDispatcher` by default; `NotWiredAlarmRpcDispatcher` is only the null fallback for tests/DI omission. Drop the "PR A.6 / A.7" prefix from the `<summary>` — the interface is now the public alarm-RPC seam.
**Resolution:** 2026-05-20 — Rewrote `IAlarmRpcDispatcher`'s `<summary>` and `<remarks>` (`src/MxGateway.Server/Sessions/IAlarmRpcDispatcher.cs`) to match the language now used on `WorkerAlarmRpcDispatcher` and on `MxAccessGatewayService.AcknowledgeAlarm` / `QueryActiveAlarms`: dropped the stale "PR A.6 / A.7" prefix from the summary, and replaced the "this PR ships a not-yet-wired default that returns a clear worker-pending diagnostic" clause with the correct statement that DI binds the production `WorkerAlarmRpcDispatcher` by default and `NotWiredAlarmRpcDispatcher` is only the null fallback for DI omission / standalone tests. Pure documentation change; no test.
**Resolution:** 2026-05-20 — Rewrote `IAlarmRpcDispatcher`'s `<summary>` and `<remarks>` (`src/MxGateway.Server/Sessions/IAlarmRpcDispatcher.cs`) to match the language now used on `WorkerAlarmRpcDispatcher` and on `MxAccessGatewayService.AcknowledgeAlarm` / `QueryActiveAlarms`: dropped the stale "PR A.6 / A.7" prefix from the summary, and replaced the "this PR ships a not-yet-wired default that returns a clear worker-pending diagnostic" clause with the correct statement that DI binds the production `WorkerAlarmRpcDispatcher` by default and `NotWiredAlarmRpcDispatcher` is only the null fallback for DI omission / standalone tests. Pure documentation change; no test. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but no longer verifiable in place — `IAlarmRpcDispatcher.cs` was deleted with the rest of the dispatcher trio in `dc9c0c9`. No stale "PR A.6/A.7" or "not-yet-wired" prose survives anywhere in Server source. Nothing to re-fix.
### Server-023
@@ -574,7 +574,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Replace the `<summary>` and `<remarks>` on `NotWiredAlarmRpcDispatcher` with text that matches the language now used on the interface and `WorkerAlarmRpcDispatcher` — "null fallback `IAlarmRpcDispatcher` used when no dispatcher is registered (DI omission / standalone tests); production wires `WorkerAlarmRpcDispatcher`." Either drop the `AcknowledgeAsync` diagnostic string's dev-rig framing entirely or shorten it to "alarm dispatcher is not registered." `#pragma warning disable CS1998` on `QueryActiveAlarmsAsync` is correct here (empty stream is intentional for the null fallback) and should stay.
**Resolution:** 2026-05-20 — Rewrote `NotWiredAlarmRpcDispatcher` summary/remarks as the null-fallback dispatcher and shortened the `AcknowledgeAsync` diagnostic to "Alarm dispatcher is not registered."; updated the two tests that asserted the old "worker"-prefixed diagnostic.
**Resolution:** 2026-05-20 — Rewrote `NotWiredAlarmRpcDispatcher` summary/remarks as the null-fallback dispatcher and shortened the `AcknowledgeAsync` diagnostic to "Alarm dispatcher is not registered."; updated the two tests that asserted the old "worker"-prefixed diagnostic. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but no longer verifiable in place — `NotWiredAlarmRpcDispatcher.cs` and its tests were deleted with the dispatcher trio in `dc9c0c9`. Nothing to re-fix.
### Server-024
@@ -637,7 +637,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Update the `ResolveCommandScope` snippet to include the four bulk-write arms. Update the Constraint Enforcement prose to enumerate the bulk read/write commands that are actually filtered, and reference the per-entry index-ordered merge that `BulkConstraintPlan.MergeDeniedInto` performs. Adding `ReadBulk` to the `InvokeRead` row of the Scope Catalog would also be useful — the table currently lists `Register`/`AddItem`/`Advise` against `InvokeRead` but not `ReadBulk`.
**Resolution:** 2026-05-20 — Updated the `ResolveCommandScope` snippet in `docs/Authorization.md` to enumerate the four bulk-write arms (`WriteBulk`/`Write2Bulk` against `InvokeWrite`, `WriteSecuredBulk`/`WriteSecured2Bulk` against `InvokeSecure`); expanded the Constraint Enforcement prose to list `ReadBulk` and all four bulk-write commands and to call out `BulkConstraintPlan.MergeDeniedInto`'s index-ordered merge; added `ReadBulk` to the `InvokeRead` row of the Scope Catalog.
**Resolution:** 2026-05-20 — Updated the `ResolveCommandScope` snippet in `docs/Authorization.md` to enumerate the four bulk-write arms (`WriteBulk`/`Write2Bulk` against `InvokeWrite`, `WriteSecuredBulk`/`WriteSecured2Bulk` against `InvokeSecure`); expanded the Constraint Enforcement prose to list `ReadBulk` and all four bulk-write commands and to call out `BulkConstraintPlan.MergeDeniedInto`'s index-ordered merge; added `ReadBulk` to the `InvokeRead` row of the Scope Catalog. Re-verified present 2026-08-18 (feat/followups-tickets) — all three corrections still stand in `docs/Authorization.md`: the snippet carries the four bulk-write arms, the Constraint Enforcement prose names `ReadBulk` plus the four bulk-write commands and `BulkConstraintPlan.MergeDeniedInto`, and `ReadBulk` is in the `InvokeRead` catalog row.
### Server-028
@@ -670,7 +670,7 @@ closed. New findings filed against this pass: Server-051..053.
**Recommendation:** Either (a) extend the advertised list with `bulk-read-command` and `bulk-write-commands` (`WriteBulk` / `Write2Bulk` / `WriteSecuredBulk` / `WriteSecured2Bulk` collectively), or (b) document in `gateway.md` and `docs/Contracts.md` that `Capabilities` is informational only and not the contract version. Option (a) is the simplest forward-compatible fix and keeps the capability token shape clients are already familiar with.
**Resolution:** 2026-05-20 — Extended the `OpenSession` capabilities list with `bulk-read-commands` and `bulk-write-commands` alongside the existing `bulk-subscribe-commands` token, so clients that gate on capability strings have an explicit signal for the bulk-read and bulk-write families.
**Resolution:** 2026-05-20 — Extended the `OpenSession` capabilities list with `bulk-read-commands` and `bulk-write-commands` alongside the existing `bulk-subscribe-commands` token, so clients that gate on capability strings have an explicit signal for the bulk-read and bulk-write families. Re-verified present 2026-08-18 (feat/followups-tickets) — `MxAccessGatewayService.OpenSession` still adds all three tokens.
### Server-030
@@ -839,7 +839,7 @@ Add a regression test that advises N items without an active `StreamEvents` cons
**Recommendation:** Before the EventsHub is exercised by Admin-only sessions or session-scoped Viewer roles, gate `SubscribeSession` on a session-access check — either via a per-session role check in the hub method itself, or by storing a per-user allowed-session-id set in the connection's `Context.Items` at connect time and rejecting subscribes outside that set. The current dashboard surfaces only a per-page Session Details view that the page can prove it's authorized for, but as soon as a Viewer role exists the gap matters.
**Resolution:** 2026-05-24 — Documented the v1 acceptance per the prompt's "practical fix for v1" direction. Added a detailed `<remarks>` block to `EventsHub.SubscribeSession` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs`) stating that (a) in v1 the hub-level `HubClientsPolicy` only requires one of the dashboard roles (Admin or Viewer) and both may subscribe to any session id, (b) this is acceptable today because the dashboard's per-session views show non-secret session metadata any authenticated user can already see and value logging is gated by the same redaction policy, and (c) the per-session ACL that gates the gRPC `StreamEvents` RPC is intentionally not yet mirrored here. Added an explicit `TODO(per-session-acl)` describing the future enforcement seam — once a role/scope is introduced that scopes a Viewer to a specific session or tenant, add a session-access check at this method (inline on `Context.User` claims/`Context.Items`, or via a dedicated authorization policy applied to the hub method). No code-behavior change in this pass; the per-session ACL data model design is out of scope for the resolution window. No new regression test (the change is documentation-only).
**Resolution:** 2026-05-24 — Documented the v1 acceptance per the prompt's "practical fix for v1" direction. Added a detailed `<remarks>` block to `EventsHub.SubscribeSession` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs`) stating that (a) in v1 the hub-level `HubClientsPolicy` only requires one of the dashboard roles (Admin or Viewer) and both may subscribe to any session id, (b) this is acceptable today because the dashboard's per-session views show non-secret session metadata any authenticated user can already see and value logging is gated by the same redaction policy, and (c) the per-session ACL that gates the gRPC `StreamEvents` RPC is intentionally not yet mirrored here. Added an explicit `TODO(per-session-acl)` describing the future enforcement seam — once a role/scope is introduced that scopes a Viewer to a specific session or tenant, add a session-access check at this method (inline on `Context.User` claims/`Context.Items`, or via a dedicated authorization policy applied to the hub method). No code-behavior change in this pass; the per-session ACL data model design is out of scope for the resolution window. No new regression test (the change is documentation-only). Re-verified 2026-08-18 (feat/followups-tickets): the documented v1 gap was subsequently **closed**, not regressed — `EventsHub` now takes `IDashboardSessionAcl` and `SubscribeSession` throws `HubException("Not authorized for this session.")` when `CanViewSession` denies (SEC-25 / TST-15). The `TODO(per-session-acl)` is correctly gone and the `<remarks>` block now describes the shipped tag-scoped gate (admin bypass first, tag intersection, `Dashboard:UntaggedSessionVisibility`, phantom-id denial for non-Admins). Nothing to re-fix; the finding's own recommendation is what shipped.
### Server-039
@@ -869,7 +869,7 @@ Add a regression test that advises N items without an active `StreamEvents` cons
**Recommendation:** Add a one-line comment above the loop explaining the precedence: full DN/CN literal first, leading-RDN fallback second. Mention the case-insensitive map comparer (`OrdinalIgnoreCase`) so the next reader doesn't ask why `"GwAdmin"` matches `"gwadmin"`.
**Resolution:** 2026-05-24 — Added a precedence comment block above the lookup in `MapGroupsToRoles` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs:156-163`) explaining that the full literal group string is tried first and the leading-RDN value (e.g. `GwAdmin` extracted from `ou=GwAdmin,ou=groups,...`) is the fallback, and back-referencing `DashboardOptions.GroupToRole` as the source of the `OrdinalIgnoreCase` comparer so a maintainer sees why `"GwAdmin"` matches `"gwadmin"`. No code change — existing `DashboardAuthenticatorTests.MapGroupsToRoles_ResolvesByShortNameAndDistinguishedName` already pins both the full-match and RDN-fallback paths and the case-insensitive lookup; pure documentation-only resolution, no new test.
**Resolution:** 2026-05-24 — Added a precedence comment block above the lookup in `MapGroupsToRoles` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs:156-163`) explaining that the full literal group string is tried first and the leading-RDN value (e.g. `GwAdmin` extracted from `ou=GwAdmin,ou=groups,...`) is the fallback, and back-referencing `DashboardOptions.GroupToRole` as the source of the `OrdinalIgnoreCase` comparer so a maintainer sees why `"GwAdmin"` matches `"gwadmin"`. No code change — existing `DashboardAuthenticatorTests.MapGroupsToRoles_ResolvesByShortNameAndDistinguishedName` already pins both the full-match and RDN-fallback paths and the case-insensitive lookup; pure documentation-only resolution, no new test. Regressed or never applied; re-fixed 2026-08-18 in `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs` (feat/followups-tickets). Chain of events: the lookup moved out of `DashboardAuthenticator` into the shared `DashboardGroupRoleMapping` helper in `792e3f9` (the `IGroupRoleMapper<string>` seam) **with the comment intact**, then `fca978d` ("docs(src): add missing XML docs and strip tracking-ID comments") deleted the entire block rather than just the `(Server-040)` marker inside it. That also took out a second, substantive paragraph added later — the note that the shared `ZB.MOM.WW.Auth.Ldap` provider already strips groups to short RDN names, making the RDN fallback a no-op on the live login path and making a full-DN `GroupToRole` **key** unsupported. Both paragraphs are restored, minus the tracking IDs (that part of the sweep's intent is respected).
### Server-041
@@ -914,7 +914,7 @@ Add a regression test that advises N items without an active `StreamEvents` cons
**Recommendation:** Add a `<remarks>` block to `HubTokenService` noting "Registered as a singleton in `AddGatewayDashboard`; the underlying `ITimeLimitedDataProtector` is thread-safe and shared across hub-token issuance and validation." Optionally add a comment near the DI registration explaining the lifetime contract.
**Resolution:** 2026-05-24 — Added a `<remarks>` block to `HubTokenService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/HubTokenService.cs`) documenting that the service is registered as a singleton in `DashboardServiceCollectionExtensions.AddGatewayDashboard` and is shared by two consumer scopes — `DashboardHubConnectionFactory` (scoped, per-circuit; calls `Issue` from the cookie-authenticated dashboard) and `HubTokenAuthenticationHandler` (transient, per-request; calls `Validate` from the SignalR negotiate / connection path). Notes that the underlying `ITimeLimitedDataProtector` is thread-safe so concurrent mint/validate from any number of callers is safe, and explicitly asks future maintainers to preserve the singleton lifetime to keep the protector instance stable. Pure documentation change; no test.
**Resolution:** 2026-05-24 — Added a `<remarks>` block to `HubTokenService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/HubTokenService.cs`) documenting that the service is registered as a singleton in `DashboardServiceCollectionExtensions.AddGatewayDashboard` and is shared by two consumer scopes — `DashboardHubConnectionFactory` (scoped, per-circuit; calls `Issue` from the cookie-authenticated dashboard) and `HubTokenAuthenticationHandler` (transient, per-request; calls `Validate` from the SignalR negotiate / connection path). Notes that the underlying `ITimeLimitedDataProtector` is thread-safe so concurrent mint/validate from any number of callers is safe, and explicitly asks future maintainers to preserve the singleton lifetime to keep the protector instance stable. Pure documentation change; no test. Re-verified present 2026-08-18 (feat/followups-tickets) — the `<remarks>` block still names the `AddGatewayDashboard` singleton registration, the two consumer scopes, the thread-safe `ITimeLimitedDataProtector`, and the preserve-the-lifetime request. One consumer was renamed since (the issuing side is now the `/hubs/token` endpoint rather than `DashboardHubConnectionFactory`) and the doc tracked that change correctly.
## Re-review 2026-05-24 (commit 42b0037)
@@ -983,7 +983,7 @@ The user-visible difference: rotating/revoking/deleting a key vs closing/killing
**Recommendation:** Align `ApiKeysPage.ConfirmPendingAsync` with the sessions pages: hold `PendingAction`, set `IsBusy = true`, run the action, then clear `PendingAction` in the `finally`. The current ApiKeysPage shape was inherited from before the dialog existed (when the confirmation was a `confirm()` JS call); the dialog component change can flatten the difference now. As a smaller alternative, document the divergence on the component's XML doc — but the shared component should ideally be used consistently.
**Resolution:** 2026-05-24 — Took the recommended alignment. `ApiKeysPage.ConfirmPendingAsync` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor`) now holds `PendingAction` for the duration of the awaited action (so the shared `ConfirmDialog` renders its `IsBusy` in-flight state on the dialog itself, matching the sessions pages) and clears it in `finally` regardless of outcome. The action is captured up front so a clear in `finally` works even when the action throws. `RunManagementActionAsync` continues to drive `IsBusy = true` inside its own `try/finally`, so the dialog now correctly disables Confirm/Cancel while the awaited service call runs. Pure UX-consistency change; no new automated test (no bUnit harness in the test project — same precedent as Server-010).
**Resolution:** 2026-05-24 — Took the recommended alignment. `ApiKeysPage.ConfirmPendingAsync` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor`) now holds `PendingAction` for the duration of the awaited action (so the shared `ConfirmDialog` renders its `IsBusy` in-flight state on the dialog itself, matching the sessions pages) and clears it in `finally` regardless of outcome. The action is captured up front so a clear in `finally` works even when the action throws. `RunManagementActionAsync` continues to drive `IsBusy = true` inside its own `try/finally`, so the dialog now correctly disables Confirm/Cancel while the awaited service call runs. Pure UX-consistency change; no new automated test (no bUnit harness in the test project — same precedent as Server-010). Re-verified present 2026-08-18 (feat/followups-tickets) — `ApiKeysPage.ConfirmPendingAsync` still captures the action up front, awaits inside `try`, and clears `PendingAction` in `finally`; the explanatory comment survives.
### Server-048
@@ -1017,7 +1017,7 @@ The user-visible difference: rotating/revoking/deleting a key vs closing/killing
**Recommendation:** Add `<summary>` blocks to `IDashboardSessionAdminService.CanManage` (states the Admin-role gate), `CloseSessionAsync` and `KillWorkerAsync` (state that missing sessions return `DashboardSessionAdminResult.Fail(...)` rather than throwing, and that the audit log captures actor + remote IP). Add `<param>` and `<returns>` for the request/response shape. The same sweep can pick up the longstanding gap on `IDashboardApiKeyManagementService` if the team wants — but the new file is the load-bearing one.
**Resolution:** 2026-05-24 — Added `<summary>` + `<remarks>` blocks to every member of `IDashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardSessionAdminService.cs`): an interface-level `<remarks>` describing the Admin-role gate, audit log shape, and `DashboardSessionAdminResult.Fail` semantics; per-member docs on `CanManage`, `CloseSessionAsync`, and `KillWorkerAsync` calling out the missing-session-returns-Fail contract and the `dashboard-admin-kill` reason constant that reaches the worker-kill audit log and `mxgateway.workers.killed` counter tag. `DashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs`) picked up a class-level `<summary>` + `<remarks>` describing the per-page audit-log seam, plus `<inheritdoc />` on each public method. Pure documentation change; no test (the behavioral contracts the docs describe are already exercised by the existing `DashboardSessionAdminServiceTests` cases).
**Resolution:** 2026-05-24 — Added `<summary>` + `<remarks>` blocks to every member of `IDashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardSessionAdminService.cs`): an interface-level `<remarks>` describing the Admin-role gate, audit log shape, and `DashboardSessionAdminResult.Fail` semantics; per-member docs on `CanManage`, `CloseSessionAsync`, and `KillWorkerAsync` calling out the missing-session-returns-Fail contract and the `dashboard-admin-kill` reason constant that reaches the worker-kill audit log and `mxgateway.workers.killed` counter tag. `DashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs`) picked up a class-level `<summary>` + `<remarks>` describing the per-page audit-log seam, plus `<inheritdoc />` on each public method. Pure documentation change; no test (the behavioral contracts the docs describe are already exercised by the existing `DashboardSessionAdminServiceTests` cases). Re-verified present 2026-08-18 (feat/followups-tickets) — the interface-level `<remarks>` and all three per-member blocks are intact, including the `dashboard-admin-kill` reason constant and the missing-session-returns-`Fail` contract.
### Server-050
@@ -1078,7 +1078,7 @@ catch (Exception ex) { _logger.LogWarning(ex, "...continuing with configuration-
**Recommendation:** For (1), align the `IAlarmWatchListResolver` doc with whatever Server-051 settles on. For (2), either restrict the exclude to GR-discovered rows (apply `RemoveAll` before appending the `IncludeAttributes` entries) or update the option XML doc and `GatewayConfiguration.md` to say excludes are applied to the merged GR-plus-include list and therefore also suppress matching explicit includes.
**Resolution:** Resolved 2026-06-15. (1) No longer over-promises: the Server-051 fix makes the implementation propagate `OperationCanceledException`, so the `IAlarmWatchListResolver.ResolveAsync` `<returns>` doc is now accurate and was left unchanged. (2) Kept the "excludes win" code behaviour (excludes applied to the merged GR-plus-include list) and corrected the prose to match: `AlarmDiscoveryOptions.ExcludeAttributes` XML doc and `docs/GatewayConfiguration.md:247` now state the exclude runs after the GR rows and explicit `IncludeAttributes` are combined, so an exclude matching an explicit include suppresses it too. The "excludes win" precedence is pinned by `AlarmWatchListResolverTests.ResolveAsync_ExcludeAlsoSuppressesMatchingExplicitInclude`.
**Resolution:** Resolved 2026-06-15. (1) No longer over-promises: the Server-051 fix makes the implementation propagate `OperationCanceledException`, so the `IAlarmWatchListResolver.ResolveAsync` `<returns>` doc is now accurate and was left unchanged. (2) Kept the "excludes win" code behaviour (excludes applied to the merged GR-plus-include list) and corrected the prose to match: `AlarmDiscoveryOptions.ExcludeAttributes` XML doc and `docs/GatewayConfiguration.md:247` now state the exclude runs after the GR rows and explicit `IncludeAttributes` are combined, so an exclude matching an explicit include suppresses it too. The "excludes win" precedence is pinned by `AlarmWatchListResolverTests.ResolveAsync_ExcludeAlsoSuppressesMatchingExplicitInclude`. Re-verified present 2026-08-18 (feat/followups-tickets) — both doc corrections stand: `IAlarmWatchListResolver.ResolveAsync`'s `<returns>` still documents cancellation propagation (and the implementation still honors it), and the "excludes win over explicit includes" wording is present on `AlarmDiscoveryOptions.ExcludeAttributes` (now in `Configuration/AlarmFallbackOptions.cs`) and in the `docs/GatewayConfiguration.md` option table.
### Server-053
@@ -1113,7 +1113,7 @@ Additionally, `GatewayAlarmMonitor.ApplyProviderModeChangeAsync` increments the
**Recommendation:** Update both `DesignDecisions.md` sections and the revisit list to describe the shipped behavior (gated by `AllowMultipleEventSubscribers`, `DetachGraceSeconds`, replay options), and amend the CLAUDE.md convention bullet.
**Resolution:** 2026-06-16: updated `docs/DesignDecisions.md` (Session Reconnect section rewritten to describe the shipped detach-grace + replay-on-reconnect behavior with config references; Event Subscribers section rewritten to describe the config-gated multi-subscriber fan-out, mode-dependent `FailFast` semantics, and internal vs external subscriber distinction; Later Revisit Items list removes the two shipped items and records them as shipped with config cross-references) and the `CLAUDE.md` conventions bullet to describe the shipped config-gated multi-subscriber + reconnect-replay behavior while preserving the one-worker-per-session invariant.
**Resolution:** 2026-06-16: updated `docs/DesignDecisions.md` (Session Reconnect section rewritten to describe the shipped detach-grace + replay-on-reconnect behavior with config references; Event Subscribers section rewritten to describe the config-gated multi-subscriber fan-out, mode-dependent `FailFast` semantics, and internal vs external subscriber distinction; Later Revisit Items list removes the two shipped items and records them as shipped with config cross-references) and the `CLAUDE.md` conventions bullet to describe the shipped config-gated multi-subscriber + reconnect-replay behavior while preserving the one-worker-per-session invariant. Re-verified present 2026-08-18 (feat/followups-tickets) — `docs/DesignDecisions.md` still records the "no reconnectable sessions" constraint as superseded and documents `DetachGraceSeconds` / `AllowMultipleEventSubscribers` / `ReplayBufferCapacity`, the Later Revisit Items list still cross-references them as shipped, and the `CLAUDE.md` bullet still carries the config-gated wording.
### Server-055
@@ -1158,7 +1158,7 @@ Additionally, `GatewayAlarmMonitor.ApplyProviderModeChangeAsync` increments the
**Recommendation:** Either (a) extend `NormalizeOutboundCommand` and the `MapCommand` tracking path to normalize each `AddItemBulk.TagAddresses` entry (and `AddBufferedItem.ItemDefinition`) the same `IsArray`-gated way, keeping the constraint check, the worker bind, and the stored `SessionItemRegistration.TagAddress` consistent; or (b) if bulk-add normalization is intentionally out of scope for this feature, state that explicitly in `gateway.md` and the client READMEs (alongside the existing `ReadBulk` carve-out) so clients know bulk-added array handles must carry the `[]` suffix themselves to be writable.
**Resolution:** 2026-06-18 — Took option (a). Root cause confirmed: `NormalizeOutboundCommand` had no `AddItemBulk`/`AddBufferedItem` case, so the worker bound bare (non-write-capable) array handles for those paths while single-add was already fixed. Added `AddItemBulk` (normalizes each `TagAddresses` entry in place) and `AddBufferedItem` (normalizes `ItemDefinition`) cases to `NormalizeOutboundCommand`; added the matching `AddBufferedItem` normalization to the `TrackCommandReply`/`MapCommand` tracking path (its registration keys off the command's `ItemDefinition`). `AddItemBulk` tracking needs no change — the worker echoes the already-suffixed address back in each `SubscribeResult.TagAddress`, which `TrackBulkItems` stores. Authz is unchanged and consistent: `FilterTagBulkAsync` checks the bare address through `ConstraintEnforcer.ResolveTarget`'s `[]` fallback, mirroring single-add. Updated `gateway.md` and all five client READMEs (dotnet/go/python/rust/java) so the add-family normalization no longer reads as AddItem-only; the `ReadBulk` carve-out stays. Regression tests: `GatewayArrayWriteWiringTests.AddItemBulk_BareArrayAddress_NormalizedOnWireAndInRegistration`, `.AddBufferedItem_BareArrayAddress_NormalizedOnWireAndInRegistration`.
**Resolution:** 2026-06-18 — Took option (a). Root cause confirmed: `NormalizeOutboundCommand` had no `AddItemBulk`/`AddBufferedItem` case, so the worker bound bare (non-write-capable) array handles for those paths while single-add was already fixed. Added `AddItemBulk` (normalizes each `TagAddresses` entry in place) and `AddBufferedItem` (normalizes `ItemDefinition`) cases to `NormalizeOutboundCommand`; added the matching `AddBufferedItem` normalization to the `TrackCommandReply`/`MapCommand` tracking path (its registration keys off the command's `ItemDefinition`). `AddItemBulk` tracking needs no change — the worker echoes the already-suffixed address back in each `SubscribeResult.TagAddress`, which `TrackBulkItems` stores. Authz is unchanged and consistent: `FilterTagBulkAsync` checks the bare address through `ConstraintEnforcer.ResolveTarget`'s `[]` fallback, mirroring single-add. Updated `gateway.md` and all five client READMEs (dotnet/go/python/rust/java) so the add-family normalization no longer reads as AddItem-only; the `ReadBulk` carve-out stays. Regression tests: `GatewayArrayWriteWiringTests.AddItemBulk_BareArrayAddress_NormalizedOnWireAndInRegistration`, `.AddBufferedItem_BareArrayAddress_NormalizedOnWireAndInRegistration`. Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; `gateway.md` still describes normalization across the whole add family (`AddItem`/`AddItem2`/`AddItemBulk`/`AddBufferedItem`) and all five client READMEs still name the bulk and buffered members (Python in its snake_case form: `add_item_bulk` / `add_buffered_item`).
### Server-058
+141 -2
View File
@@ -24,13 +24,13 @@ the blocker rather than rediscovering it.
|---|---|
| GUID stable across polls and `ALM → RTN` | Answered — yes (2026-05-01 capture in `AlarmClientDiscovery.md`, reconfirmed 2026-08-18) |
| GUID stable across clear-then-re-raise | Answered 2026-08-18 — **no**, a re-raise mints a new GUID |
| GUID stable across `UNACK → ACK` | **Open** — the rig cannot be driven into an acknowledged state at all |
| GUID stable across `UNACK → ACK` | **Open** — the rig cannot be driven into an acknowledged state; 2026-08-18 (third attempt) identifies the reason as the test attribute's `MxSecurityOperate` classification, which no non-interactive ack surface on the rig can satisfy |
| `COUNT` = total active vs records-in-reply under a capped fetch | Answered 2026-08-18 — **records in the reply** |
The 2026-08-17 run below is kept because it is the record of the wrong-verb blocker. The
2026-08-18 run cleared that blocker with `AuthenticateUser` + `WriteSecured` and answered
two of the three questions; the acknowledge leg is now blocked on something narrower and
different, described in "Second attempt".
different, described in "Second attempt" and diagnosed in "Third attempt".
## First attempt (2026-08-17): plain `Write`
@@ -264,8 +264,147 @@ MXAccess-side entry point and the wnwrap-side entry point is inert.
spurious Clear plus a spurious Raise, which is the same shape the now-observed re-raise
behaviour produces and which `ComputeTransitions` already handles as two instances.
## Third attempt (2026-08-18): why the acknowledge is refused
The second attempt left two candidate explanations for `AlarmAckByName` returning `rc=0`
and changing nothing: the rig enforces an acknowledgement security requirement the wnwrap
consumer cannot meet, or wnwrap's ack is simply broken here. This attempt was read-only —
no writes, no alarms raised, no configuration touched — and settles the first question:
the requirement is **enforced**, and the alarm attribute's security classification is what
enforces it.
### Method
Read-only inspection of the `ZB` Galaxy Repository over `sqlcmd -S localhost -d ZB -E`,
plus the already-built `mxa` CLI (`C:\Users\dohertj2\Desktop\wwtools\mxaccesscli\src\MxAccess.Cli\bin\x86\Release\net48\mxa.exe`)
for runtime reads. Nothing was written and nothing was installed. The `lmxopcua\gr` schema
notes referenced elsewhere in this repo are **not present on this box** (`Test-Path` is
`False`), so the schema was located by querying `sys.tables` / `sys.columns` directly.
### The test attribute is classified `MxSecurityOperate`
UDA security classification lives in `dynamic_attribute.security_classification`, keyed by
`gobject_id` — for the `$TestMachine` template that is `1055`:
```
attribute_name | security_classification | mx_attribute_category
ProtectedValue | 2 | 10
ProtectedValue1 | 3 | 10
TestAlarm001 | 1 | 10
TestAlarm002 | 1 | 10
TestAlarm003 | 1 | 10
TestChangingInt | 1 | 10
```
The enum is pinned by two independent sources rather than assumed. `ProtectedValue` and
`ProtectedValue1` are the mxaccess analysis project's documented *secured-write* and
*verified-write* fixtures (`C:\Users\dohertj2\Desktop\mxaccess\docs\galaxy-test-fixtures.md`),
and `docs/NMX-COM-Contracts.md` in the same project records "Galaxy security classification
(`2` for `SecuredWrite`, `3` for VerifiedWrite)" — so `2` and `3` land on exactly the two
attributes that are supposed to carry them. `galaxy-test-fixtures.md` also records the
provisioning verb used for every UDA in that inventory: `--security MxSecurityOperate`.
`TestAlarm001` therefore reads `1` = **`MxSecurityOperate`**.
That is the missing piece from 2026-08-17. `Operate` is not free access: it requires an
authenticated galaxy identity holding Operate permission on the object's security group.
An unauthenticated `Write` is refused with `SecurityError` `1008`, and the same write after
`AuthenticateUser` succeeds — which is precisely the pair of results both prior attempts
recorded. Galaxy security is live on this rig, not disabled.
`wwAlarmConsumerClass.AlarmAckByName` carries an operator *name* string and a comment. It
carries no authenticated user id, no credential, and no token — there is no parameter on
either the 6-arg or the 8-arg overload that could convey one. A consumer calling it cannot
satisfy an `Operate` classification, and `rc=0` followed by no state change is what an ack
dropped downstream of an accepted call looks like.
The one step this stops short of is a direct experiment: the rig's security configuration
is out of scope for a read-only probe, so "an `Operate` alarm cannot be acknowledged
without an authenticated identity" is inferred from the classification plus the observed
`1008`/`rc=0` pattern rather than observed by relaxing the classification and watching the
ack start working. The human step below is exactly that experiment.
### There is no writeable `.Ack` attribute — the second attempt targeted the only one there is
A plausible reading of the second attempt was that it wrote the wrong sub-attribute:
ArchestrA alarm extensions are commonly described as exposing a writeable `.Ack` alongside
the read-only `.Acked`. On this galaxy they do not. Every `Ack`-named attribute the alarm
primitives define:
```
attribute_name | security_classification | mx_attribute_category
Acked | -1 | 2
AckMsg | 0 | 6
Bad.Acked | -1 | 2
Bad.AckMsg | 0 | 6
TimeAlarmAcked | -1 | 2
AlarmAckCnt | -1 | 2
AlarmAckErrorsCnt | -1 | 2
AlarmMostUrgentAcked | -1 | 2
AlarmUnAckedCnt | -1 | 2
```
No `Ack`. `Acked` carries `security_classification = -1` — no classification at all, which
is what a non-writeable attribute carries, as distinct from the `1` on the writeable
`TestAlarm001`. That is the configuration-side counterpart of the `detail=1007`
operational refusal the second attempt got when it wrote `.Acked`: not a permission
refusal, but an attribute that has no write path to refuse. The second attempt had already
found the only MXAccess-side candidate, and it is read-only by definition.
### No non-interactive acknowledge surface is installed
The `wwtools` collection on the box (`aalogcli`, `aot`, `graccesscli`, `grdb`, `histdb`,
`mbproxy`, `mxaccesscli`, `secrets`) is the most likely home for a scriptable ack. There
isn't one. `mxa --help` lists `diag`, `info`, `read`, `read-batch`, `subscribe`,
`subscribe-batch`, `write`, `write-batch` — a tag data-plane only, with no alarm surface.
`graccesscli` is a Galaxy Repository configuration tool (`object uda add`, `instance
deploy`), which acts at configure/deploy time and not on live alarm state. Reading the
galaxy's own authentication mode at runtime is also unavailable: `Galaxy.AuthenticationMode`
does not resolve over MXAccess (`Category=4 Detail=6`), and the value is not in
`dynamic_attribute` — only the attribute *definition* names `AuthenticationMode` and
`_AuthenticationModeEnum` exist in `attribute_definition`.
So every remaining acknowledge surface on this rig is interactive: the IDE's alarm client,
InTouch, or an ArchestrA graphic bound to the alarm. Driving those is out of scope.
### Status of the acknowledge leg
**Unavailable by configuration, and the GUID question stays assumed.** The two are separate
statements and both matter:
- The wnwrap ack path is unavailable on this rig as configured, for an identified reason
rather than an unknown one. This is a real finding for the gateway: `AcknowledgeByName`
will behave the same way — accepted, inert — on any customer galaxy whose alarmed
attributes carry a non-free-access security classification. It is worth noting in the
alarm client's documentation that a silent `rc=0` is not proof of acknowledgement.
- Whether wnwrap re-mints the record GUID on `UNACK_ALM → ACK_ALM` is still unobserved, and
after three attempts it stays assumed. As the second attempt already noted, this remains
a documentation gap rather than a correctness one: a re-minted GUID produces a spurious
Clear plus a spurious Raise, the same shape the observed re-raise behaviour produces, and
`ComputeTransitions` already handles that correctly as two instances.
#### What a human would need to do
Either of these answers it; the second is cheaper and also confirms or refutes the
classification hypothesis above, which the read-only probe could only infer.
1. Acknowledge `TestMachine_001.TestAlarm001` from an interactive System Platform client
(IDE alarm client, InTouch, or an ArchestrA graphic) while a wnwrap probe polls
`GetXmlCurrentAlarms2` against `\\DESKTOP-6JL3KKO\Galaxy!TestArea`, and record whether
`STATE` reaches `ACK_ALM` and whether `GUID` survives the transition. Raise the alarm
first with `AuthenticateUser` + `WriteSecured` as the second attempt did.
2. Reclassify `TestAlarm001` on the `$TestMachine` template to free access and redeploy —
`graccesscli object uda ... --security MxSecurityFreeAccess` against `$TestMachine`,
then `instance deploy TestMachine_001` — and re-run the second attempt's ack probe
unchanged. If `AlarmAckByName` then moves `STATE` to `ACK_ALM`, the classification is
confirmed as the blocker and the GUID question is answered in the same run. Restore the
classification to `MxSecurityOperate` afterwards, since the secured-write fixtures in
`WorkerLiveMxAccessSmokeTests` depend on the alarm UDAs being secured.
### Rig state left behind
The third attempt changed nothing. `TestMachine_001.TestAlarm001` and its `.Acked` subtag
both read `false` at the end of the session, matching the state the second attempt left.
The three `TestMachine_00{1,2,3}.TestAlarm001` UDAs are back to `false` and their
`.InAlarm` subtags read `false`, but each leaves a `UNACK_RTN` record in the wnwrap
snapshot, since nothing can acknowledge them away. `SnapshotActiveAlarms` counts only
+11
View File
@@ -175,6 +175,17 @@ optional serialized constraints, and the `created_utc`, `last_used_utc`,
belong to `ZB.MOM.WW.Auth.ApiKeys`, this document does not restate their column
readers or SQL; consult the library for that detail.
One connection-level behavior is worth stating here because it is what keeps the
store usable under concurrency rather than an implementation detail of the schema:
the library's `AuthSqliteConnectionFactory.OpenConnectionAsync` opens pooled
connections with a non-zero command timeout and applies `PRAGMA journal_mode=WAL`
and `PRAGMA busy_timeout` (5 s). Last-used stamping runs on every authenticated
request and the audit store appends on every denial, so without WAL and a busy
timeout those concurrent writers would surface `SQLITE_BUSY` as a hard failure on
the request path instead of degrading gracefully. WAL is a persistent
database-level setting, so re-applying it per connection is a cheap no-op;
`busy_timeout` is per-connection state and must be set each time.
### Audit trail
The library emits its own API-key audit entries (from the admin verbs — create,
+4 -4
View File
@@ -186,7 +186,7 @@ hubs stay for the audience that genuinely needs a wire.
|---|---|---|
| every page deriving from `DashboardPageBase` | `IDashboardSnapshotFeed.WatchAsync` | `DashboardSnapshotFeed` (singleton) multicasting one `IDashboardSnapshotService.WatchSnapshotsAsync` enumeration |
| `SessionDetailsPage` | `IDashboardSessionEventSubscriber.Subscribe(sessionId)` | `DashboardEventBroadcaster` — the same singleton the session mirror publishes to, registered behind both interfaces |
| `AlarmsPage` | `IGatewayAlarmService.StreamAsync` | the central alarm monitor, **provider status only**; the alarm rows still come from the 3 s `QueryAlarmsAsync` poll |
| `AlarmsPage` | `IGatewayAlarmService.StreamAsync` | the central alarm monitor, **gateway-status frames only** — `provider_status` for the badge and `snapshot_status` for the truncated-snapshot banner; the alarm rows still come from the 3 s `QueryAlarmsAsync` poll |
The snapshot feed multicasts rather than handing each page its own enumeration:
`WatchSnapshotsAsync` is not multicast on its own — each enumeration owns a timer
@@ -288,9 +288,9 @@ Both seams consume the same producing services, so they share these cadences:
to its internal dashboard-mirror subscriber (independent of any gRPC `StreamEvents`);
- the alarms page's status feed resubscribes one second after its
`IGatewayAlarmService.StreamAsync` enumeration ends — the monitor completes a
subscriber's stream when it falls behind and again when it restarts, both
recoverable by resubscribing — and the badge and banner hold their last values in
between. That feed carries both gateway-status frames: `provider_status` drives the
subscriber's stream only when that subscriber has fallen behind, which resubscribing
recovers (a monitor restart keeps the channel and pushes cleared status frames through
it) — and the badge and banner hold their last values in between. That feed carries both gateway-status frames: `provider_status` drives the
badge, and `snapshot_status` drives the truncated-snapshot banner, so the caveat
appears on the monitor's verdict change rather than up to three seconds later. Every
subscriber is primed with a `snapshot_status` frame at open, so a page attaching
@@ -258,10 +258,13 @@
}
catch
{
// The monitor completes a subscriber's stream when it falls behind, and
// again when the monitor restarts. Both are recoverable by resubscribing;
// the badge and banner hold their last values in the meantime, and the
// resubscribe is primed with the current ones.
// The monitor drops a subscriber whose queue it cannot write to, completing
// that stream with an error; short of cancellation or disposal that is the
// only way this enumeration ends. A monitor restart is NOT one of them — it
// keeps the channel and pushes the cleared status frames through it — so this
// catch is the fell-behind case, recoverable by resubscribing. The badge and
// banner hold their last values meanwhile, and the resubscribe is primed with
// the current ones.
}
try
@@ -31,6 +31,23 @@ internal static class DashboardGroupRoleMapping
{
string normalizedGroup = group.Trim();
// Lookup precedence: the full literal group string is tried first; only if
// that misses do we fall back to the leading RDN value (e.g. "GwAdmin"
// extracted from "ou=GwAdmin,ou=groups,..."). The map's comparer is
// OrdinalIgnoreCase (see DashboardOptions.GroupToRole), so "GwAdmin" and
// "gwadmin" both match.
//
// With the shared ZB.MOM.WW.Auth.Ldap provider, groups arrive here already
// stripped to short RDN names (the library calls FirstRdnValue before
// returning them). So through the live login path the full-string branch
// only ever sees short names and the RDN fallback is effectively a no-op —
// they collapse to the same key. The fallback is retained because this
// mapping is also reachable directly via the IGroupRoleMapper<string> seam
// (DashboardGroupRoleMapper), where a caller could still pass a full DN.
// CONSEQUENCE: configuring a full-DN GroupToRole *key* (e.g.
// "ou=GwAdmin,ou=groups,...") is UNSUPPORTED with the shared library — the
// incoming group is a short name, so it will never equal a full-DN key.
// Keep GroupToRole keys as short group names.
if (groupToRole.TryGetValue(normalizedGroup, out string? mapped)
|| groupToRole.TryGetValue(ExtractFirstRdnValue(normalizedGroup), out mapped))
{
@@ -134,6 +134,44 @@ public sealed class AlarmsPageTruncationBannerTests
"banner to clear after a complete snapshot_status frame");
}
/// <summary>
/// Attach while the verdict is already truncated. The page carries no
/// priming logic of its own — it relies on <c>StreamAsync</c> opening every
/// subscription with a <c>snapshot_status</c> baseline — so the caveat has
/// to come up off the open sequence alone, with no edge pushed afterwards.
/// A page that only handled the edge would show an un-caveated alarm list
/// to every operator who opened it after the truncation began.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task AlarmsPage_AttachingToAnAlreadyTruncatedFeed_RaisesTheBannerFromThePriming()
{
// The monitor's open sequence: provider status, then the unconditional
// completeness baseline. Nothing is pushed after this.
ScriptedAlarmFeed feed = new()
{
Priming =
[
new AlarmFeedMessage { ProviderStatus = new AlarmProviderStatus() },
SnapshotStatusFrame(truncated: true),
],
};
await using ServiceProvider provider = BuildPushServices(feed);
await using HtmlRenderer renderer = new(
provider,
provider.GetRequiredService<ILoggerFactory>());
HtmlRootComponent page = await renderer.Dispatcher.InvokeAsync(
() => renderer.RenderComponentAsync<AlarmsPage>());
await WaitForHtmlAsync(
renderer,
page,
html => html.Contains(BannerMarker, StringComparison.Ordinal),
"banner to appear from the feed's open-time snapshot_status baseline");
}
private static AlarmFeedMessage SnapshotStatusFrame(bool truncated)
{
return new AlarmFeedMessage
@@ -280,6 +318,14 @@ public sealed class AlarmsPageTruncationBannerTests
/// <inheritdoc />
public bool SnapshotTruncated { get; set; }
/// <summary>
/// Frames replayed at the head of every subscription, standing in for the
/// monitor's open sequence (provider status, then the unconditional
/// completeness baseline). Empty means the subscriber sees only what the
/// test pushes.
/// </summary>
public IReadOnlyList<AlarmFeedMessage> Priming { get; init; } = [];
/// <summary>Pushes one frame onto the feed the page is subscribed to.</summary>
/// <param name="message">The feed frame to deliver.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
@@ -290,6 +336,12 @@ public sealed class AlarmsPageTruncationBannerTests
string? alarmFilterPrefix,
[EnumeratorCancellation] CancellationToken cancellationToken)
{
foreach (AlarmFeedMessage primed in Priming)
{
cancellationToken.ThrowIfCancellationRequested();
yield return primed;
}
await foreach (AlarmFeedMessage message in _frames.Reader
.ReadAllAsync(cancellationToken)
.ConfigureAwait(false))
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.IO.Pipes;
using System.Runtime.InteropServices;
@@ -1451,17 +1452,17 @@ public sealed class WorkerPipeSessionTests
/// refresh <c>LastActivityUtc</c> on every wait iteration, so a healthy
/// <c>ReadBulk</c> holding the STA far longer than
/// <c>HeartbeatStuckCeiling</c> (75 s in production) keeps its activity
/// timestamp fresh. This test compresses the clock — a 100 ms ceiling
/// with a command in flight across a window many multiples longer — and
/// models the pump refresh with
/// timestamp fresh. This test compresses the clock — a 1 s ceiling with
/// a command in flight across a window twice as long — and models the
/// pump refresh with
/// <see cref="FakeRuntimeSession.RefreshStaActivityOnCapture"/>, which
/// stamps activity at every heartbeat capture exactly as the pump's
/// per-iteration <c>MarkActivity()</c> does. The refresh has to be in
/// effect from construction, not from the moment the command blocks:
/// with a 50 ms grace, the idle window covering handshake and startup
/// carries no correlation id for the watchdog to suppress on, so a fake
/// whose activity timestamp is frozen at construction is reported
/// <c>StaHung</c> before the scenario under test even starts. Contrast
/// effect from construction, not from the moment the command blocks: the
/// idle window covering handshake and startup carries no correlation id
/// for the watchdog to suppress on, so a fake whose activity timestamp is
/// frozen at construction is reported <c>StaHung</c> before the scenario
/// under test even starts. Contrast
/// <see cref="RunAsync_WhenStaActivityIsStaleBeyondCeilingWithCommandInFlight_WritesWatchdogFault"/>,
/// where a frozen timestamp beyond the ceiling correctly faults; here
/// the refreshed timestamp must keep the fault suppressed and let the
@@ -1489,9 +1490,15 @@ public sealed class WorkerPipeSessionTests
runtime,
new WorkerPipeSessionOptions
{
// Compressed relative to production (75 s ceiling), but no further than the real
// pipe underneath can carry. ReportWatchdogFaultIfNeededAsync measures staleness
// AFTER the heartbeat frame has been written and flushed, so any beat whose pipe
// I/O outlasts the ceiling faults a healthy session. At a 100 ms ceiling that is a
// plausible stall on a loaded box; at 1 s it is not — and this is the one test
// asserting the watchdog NEVER fires, so it has to hold under load.
HeartbeatInterval = TimeSpan.FromMilliseconds(20),
HeartbeatGrace = TimeSpan.FromMilliseconds(50),
HeartbeatStuckCeiling = TimeSpan.FromMilliseconds(100),
HeartbeatGrace = TimeSpan.FromMilliseconds(200),
HeartbeatStuckCeiling = TimeSpan.FromSeconds(1),
});
Task runTask = session.RunAsync(cancellation.Token);
await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token);
@@ -1514,16 +1521,22 @@ public sealed class WorkerPipeSessionTests
lastEventSequence: 0,
currentCommandCorrelationId: "long-bulk-read"));
// Inspect a bounded number of frames over a window many multiples of the
// 100 ms ceiling (at least 30 heartbeats at 20 ms ~ 600 ms). None may be
// a WorkerFault while activity is continuously refreshed.
const int framesToInspect = 30;
// Inspect frames across a window twice the stuck ceiling — long enough that a fake whose
// activity timestamp stopped advancing would accumulate staleness past the ceiling and
// fault — and require the beats to have actually flowed while it ran, so an inspection
// that saw a couple of frames and timed out cannot pass for a clean window. None may be a
// WorkerFault while activity is continuously refreshed. The window stays well inside
// FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight
// throughout.
TimeSpan inspectionWindow = TimeSpan.FromSeconds(2);
const int minimumFramesInspected = 30;
Stopwatch inspection = Stopwatch.StartNew();
int frameIndex = 0;
for (; frameIndex < framesToInspect; frameIndex++)
while (inspection.Elapsed < inspectionWindow || frameIndex < minimumFramesInspected)
{
WorkerEnvelope envelope = await pipePair.GatewayReader
.ReadAsync(cancellation.Token);
AssertNotFault(envelope, frameIndex);
AssertNotWorkerFault(envelope, frameIndex++);
}
// Release the command with the pump still running — as it is in
@@ -1537,7 +1550,7 @@ public sealed class WorkerPipeSessionTests
{
WorkerEnvelope envelope = await pipePair.GatewayReader
.ReadAsync(cancellation.Token);
AssertNotFault(envelope, frameIndex++);
AssertNotWorkerFault(envelope, frameIndex++);
if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerCommandReply
&& envelope.CorrelationId == "long-bulk-read")
{
@@ -2224,12 +2237,6 @@ public sealed class WorkerPipeSessionTests
cancellationToken);
}
/// <summary>Reads frames until one matches the expected body type and predicate.</summary>
/// <param name="reader">Frame reader.</param>
/// <param name="expectedBody">Expected body case.</param>
/// <param name="predicate">Predicate to match against envelope.</param>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>The matching envelope.</returns>
/// <summary>
/// Fails when the frame is a <c>WorkerFault</c>, naming the category and diagnostic message.
/// A bare body-case comparison reports only "expected not WorkerFault", which says nothing
@@ -2238,7 +2245,7 @@ public sealed class WorkerPipeSessionTests
/// </summary>
/// <param name="envelope">Frame read from the gateway end.</param>
/// <param name="frameIndex">Ordinal of the frame within the inspected run.</param>
private static void AssertNotFault(WorkerEnvelope envelope, int frameIndex)
private static void AssertNotWorkerFault(WorkerEnvelope envelope, int frameIndex)
{
if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerFault)
{
@@ -2250,6 +2257,12 @@ public sealed class WorkerPipeSessionTests
+ envelope.WorkerFault.DiagnosticMessage);
}
/// <summary>Reads frames until one matches the expected body type and predicate.</summary>
/// <param name="reader">Frame reader.</param>
/// <param name="expectedBody">Expected body case.</param>
/// <param name="predicate">Predicate to match against envelope.</param>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>The matching envelope.</returns>
private static async Task<WorkerEnvelope> ReadUntilAsync(
WorkerFrameReader reader,
WorkerEnvelope.BodyOneofCase expectedBody,