chore(GWC-36,IPC-38,TST-33,TST-34,TST-36): close cycle-3 currency gaps

This commit is contained in:
Joseph Doherty
2026-08-25 10:52:39 -04:00
parent 4614080e24
commit 92aae28f07
10 changed files with 56 additions and 27 deletions
+3
View File
@@ -91,6 +91,9 @@ jobs:
shell: pwsh shell: pwsh
run: ./scripts/check-client-versions.ps1 run: ./scripts/check-client-versions.ps1
- name: Code-reviews README consistency
run: python code-reviews/regen-readme.py --check
- name: Gateway fake-worker tests - name: Gateway fake-worker tests
run: dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj -c Release --no-build run: dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj -c Release --no-build
+1 -1
View File
@@ -157,7 +157,7 @@ External analysis sources referenced by design docs:
Gateway gRPC clients authenticate with an API key in metadata: `authorization: Bearer mxgw_<key-id>_<secret>`. Keys are stored hashed (with a peppered SHA) in a gateway-owned SQLite DB (default `C:\ProgramData\MxGateway\gateway-auth.db`). Scopes (`session:open`, `session:close`, `invoke:read`, `invoke:write`, `invoke:secure`, `events:read`, `metadata:read`, `admin`) gate specific RPCs; missing → `Unauthenticated`, insufficient → `PermissionDenied`. The `apikey` subcommand on the server exe manages keys; see `src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/`. Gateway gRPC clients authenticate with an API key in metadata: `authorization: Bearer mxgw_<key-id>_<secret>`. Keys are stored hashed (with a peppered SHA) in a gateway-owned SQLite DB (default `C:\ProgramData\MxGateway\gateway-auth.db`). Scopes (`session:open`, `session:close`, `invoke:read`, `invoke:write`, `invoke:secure`, `events:read`, `metadata:read`, `admin`) gate specific RPCs; missing → `Unauthenticated`, insufficient → `PermissionDenied`. The `apikey` subcommand on the server exe manages keys; see `src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/`.
Session event streaming is **owner-scoped**: the API key that opened a session is recorded on the session, and every `StreamEvents` attach/reattach is rejected with `PermissionDenied` unless the caller's key id matches the owner. Possessing the `event` scope and knowing a session id is not sufficient — this closes the reconnect/fan-out trust boundary (detach-grace and replay retention are on by default) so an `event`-scoped key cannot attach to another key's retained session. Session event streaming is **owner-scoped**: the API key that opened a session is recorded on the session, and every `StreamEvents` attach/reattach is rejected with `PermissionDenied` unless the caller's key id matches the owner. Possessing the `events:read` scope and knowing a session id is not sufficient — this closes the reconnect/fan-out trust boundary (detach-grace and replay retention are on by default) so an `events:read`-scoped key cannot attach to another key's retained session.
Dashboard auth is LDAP-backed (separate from the gRPC API-key model). `/login` binds against `MxGateway:Ldap` and maps the user's LDAP groups to `Admin` or `Viewer` via `MxGateway:Dashboard:GroupToRole`, then issues an HTTP-only secure cookie named `__Host-MxGatewayDashboard` when `Dashboard:RequireHttpsCookie` is true (default) and no `Dashboard:CookieName` override is set, else the plain `MxGatewayDashboard` (the `__Host-` prefix requires a Secure cookie). SignalR hubs at `/hubs/{snapshot,alarms,events}` accept either the cookie or a 5-minute bearer minted at `/hubs/token`; `/hubs/alarms` and the `/alarms` page additionally require a **real authenticated dashboard role** (`MxGateway.Dashboard.Alarms`, a requirement type the environment-bypass handler does not handle), because the alarm feed is gateway-central with no per-session provenance and so cannot be tag-scoped — its metadata stays galaxy-wide for authenticated Viewers, but anonymous localhost and `Authentication:Mode=Disabled` do not receive it at all. Dashboard event visibility is **tag-scoped per session** (`IDashboardSessionAcl`, gating both the events hub and the session-details page's in-process subscribe): an Administrator sees every session, while any other caller sees a session only when its tags — inherited from the owning API key's `apikey --dashboard-tags`, never from the client's request — intersect the tags their LDAP groups grant via `Dashboard:GroupToTag`. That grant is **re-resolved from the caller's LDAP group claims against current options on every subscribe** (not read from the tag claims stamped at login, which would outlive a config-revoked grant for the cookie's 8-hour sliding life); login-stamped tag claims are the fallback only for a principal carrying no group claims — i.e. the hub-token principal, whose grant the mint already re-resolved. Untagged sessions follow `Dashboard:UntaggedSessionVisibility` (default `AdminOnly`), and a principal with neither claim kind (anonymous localhost included) is an empty-grant Viewer. `Dashboard:AllowAnonymousLocalhost` (default `true`) grants **read-only** loopback access — it satisfies the Viewer requirement but never the Admin-only requirement, so anonymous localhost can view the dashboard but not reach API-key CRUD, session Close/Kill, or the alarm feed (`Authentication:Mode=Disabled` is scoped the same way). `Dashboard:DisableLogin` (default `false`) auto-authenticates every dashboard request — including remote browsers — as `Dashboard:AutoLoginUser` (default `multi-role`) with both Admin and Viewer roles; dev/test only, never enable in production. Dashboard auth is LDAP-backed (separate from the gRPC API-key model). `/login` binds against `MxGateway:Ldap` and maps the user's LDAP groups to `Admin` or `Viewer` via `MxGateway:Dashboard:GroupToRole`, then issues an HTTP-only secure cookie named `__Host-MxGatewayDashboard` when `Dashboard:RequireHttpsCookie` is true (default) and no `Dashboard:CookieName` override is set, else the plain `MxGatewayDashboard` (the `__Host-` prefix requires a Secure cookie). SignalR hubs at `/hubs/{snapshot,alarms,events}` accept either the cookie or a 5-minute bearer minted at `/hubs/token`; `/hubs/alarms` and the `/alarms` page additionally require a **real authenticated dashboard role** (`MxGateway.Dashboard.Alarms`, a requirement type the environment-bypass handler does not handle), because the alarm feed is gateway-central with no per-session provenance and so cannot be tag-scoped — its metadata stays galaxy-wide for authenticated Viewers, but anonymous localhost and `Authentication:Mode=Disabled` do not receive it at all. Dashboard event visibility is **tag-scoped per session** (`IDashboardSessionAcl`, gating both the events hub and the session-details page's in-process subscribe): an Administrator sees every session, while any other caller sees a session only when its tags — inherited from the owning API key's `apikey --dashboard-tags`, never from the client's request — intersect the tags their LDAP groups grant via `Dashboard:GroupToTag`. That grant is **re-resolved from the caller's LDAP group claims against current options on every subscribe** (not read from the tag claims stamped at login, which would outlive a config-revoked grant for the cookie's 8-hour sliding life); login-stamped tag claims are the fallback only for a principal carrying no group claims — i.e. the hub-token principal, whose grant the mint already re-resolved. Untagged sessions follow `Dashboard:UntaggedSessionVisibility` (default `AdminOnly`), and a principal with neither claim kind (anonymous localhost included) is an empty-grant Viewer. `Dashboard:AllowAnonymousLocalhost` (default `true`) grants **read-only** loopback access — it satisfies the Viewer requirement but never the Admin-only requirement, so anonymous localhost can view the dashboard but not reach API-key CRUD, session Close/Kill, or the alarm feed (`Authentication:Mode=Disabled` is scoped the same way). `Dashboard:DisableLogin` (default `false`) auto-authenticates every dashboard request — including remote browsers — as `Dashboard:AutoLoginUser` (default `multi-role`) with both Admin and Viewer roles; dev/test only, never enable in production.
+3 -2
View File
@@ -133,8 +133,9 @@ finding carries an unrecognised Status value. The PowerShell wrapper
`scripts/check-code-reviews-readme.ps1` runs that check and is the intended hook `scripts/check-code-reviews-readme.ps1` runs that check and is the intended hook
for CI or a pre-commit step. for CI or a pre-commit step.
> The repo's installed `python` is the real interpreter; the bare `python3` > On Windows, the installed interpreter may be available only as `python`
> alias resolves to the Windows Store stub and fails. Use `python`. > because `python3` can resolve to the Windows Store stub. macOS and Linux
> normally use `python3`; CI uses the configured `python` executable.
The per-module `findings.md` files are the source of truth; `README.md` is the The per-module `findings.md` files are the source of truth; `README.md` is the
aggregated index and must always agree with them — which the script guarantees. aggregated index and must always agree with them — which the script guarantees.
@@ -155,6 +155,14 @@ Sequence these together rather than piecemeal — several are one change set spa
- ~~When CLI-38 lands, close old **CLI-08** with a pointer here.~~ Done 2026-08-07: CLI-38 landed and old CLI-08 is now `Done` in the first-cycle tracker, pointing at [CLI-38](50-clients.md#cli-38--align-netgojava-on-hresult--0-lands-prior-cli-08-cures-the-doc-drift---medium--p1). - ~~When CLI-38 lands, close old **CLI-08** with a pointer here.~~ Done 2026-08-07: CLI-38 landed and old CLI-08 is now `Done` in the first-cycle tracker, pointing at [CLI-38](50-clients.md#cli-38--align-netgojava-on-hresult--0-lands-prior-cli-08-cures-the-doc-drift---medium--p1).
- ~~When WRK-26 lands, its doc section also discharges the WorkerFrameProtocol gap~~ Done 2026-08-07: WRK-26 landed and **IPC-29** is discharged, pointing at [WRK-26](20-worker.md#wrk-26--write-priority-and-overflow-doc-drift-from-the-wrk-07-change---low--p1). ~~When TST-25 lands, revisit old **TST-05** (scheduled live smoke) and **TST-24** (client wire tests), which it unlocks.~~ TST-05 revisited 2026-08-10: `Partially done` in the first-cycle tracker — the `nightly-windev` job closes the scheduled-cadence half, but the finding's coverage-audit half stays open (the live suite reaches all six late-added COM commands and none of the five control commands). TST-24 revisited in the same change and closed `Done`: Go/Rust/Java already had real-server wire tests, and the two genuine gaps (.NET, Python) now have them plus a CI step. - ~~When WRK-26 lands, its doc section also discharges the WorkerFrameProtocol gap~~ Done 2026-08-07: WRK-26 landed and **IPC-29** is discharged, pointing at [WRK-26](20-worker.md#wrk-26--write-priority-and-overflow-doc-drift-from-the-wrk-07-change---low--p1). ~~When TST-25 lands, revisit old **TST-05** (scheduled live smoke) and **TST-24** (client wire tests), which it unlocks.~~ TST-05 revisited 2026-08-10: `Partially done` in the first-cycle tracker — the `nightly-windev` job closes the scheduled-cadence half, but the finding's coverage-audit half stays open (the live suite reaches all six late-added COM commands and none of the five control commands). TST-24 revisited in the same change and closed `Done`: Go/Rust/Java already had real-server wire tests, and the two genuine gaps (.NET, Python) now have them plus a CI step.
## 2026-08-25 documentation correction
The 2026-08-07 IPC-23 change-log entry's `docs/Grpc.md` deviation rationale is inaccurate:
`DrainEvents` is not private worker IPC. It is a public `MxCommandKind` accepted by the
`Invoke` RPC. Cycle-3 IPC-38 adds the missing public count-cap, byte-cap,
drain-until-empty, and destructive-stream interaction contract to `docs/Grpc.md` while
preserving the historical entry below.
## Change log ## Change log
| Date | Change | | Date | Change |
@@ -27,7 +27,8 @@ This plan is executed as **seven parallel batches**, each by one subagent workin
|---|---|---| |---|---|---|
| TST-32 | Blocked (external) | Gitea runner registration-token reset is a web-UI action on the Gitea admin page; no repo change. Operator to-do. | | TST-32 | Blocked (external) | Gitea runner registration-token reset is a web-UI action on the Gitea admin page; no repo change. Operator to-do. |
| SEC-37 | Blocked (external) | Family-wide: TLS on the shared GLAuth lives in `scadaproj/infra/glauth/` (separate repo + docker host). Plan separately; after it lands, move the deployed service to `Production`. | | SEC-37 | Blocked (external) | Family-wide: TLS on the shared GLAuth lives in `scadaproj/infra/glauth/` (separate repo + docker host). Plan separately; after it lands, move the deployed service to `Production`. |
| CLI-47/48/49/50, IPC-36/37/38, TST-33/34/35/36, GWC-33/35/36, SEC-38/39 | Not started | Activated by the 2026-08-25 P2 continuation below. | | CLI-47/48/49/50, IPC-36/37, TST-35, GWC-33/35, SEC-38/39 | Not started | Activated by the 2026-08-25 P2 continuation below. |
| GWC-36, IPC-38, TST-33/34/36 | Done | Closed by continuation batch H; verification and commit are recorded below. |
| WRK-34 | Done | Completed opportunistically in batch F (`3679e59`): watchdog faults use `CommandMethod=Watchdog` and carry the stuck correlation ID in the diagnostic message. Tracker correction recorded by the continuation audit. | | WRK-34 | Done | Completed opportunistically in batch F (`3679e59`): watchdog faults use `CommandMethod=Watchdog` and carry the stuck correlation ID in the diagnostic message. Tracker correction recorded by the continuation audit. |
## Batch specifications ## Batch specifications
@@ -104,7 +105,7 @@ The continuation runs serially in the isolated `.worktrees/c3-p2-followups` work
| Batch | Findings / follow-ups | Codex model | Status | Exit gate | | Batch | Findings / follow-ups | Codex model | Status | Exit gate |
|---|---|---|---|---| |---|---|---|---|---|
| H — tracker and currency | WRK-34 record, GWC-36, IPC-38, TST-33/34/36 | `gpt-5.6-terra` medium | Not started | Currency greps clean; README check in portable CI; NonWindows build clean | | H — tracker and currency | WRK-34 record, GWC-36, IPC-38, TST-33/34/36 | `gpt-5.6-terra` medium | Done | Currency greps clean; README check in portable CI; NonWindows build clean |
| I — gateway correctness/security | GWC-33/35, SEC-38/39 | `gpt-5.6-sol` high | Not started | Focused regression tests green; NonWindows build clean | | I — gateway correctness/security | GWC-33/35, SEC-38/39 | `gpt-5.6-sol` high | Not started | Focused regression tests green; NonWindows build clean |
| J — secondary contracts | IPC-36/37 | `gpt-5.6-sol` high | Not started | Shared launch constants and descriptor parity tests green | | J — secondary contracts | IPC-36/37 | `gpt-5.6-sol` high | Not started | Shared launch constants and descriptor parity tests green |
| K — client alarm parity | CLI-47/48/49/50 | `gpt-5.6-terra` high | Not started | All five client suites green | | K — client alarm parity | CLI-47/48/49/50 | `gpt-5.6-terra` high | Not started | All five client suites green |
@@ -116,14 +117,20 @@ The continuation runs serially in the isolated `.worktrees/c3-p2-followups` work
#### H — tracker and currency #### H — tracker and currency
- [ ] Correct WRK-34 from Deferred to Done with commit `3679e59`. - [x] Correct WRK-34 from Deferred to Done with commit `3679e59`.
- [ ] GWC-36: correct `EventSubscriberReplayAttachment.OldestAvailableSequence` semantics. - [x] GWC-36: correct `EventSubscriberReplayAttachment.OldestAvailableSequence` semantics.
- [ ] IPC-38: add public `DrainEvents` behavior to `docs/Grpc.md` and correct the cycle-2 omission rationale. - [x] IPC-38: add public `DrainEvents` behavior to `docs/Grpc.md` and correct the cycle-2 omission rationale.
- [ ] TST-33: reconcile the five stale/self-contradictory `stillpending.md` claims and mark its live-status boundary. - [x] TST-33: reconcile the five stale/self-contradictory `stillpending.md` claims and mark its live-status boundary.
- [ ] TST-34: run `code-reviews/regen-readme.py --check` in portable CI and qualify the interpreter note. - [x] TST-34: run `code-reviews/regen-readme.py --check` in portable CI and qualify the interpreter note.
- [ ] TST-36: replace prose ``event`` scope references with canonical ``events:read``. - [x] TST-36: replace prose ``event`` scope references with canonical ``events:read``.
- [ ] Record batch-H commit and verification evidence here. - [ ] Record batch-H commit and verification evidence here.
Batch-H verification (2026-08-25): `python3 code-reviews/regen-readme.py --check`
reported the generated README current and consistent; the TST-33 and live-document
TST-36 stale-pattern greps returned no matches; `docs/Grpc.md` contains the public
`DrainEvents` contract; and `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx`
succeeded with **0 warnings / 0 errors** in 5.69 s. Commit recorded after creation.
#### I — gateway correctness and security #### I — gateway correctness and security
- [ ] GWC-33: unregister terminal clients from the event-queue-depth gauge without shortening fault retention. - [ ] GWC-33: unregister terminal clients from the event-queue-depth gauge without shortening fault retention.
+3 -3
View File
@@ -90,9 +90,9 @@ detach-grace and replay-retention windows are on by default, the reconnect
surface is a trust boundary — a retained session outlives the stream that opened surface is a trust boundary — a retained session outlives the stream that opened
it. The session records the opening key id (`GatewaySession.OwnerKeyId`) and it. The session records the opening key id (`GatewaySession.OwnerKeyId`) and
every `StreamEvents` attach/reattach is rejected with `PermissionDenied` unless every `StreamEvents` attach/reattach is rejected with `PermissionDenied` unless
the caller's key id matches the owner. Gating on the `event` scope alone was the caller's key id matches the owner. Gating on the `events:read` scope alone was
rejected: it would let any `event`-scoped key that learned a session id attach to rejected: it would let any `events:read`-scoped key that learned a session id
another key's retained session and receive its replayed and live data. The check attach to another key's retained session and receive its replayed and live data. The check
runs before any subscriber is attached, so a foreign key never touches the runs before any subscriber is attached, so a foreign key never touches the
replay ring. Admin-scope override is deferred. replay ring. Admin-scope override is deferred.
+7 -1
View File
@@ -84,6 +84,12 @@ Carrying the enqueue timestamp into the worker layer is what lets queue-wait tim
An accepted gRPC command payload can still be too large for the worker pipe: the envelope built around it must fit `MxGateway:Worker:MaxMessageBytes`, which is validated at startup to sit at least a fixed envelope-overhead reserve above `MaxGrpcMessageBytes` (see the headroom rule in [Gateway Configuration](./GatewayConfiguration.md)) so this should not occur for a conformant payload, but if it does, `WorkerClient` raises `WorkerClientException(CommandTooLarge)` and `Invoke` reports `ResourceExhausted` for that command — the session is not faulted, so a client can retry with a smaller payload without reopening the session. An accepted gRPC command payload can still be too large for the worker pipe: the envelope built around it must fit `MxGateway:Worker:MaxMessageBytes`, which is validated at startup to sit at least a fixed envelope-overhead reserve above `MaxGrpcMessageBytes` (see the headroom rule in [Gateway Configuration](./GatewayConfiguration.md)) so this should not occur for a conformant payload, but if it does, `WorkerClient` raises `WorkerClientException(CommandTooLarge)` and `Invoke` reports `ResourceExhausted` for that command — the session is not faulted, so a client can retry with a smaller payload without reopening the session.
Selected control commands have additional public behavior beyond their protobuf shape:
| Command | Public behavior |
|---|---|
| `DrainEvents` | Returns a destructive diagnostic snapshot of the worker's outbound event queue. `max_events = 0` selects the default cap; positive values may not exceed `GatewayContractInfo.MaxDrainEventsPerCommand` (10,000). The worker also applies a serialized-byte budget derived from the negotiated frame limit, so a reply may contain fewer events than the requested count. When either cap leaves queued events, `MxCommandReply.diagnostic_message` reports the truncation and callers repeat `DrainEvents` until an empty reply. Events included in a drain are removed from the worker queue and therefore do not later appear on the normal session `StreamEvents` path; the background stream drain may concurrently consume other events first. |
### `StreamEvents` ### `StreamEvents`
`StreamEvents` is a server-streaming RPC. The handler delegates the full pipeline to `IEventStreamService` and just forwards each `MxEvent` onto the response stream. Keeping the channel and producer/consumer machinery out of the handler means cancellation, exception mapping, and metric bookkeeping live in one place. `StreamEvents` is a server-streaming RPC. The handler delegates the full pipeline to `IEventStreamService` and just forwards each `MxEvent` onto the response stream. Keeping the channel and producer/consumer machinery out of the handler means cancellation, exception mapping, and metric bookkeeping live in one place.
@@ -100,7 +106,7 @@ An `OK` response means the alarm provider accepted the acknowledgement, not that
### `QueryActiveAlarms` ### `QueryActiveAlarms`
`QueryActiveAlarms` is a server-streaming, **session-less** RPC that returns a point-in-time snapshot of the currently-active alarm set. Unlike `StreamAlarms`, it does not stay attached for live transitions — it writes the snapshot and completes, which clients use after a reconnect to reseed alarm state or to reconcile alarms that may have been missed during a transport blip. It is served from the always-on `GatewayAlarmMonitor` cache (`IGatewayAlarmService.CurrentAlarms`), so no worker session is opened. The handler does not run through `MxAccessGrpcRequestValidator`; it only null-checks the request inline, then streams one `ActiveAlarmSnapshot` per cached alarm, honouring cancellation between writes. When `alarm_filter_prefix` is non-empty, only alarms whose `alarm_full_reference` starts with that prefix (ordinal `StartsWith`) are emitted; an empty prefix returns the full set. Exceptions are translated by the same `MapException` path as the other handlers. The RPC requires the `event` scope (`GatewayScopes.EventsRead`), the same scope as `StreamEvents` and `StreamAlarms`. `QueryActiveAlarms` is a server-streaming, **session-less** RPC that returns a point-in-time snapshot of the currently-active alarm set. Unlike `StreamAlarms`, it does not stay attached for live transitions — it writes the snapshot and completes, which clients use after a reconnect to reseed alarm state or to reconcile alarms that may have been missed during a transport blip. It is served from the always-on `GatewayAlarmMonitor` cache (`IGatewayAlarmService.CurrentAlarms`), so no worker session is opened. The handler does not run through `MxAccessGrpcRequestValidator`; it only null-checks the request inline, then streams one `ActiveAlarmSnapshot` per cached alarm, honouring cancellation between writes. When `alarm_filter_prefix` is non-empty, only alarms whose `alarm_full_reference` starts with that prefix (ordinal `StartsWith`) are emitted; an empty prefix returns the full set. Exceptions are translated by the same `MapException` path as the other handlers. The RPC requires the `events:read` scope (`GatewayScopes.EventsRead`), the same scope as `StreamEvents` and `StreamAlarms`.
#### Provider status on the alarm feed #### Provider status on the alarm feed
+1 -1
View File
@@ -247,7 +247,7 @@ A session that faults (for example, a slow single-subscriber client that overflo
A client that drops mid-stream reconnects by re-issuing `StreamEvents` with `StreamEventsRequest.after_worker_sequence` set to the last `worker_sequence` it observed. A non-zero `after_worker_sequence` means *resume*; `0` means *fresh stream* and behaves exactly as a first-time subscribe — no replay, no sentinel. A client that drops mid-stream reconnects by re-issuing `StreamEvents` with `StreamEventsRequest.after_worker_sequence` set to the last `worker_sequence` it observed. A non-zero `after_worker_sequence` means *resume*; `0` means *fresh stream* and behaves exactly as a first-time subscribe — no replay, no sentinel.
**Owner-scoped attach (security control).** `OpenSession` records the caller's API key id on the session as `GatewaySession.OwnerKeyId`. Every `StreamEvents` attach and reattach is owner-checked: `EventStreamService.StreamEventsAsync` compares the caller's key id (threaded from `GatewayGrpcAuthorizationInterceptor` via `MxAccessGatewayService.StreamEvents`) to the session owner and throws `SessionManagerException(PermissionDenied)` — surfaced as gRPC `PermissionDenied` — when they differ, before any subscriber is attached and before any replayed or live event is delivered. Possessing the `event` scope and knowing a session id is not enough. This is what makes the default-on detach-grace and replay-retention windows safe: without the check, any `event`-scoped key that learned a session id could attach to another key's retained session and receive its replayed and live data. A `null` owner (session opened with authentication disabled) matches only a `null` caller key. **Owner-scoped attach (security control).** `OpenSession` records the caller's API key id on the session as `GatewaySession.OwnerKeyId`. Every `StreamEvents` attach and reattach is owner-checked: `EventStreamService.StreamEventsAsync` compares the caller's key id (threaded from `GatewayGrpcAuthorizationInterceptor` via `MxAccessGatewayService.StreamEvents`) to the session owner and throws `SessionManagerException(PermissionDenied)` — surfaced as gRPC `PermissionDenied` — when they differ, before any subscriber is attached and before any replayed or live event is delivered. Possessing the `events:read` scope and knowing a session id is not enough. This is what makes the default-on detach-grace and replay-retention windows safe: without the check, any `events:read`-scoped key that learned a session id could attach to another key's retained session and receive its replayed and live data. A `null` owner (session opened with authentication disabled) matches only a `null` caller key.
On a resume, `EventStreamService.StreamEventsAsync` attaches through `GatewaySession.AttachEventSubscriberWithReplay`, which calls `SessionEventDistributor.RegisterWithReplay`. That method snapshots the session's replay ring for events newer than `after_worker_sequence` **and** registers the live subscriber inside a single `_replayLock` critical section. This atomicity is what makes the replay→live handoff free of gaps and duplicates: the pump appends each event to the replay ring (under `_replayLock`) before fanning it to subscriber channels, so relative to that one critical section every event is either in the replay snapshot or fanned into the freshly-registered live channel — never both observably, never neither. On a resume, `EventStreamService.StreamEventsAsync` attaches through `GatewaySession.AttachEventSubscriberWithReplay`, which calls `SessionEventDistributor.RegisterWithReplay`. That method snapshots the session's replay ring for events newer than `after_worker_sequence` **and** registers the live subscriber inside a single `_replayLock` critical section. This atomicity is what makes the replay→live handoff free of gaps and duplicates: the pump appends each event to the replay ring (under `_replayLock`) before fanning it to subscriber channels, so relative to that one critical section every event is either in the replay snapshot or fanned into the freshly-registered live channel — never both observably, never neither.
@@ -24,9 +24,11 @@ namespace ZB.MOM.WW.MxGateway.Server.Sessions;
/// replay batch. /// replay batch.
/// </param> /// </param>
/// <param name="OldestAvailableSequence"> /// <param name="OldestAvailableSequence">
/// The oldest worker sequence still retained and replayable; <c>0</c> when nothing is /// Populates the <c>ReplayGap.oldest_available_sequence</c> field. When
/// retained. Populates the <c>ReplayGap.oldest_available_sequence</c> field. Meaningful /// <paramref name="Gap"/> is <see langword="true"/>, this is the oldest worker sequence
/// only when <paramref name="Gap"/> is <see langword="true"/>. /// still retained and replayable, or the highest observed sequence plus one when the ring
/// is empty. It is <c>0</c> only when <paramref name="Gap"/> is
/// <see langword="false"/>, where the value is not emitted.
/// </param> /// </param>
/// <param name="LiveResumeSequence"> /// <param name="LiveResumeSequence">
/// The worker sequence the live channel must resume strictly after: the highest replayed /// The worker sequence the live channel must resume strictly after: the highest replayed
+10 -8
View File
@@ -2,9 +2,11 @@
**Generated:** 2026-06-15 · **Commit:** `c7f754c` (main) · **Method:** six parallel read-only audits (Server, Worker, Contracts/proto, all five clients, docs/design/plans, tests + review backlog). Every item cites a verified `file:line`. **Generated:** 2026-06-15 · **Commit:** `c7f754c` (main) · **Method:** six parallel read-only audits (Server, Worker, Contracts/proto, all five clients, docs/design/plans, tests + review backlog). Every item cites a verified `file:line`.
> **Historical inventory:** This file preserves the June 2026 audit and its resolution annotations. It is not the live remediation register. Current finding status and verification evidence live under `archreview/`, including the active cycle tracker at `archreview/2026-08-19/remediation/00-tracking.md`.
> **Resolution update (2026-06-15, branch `feat/stillpending-completion`):** The actionable items were implemented and verified per `docs/plans/2026-06-15-stillpending-completion.md`. **§1.1** (all 11 worker command kinds), **§1.2** (audit CorrelationId), and the **§4** client CLI/helper parity gaps are **Resolved** — see per-item annotations below. Worker COM commands are live-verified on the dev rig (`efd9971`, `f7ada90`). Remaining open items are the documented residuals (**§1.3**, **§1.4**, the **§3** vendor/capture-gated questions incl. the new **§3.2** multi-sample buffered residual) and the deliberate v1 scope of **§2**. Zero `.proto` changes were needed (all reply messages already existed). > **Resolution update (2026-06-15, branch `feat/stillpending-completion`):** The actionable items were implemented and verified per `docs/plans/2026-06-15-stillpending-completion.md`. **§1.1** (all 11 worker command kinds), **§1.2** (audit CorrelationId), and the **§4** client CLI/helper parity gaps are **Resolved** — see per-item annotations below. Worker COM commands are live-verified on the dev rig (`efd9971`, `f7ada90`). Remaining open items are the documented residuals (**§1.3**, **§1.4**, the **§3** vendor/capture-gated questions incl. the new **§3.2** multi-sample buffered residual) and the deliberate v1 scope of **§2**. Zero `.proto` changes were needed (all reply messages already existed).
> >
> **Resolution update (2026-06-16, `main`):** The **session-resilience epic** (`docs/plans/2026-06-15-session-resilience.md`) landed its first **12 of 28 tasks** on `main` (through merge `c446bef`), which moves several **§2** items off "deferred": **multi-subscriber fan-out is now Resolved**, and **reconnectable sessions** has its server-side core (detach-grace window + replay-on-reconnect + a new `ReplayGap` signal on `MxEvent`). Still pending in that epic: reconnect Tasks 13–15 (owner re-validation, client `ReplayGap` handling, integration test), **Phase 4** per-session dashboard ACL (Tasks 16–19 — the §2/§7.6/§8 EventsHub-ACL item), and **Phase 5** orphan-worker reattach (Tasks 20–28). Separately, a `MxGateway:Dashboard:DisableLogin` dev flag shipped (`ca443b1`) — auto-authenticates the dashboard as a multi-role admin; default off, **enabled on the 10.100.0.48 deployment**. Per-item §2 status annotated below; remaining epic tasks tracked in `docs/plans/2026-06-15-session-resilience.md.tasks.json`. > **Resolution update (2026-08-25, `main`):** The **session-resilience epic** (`docs/plans/2026-06-15-session-resilience.md`) subsequently completed reconnect owner validation, typed `ReplayGap` handling in all five clients, the reconnect integration test, and the Phase 4 per-session dashboard ACL. Orphan-worker reattach remains deliberately deferred and is not planned. Separately, a `MxGateway:Dashboard:DisableLogin` dev flag shipped (`ca443b1`) — auto-authenticates the dashboard as a multi-role admin; default off, **enabled on the 10.100.0.48 deployment**. See the current archreview tracker for live status.
## How to read this ## How to read this
@@ -51,9 +53,9 @@ Items are graded by what they actually are, because most "pending" surface in th
These are documented, deliberate, and mostly enforced. Listed so the deferred surface is in one place — **none are bugs.** Canonical register: `docs/DesignDecisions.md:466-474` ("Later Revisit Items") + `gateway.md` "Post-v1 revisit items". These are documented, deliberate, and mostly enforced. Listed so the deferred surface is in one place — **none are bugs.** Canonical register: `docs/DesignDecisions.md:466-474` ("Later Revisit Items") + `gateway.md` "Post-v1 revisit items".
- 🟡 **Reconnectable sessions — server-side core landed, not yet complete (2026-06-16, `main`).** Epic Phase 3 added the detach-grace retention window, replay-on-reconnect from the bounded replay ring, and the `ReplayGap` signal on `MxEvent` (Tasks 10–12, merged). Still pending: owner re-validation on reconnect (Task 13), client `ReplayGap` handling across all five clients (Task 14), and the fake-worker reconnect integration test (Task 15) — so reconnect is **not** safe to rely on end-to-end yet. Overturns `docs/DesignDecisions.md:63-73`. See `docs/plans/2026-06-15-session-resilience.md` Phase 3. - ✅ **Reconnectable sessions — RESOLVED.** Detach-grace retention, replay-on-reconnect, the `ReplayGap` signal, owner re-validation, typed handling in all five official clients, and fake-worker end-to-end coverage have shipped. See `docs/DesignDecisions.md`, `docs/Sessions.md`, and `GatewayEndToEndReconnectReplayTests` for the current contract.
- ✅ **Multi-event-subscriber fan-out — RESOLVED (2026-06-16, `main`).** Epic Phase 2 (Tasks 7–9) removed the `GatewayOptionsValidator` rejection, added a `MaxEventSubscribersPerSession` cap (default 8), and built fan-out on the new `SessionEventDistributor` (single pump → N bounded per-subscriber channels with per-subscriber backpressure), with FakeWorkerHarness end-to-end coverage. Was: validator-blocked. `docs/plans/2026-06-15-session-resilience.md` Phase 2. - ✅ **Multi-event-subscriber fan-out — RESOLVED (2026-06-16, `main`).** Epic Phase 2 (Tasks 7–9) removed the `GatewayOptionsValidator` rejection, added a `MaxEventSubscribersPerSession` cap (default 8), and built fan-out on the new `SessionEventDistributor` (single pump → N bounded per-subscriber channels with per-subscriber backpressure), with FakeWorkerHarness end-to-end coverage. Was: validator-blocked. `docs/plans/2026-06-15-session-resilience.md` Phase 2.
- 🟡 **Gateway restart does not reattach orphan workers — still true on `main`, planned (epic Phase 5, not started).** Workers are still terminated on startup. Epic Phase 5 (Tasks 20–28) is designed to reverse this — stable pipe naming, a SQLite adoption manifest, a worker adopt/reconnect proto frame, and nonce-validated re-adoption behind an `EnableOrphanReattach` flag (default off). Reverses the hard rule in `docs/DesignDecisions.md:65-69` and `CLAUDE.md`. `docs/plans/2026-06-15-session-resilience.md` Phase 5. - 🔵 **Gateway restart does not reattach orphan workers — deliberately deferred, not planned.** Workers are terminated on startup and no code path may assume adoption after a gateway restart. The old epic Phase 5 sketch remains historical context only; `docs/DesignDecisions.md` and `CLAUDE.md` are authoritative.
- 🔵 **Workers run as the gateway service identity** — restricted service account is a reserved extension point. `docs/DesignDecisions.md:179-184`. - 🔵 **Workers run as the gateway service identity** — restricted service account is a reserved extension point. `docs/DesignDecisions.md:179-184`.
- 🔵 **Fail-fast event backpressure, no coalescing** — opt-in coalescing is post-v1. `docs/DesignDecisions.md:187-203`. - 🔵 **Fail-fast event backpressure, no coalescing** — opt-in coalescing is post-v1. `docs/DesignDecisions.md:187-203`.
- 🔵 **No public command batching** — `docs/DesignDecisions.md:206-212`. - 🔵 **No public command batching** — `docs/DesignDecisions.md:206-212`.
@@ -108,14 +110,14 @@ Library RPC surface is at **full parity**: all gateway + GalaxyRepository RPCs a
All live/integration paths are opt-in; the default unit suites do not exercise them. All live/integration paths are opt-in; the default unit suites do not exercise them.
- ⚪ **Live MXAccess COM + STA + message pump** — `Worker.Tests/MxAccess/MxAccessLiveComCreationTests.cs` (5 `[LiveMxAccessFact]`), gated `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`. - ⚪ **Live MXAccess COM + STA + message pump** — `Worker.Tests/MxAccess/MxAccessLiveComCreationTests.cs` (5 `[LiveMxAccessFact]`), gated `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`.
- ⚪ **Live gateway↔worker↔MXAccess round-trip** — `IntegrationTests/WorkerLiveMxAccessSmokeTests.cs` (6 `[LiveMxAccessFact]`). - ⚪ **Live gateway↔worker↔MXAccess round-trip** — `IntegrationTests/WorkerLiveMxAccessSmokeTests.cs` (8 `[LiveMxAccessFact]`).
- ⚪ **Live Galaxy Repository SQL** — `IntegrationTests/Galaxy/GalaxyRepositoryLiveTests.cs` (4 `[LiveGalaxyRepositoryFact]`), gated `MXGATEWAY_RUN_LIVE_GALAXY_TESTS=1`. - ⚪ **Live Galaxy Repository SQL** — `IntegrationTests/Galaxy/GalaxyRepositoryLiveTests.cs` (4 `[LiveGalaxyRepositoryFact]`), gated `MXGATEWAY_RUN_LIVE_GALAXY_TESTS=1`.
- ⚪ **Live LDAP dashboard auth** — `IntegrationTests/DashboardLdapLiveTests.cs` (5 `[LiveLdapFact]`), gated `MXGATEWAY_RUN_LIVE_LDAP_TESTS=1`. - ⚪ **Live LDAP dashboard auth** — `IntegrationTests/DashboardLdapLiveTests.cs` (5 `[LiveLdapFact]`), gated `MXGATEWAY_RUN_LIVE_LDAP_TESTS=1`.
- ⚪ **Alarm runtime/discovery probes (dev-rig)** — `Worker.Tests/Probes/{WnWrapConsumerProbeTests,AlarmClientWmProbeTests}.cs`, `AlarmClientDiscoveryTests.cs` — hard `[Fact(Skip=...)]`. - ⚪ **Alarm runtime/discovery probes (dev-rig)** — `Worker.Tests/Probes/{WnWrapConsumerProbeTests,AlarmClientWmProbeTests}.cs`, `AlarmClientDiscoveryTests.cs` — hard `[Fact(Skip=...)]`.
- ⚪ **Live alarm + subtag-fallback smoke** — `Worker.Tests/Probes/{AlarmSubtagLiveSmokeTests,AlarmsLiveSmokeTests}.cs` — `Skip` + one `[LiveMxAccessFact]`; Clear path remains undrivable even when enabled. - ⚪ **Live alarm + subtag-fallback smoke** — `Worker.Tests/Probes/{AlarmSubtagLiveSmokeTests,AlarmsLiveSmokeTests}.cs` — `Skip` + one `[LiveMxAccessFact]`; Clear path remains undrivable even when enabled.
- ⚪ **Python loopback TLS** — `clients/python/tests/test_tls.py:111-112` — gated `MXGATEWAY_RUN_TLS_TESTS=1` + openssl; only cert-config parsing runs by default. - ⚪ **Python loopback TLS** — `clients/python/tests/test_tls.py:111-112` — gated `MXGATEWAY_RUN_TLS_TESTS=1` + openssl; only cert-config parsing runs by default.
- ⚪ **.NET client live browse smoke** — `clients/dotnet/.../BrowseChildrenSmokeTests.cs:17-18` — hard `[Fact(Skip=...)]`. - ⚪ **.NET client live browse smoke** — `clients/dotnet/.../BrowseChildrenSmokeTests.cs:17-18` — hard `[Fact(Skip=...)]`.
- ⚪ **Cross-language client↔gateway wire behavior** — no per-client integration unit tests; only `scripts/run-client-e2e-tests.ps1` against a live gateway (`MXGATEWAY_INTEGRATION=1`). All client wire behavior is unverified in default unit runs. - ✅ **Cross-language client↔gateway wire behavior — RESOLVED (TST-24).** All five clients run real-server transport tests in their default suites: .NET and Python use loopback servers, while Go (`bufconn`), Rust, and Java use in-process servers/channels. The opt-in live matrix in `scripts/run-client-e2e-tests.ps1` (`MXGATEWAY_INTEGRATION=1`) remains the separate deployed-gateway check.
No placeholder/empty/`Assert.True(true)` tests were found anywhere. No placeholder/empty/`Assert.True(true)` tests were found anywhere.
@@ -144,10 +146,10 @@ No placeholder/empty/`Assert.True(true)` tests were found anywhere.
## 8. Deferred test-coverage follow-ups (noted in resolutions, never filed as findings) ## 8. Deferred test-coverage follow-ups (noted in resolutions, never filed as findings)
> **Resolution update (2026-06-16, branch `feat/stillpending-section8`):** Three of the four items below are **Resolved** per `docs/plans/2026-06-16-stillpending-section8.md` (Java CLI coverage, Worker-Ready wait, duplicated `FakeWorkerProcess`). Java work verified on the windev toolchain (full cli suite green); C# verified locally. The remaining item (per-session-ACL TODO) was correctly out of scope — it is session-resilience epic Phase 4. > **Resolution update (2026-08-25):** All four items below are **Resolved**. The first three landed through `docs/plans/2026-06-16-stillpending-section8.md`; the per-session dashboard ACL subsequently shipped as archreview TST-15.
- ✅ **Java CLI bulk-subcommand coverage — RESOLVED.** All 10 previously-untested subcommands now have CLI-layer round-trip tests in `clients/java/zb-mom-ww-mxgateway-cli/src/test/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCliTests.java`: the bulk family (`read-bulk`, `write-bulk`, `write2-bulk`, `write-secured-bulk`, `write-secured2-bulk`, `bench-read-bulk`) + `close-session` via the `FakeSession` seam; `stream-events`, `galaxy-discover`, `galaxy-watch` via a new in-process gRPC harness (`InProcessGatewayHarness`) that drives the real client over an in-process channel. A `GalaxyClientFactory` seam was added to `MxGatewayCli` (behavior-preserving default) to make the galaxy commands testable. Was: `code-reviews/Client.Java/findings.md:495` (Client.Java-026). - ✅ **Java CLI bulk-subcommand coverage — RESOLVED.** All 10 previously-untested subcommands now have CLI-layer round-trip tests in `clients/java/zb-mom-ww-mxgateway-cli/src/test/java/com/zb/mom/ww/mxgateway/cli/MxGatewayCliTests.java`: the bulk family (`read-bulk`, `write-bulk`, `write2-bulk`, `write-secured-bulk`, `write-secured2-bulk`, `bench-read-bulk`) + `close-session` via the `FakeSession` seam; `stream-events`, `galaxy-discover`, `galaxy-watch` via a new in-process gRPC harness (`InProcessGatewayHarness`) that drives the real client over an in-process channel. A `GalaxyClientFactory` seam was added to `MxGatewayCli` (behavior-preserving default) to make the galaxy commands testable. Was: `code-reviews/Client.Java/findings.md:495` (Client.Java-026).
- 🟡 **Per-session-ACL TODO** at `Server/Dashboard/Hubs/EventsHub.cs` (`code-reviews/Server/findings.md:765`) — still scheduled as session-resilience epic Phase 4 (Tasks 16–19); not yet started. (Out of scope for the §8 completion branch.) - ✅ **Per-session dashboard ACL — RESOLVED (archreview TST-15).** Both the Events hub and session-details in-process subscription enforce `IDashboardSessionAcl`; the stale `TODO(per-session-acl)` is gone. See `docs/Authorization.md`.
- ✅ **Worker-Ready retry race — RESOLVED.** `GatewaySession.GetReadyWorkerClient` is now an opt-in bounded ready-wait (`GetReadyWorkerClientAsync` + `MxGateway:Sessions:WorkerReadyWaitTimeoutMs`, **default `0` = unchanged fail-fast**): a session that is `Ready` while its worker is transiently `Handshaking`/`Created` waits up to the configured window for `Ready`; terminal worker states fail fast immediately with the both-states diagnostic preserved. Was: `code-reviews/Server/findings.md:611`. - ✅ **Worker-Ready retry race — RESOLVED.** `GatewaySession.GetReadyWorkerClient` is now an opt-in bounded ready-wait (`GetReadyWorkerClientAsync` + `MxGateway:Sessions:WorkerReadyWaitTimeoutMs`, **default `0` = unchanged fail-fast**): a session that is `Ready` while its worker is transiently `Handshaking`/`Created` waits up to the configured window for `Ready`; terminal worker states fail fast immediately with the both-states diagnostic preserved. Was: `code-reviews/Server/findings.md:611`.
- ✅ **Duplicated `FakeWorkerProcess` harness — RESOLVED.** The three private nested `FakeWorkerProcess` copies were consolidated onto the canonical `src/ZB.MOM.WW.MxGateway.Tests/TestSupport/FakeWorkerProcess.cs` (one definition remains). Was a latent regression vector — `code-reviews/Tests/findings.md:463`. - ✅ **Duplicated `FakeWorkerProcess` harness — RESOLVED.** The three private nested `FakeWorkerProcess` copies were consolidated onto the canonical `src/ZB.MOM.WW.MxGateway.Tests/TestSupport/FakeWorkerProcess.cs` (one definition remains). Was a latent regression vector — `code-reviews/Tests/findings.md:463`.
@@ -162,6 +164,6 @@ No placeholder/empty/`Assert.True(true)` tests were found anywhere.
- **§1.4 / §3.4 / §3.5** — the AVEVA 8-arg `AlarmAckByName` is a vendor stub (−55) and `AlarmAckByGUID` is `E_NOTIMPL`; the `domain`/`full_name` fields stay forward-compat-only until AVEVA implements them. - **§1.4 / §3.4 / §3.5** — the AVEVA 8-arg `AlarmAckByName` is a vendor stub (−55) and `AlarmAckByGUID` is `E_NOTIMPL`; the `domain`/`full_name` fields stay forward-compat-only until AVEVA implements them.
- **§3.2** — buffered commands work and the empty bootstrap converts cleanly live, but a multi-sample buffered batch is undrivable on the rig (unit-tested only). - **§3.2** — buffered commands work and the empty bootstrap converts cleanly live, but a multi-sample buffered batch is undrivable on the rig (unit-tested only).
- **§3.1 / §3.3 / §3.6 / §3.7** — await live MXAccess captures. - **§3.1 / §3.3 / §3.6 / §3.7** — await live MXAccess captures.
- **§2** — mostly deliberate v1 scope, but the session-resilience epic (12/28 tasks merged to `main`) has since **resolved multi-subscriber fan-out** and landed the **reconnect server-side core**; reconnect Tasks 13–15, per-session ACL (Phase 4), and orphan-worker reattach (Phase 5) remain (see `docs/plans/2026-06-15-session-resilience.md`, `docs/plans/2026-06-15-session-resilience.md.tasks.json`). **§5** — opt-in verification gates. **§7.6** — accepted `Won't Fix` review findings. - **§2** — deliberate v1 scope. Multi-subscriber fan-out, reconnect, and the per-session dashboard ACL are resolved; orphan-worker reattach remains deliberately deferred and not planned. **§5** — opt-in verification gates. **§7.6** — accepted `Won't Fix` review findings.
MXAccess **event/data/value/write** mapping, the **Galaxy** RPC surface, and now the **full command surface** are complete; no `NotImplementedException`s, stubbed RPC bodies, or empty tests remain in the production paths. MXAccess **event/data/value/write** mapping, the **Galaxy** RPC surface, and now the **full command surface** are complete; no `NotImplementedException`s, stubbed RPC bodies, or empty tests remain in the production paths.