docs(archreview): cycle-3 architecture review (2026-08-19 @ 8b3e3b3)

Six domain reports + overall synthesis. All 47 cycle-2 Done claims verified
at HEAD with zero false Dones and zero regressions; 34 new findings
(0 Critical, 0 High, 4 Medium, 25 Low, 5 Info): GWC-31..38, WRK-32..34,
IPC-33..39, SEC-37..41, CLI-46..50, TST-31..36.

archreview/2026-08-19/00-overall.md

@@ -0,0 +1,117 @@
+# MxAccessGateway — Architecture Review, Cycle 3
+
+Date: 2026-08-19. Branch: `main` at `8b3e3b3` (clean; ~288 commits since the cycle-2 baseline `4f5371f`). Working tree: macOS, **static review only** — no builds, no test runs, no source changes, no remote hosts (one sanctioned read-only script, `regen-readme.py --check`, was executed by the testing reviewer).
+
+## Scope and method
+
+Third run of the review series ([cycle 1](../00-overall.md) 2026-07-08, [cycle 2](../2026-07-12/00-overall.md) 2026-07-12). Same shape: six domain reviews, each with three passes — (1) verify every cycle-2 finding's tracked status (`../2026-07-12/remediation/00-tracking.md`, plus the NEXT-xx candidate rows in `90-candidate-findings-next-cycle.md`) against code read at HEAD, (2) deep-review the remediation code itself and everything else landed in the period, (3) a fresh sweep. Every claim in the domain reports carries a `file:line` citation read at HEAD.
+
+| Report | Domain |
+|---|---|
+| [10-gateway-core.md](10-gateway-core.md) | Gateway server: sessions, worker lifecycle, IPC client, gRPC streaming, alarm monitor, GalaxyRepository package adoption |
+| [20-worker.md](20-worker.md) | Worker process: STA pump, COM lifetime, pipe session, event queue, write-completion correlation, subtag failover |
+| [30-contracts-ipc.md](30-contracts-ipc.md) | Protos, frame protocol (both sides), codegen/descriptor pipeline, launch-environment contract |
+| [40-security-dashboard.md](40-security-dashboard.md) | API-key auth + failure limiter, LDAP dashboard, tag ACL, secrets store, SignalR hubs, redaction |
+| [50-clients.md](50-clients.md) | .NET, Go, Java, Python, Rust clients; publish pipeline; cross-client parity |
+| [60-testing-docs-gaps.md](60-testing-docs-gaps.md) | Test architecture, CI (incl. the windev tier), documentation currency, repo hygiene |
+
+The period under review is the largest yet: the full cycle-2 remediation train (47 findings), the SEC-25 tag-ACL epic, the secrets-store adoption, the alarm truncation / `snapshot_status` rollout, the OnWriteComplete correlation feature, a worker perf wave, the 0.2.0/0.2.1 client publish, the windev CI tier, and the fca978d marker-sweep restoration.
+
+## Overall verdict
+
+**The cycle-2 remediation held completely.** All 47 cycle-2 findings tracked `Done` were verified present, sound, and tested at HEAD — **zero false `Done` claims, zero regressions in previously verified fixes, across all six domains.** Four gateway/IPC items carry a caveat in the precise sense of cycle 2 ("fix is correct for the filed defect; a residual failure mode one layer out survives"), each filed as a new Low (GWC-32/34/35, IPC-33). The worker, security, clients, and testing domains reported no caveats at all.
+
+The new top line is **0 Critical, 0 High, 4 Medium, 25 Low, 5 Info** across 34 new findings — down from 0/1/14/28/4 (47) in cycle 2 and 1/12/54/39 in cycle 1. For the first time in the series, **none of the new findings is a correctness defect in the core data path that a client can trigger in default configuration.** The four Mediums are: one deployment-posture gap outside this repo's control (SEC-37, GLAuth has no TLS), one operator action outstanding from a tracked runbook (TST-32, runner-token reset), one version-hygiene repeat (CLI-46, constants equal to already-published artifacts — for the second cycle running), and one stale fixture doc with live-test fallout (TST-31, `glauth.md` credential table).
+
+The residual risk profile is now dominated by **edges of new machinery and process residue**, not product defects: the recency-based write-completion correlation (WRK-32), the fail-open browse scope under a missing ambient identity (GWC-38), the un-tag-scoped alarms hub (SEC-40), the undeclared launch-env-var contract (IPC-36), and a client family that keeps shipping post-publish changes under the published version number (CLI-46).
+
+## Prior-finding verification
+
+| Domain | Cycle-2 Done claims | Verified | Caveats (filed as new) | Cycle-1 leftovers |
+|---|:-:|:-:|---|---|
+| Gateway core | 7 (GWC-24..30) | 7/7 | GWC-24→**GWC-32**/33, GWC-26→**GWC-35**, GWC-27→**GWC-34** | 13 re-confirmed open, none regressed; GWC-16 recommended Won't-fix (documented deferral) |
+| Worker | 8 (WRK-21..28) + NEXT-04/05 | 8/8 + 2/2 | none — WRK-21 fix exceeded its design (enqueue-time sizing) | WRK-01/04/06 re-verified intact |
+| Contracts & IPC | 8 (IPC-23..32) | 8/8 | IPC-23→**IPC-33** | IPC-08 substantially resolved by `7c1ea12` (recommend closing; residual = IPC-34); IPC-10/11/12 open; IPC-31 superseded by GWC-28 (flip to Resolved) |
+| Security & dashboard | 6 (SEC-31..36) | 6/6 | none; SEC-36 includes the executed operator rotation | SEC-19/21/26 resolved in the period; SEC-13 impact reduced; SEC-24 narrowed; 13/14/15(accepted)/16/17/18/23/27 open |
+| Clients | 11 (CLI-35..45) + NEXT-02 | 11/11 + 1/1 | none — including the halves prior cycles skipped (CLI-21 tag guard, CLI-02/42 docs, CLI-26 drift test) | CLI-05/08 partly, CLI-09/13/17/32 divergence table updated |
+| Testing, docs & gaps | 6 (TST-25..30) | 6/6 | TST-30 carries one tracked operator residual (→ **TST-32**) | TST-06 materially narrowed (tracker stale); TST-07/09/14 partially addressed; TST-17 row needs refresh; TST-05/10/18–22 open |
+
+**Tracker housekeeping recommended:** close GWC-16 (Won't-fix, recorded), IPC-08 (resolved, residual → IPC-34), IPC-31 (superseded by GWC-28); refresh TST-06 and TST-17 rows; note that `e913dab` burned **WRK-31** (and possibly 29/30) as interim IDs and the 2026-08-15 perf pass stamped unregistered **GWC-3x** labels into code comments (GWC-31) — the register must stay the sole ID authority.
+
+## Severity roll-up (new findings only)
+
+| Domain | Critical | High | Medium | Low | Info | IDs |
+|---|:-:|:-:|:-:|:-:|:-:|---|
+| Gateway core | — | — | — | 6 | 2 | GWC-31..38 |
+| Worker | — | — | — | 3 | — | WRK-32..34 |
+| Contracts & IPC | — | — | — | 5 | 2 | IPC-33..39 |
+| Security & dashboard | — | — | 1 | 3 | 1 | SEC-37..41 |
+| Clients | — | — | 1 | 4 | — | CLI-46..50 |
+| Testing, docs & gaps | — | — | 2 | 4 | — | TST-31..36 |
+| **Total** | **0** | **0** | **4** | **25** | **5** | **34** |
+
+(Cycle 2: 0 Critical, 1 High, 14 Medium, 28 Low, 4 Info. Cycle 1: 1 Critical, 12 High, 54 Medium, 39 Low.)
+
+## Headline findings
+
+### Medium
+
+- **SEC-37 — the Production plaintext-LDAP hard-stop is unsatisfiable.** The shared GLAuth (`10.100.0.35:3893`) offers no TLS, so every host in the family that authenticates against it cannot run with `DOTNET_ENVIRONMENT=Production` — which is why the deployed service runs as `Staging`. Promotion of NEXT-08; the fix is family-wide (GLAuth TLS in `scadaproj/infra/glauth/`), not in this repo. ([40](40-security-dashboard.md))
+- **CLI-46 — version constants again equal the already-published 0.2.0/0.2.1** while post-publish changes shipped — including the fix for the published-broken Python `connect()` found by TST-24. Same shape as cycle-2's CLI-39; the publish-pipeline guards (registry-collision hard-stop) will now *block* the next publish rather than ship a duplicate, but the bump discipline still isn't happening at change time. ([50](50-clients.md))
+- **TST-31 — `glauth.md`'s user table contradicts the directory**, and the wrong credential propagated into the live-LDAP test surface (promotes NEXT-10). ([60](60-testing-docs-gaps.md))
+- **TST-32 — the Gitea runner registration token (world-readable ~5 months, proven live) still awaits its web-UI reset** — the one operator residual of the otherwise-complete TST-30. ([60](60-testing-docs-gaps.md))
+
+### Low — items worth a second look
+
+- **WRK-32 — write-completion correlation is recency-based per handle**: a late `OnWriteComplete` from an earlier write can be reported as the current write's outcome. Parity-clean otherwise (no synthesized events), but the docs should state the attribution rule. ([20](20-worker.md))
+- **GWC-38 — `GatewayBrowseScopeProvider` fails open** to the full Galaxy hierarchy when the ambient identity is absent, resting a scoping control on an undeclared interceptor-ordering invariant. ([10](10-gateway-core.md))
+- **SEC-40 / SEC-41 — tag ACL edges**: the session-less alarms hub broadcasts galaxy-wide alarm metadata to every Viewer including empty-grant and anonymous-localhost principals; revoked `GroupToTag` grants survive on cookie principals for up to the 8 h sliding life. ([40](40-security-dashboard.md))
+- **SEC-38 — limiter aggregate sidestep**: pre-filling the per-peer key-id cap (SEC-32) routes later failures to the fallback partition, which doesn't feed the cross-peer aggregate, restoring the `limit × addresses` guessing bound. ([40](40-security-dashboard.md))
+- **IPC-33 — DrainEvents byte budget is sized against the pipe max, not gRPC `MaxSendMessageSize`**; with a legal larger `Worker:MaxMessageBytes` the reply crosses the pipe and fails at the gRPC send with events already dequeued — the cycle-2 "no event loss" invariant violated one boundary later. ([30](30-contracts-ipc.md), [20](20-worker.md))
+- **IPC-34 — `WorkerCancel` fires only on the gateway's own timeout**; a client deadline/disconnect leaves the command on the STA queue (the IPC-08 residual). ([30](30-contracts-ipc.md))
+- **IPC-36 — six launch env-vars form an undeclared second gateway↔worker contract**, four new since baseline, duplicated as literals on both sides, two documented. ([30](30-contracts-ipc.md))
+- **GWC-37 — the subtag watch-list is resolved once per monitor lifecycle**; a Galaxy-Repository outage at startup empties the failover's coverage for the session's life. ([10](10-gateway-core.md))
+- **GWC-32 / GWC-34 — dispose races**: `WorkerClient.DisposeAsync` never enters a terminal state (the GWC-24 overflow fault can misfire); `EnsureDistributorCreated` recreates a pump-less distributor after teardown and a racing attach hangs silently. ([10](10-gateway-core.md))
+- **CLI-47/48/50 — alarm-feed CLI divergence**: `snapshot_status` rendering differs per language, the new renderer arms are untested in three CLIs (Go has no `stream-alarms` test at all), and the Python CLI's default `--max-messages 1` returns only the provider-status preamble so the README example fetches no alarm data. ([50](50-clients.md))
+- **TST-35 — the Windows-host run of the *gateway* (net10) suite is still manual-only**, and its failure class recurred three times in August; TST-34 — the code-reviews README check is wired into no CI job. ([60](60-testing-docs-gaps.md))
+
+## Cross-cutting themes
+
+1. **"One layer out" is now the only residual pattern in the core.** Every caveated fix this cycle (GWC-24/26/27 → GWC-32/34/35, IPC-23 → IPC-33) is the cycle-2 pattern: the invariant holds at the boundary that was filed and leaks at the next one (pipe max vs gRPC max; shutdown state vs dispose state; unacked→acked vs acked→unacked). The fix shape remains the same advice as last cycle: specify against the invariant, not the symptom — but the blast radius has shrunk from "session-fatal" to "misdiagnosed fault / silent hang in a race".
+2. **The tag-ACL epic is sound at its two designed seams and leaky at the one it didn't name.** Events hub and session-details subscribe are pinned with decision-table tests and live-LDAP coverage; the alarms hub (SEC-40) was never in scope, the cookie lifetime (SEC-41) wasn't either, and the browse scope (GWC-38) fails open. A one-page "which surfaces are tag-scoped and what principal each trusts" table would close all three or document them as accepted.
+3. **Version bumps are still not part of the change, for the second cycle** (CLI-39 → CLI-46). The pipeline guards now make this a blocked publish instead of a duplicate publish, which is the right failure — but the process fix is a "did any `clients/**` source change since the last publish tag" check at PR time, not at pack time.
+4. **Second contracts exist outside the proto.** The launch env-vars (IPC-36) and the GalaxyRepository wire-identity (IPC-37, still unguarded though now testable) are both gateway↔peer agreements with no schema, no test, and partial docs. The codegen guard is now excellent for everything *in* the protos; these are what's left.
+5. **ID discipline slipped in the period.** Interim WRK-31 and unregistered GWC-3x labels in code comments (GWC-31), plus stale tracker rows (TST-06/17, IPC-08/31, GWC-16) — cheap to fix, and the register is the only thing that makes cycle-over-cycle verification possible.
+6. **Documentation currency is high and the one big wound healed.** The fca978d sweep's collateral damage was systematically restored (816 lines judged, 17 restorations), every currency spot-check matched code, and the remaining doc findings (TST-31/33/36, GWC-36, IPC-38) are inherited debris rather than new drift.
+
+## Prioritized roadmap
+
+**P0 — small, do now**
+1. CLI-46: bump all five client versions past 0.2.0/0.2.1 before anything else touches `clients/`; add the PR-time "source changed since publish tag" check.
+2. TST-32: reset the Gitea runner registration token (web UI); TST-31: fix `glauth.md` and the propagated live-test credential.
+3. Tracker housekeeping: close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31 and retire the GWC-3x comment labels (GWC-31).
+
+**P1 — hardening, each a contained change**
+4. Tag-ACL completeness: SEC-40 (scope the alarms hub or document it as galaxy-wide-by-design), SEC-41 (re-resolve tag claims on restart / shorten validity), GWC-38 (fail closed when identity is absent and assert interceptor order).
+5. IPC-33 + IPC-39: budget DrainEvents against `min(pipe max, gRPC send max)`; fold the sequence-stamp window into the pre-check.
+6. IPC-34: forward `WorkerCancel` on caller cancellation, not only gateway timeout; IPC-35: surface handshake `WorkerFault`s structurally.
+7. WRK-32: document (or tighten) the recency-based completion attribution; WRK-33: give the discarded command-task observer its fault-observing continuation.
+8. GWC-32/34: terminal-state on dispose; refuse `EnsureDistributorCreated` after disposal. GWC-37: re-resolve the subtag watch-list on a timer or on Galaxy-Repository recovery.
+9. SEC-37: family-wide — add TLS to the shared GLAuth (`scadaproj/infra/glauth/`), then move the deployed service to `Production`.
+
+**P2 — the rest**
+10. Client CLI parity batch (CLI-47/48/49/50), IPC-36 launch-env contract (single shared constants class + doc table), IPC-37 wire-identity test, TST-34/35 CI wiring, doc debris (TST-33/36, GWC-36, IPC-38, SEC-39 comment), alongside the unchanged cycle-1 backlog.
+
+## What is demonstrably good
+
+Everything cycle 2 asked to preserve is intact: STA pump and COM teardown discipline, write-time sequence stamping (now peek-stamp-commit on the worker, GWC-28 on the gateway), negotiated frame maxima with startup cross-validation, additive-only proto evolution with every generated tree regenerated in the same commit as the proto change, fail-closed auth with constant-time compare, owner-scoped `StreamEvents`, and the layered fake-worker test architecture.
+
+New this period and worth protecting under change:
+
+- **The windev CI tier** — per-push x86 worker build + Worker.Tests and a nightly live-MXAccess run, hardened by its own acceptance checks (base64-stored key, `MXGW_CI_LOCK_HELD` handoff, SHA shape validation, fail-red on unreachable host), and the full-suite testhost hang root-caused to a zero-buffer pipe and structurally fixed (windev baseline = macOS = 879).
+- **Byte-budgeted DrainEvents with enqueue-time sizing** — the cycle-2 headline defect fixed beyond its design; the IPC-33 residual is a configuration edge, not the default path.
+- **The failure limiter's composite partition + aggregate + probe admission** — the cycle-2 lockout-DoS is closed; SEC-38/39 are edge refinements.
+- **Fixtures-first client conformance** — ten shared command-reply fixtures consumed by all five suites, so HRESULT sign, category-verdict, scrub, and malformed-reply rules are pinned in bytes; plus real-wire tests in all five clients (which immediately found the published-broken Python `connect()`).
+- **Three-seam value redaction** (events mirror, alarms hub, `/browse`), clone-only, with the config doc telling the truth about all three.
+- **The codegen guard** — descriptor set, `Contracts/Generated`, Rust vendored protos in both directions, Go/Python bindings with pinned generators, Java aggregates via the java job's diff gate, and Check 4 now Windows-runnable.
+- **Per-correlation `CommandTooLarge` → `ResourceExhausted`** remains the model for fault isolation the residual IPC-33/34/35 items should copy.

archreview/2026-08-19/10-gateway-core.md

@@ -0,0 +1,144 @@
+# Gateway Server Core — Cycle-3 Review
+
+- **Date:** 2026-08-19
+- **Commit:** `8b3e3b3` (`main`, clean)
+- **Baseline:** `4f5371f` (2026-07-12 cycle-2 review; ~288 commits since, ~99 of them touching `src/ZB.MOM.WW.MxGateway.Server`)
+- **Method:** static review on macOS; every cited file read at HEAD. No build, no test, no source change.
+- **Scope:** gateway server process — `Sessions/`, `Workers/`, `Grpc/` (`MxAccessGatewayService`, `EventStreamService`, mapper, validator), `Alarms/` (monitor, watch-list resolver, subtag-fallback gateway half), the session/alarm-feed plumbing the dashboard consumes (`DashboardLiveDataService` session use, `AlarmsHubPublisher` feed consumption), and the `ZB.MOM.WW.GalaxyRepository` package adoption (`AddZbGalaxyRepository`/`MapZbGalaxyRepository` wiring, `GatewayBrowseScopeProvider`, `DashboardGalaxySummaryProjector`). Excludes `Security/` internals and dashboard UI (security-dashboard reviewer), except the browse-scope provider assigned here.
+- **Prior reports:** `archreview/2026-07-12/10-gateway-core.md`; remediation status source `archreview/2026-07-12/remediation/00-tracking.md` (+ `10-gateway-core.md`); first-cycle backlog `archreview/remediation/00-tracking.md`.
+
+## Prior-finding verification
+
+### Cycle-2 findings (GWC-24..30, all tracked Done)
+
+Every Done claim was verified against code read at HEAD. **All 7 hold; 3 are verified with caveats** — in each case the fix is present and sound for the reported defect, but a residual failure mode one layer out survives and is filed as a new finding.
+
+| ID | Claimed | Verified? | Evidence (HEAD) | Notes |
+|----|---------|-----------|-----------------|-------|
+| GWC-24 | Done | ✅ holds, **with caveat** | `Workers/WorkerClient.cs:107-119` (staging channel bounded at `checked(2 * EventChannelCapacity)`, Wait+TryWrite idiom documented), `:646-676` (`StageWorkerEvent` faults `ProtocolViolation` on a rejected `TryWrite`, message names the bound, the depth, and the actionable fix), `:653-658`/`:350-359` (single `_eventQueueDepth` counter incremented at staging, decremented at consumer read — total undelivered depth), `:120-121` + `Metrics/GatewayMetrics.cs:524` (pull-model gauge registration), `:44-48` (field comment states bound + fault + gauge coverage); startup overflow guard `Configuration/GatewayOptionsValidator.cs:343-357` (`QueueCapacity ≤ int.MaxValue / 2` so the 2× derivation cannot overflow); regression tests `Tests/Gateway/Workers/WorkerClientTests.cs:781` (`StagingChannelOverflowFaultsWorkerWithoutWaitingForFullModeTimeout`), `:865` (`WorkerEventQueueDepthGaugeCountsStagedEvents`) | Implemented exactly per the remediation design, including the shutdown-drop guard (`IsTerminalState()`), and the design's extra validator guard. Caveat: the shutdown-drop guard does not cover `DisposeAsync`, which completes the channels **without** entering a terminal state — a dispose-race `TryWrite` failure is then misdiagnosed as the backpressure fault → **GWC-32**. A faulted-but-retained client also keeps contributing stale depth to the gauge until disposal → **GWC-33** (Info). |
+| GWC-25 | Done | ✅ holds | `Sessions/SessionEventDistributor.cs:519-535` (empty-ring branch: `oldestAvailableSequence = gap ? _highestSequenceSeen + 1 : 0`, with a full explanatory comment citing the finding), out-param contract updated `:430-440`; emitted verbatim `Grpc/EventStreamService.cs:139-145`, `:227-239`; proto comment amended `Contracts/Protos/mxaccess_gateway.proto:797-798` ("When nothing is retained … `highest observed + 1` … resume formula remains valid"); `docs/Sessions.md:262` (dedicated empty-ring paragraph); tests `Tests/Gateway/Sessions/SessionEventDistributorTests.cs:935` (`ResumeUsingSentinelFormulaAfterEmptyRingGapDeliversLiveEvents`) and `Tests/Gateway/GatewayEndToEndReconnectReplayTests.cs:246` (`ReconnectAfterFullAgeEvictionResumesWithSentinelFormula`) | Sound end to end. The `afterSequence == ulong.MaxValue` wrap guards in both branches (`:521`, `:540`) are preserved, so the old dead-stream input is also inert. Proto + docs + server + tests all landed as one story; the client halves (CLI-35/36, NEXT-02) are closed in the tracking record. |
+| GWC-26 | Done | ✅ holds, **with caveat** | `Alarms/GatewayAlarmMonitor.cs:246-258` (internal lease attached **before** `SubscribeAlarmsAsync` + first `ReconcileAsync`, with the window rationale in the comment), `:272-281` (drain the same lease after reconcile; order-safety noted), `ApplyReconcile` acked-delta repair `:674-684` (both-present, `existing.CurrentState != incoming.CurrentState && incoming == ActiveAcked` → `Acknowledge` feed transition); `ISessionManager.ReadAlarmEventsAsync` removed (grep: absent from `Sessions/ISessionManager.cs` and `SessionManager.cs`); repair-scope + at-least-once contract documented `:607-633`; regression tests `Tests/Alarms/GatewayAlarmMonitorAttachOrderTests.cs` | Implemented per design, including the method-removal half and the "never synthesize MxEvents" boundary note (`:610-614`: repairs are `AlarmFeedMessage`s, never `MxEvent`s). Caveat: the state-delta repair is one-directional — only unacked→acked is repaired; an acked alarm silently replaced by a new **unacked** instance is absorbed without a feed transition → **GWC-35**. |
+| GWC-27 | Done | ✅ holds, **with caveat** | `Sessions/GatewaySession.cs:590-617` (`AttachInternalEventSubscriber` readiness gate under `_syncRoot` before `EnsureDistributorCreated`, same exception code/message shape as `AttachEventSubscriber:955-961`; the poison mechanism is spelled out in the comment); test `Tests/Gateway/Sessions/GatewaySessionTests.cs:683` (`AttachInternalEventSubscriberBeforeReadyThrowsAndDoesNotPoisonDistributor`) | The premature-attach poison path is closed. Caveat: the disposal-side twin remains — `EnsureDistributorCreated` recreates a never-started distributor after `DisposeAsync` nulls `_eventDistributor`, silently hanging a racing attach → **GWC-34**. |
+| GWC-28 | Done | ✅ holds | `Workers/WorkerClient.cs:463-472` (`WriteLoopAsync` stamps `envelope.Sequence = unchecked(++_nextSequence)` immediately before `_writer.WriteAsync`, single-consumer rationale in comment), `:49-51` (field is a plain `ulong` touched only by the write loop, no interlock), `CreateEnvelope:1225-1241` (sequence deliberately unset, comment cites GWC-28/WRK-04); test `Tests/Gateway/Workers/WorkerClientTests.cs:228` (`ConcurrentInvokesEmitStrictlyIncreasingSequencesOnTheWire`) | Exactly the WRK-04-mirror design. All outbound envelopes flow through `_outboundEnvelopes` (hello `:171`, commands `:251`, cancel `:849/:858`, shutdown `:383`), so coverage is total. Note the `InvokeAsync` size pre-check comment (`:236-241`) correctly explains why the computed size cannot be reused as the writer's `knownSize` post-GWC-28 — the interaction was thought through. |
+| GWC-29 | Done | ✅ holds | `Grpc/MxAccessGrpcMapper.cs:43-55` (new `MapCommand(MxCommand)` overload; the one required clone kept with the aliasing rationale), `:27-33` (request overload delegates); `Grpc/MxAccessGatewayService.cs:131-136` (`mapper.MapCommand(commandToInvoke)` — the request clone and `invokeRequest` local are gone; `TrackCommandReply` still reads `commandToInvoke` at `:147`, which is why the inner clone must stay and does) | Sound. `WorkerClient.CreateCommandEnvelope`'s no-aliasing invariant comment (`:1194-1198`) remains accurate. |
+| GWC-30 | Done | ✅ holds | `Workers/WorkerFrameReader.cs:22-24` (per-reader `_lengthPrefix` scratch buffer), `:46` (used per frame), `:11-16` (class remark documents the non-reentrancy contract) | As designed. (Unrelatedly, the *label* "GWC-30" has since been reused by an off-register perf pass for a different change — see **GWC-31**.) |
+
+### Cycle-1 findings still open at cycle 2 — re-checked at HEAD
+
+All were re-read at HEAD. **None regressed; none silently fixed; two materially changed shape** (GWC-16 is now an explicitly documented deferral; GWC-20's client-side half improved).
+
+| ID | Cycle-2 status | HEAD status | Evidence |
+|----|----------------|-------------|----------|
+| GWC-05 | Open | **Still open** | `Sessions/SessionWorkerClientFactory.cs:175-185` — pipe created with explicit 128 KiB buffers (the zero-quota fix, `da84635`) but still no ACL and no `PipeOptions.CurrentUserOnly`. The local pipe-squatting exposure and the `gateway.md` pipe-security contract gap are unchanged. |
+| GWC-09 | Open | **Still open** | `Workers/WorkerProcessStartedProbe.cs:5-19` (no-op `HasExited` probe throwing `WorkerProcessLaunchException`); `Workers/WorkerProcessLauncher.cs:331-339` (`ShouldRetryStartupProbe` still excludes `WorkerProcessLaunchException`); `Configuration/WorkerOptions.cs:19-22` (both dead retry options remain). The retry pipeline still can never retry. |
+| GWC-10 | Open | **Still open** | `Workers/WorkerEnvelopeValidator.cs:15-39` — version/session/body only; no inbound sequence tracking anywhere in `WorkerClient`. (The outbound half was fixed by GWC-28; enforcement remains unimplemented on both sides, consistent with IPC-31's recorded decision.) |
+| GWC-11 | Open | **Still open** | `Sessions/GatewaySession.cs:23` (`_workerClient` not volatile); lock-free reads at `:308` (`WorkerProcessId`), `:313` (`WorkerClient`), `:1579` (`CloseAsync`), `:1732`, `:1765`, `:1877` (`DisposeAsync`). |
+| GWC-12 | Open | **Still open** | `Workers/WorkerClient.cs:419-426` — plain `if (_disposed) return; _disposed = true;`, no interlock. (Now compounds with new GWC-32: dispose also never enters a terminal state.) |
+| GWC-13 | Open | **Still open** | `Sessions/GatewaySession.cs:1956-2024` — 25 ms poll loop, routed through the injected `TimeProvider` (unchanged since cycle 2). |
+| GWC-16 | Open | **Still open — now a recorded deferral; recommend closing as Won't-fix** | `Grpc/MxAccessGatewayService.cs:111-115` — a `PERF(followup)` comment now documents the double session lookup as measured-negligible and deliberately not collapsed (the fix would duplicate `InvokeAsync`'s fault mapping at a second entry point). The rationale is sound; the tracker row should be flipped to `Won't fix` citing this comment rather than left `Not started`. |
+| GWC-17 | Open | **Still open** | `Sessions/SparseArrayExpander.cs:306` (`RpcException` factory below the gRPC layer), `:75` (GWC-03's cap rejection still flows through the same leak); invocation comment at `Sessions/GatewaySession.cs:1103-1111` still documents the layering leak explicitly. |
+| GWC-18 | Open | **Still open** | `Sessions/GatewaySession.cs:14` — no `IAsyncDisposable`; `DisposeAsync` at `:1787`. |
+| GWC-19 | Open | **Still open** | `Sessions/GatewaySession.cs:1730-1734` — `KillWorker(string)`; grep across `src/` + `clients/` finds zero callers (one doc-comment mention in `SessionManagerTests.cs:1433`). |
+| GWC-20 | Open | **Still open (client half improved)** | `Configuration/WorkerOptions.cs:52` — the config key is still `HeartbeatIntervalSeconds` ("interval … for worker heartbeats") while it is bound to the **check** interval at `Sessions/SessionWorkerClientFactory.cs:98`; the `WorkerClientOptions` field was renamed to `HeartbeatCheckInterval` (`Workers/WorkerClientOptions.cs:40`), which fixes the client-side naming but leaves the config-key conflation. The heartbeat loop still uses raw `Task.Delay` without `TimeProvider` (`Workers/WorkerClient.cs:534`). |
+| GWC-21 | Open | **Still open** | `Sessions/SessionWorkerClientFactory.cs:95-101` populates 4 of 6 `WorkerClientOptions`; `EventChannelFullModeTimeout` / `HeartbeatStuckCeiling` remain fixed defaults (`Workers/WorkerClientOptions.cs:13`, `:23`). Post-GWC-24 the staging bound is *derived* from `EventChannelCapacity` (which is configurable), so the exposure gap is narrower than at cycle 2 but the two knobs are still hard-coded. |
+| GWC-22 | Open | **Still open** | `Grpc/EventStreamService.cs:217` — `metrics.StreamDisconnected("Detached")` hardcoded in the `finally`; overflow/fault exits are not differentiated. |
+| GWC-23 | N/A | Unchanged | Decision stands; multi-subscriber mode remains config-gated and the validator comment survives. |
+
+### NEXT rows touching this domain
+
+- **NEXT-01** (macOS `sun_path` pipe names) — **verified resolved**: `Sessions/SessionManager.cs:532-536` builds `mxgw-{Environment.ProcessId}-{sessionUid}` with the length-budget rationale in the comment; `CLAUDE.md`/`gateway.md` describe the same format.
+- **NEXT-03** (reconcile/live duplicate broadcasts) — **verified resolved as designed (best-effort, positive-match-only)**: `Alarms/GatewayAlarmMonitor.cs:483-489` (`IsDuplicateOfCachedState`, timestamp+state marker), `:496-507` (`IsDuplicateOfReconcileClear`, tombstone consumed on match), `:645-664` (tombstones recorded per reconcile generation, cleared at `:649`); at-least-once contract restated at `:616-625`; race-driving regressions in `Tests/Alarms/GatewayAlarmMonitorAttachOrderTests.cs:159`, `:218`. The one-generation tombstone lifetime and the unset-timestamp passthrough match the recorded design; consumers must (and are documented to) treat transitions idempotently.
+- NEXT-08 / NEXT-10 (GLAuth TLS posture, glauth.md table) are security/docs-domain rows, not promoted here.
+
+### Also verified in passing (other-domain fixes landing in gateway-core files)
+
+- **WRK-21/WRK-28/IPC-23 gateway half** — `Grpc/MxAccessGrpcRequestValidator.cs:74-88` rejects `DrainEvents.max_events` above the now-shared `GatewayContractInfo.MaxDrainEventsPerCommand` constant, with the byte-cap division of labor documented in the comment.
+- **Zero-quota pipe fix** (`da84635`) — `SessionWorkerClientFactory.cs:15-25`, `:175-185`: explicit 128 KiB in/out buffers with the windev-wedge rationale; this was the root cause of the historical full-suite testhost hang.
+- **Dead `ISessionManager.ReadEventsAsync` chain removed** (`fa9eb0c`) and `StreamEventsAsync` now consumes the subscriber channel directly (`Grpc/EventStreamService.cs:110-204`, `935f002`) — the WaitToReadAsync/TryRead loop preserves the fault-surface semantics (`:158-204`) including the worker-fault → `MarkFaulted` + metric path.
+- **Reply ownership transfer** (`3ff073d`) — `Grpc/MxAccessGrpcMapper.cs:62-85`: single-consumer argument for moving `reply.Reply` is documented and holds (one frame → one `PendingCommand` awaiter → one mapper call; `MergeDeniedInto` splices in place on the owned instance).
+- **GalaxyRepository adoption** — `GatewayApplication.cs:151-176`: scope provider registered *before* `AddZbGalaxyRepository` so the library's `TryAddSingleton` null default cannot win (order-sensitive, correctly commented); snapshot-path default seeded pre-bind with `GalaxyRepositoryOptionsValidator` + `ValidateOnStart` (SEC-33); `MapZbGalaxyRepository` at `:351`. `A2-galaxyrepository-adoption-handoff.md`'s caveats are all marked resolved and match the code. One hardening gap filed as **GWC-38**.
+- **Exception-total shutdown drain** (`95f8ba9`/`0bc13b5`) — `Sessions/SessionManager.cs:354-427`: bounded-parallel (4) close with an exception-total body, kill fallback on `CancellationToken.None`, and the deliberate token asymmetry against `CloseExpiredLeasesAsync` documented at length; `ISessionManager.ShutdownAsync`'s doc (`ISessionManager.cs:100-110`) states the bounded overrun honestly. Sweep selection stays sequential with the TOCTOU re-check inside `TryBeginCloseIfExpired` (`GatewaySession.cs:1671-1695`) — the cycle-2 close-gate invariants are preserved under the parallelization.
+
+## New findings
+
+IDs continue from the register. **Note on ID allocation:** the codebase already contains comment references to "GWC-31" (and a re-used "GWC-30") coined by an off-register remediation pass — those labels are *not* register entries (see GWC-31 below, which claims the ID and records the collision). Register IDs assigned here are authoritative.
+
+### GWC-31 — The 2026-08-15 perf remediation stamped unregistered "GWC-3x" IDs into code comments, colliding with this register
+
+- **Severity:** Low
+- **Category:** Documentation & comments
+- **Location:** `Workers/WorkerClient.cs:44` ("GWC-30" = pull-model queue-depth gauge), `:236` ("PERF(GWC-31)"), `:253` ("GWC-31: one pooled timer…"); `Metrics/GatewayMetrics.cs:46`, `:213`, `:284`, `:463`, `:524` ("GWC-30" = gauge pull model); `Tests/Gateway/Workers/WorkerClientTests.cs:163`, `:179` ("GWC-31" = WorkerCancel forwarding); analogous worker-side citations (`Worker/Ipc/WorkerFrameReader.cs`/`WorkerFrameWriter.cs`, per `docs/plans/2026-08-15-perf-review-remediation.md:544-548`)
+- **Description:** The 2026-08-15 performance review (whose findings live only in `docs/plans/2026-08-15-*.md`, not in `archreview/`) labeled its changes with GWC-series IDs that were never added to any register. "GWC-30" now means two different things — the archreview register says *frame-reader 4-byte prefix allocation*; the code comments say *pull-model worker queue-depth gauge* — and "GWC-31" exists only in comments. Finding IDs are the repo's cross-reference currency ("cite it in commits, branches, and PRs" — remediation tracker); a reader following a code comment to the register lands on the wrong finding, and future review cycles (this one included) must dodge already-burned numbers. REVIEW-PROCESS.md's "never reused" rule is violated in spirit.
+- **Recommendation:** Sweep the perf-pass citations to a distinct prefix (e.g. `PERF-nn` keyed to `docs/plans/2026-08-15-perf-review-remediation.md` task numbers) or append the perf findings to the register under fresh, unclaimed IDs and fix the comments to match. Add a line to REVIEW-PROCESS.md stating that GWC/WRK/IPC/SEC/CLI/TST prefixes are reserved for the archreview registers.
+
+### GWC-32 — `WorkerClient.DisposeAsync` never enters a terminal state, so the GWC-24 staging-overflow fault can misfire during a dispose race and the "already terminal" shutdown-drop claim is not guaranteed
+
+- **Severity:** Low
+- **Category:** Concurrency & thread safety (residual of GWC-24)
+- **Location:** `Workers/WorkerClient.cs:419-434` (`DisposeAsync` sets `_disposed`, cancels, completes all three channels — but `_state` is untouched), `:661-676` (`StageWorkerEvent` fallback: `TryWrite` false → `if (IsTerminalState()) return;` → otherwise `ProtocolViolation` fault), `:632-644` (XML doc: a completed staging channel is "safely dropped because the client is already terminal")
+- **Description:** `MarkClosed` and `SetFaulted` both flip `_state` to a terminal value *before* completing the channels, so the `IsTerminalState()` guard in `StageWorkerEvent` correctly classifies a completed-channel `TryWrite` miss as shutdown. `DisposeAsync` is the third channel-completing path and does not: a client disposed while still `Ready` keeps reporting `IsTerminalState() == false`. If the read loop is inside `DispatchEnvelope` when `DisposeAsync` completes `_eventStaging` (the `_stopCts.Cancel()` only interrupts the *next* `ReadAsync`), the in-flight `StageWorkerEvent` takes the overflow arm: a spurious `ProtocolViolation` session fault with the misleading "consumer is draining slower than the worker produces" message, a `worker-event-staging` overflow metric, a redundant `KillOwnedProcess`, and a `WorkerStopped("ProtocolViolation")` metric — all during an intentional teardown. Reachability is narrow: in the normal teardown order (`CloseAsync`/`Kill` → `DisposeAsync`) the state is already terminal; the exposed path is disposing a still-Ready client, which `SessionManager.OpenSessionAsync`'s failure arm does (`Sessions/SessionManager.cs:133-140` — `MarkFaulted` faults the *session*, not the worker client) and `SessionWorkerClientFactory`'s catch avoids only because it calls `Kill` first (`SessionWorkerClientFactory.cs:120-132`). Impact is corrupted diagnostics/metrics, not data loss — but the fault fires on exactly the code path an operator investigates after a failed open, pointing them at backpressure that never happened. It also makes the `StageWorkerEvent` XML doc's "already terminal" claim false on one of the three completion paths.
+- **Recommendation:** Treat disposal as terminal in the guard — `if (_disposed || IsTerminalState()) return;` — or have `DisposeAsync` transition `_state` to `Closed` under `_syncRoot` before completing the channels (which would also give GWC-12's double-dispose interlock a natural home). Update the `StageWorkerEvent` doc to name all three completion paths.
+
+### GWC-33 — A faulted-but-retained worker client keeps contributing its stale undelivered-event depth to the pull-model gauge until session disposal
+
+- **Severity:** Info
+- **Category:** Performance & resource management (observability)
+- **Location:** `Workers/WorkerClient.cs:120-121` (gauge source registered at construction), `:426-429` (unregistered only in `DisposeAsync`), `:1011-1039` (`SetFaulted` completes channels but leaves staged+queued events uncounted-down); retention window: `Configuration/SessionOptions.cs` `FaultedGraceSeconds` + `Sessions/SessionManager.cs:283-289` (faulted sessions swept only when reapable)
+- **Description:** The GWC-24 gauge counts staged + queued events and is decremented only on consumer read. When a client faults, its channels are completed with the fault and never drained, so `_eventQueueDepth` freezes at whatever was undelivered. The registration is removed only in `DisposeAsync`; with a positive `FaultedGraceSeconds` (deliberately configurable so operators can observe faults before the reap) the dead client's frozen depth inflates the aggregate `worker event queue depth` gauge for the whole grace window. With the default `FaultedGraceSeconds = 0` the window is one sweep interval, so the default posture is fine — recorded so the next metrics pass knows it was seen.
+- **Recommendation:** Optional: dispose the gauge registration inside `SetFaulted`/`MarkClosed` (the comment at `:427-428` already articulates the principle: "the sum must not keep counting a client that is going away").
+
+### GWC-34 — `EnsureDistributorCreated` recreates a never-started distributor after `DisposeAsync`; an attach that races a session teardown registers on a pump-less distributor and hangs silently
+
+- **Severity:** Low
+- **Category:** Concurrency & thread safety (residual of GWC-27 — the disposal-side twin)
+- **Location:** `Sessions/GatewaySession.cs:1865-1875` (`DisposeAsync` nulls `_eventDistributor`; `_eventDistributorStarted` is never reset — `:41`, `:554-559`), `:535-563` (`EnsureDistributorCreated`: `_eventDistributor is null` → constructs a fresh distributor; `startNow` is false because the started flag is already latched), `:946-998` (`AttachEventSubscriber`: readiness gate under `_syncRoot`, then the distributor calls run *outside* the lock)
+- **Description:** GWC-27 closed the premature-attach poison; the mirror-image hole remains open on the teardown side. An `AttachEventSubscriber` (or `AttachEventSubscriberWithReplay` / `AttachInternalEventSubscriber`) that passes its readiness gate while the session is still `Ready`, then loses the CPU until `CloseAsync` + `DisposeAsync` complete, reaches `EnsureDistributorCreated` after `_eventDistributor` was nulled: a **new** distributor is constructed, `startNow` comes back `false` (the latch is one-way), the subscriber registers on a distributor whose pump will never start and whose channel will never be completed — `WaitToReadAsync` in `EventStreamService.cs:167` then blocks until the client's own deadline/cancel. No error, no log, no fault; the replacement distributor is also never disposed. Contrast the adjacent race that *is* handled: if disposal instead wins between `EnsureDistributorCreated` returning the **old** distributor and `Register`, the old distributor's disposed check throws `ObjectDisposedException` loudly and the attach rolls back (`RegisterSubscriber`, `SessionEventDistributor.cs:361-386`). Reachability is small (the attach must stall across an entire multi-second close + dispose between two adjacent statements), but the failure mode is the worst kind — silent, per-stream, and invisible to metrics (the backlog registration reports 0 forever). This is the same "one distributor per session, never reset" lifecycle assumption GWC-27 documented, violated from the other end.
+- **Recommendation:** Make the latch symmetric with the null-out: in `DisposeAsync`, set a `_eventDistributorDisposed` flag (or reuse a session-terminal check) under `_syncRoot` when nulling `_eventDistributor`, and have `EnsureDistributorCreated` throw `SessionManagerException(SessionNotReady)` instead of re-creating when it is set. One flag, one throw — the attach paths' existing catch/rollback already handles the exception.
+
+### GWC-35 — `ApplyReconcile`'s state-delta repair is one-directional: an acked alarm replaced by a new unacked instance (or any acked→active regression) is absorbed silently by the snapshot replace
+
+- **Severity:** Low
+- **Category:** Correctness & logic (alarm feed completeness; residual of GWC-26)
+- **Location:** `Alarms/GatewayAlarmMonitor.cs:666-691` (`ApplyReconcile` both-present loop: broadcasts only when `incoming.Value.CurrentState == AlarmConditionState.ActiveAcked`), snapshot replace at `:687-691`
+- **Description:** The GWC-26 repair covers three legs — missing→present (`Raise`), present→missing (`Clear`), and unacked→acked (`Acknowledge`). The fourth leg is absent: an alarm the cache holds as `ActiveAcked` that the worker snapshot now reports as `Active` (the acked instance cleared and re-raised as a new unacked instance while the live `Clear`+`Raise` transitions were lost — e.g. the internal alarm lease overflowed and disconnected, or worker-side event loss) is absorbed by the `_alarms` replace with no feed transition. `CurrentAlarms`/`QueryActiveAlarms` are correct (they read the replaced cache), but live `StreamAlarms`/dashboard subscribers keep rendering the alarm as acknowledged until its next genuine transition — for a re-raised alarm, precisely the state an operator must not misread. Reachability is much lower than the pre-GWC-26 window (a lease overflow restarts the whole monitor lifecycle, which re-snapshots every subscriber), so this is a completeness gap in the defense-in-depth rather than a live bug.
+- **Recommendation:** Generalize the both-present delta to any `CurrentState` change: broadcast `Acknowledge` for `→ ActiveAcked` and `Raise` (with the incoming snapshot's new instance identity) for `→ Active`. The existing `TransitionFromSnapshot` already carries the timestamps the NEXT-03 dedup needs to suppress the live duplicates.
+
+### GWC-36 — `EventSubscriberReplayAttachment.OldestAvailableSequence` doc still describes the pre-GWC-25 empty-ring semantics
+
+- **Severity:** Info
+- **Category:** Documentation & comments
+- **Location:** `Sessions/EventSubscriberReplayAttachment.cs:26-30` ("The oldest worker sequence still retained and replayable; `0` when nothing is retained.")
+- **Description:** GWC-25 changed the empty-ring-with-gap value to `highest observed + 1`; the distributor's out-param doc (`SessionEventDistributor.cs:430-440`), the proto, and `docs/Sessions.md` were all updated, but this record's XML doc was missed. "`0` when nothing is retained" is now exactly the value the fix exists to avoid — a reader implementing against this doc re-introduces the dead-stream formula.
+- **Recommendation:** Mirror the distributor's out-param wording (retained-oldest, or `highest observed + 1` when the ring is empty; `0` only when `Gap` is false).
+
+### GWC-37 — The alarm subtag watch-list is resolved once per monitor lifecycle: a Galaxy-Repository outage at startup silently empties the subtag failover's coverage for the session's entire lifetime
+
+- **Severity:** Low
+- **Category:** Error handling & resilience (alarm subtag fallback)
+- **Location:** `Alarms/GatewayAlarmMonitor.cs:330-357` (`SubscribeAlarmsAsync` resolves the watch list once and sends it in the `SubscribeAlarms` command); `Alarms/AlarmWatchListResolver.cs:54-83` (GR discovery failure → warn and continue with a config-only, typically empty set), `:141-145` (zero-target warning); no re-resolve anywhere — `ApplyProviderModeChangeAsync` (`GatewayAlarmMonitor.cs:513-566`) triggers only a reconcile on failover
+- **Description:** The subtag fallback's coverage is fixed at `SubscribeAlarms` time: the resolved `AlarmSubtagTarget` list travels once to the worker and is never refreshed while the monitor session lives. Discovery failure is deliberately non-fatal (`ResolveAsync` catches, warns, and returns whatever config contributes) — reasonable for the resolver, but the monitor then subscribes with an empty watch list and runs indefinitely. In `Auto` mode the alarmmgr primary works fine, so nothing looks wrong until the moment a failover to subtag fires — at which point the degraded provider covers zero alarms, the feed goes quiet, and the only evidence is a single `LogWarning` emitted possibly weeks earlier at startup. The same staleness applies to Galaxy objects added after monitor start (a documented design property), but the outage-at-startup case differs in kind: the operator configured discovery, it silently produced nothing, and the safety net the feature exists to provide is empty. The monitor's supervisor restarts (and thus re-resolves) only when the session faults — a healthy alarmmgr session never does.
+- **Recommendation:** Re-resolve and re-subscribe on the failover edge (`ApplyProviderModeChangeAsync` when `toMode == Subtag`), or treat "GR discovery failed AND the effective mode can need the watch list (Auto/ForceSubtag) AND zero targets resolved" as a lifecycle failure so the supervisor's backoff loop retries resolution instead of latching the empty set. At minimum, surface the empty-watch-list condition on the provider status (`Degraded`/`Reason`) rather than only in a startup log line.
+
+### GWC-38 — `GatewayBrowseScopeProvider` fails open to the full Galaxy hierarchy when the ambient identity is absent, resting the browse-scoping control on an undeclared interceptor-ordering invariant
+
+- **Severity:** Low
+- **Category:** Security (defense-in-depth; GalaxyRepository adoption)
+- **Location:** `Security/Authorization/GatewayBrowseScopeProvider.cs:14-25` (`identityAccessor.Current?.EffectiveConstraints ?? ApiKeyConstraints.Empty` → empty `BrowseSubtrees` → library treats as "no scoping"); registration-order dependency documented at `GatewayApplication.cs:151-154`
+- **Description:** The per-key browse-subtree scoping — the one piece of authorization mxaccessgw adds on top of the shared GalaxyRepository package — resolves to *unscoped* (full hierarchy) whenever `IGatewayRequestIdentityAccessor.Current` is null. The code's own comment says this is "safe only because the global interceptor has already authenticated and authorized the request first. Do not reorder the interceptor without revisiting this." That is a correct description of today's wiring and an honest warning, but it makes the control's fail state *open*: any future path that reaches the library's galaxy service without the interceptor populating the ambient identity (a new endpoint mapping, an interceptor scoped per-service instead of globally, a library upgrade changing pipeline order) silently grants every scoped key the full hierarchy, with nothing failing and nothing logged. Scoping is a constraint-narrowing control, so its absence is invisible in normal operation. (Whether identity-absent is legitimately reachable under `Authentication:Mode=Disabled` is the security reviewer's domain; this finding is about the fail-open shape itself.)
+- **Recommendation:** Distinguish "authenticated with no BrowseSubtrees constraint" (legitimately unscoped) from "no ambient identity" (invariant broken): when `Current` is null *and* API-key authentication is enabled, throw `RpcException(PermissionDenied)` (or log-and-deny) instead of returning unscoped. One branch converts a silent authorization bypass into a loud misconfiguration signal.
+
+## Severity roll-up (new findings)
+
+| Severity | Count | IDs |
+|----------|:-:|-----|
+| Critical | 0 | — |
+| High | 0 | — |
+| Medium | 0 | — |
+| Low | 6 | GWC-31, GWC-32, GWC-34, GWC-35, GWC-37, GWC-38 |
+| Info | 2 | GWC-33, GWC-36 |
+
+Prior-finding verification: 7/7 cycle-2 Done claims hold (3 with caveats filed as GWC-32/34/35); 13 still-open cycle-1 findings re-confirmed open with none regressed; GWC-16 recommended for closure as a recorded Won't-fix.
+
+## Still good / preserve under change
+
+The cycle-2 remediation wave and the subsequent perf pass landed without breaking the core invariants, and several of them got structurally *stronger*. The backpressure topology is now bounded end to end again — worker queue → pipe → bounded staging (2×) → bounded consumer channel → bounded per-subscriber channels — with a loud fail-fast fault at every seam and a single observable depth gauge across the first two gateway stages; the GWC-24 implementation even added the startup overflow guard the design didn't ask for. The replay→live handoff argument is intact after the GWC-25 change, and the empty-ring sentinel arithmetic is documented in four places that all agree (distributor, proto, docs/Sessions.md, e2e test). The GWC-28 write-loop stamping is the textbook fix, and the `InvokeAsync` size-pre-check comment proves the interaction with it was reasoned about rather than stumbled past. The alarm monitor's attach-before-subscribe plus the NEXT-03 positive-match dedup give the alarm feed a defensible, *stated* delivery contract (at-least-once, idempotent-state), and the reconcile repairs are correctly quarantined to the feed layer — the "never synthesize events" rule for `MxEvent`s is respected everywhere, with the ReplayGap sentinel still the sole, documented control signal. Session state remains single-lock disciplined through the new detach-grace/faulted-grace/close-gate arms; the parallelized sweep and shutdown preserved the TOCTOU re-check and made the shutdown body exception-total with an honestly documented token asymmetry. The GalaxyRepository adoption is clean consumer wiring: scope-provider-before-package registration order is commented, the snapshot path is validated at startup, and the host-side projector is a pure function memoizable on the cache sequence. Comment quality across the new code is exceptional — most of this cycle's findings were *found via* comments that honestly documented their own residual assumptions ("safe only because…", "already terminal") — preserve that habit; it is what keeps one-layer-out defects findable. `GatewaySession` has grown again (2,173 lines) and the long-standing split-out recommendation stands.

archreview/2026-08-19/20-worker.md

@@ -0,0 +1,108 @@
+# Worker Process — Architecture Review (cycle 3)
+
+## Scope & method
+
+Date 2026-08-19 · commit `8b3e3b3` (`main`, clean) · macOS static review — the worker targets .NET Framework 4.8 x86 and was **not built or tested** here; every claim cites file:line evidence read directly from the working tree at HEAD. Scope: `src/ZB.MOM.WW.MxGateway.Worker` (STA runtime/pump, `MxAccessSession`/`MxAccessStaSession` COM lifetime, `Ipc/` pipe session + frame reader/writer, event queue and DrainEvents, value/write-completion caches, alarm consumers incl. the subtag failover, watchdog/heartbeat, program lifecycle/exit codes) and `src/ZB.MOM.WW.MxGateway.Worker.Tests`. This is a verification pass over the 2026-07-12 review (`archreview/2026-07-12/20-worker.md`, remediation IDs WRK-21..28 in `archreview/2026-07-12/remediation/20-worker.md` / `00-tracking.md`, plus NEXT-04/NEXT-05 from `remediation/90-candidate-findings-next-cycle.md`), a re-check of the still-open cycle-1 findings, and a deep review of everything landed since `4f5371f` (~50 worker commits, 53 files, +9,014/−427).
+
+Changed-in-scope since `4f5371f`: the WRK-21/28/23/IPC-30 drain cluster (`33ba612`, `6bc3f9b`), the WRK-22/24/25/27/IPC-26 write-seam batch (`8df35cd`), the OnWriteComplete correlation feature (`fc23a65`..`b0e65d4`), the perf wave (pooled frame buffers `f56798a`, reverse tag index `afec56d`, message-driven completion waits `dc9424d`, signal-driven event drain `f4b065b`, exact-format timestamp parse `94fdc18`/`25cbe5c`, enqueue-time frame sizing `58d97ad`, value-cache borrowing `9871d47`, class-transition flush `aac7957`, drain-pass unparking `9130994`), the NEXT-04/NEXT-05 abandonment-observation batch (`84dbf20`, `3ef56be`, `b8b7b69`), session-owned transport disposal (`e913dab`, labeled WRK-31 in code), the alarm-side truncation/failover hardening (`f3e1de5`, `b5ea6bb`, `693a78d`, `b9fb0dd`), and the WorkerPipeSessionTests harness rework (`7da52b6`, `462850a`, `aaeb86b`, `753d070`).
+
+**Finding-ID note:** the tracker instruction says new IDs continue at WRK-29, but `e913dab` already consumed **WRK-31** as an interim ID (cited in code at `Ipc/WorkerPipeSession.cs:45,177` and in `Ipc/WorkerPipeSessionTests.cs`); WRK-29/WRK-30 appear nowhere in the repo or its history but may have been burned in the same interim numbering. To guarantee no reuse, this review's new findings start at **WRK-32**.
+
+## Executive summary
+
+- All eight cycle-2 Done claims (WRK-21..28) are present, sound, and tested at HEAD; none is a false Done and none carries a functional caveat. The WRK-21 fix went beyond the design: the drain sizes events at enqueue time (`58d97ad`) so the byte-budget walk no longer holds the queue lock the STA needs, and `6bc3f9b` made the budget monotonic at the reserve boundary so drain-until-empty terminates even at the 1024-byte negotiated floor.
+- NEXT-04 (fault-observing continuations on abandoned/tombstoned frames) and NEXT-05 (lazy tombstone purge as a documented decision) are both verified — the observer discipline is applied uniformly in the writer (`ObserveAbandonedFault`/`ObserveFault`), and `e913dab`/`3ef56be` extended it to the abandoned net48 pipe read and overrun background tasks, with a GC-forcing `UnobservedTaskException` regression test.
+- The two riskiest perf changes checked out. The value-cache borrow (`9871d47`) is safe because the post-publish hook runs only after `Enqueue` and every consumer of the borrowed instances is serialize-only (verified through `MxAccessSession.SucceededRead` and the write-completion cache, which deliberately still clones); the class-transition flush (`aac7957`) and drain-pass unparking (`9130994`) preserve the "written and flushed before completion" contract, are lost-wakeup-free (a walked-away lock acquisition is detached, drains, and releases — never dropped), and cannot double-release the semaphore.
+- One cycle-1 finding was fixed incidentally: **WRK-13** (unpooled writer buffers) is resolved by `f56798a` — the writer now rents one combined prefix+payload buffer from `ArrayPool` (`Ipc/WorkerFrameWriter.cs:675-687`). The old tracker should be updated. WRK-18's stale-doc half is also discharged (the design doc now describes the implemented fail-fast and honestly names the generic exit code); its dedicated-exit-code half stays open.
+- The remaining cycle-1 opens (WRK-02/03/05/08/09/10/14/16/17/18/19 and the WRK-02/03 slice of WRK-20) are all still present. WRK-05 is materially narrowed: in auto/failover mode `FailoverAlarmConsumer.RunPrimary` absorbs primary COM failures (`MxAccess/FailoverAlarmConsumer.cs:284-291`), but the default alarmmgr-only path is still one-poll-exception-fatal (`MxAccess/MxAccessStaSession.cs:392-405`).
+- Fresh sweep found no Medium or higher. Three new Lows: recency-based OnWriteComplete correlation can attribute a stale completion from an earlier write to the current write's reply (WRK-32); the discarded command-task observer is the one discarded task in the worker without a fault-observing continuation (WRK-33); the StaHung fault writes the in-flight correlation id into `WorkerFault.CommandMethod` (WRK-34, pre-existing since 2026-04-26).
+
+## Prior-finding verification — cycle 2 (all tracked Done) and NEXT items
+
+| ID | Claimed status | Verified? | Evidence (file:line at HEAD) | Notes |
+|----|----------------|-----------|------------------------------|-------|
+| WRK-21 (= IPC-23) | Done | **Yes** | Byte-budgeted `Drain(uint, int)` dequeues only events known to fit, leaves a non-fitting head queued, reports `OversizedHeadSequence` (`MxAccess/MxAccessEventQueue.cs:389-429`); per-event size memoized at enqueue outside the drain's lock hold (`:248-281`, `:502-509`, slack constants `:73-81`); reply builder consumes it with truncation `DiagnosticMessage` (`Ipc/WorkerPipeSession.cs:978-1020`); budget = `max(MaxMessageBytes − 64 KiB, MaxMessageBytes/2)` so it stays monotonic at the reserve boundary (`:1034-1039`, `6bc3f9b`); shared result type is net48-safe (`MxAccess/WorkerEventDrainResult.cs`). Backstops: control-reply write catches `MessageTooLarge` and answers with a small `InvalidRequest` reply (`Ipc/WorkerPipeSession.cs:847-864`), STA command replies get the same catch (`:1136-1147`), and the fallback write itself is size-guarded so a degenerate negotiated max cannot make the backstop fatal (`:876-900`). Tests: `WorkerPipeSessionTests.DrainEvents_ByteHeavyQueue_ReplyIsBoundedAndSessionSurvives` (:571), `DrainEvents_RepeatedCalls_RecoverAllEventsWithoutLoss` (:630), `ControlReplyTooLarge_WritesErrorReplyInsteadOfDying` (:770), `CommandReplyTooLarge_WritesErrorReplyInsteadOfFaulting` (:815), plus `MxAccessEventQueueTests` byte-budget cases. | Sound, and stronger than designed (no sizing walk under the STA-shared lock). No diagnostics command is session-fatal on size anymore. |
+| WRK-22 (= IPC-26) | Done | **Yes** | `PendingFrame.Claimed` interlock documented and mutated only under `_gate` (`Ipc/WorkerFrameWriter.cs:54-62`); cancelled waiter tombstones only unclaimed frames (`:169-183`, `:308-338`); `DequeueNext` skips cancelled frames and claims atomically in the same critical section (`:584-618`); contract documented in the method doc (`:105-112`) and `docs/WorkerFrameProtocol.md:203-211`. Tests `WorkerFrameProtocolTests.WriteAsync_CancelledWhileWaitingForLock_FrameIsNeverWritten` (:695), `WriteAsync_CancelledEventFrame_DoesNotTrailShutdownAck` (:800). | Sound. The claimed-mid-write residual window is documented as by-design. |
+| WRK-23 | Done | **Yes** | Peek-stamp-commit: candidate stamped before the size checks (sequence participates in `CalculateSize`), counter committed only once the frame is certain to be written, so a per-frame rejection burns no number (`Ipc/WorkerFrameWriter.cs:648-666`; comment `:72-76`). | Wire sequence stays contiguous across rejections; the gap-free concurrent-sequence test's guarantee is now genuinely true. |
+| WRK-24 | Done | **Yes** | `MinNegotiableFrameBytes = 1024` matching the gateway validation floor, rejected at the handshake with `InvalidConfiguration` (`Ipc/WorkerFrameProtocolOptions.cs:30`, `:144-150`); range documented in the property doc and `docs/WorkerFrameProtocol.md`. | Also closes the degenerate config that could have made WRK-21's fallback swallow reachable (`Ipc/WorkerPipeSession.cs:866-875` documents that chain). |
+| WRK-25 | Done | **Yes** | `WriteBatchAsync` enqueues the whole batch under one `_gate` acquisition, drains under one lock, awaits every completion so no per-frame rejection faults unobserved, rethrows the first failure in batch order (`Ipc/WorkerFrameWriter.cs:236-306`); the event drain loop submits drained batches through it (`Ipc/WorkerPipeSession.cs:615-630`). Tests `WriteBatchAsync_FlushesOnceAndPreservesOrder` (:839), `WriteBatchAsync_ControlFrameQueuedDuringBatch_JumpsRemainingEvents` (:876), `WorkerPipeSessionTests.EventBurst_DrainLoopCoalescesFlushes` (deflaked `50322ba`). | The WRK-12 coalescing now engages on the exact path it was built for: an uninterrupted N-event burst pays one flush. `aac7957` later refined the mixed-class case (see deep review). |
+| WRK-26 (discharges IPC-29) | Done | **Yes** | `docs/MxAccessWorkerInstanceDesign.md:717-751` — Outbound Queues rewritten to the two-class scheduler, explicitly records the accepted collapse of the five-level order and why, and rewrites the overflow paragraph to the implemented fail-fast (naming the still-generic `UnexpectedFailure` exit code honestly — WRK-18's exit-code half remains tracked); `docs/WorkerFrameProtocol.md:79+` "Write Scheduling And Sequencing" covers priority classes, write-time stamping, per-frame vs stream-failure semantics, flush coalescing, and cancellation. | No stale five-level prose remains. |
+| WRK-27 | Done | **Yes** | `volatile bool staAlarmPollInProgress` flipped on the STA exactly around `PollOnce` (`MxAccess/MxAccessStaSession.cs:64-70`, `:352-365`); surfaced via `WorkerRuntimeHeartbeatSnapshot.StaCallInProgress` (`MxAccess/WorkerRuntimeHeartbeatSnapshot.cs`, ctor default keeps net48-safe plain ctor); watchdog suppression honors it alongside the in-flight correlation id up to the ceiling (`Ipc/WorkerPipeSession.cs:1359-1370`). Test `WorkerPipeSessionTests.Watchdog_StaCallInProgress_SuppressedUntilCeiling` (:1260). | A slow-but-healthy poll now gets the 75 s ceiling, not the 15 s grace; a poll blocking past the ceiling still faults, per the ceiling's contract. |
+| WRK-28 | Done | **Yes** | `GatewayContractInfo.MaxDrainEventsPerCommand = 10_000` in Contracts (`src/ZB.MOM.WW.MxGateway.Contracts/GatewayContractInfo.cs:33`), consumed by the gateway validator (`src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcRequestValidator.cs:82-86`) and the worker clamp (`Ipc/WorkerPipeSession.cs:992-994`). | Single shared constant; the desync failure mode is removed, not merely detected. |
+| NEXT-04 | Resolved 2026-08-10 | **Yes** | Unconditional fault-observing continuation on every frame a cancelled caller abandons — `TombstoneIfUnclaimed`/`TombstoneUnclaimed` → `ObserveAbandonedFault` → `ObserveFault` (`Ipc/WorkerFrameWriter.cs:308-372`); a cancelled task never fires `OnlyOnFaulted`, so the unconditional attach is safe, and it runs outside `_gate` because an already-faulted task continues inline. Also applied to the detached lock wait (`:389-426`) and, session-side, to overrun background tasks and the abandoned pipe read (`Ipc/WorkerPipeSession.cs:521-576`). Regression test drives the exact abandonment and asserts a marker exception never reaches `TaskScheduler.UnobservedTaskException` (`WorkerFrameProtocolTests:733-785`). | Verified; the discipline is now near-uniform (the one residual gap is WRK-33). |
+| NEXT-05 | Resolved as documented decision | **Yes** | `DequeueNext` still lazily skips tombstones (`Ipc/WorkerFrameWriter.cs:588-618`); the rationale — any subsequent write drains both queues, the heartbeat bounds tombstone residency to one interval, and eager `Queue<T>` rebuilds would add ordering-invariant surface next to the WRK-22 interlock — is recorded in `docs/WorkerFrameProtocol.md:218-221`. | Decision recorded where the review asked for it. |
+
+Done claims that failed verification: **none**. Regressions in previously-verified fixes: **none** — WRK-01 (pump refresh, `Sta/StaRuntime.cs:95-100`), WRK-04/GWC-28 (write-time sequence stamping, now peek-stamp-commit), WRK-06 (converter cache, extended by `94fdc18`'s compiled accessors), WRK-07 (two-class scheduler), WRK-11 (ownership transfer, now load-bearing for both the value-cache borrow and enqueue-time sizing), WRK-12 (flush coalescing, finally effective via WRK-25), and WRK-15 (doc names) all remain intact at HEAD.
+
+## Prior-finding verification — still-open cycle-1 items
+
+| ID | Status at HEAD | Evidence | Notes |
+|----|----------------|----------|-------|
+| WRK-02 | **Still open** | `Sta/StaRuntime.cs:265-269` (loop exception stored in write-only `startupException` after startup; only `startedEvent.Set()`); `InvokeAsync` (`:161-189`) checks disposal and shutdown but not thread death — work enqueued after `ThreadMain` unwinds is consumed by nobody and its task hangs forever (`CancelQueuedCommands` in the `finally` at `:272` only drains what was queued at death). | Highest-value remaining worker item, unchanged three cycles running. |
+| WRK-03 | **Still open** | `Ipc/WorkerPipeSession.cs:1226-1229` — `TryStartCommandTask` silently `return`s when `_acceptingCommands` is false: no reply, no log. | The gateway's correlation wait times out blind. |
+| WRK-05 | **Still open, narrowed** | `MxAccess/MxAccessStaSession.cs:392-405` — any exception from one poll records a fault and stops the loop; the drain loop turns it into session termination (`Ipc/WorkerPipeSession.cs:589-598`). Narrowed: in auto mode the `FailoverAlarmConsumer` absorbs primary COM failures inside `PollOnce` (`MxAccess/FailoverAlarmConsumer.cs:284-291`, catches all but OOM and counts toward failover), so a transient alarmmgr E_FAIL no longer escapes. The default alarmmgr-only mode (`Ipc/WorkerPipeSession.cs:78`, and subtag-source failures in standby mode) is still one-failure-fatal. | Worth re-scoping to "non-failover alarm modes" if kept. |
+| WRK-08 | **Still open** | `Ipc/WorkerPipeSession.cs:487-491` — the message-loop `finally` cancels the drain loop after the shutdown ack; events already drained from the queue (`:600`) whose batch write is cancelled are tombstoned and lost, and the residual queue is never shipped. `docs/MxAccessWorkerInstanceDesign.md` still documents no discard (`grep residual/unshipped` finds nothing). | Same scale as before the batch change (the old loop also drained 128 ahead of writing). Documentation half remains the cheap part. |
+| WRK-09 | **Still open** | No `AppDomain.UnhandledException`/`TaskScheduler.UnobservedTaskException` handler anywhere under `src/ZB.MOM.WW.MxGateway.Worker/` (grep at HEAD; the only mentions are comments noting the absence). | The NEXT-04 observer network reduces the practical exposure but is not a process-level backstop. |
+| WRK-10 | **Still open** | `WorkerApplication.cs:112-139` — all three catch blocks log `exception_type` only; the redactor (`Bootstrap/WorkerLogRedactor.cs`) is available and unused here. | Still a cheap diagnostics win. |
+| WRK-13 | **Resolved (side effect of `f56798a`)** | `Ipc/WorkerFrameWriter.cs:675-687` — single `ArrayPool<byte>.Shared.Rent` for prefix+payload, one stream write, returned in `finally`; matches the reader's pooling. | Flip to Resolved in `archreview/remediation/00-tracking.md` with commit `f56798a`. |
+| WRK-14 | **Still open** | `Ipc/WorkerPipeSession.cs:33-62` `_camelCase` vs `MxAccess/MxAccessStaSession.cs:50-71` and `Sta/StaRuntime.cs:10-24` bare `camelCase`. | Cosmetic; new files followed their host file's convention, so the split persists. |
+| WRK-16 | **Still open** | `Ipc/WorkerPipeSession.cs:1449-1544` — seven `CreateEnvelope`/`CreateBaseEnvelope` pairs; `Ipc/WorkerPipeClient.cs` still has 7 public constructors. | |
+| WRK-17 | **Still open** | `WorkerApplication.cs:110-119` maps every `WorkerFrameProtocolException` (including `EndOfStream`) to exit 6 `ProtocolViolation`. | |
+| WRK-18 | **Half discharged** | Doc half done: `docs/MxAccessWorkerInstanceDesign.md:743-751` now describes the implemented fail-fast and names the generic `UnexpectedFailure` exit honestly. Code half open: a drain-loop fault still exits 1 via the generic catch (`WorkerApplication.cs:131-139`); no dedicated overflow/event-fault exit code. | |
+| WRK-19 | **Still open** | `Sta/StaCommandDispatcher.cs` — no logger field, no start/end/failure log anywhere in the file. | |
+| WRK-20 | **Residual only** | The WRK-21..28 batch added its full test matrix (byte-budget drain, size backstops, cancellation/tombstoning, batch flush, floor handshake, watchdog suppression), plus teardown `UnobservedTaskException` tests (`WorkerPipeSessionTests:2328-2434`). Still missing: STA-thread-death (WRK-02) and command-refused-during-shutdown (WRK-03) tests, because their fixes are open. | |
+
+## Deep review of the new code
+
+**Frame-writer unparking (`9130994`) and class-transition flush (`aac7957`).** `WriteAsync` now races its frame's completion against the lock acquisition (`Ipc/WorkerFrameWriter.cs:148-198`): a caller whose frame is delivered by another drainer returns at the boundary flush instead of at the winner's release, and the acquisition it walks away from is handed to `DetachLockWait` (`:389-399`), whose continuation either observes a cancelled wait (no count taken, nothing to release) or drains-and-releases (`:409-452`). I probed this seam hard: acquisition and cancellation are mutually exclusive in `SemaphoreSlim`, so no count can leak and no double-release path exists (`DrainDetachedAsync` releases in a `finally` that runs exactly once per successful acquisition); a frame enqueued between a drainer's last `DequeueNext` and its release is picked up by the detached pass, so no lost wakeup; every discarded task in the chain carries the NEXT-04 observer. The drain flushes and completes at each control→event boundary (`:484-511`), so a heartbeat/reply/fault's delivery point is never charged for the event backlog behind it; frames completed at a boundary leave `written`, so a later flush failure in the same pass cannot reach back and fail an already-delivered frame (`:543-557`) — the failure-attribution edges are all correct. Cost claims in the comments are accurate: a pure-event pass still pays one flush; only a genuinely mixed pass pays one extra per class transition. Residual (accepted, documented at `Ipc/WorkerPipeSession.cs:186-191`): a detached acquisition can schedule an empty drain pass after session teardown — harmless because the queues are empty and an empty pass writes nothing.
+
+**Value-cache borrowing (`9871d47`).** The hunt for aliasing bugs came up clean. The borrow is sound on three legs, each verified: (1) ordering — `MxAccessBaseEventSink.EnqueueEvent` runs the `postPublish` cache write only after `eventQueue.Enqueue` returns (`MxAccess/MxAccessBaseEventSink.cs:248-295`), so the event is already write-once (WRK-11 ownership plus the sequence/timestamp stamps under the queue lock); (2) consumers — `MxAccessSession.SucceededRead` copies the borrowed `Statuses` element references into a fresh `RepeatedField` on the reply and reference-shares `Value`/`SourceTimestamp` (`MxAccess/MxAccessSession.cs:1066-1097`), and everything downstream only serializes (the worker↔gateway process boundary prevents any other consumer from aliasing); (3) the size-memoization coupling — a post-enqueue mutation would now also invalidate `QueuedEvent.Size`, and the ownership contract text in both classes names that hazard explicitly (`MxAccess/MxAccessValueCache.cs:76-108`, `MxAccess/MxAccessEventQueue.cs:15-32`). The write-completion cache deliberately still clones and documents why the asymmetry is not an oversight (`MxAccess/MxAccessWriteCompletionCache.cs:77-89`). Protobuf reads (`CalculateSize`/`WriteTo`) from multiple threads on a frozen message are safe, and every cross-thread hand-off passes through a lock.
+
+**Enqueue-time frame sizing (`58d97ad`).** `CalculateWorkerEventSize` reconstructs the generated `CalculateSize` arithmetic for exactly the two stamped fields (`MxAccess/MxAccessEventQueue.cs:493-509`); I checked the tag math — `worker_sequence` (field 9, varint) and `worker_timestamp` (field 10, length-delimited) are both single-byte tags, the `WorkerEvent.event` wrapper (field 1) matches the `DrainEventsReply.events` packing shape exactly, and the +8 `RepeatedFieldOverheadBytes` slack absorbs the documented renumbering hazard (`:66-81`). Clearing the two fields before measurement makes the pre-lock measurement exact even against a caller that pre-filled them. The wake signal (`SignalWake`, `:471-491`) is a level, not a count; I walked the check-then-release race against a concurrent consumer and every interleaving either sees the event in its re-drain or leaves a pending permit — no lost wake, and the 25 ms fallback tick bounds any hypothetical future unsignalled mutation.
+
+**Session-owned transport disposal and abandonment observation (`e913dab`, `3ef56be`, WRK-31).** The session now owns the pipe stream and disposes it as the last teardown step — the only thing that unblocks a net48 pipe read parked in the kernel — then awaits the abandoned read's resulting fault (`Ipc/WorkerPipeSession.cs:143-290`). The `_pendingReadTask` bookkeeping preserves the reader's single-consumer invariant through teardown (set at issue, cleared before the loop's own await, read only after the loop's task completes — the await supplies the happens-before), and two `Debug.Assert`s pin it (`:447-449`, `:470-472`). `ObserveBackgroundTaskStopAsync` correctly separates the logging budget from observation: an overrun task gets `ObserveFaultWhenever` before the method returns (`:521-558`), so the 1 s budget is a diagnostics decision, not a correctness one. `DisposeTransportStream`'s total catch is well argued (nothing thrown while closing a handle may displace the session's terminal exception, `:212-250`), and `WorkerPipeClient` keeps its `using` as an explicitly-documented idempotent backstop (`Ipc/WorkerPipeClient.cs:153-165`).
+
+**Oversized-event death (IPC-30) through the batch path.** A `MessageTooLarge` surfacing from the batch's awaited completions is mapped back to the first envelope in batch order that overshoots the negotiated max, logged with identity-but-never-value fields, written as a structured `ProtocolViolation` fault, and then thrown (`Ipc/WorkerPipeSession.cs:626-725`). `FindOversizedEvent`'s recomputation is consistent with what the writer rejected (the rejected envelope retains its tentative sequence stamp, so `CalculateSize` agrees), and the unreachable-fallback comment is honest. Other per-frame rejection codes still unwind the session as worker bugs — the deliberate IPC-30 scope.
+
+**Write-completion correlation (`fc23a65`..`b0e65d4`).** Parity-clean in the load-bearing respects: no synthesized events (the completion rides the unary reply's `statuses` only), acceptance-only `Hresult`/`ProtocolStatus` either way, expiry yields empty statuses ("unconfirmed, not failed"), zero timeout restores pure fire-and-forget, and the pump-wait keeps the STA dispatching for the whole wait via the shared `MsgWaitForMultipleObjectsEx` declaration (`Sta/StaNativeMethods.cs`, single-copy rationale documented). `StaWaitHelper` handles the failed-wait degrade (one-shot trace + blind sleep, `Sta/StaWaitHelper.cs:104-112`) and documents the `MWMO_INPUTAVAILABLE` drain-to-empty contract that would otherwise busy-spin. The one soft spot is attribution across successive writes to the same handle — WRK-32 below.
+
+**Alarm subtag failover and truncation.** The production wiring works despite the `standbyFactory: null` default: the `comFactory` threaded through `MxAccessStaSession.StartAsync` lets `AlarmCommandHandler.BuildStandby` construct the `SubtagAlarmConsumer` over `LmxSubtagAlarmSource` (`MxAccess/AlarmCommandHandler.cs:206-248`); mode selection (alarmmgr / subtag / auto-failover) matches the documented matrix, provider-mode changes are surfaced as queue events with handler detach on unsubscribe/dispose (`:159-183`, `:258-267`). WnWrap truncation handling is careful where it matters: a truncated fetch merges into the retained snapshot instead of clearing it (`ApplySnapshotUpdate`), so Clear transitions are never synthesized from records that merely fell off a truncated fetch, and the truncation verdict rides out of the same locked fold that produced the transitions (`FoldFetch`), with a rate-limited warning. STA affinity for the wnwrap COM object is asserted at every consumer entry (`MxAccess/MxAccessStaSession.cs:410-440`), and the affinity-violation exception is deliberately routed to the fault path rather than the graceful-stop path (`:378-405`).
+
+**Handle-registry reverse index (`afec56d`).** Every mutation maintains all four secondary indexes and the three memoized views; I walked the consistency edges — re-registration with a changed definition drops the stale index entry first, per-server teardown removes exactly the owned sets, advice de-indexing trims empty containers, and the sorted-insertion keeps `GetItemHandlesForDefinition` in the same order the old linear scan visited (`MxAccess/MxAccessHandleRegistry.cs`). The registry is STA-confined by documented contract, so the lock-free indexes are sound. The returned live-view caveat is documented at the API.
+
+**Timestamp fast path (`94fdc18`, `25cbe5c`).** The per-culture derived formats are validated by round-tripping a probe instant before they ever reach the event path, so a pathological culture pattern (quoted `ss`, existing fraction) degrades to no-fast-path instead of a throwing `TryParseExact`; the fast path is provably a shortcut, never a different answer, because the formats are derived from the same culture the general parse would use (`MxAccess/MxAccessEventMapper.cs:373-509`).
+
+**Test harness rework.** `FakeRuntimeSession` now mirrors production's coalesced wake signal (never-disposed `SemaphoreSlim`, same rationale comment), models STA-pump-at-heartbeat (`refreshStaActivityOnCapture`), and its 30 s dispatch safety net is decoupled from scenario timing with the reasoning written down (`TestSupport/FakeRuntimeSession.cs:23-51`). The teardown tests arm `TaskScheduler.UnobservedTaskException` and force GC — the strongest possible assertion shape for the NEXT-04/WRK-31 class of fix. Test counts in scope: 40 facts/theories in `WorkerPipeSessionTests`, 31 in `WorkerFrameProtocolTests`, 23 in `MxAccessEventQueueTests`, plus new suites for the completion cache, wait helper, registry, and WnWrap XML parsing.
+
+## New findings
+
+**WRK-32 — Low (error handling / documentation).** Write-completion correlation is recency-based per handle; a late `OnWriteComplete` from an earlier write can be reported as the current write's outcome.
+Evidence: the baseline is `CurrentVersion` captured before the COM call (`MxAccess/MxAccessCommandExecutor.cs:447-450`, `:522-528`), and `AwaitWriteCompletion` copies onto the reply *any* completion whose version exceeds it (`:1005-1027`; version test only, `MxAccess/MxAccessWriteCompletionCache.cs:162-169`). MXAccess's `OnWriteComplete` carries no correlation token, only the handle pair — so if write A to a tag times out unconfirmed (its callback still undelivered) and the client immediately writes B to the same tag, A's stale callback dispatching during B's pump-wait bumps the version past B's baseline and A's status rows ride B's reply. The executor doc comment claims "the OnWriteComplete row *matching* a WriteSecured/WriteSecured2 call" (`:996-1003`), and neither `docs/MxAccessWorkerInstanceDesign.md` nor the proto comments mention the ambiguity (grep for misattribution/stale/earlier-write finds nothing). Impact is bounded — the statuses are an enrichment field, `Hresult`/`ProtocolStatus` stay acceptance-only, and back-to-back same-tag writes with a timed-out predecessor is the only exposed sequence — and the ambiguity is inherent to the COM surface (a native client correlating by handle has the same problem, so mechanical parity is arguably preserved).
+Recommendation: document the recency-based contract at the executor, in the design doc's write-completion section, and in the reply-field proto comment; additionally, run one `pumpStep()` immediately before capturing the baseline so an already-queued stale callback is folded into the baseline instead of the wait window. Full closure is impossible without an MXAccess-side token — say so where consumers will read it.
+
+**WRK-33 — Low (concurrency / NEXT-04 discipline gap).** The discarded command-task observer is the one discarded task in the worker without a fault-observing continuation.
+Evidence: `TryStartCommandTask` discards its wrapper — `_ = ObserveCommandTaskAsync(commandTask);` (`Ipc/WorkerPipeSession.cs:1235`) — and `ObserveCommandTaskAsync` catches only `OperationCanceledException` (`:1238-1254`). `ProcessCommandAsync` handles its own failures, but its catch path ends in `TryWriteFaultAsync`, whose filter is exactly `IOException`/`ObjectDisposedException`/`WorkerFrameProtocolException` (`:1440-1447`); any other exception escaping that write — or a future edit anywhere in the catch path — propagates out of `ProcessCommandAsync`, faults the discarded wrapper, and surfaces only as a finalizer-time unobserved-task exception in a process that installs no handler (WRK-09). Every other discarded task in this codebase now carries an observer (`ObserveFaultWhenever` lives 660 lines up in this same file); the gap is unlikely to fire but contradicts the discipline the NEXT-04/WRK-31 batch established and tested.
+Recommendation: `ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask));`, or broaden `ObserveCommandTaskAsync` to catch-all-and-log (the redaction-safe fields are already at hand in `LogCommandResultDropped`'s shape).
+
+**WRK-34 — Low (documentation / diagnostics).** The StaHung fault writes the in-flight correlation id into `WorkerFault.CommandMethod`.
+Evidence: `ReportWatchdogFaultIfNeededAsync` builds its fault with `CreateFault(WorkerFaultCategory.StaHung, snapshot.CurrentCommandCorrelationId, $"STA activity is stale by {staleFor}.")` (`Ipc/WorkerPipeSession.cs:1384-1389`), whose second parameter is `commandMethod` (`:1712-1728`) — so the wire fault's `command_method` field carries a correlation id (or empty), a correlation id never lands in a correlation-shaped field, and a gateway/dashboard consumer reading `CommandMethod` as a method name gets an opaque id string. Pre-existing since `4a3560c` (2026-04-26), never previously flagged; noted now because the WRK-27 work touched this exact branch and the field's semantics matter more once polls also suppress here.
+Recommendation: set `CommandMethod` to a stable label (empty, or `"Watchdog"`), and carry the stuck command's correlation id in the fault's diagnostic message or the envelope's `CorrelationId` where the gateway already correlates.
+
+## Severity roll-up (new findings)
+
+| Severity | Count | IDs |
+|----------|-------|-----|
+| High | 0 | — |
+| Medium | 0 | — |
+| Low | 3 | WRK-32, WRK-33, WRK-34 |
+
+## Still good / preserved
+
+Every structural strength recorded in the prior two reviews is intact at HEAD, and several were deepened. The STA core still runs the canonical `MsgWaitForMultipleObjectsEx` + `PeekMessage`/`DispatchMessage` loop with the wake event and 50 ms idle pump (`Sta/StaRuntime.cs:255-261`), and the subtle Win32 wait contract now has a single shared declaration with the drift hazard written down (`Sta/StaNativeMethods.cs`). All MXAccess COM calls remain on the dedicated STA (commands via the dispatcher, alarm polls via `InvokeAsync` with an affinity assertion, the consumer factory on-thread at `MxAccess/MxAccessStaSession.cs:282-296`); COM lifetime discipline (`FinalReleaseComObject`, sink-detach-before-release, UnAdvise → RemoveItem → Unregister, alarm handler disposed on the STA before data-side teardown) is unchanged. Nothing synthesizes events — the completion-correlation feature explicitly kept `OperationComplete` untouchable and the truncation work explicitly refuses to fabricate Clear transitions. Fail-fast backpressure stands (no drop policy; overflow self-records the first fault; the dispatcher's 128-entry bound rejects loudly). Credentials and values stay out of logs (the oversized-event fault logs sizes and identity only, `AuthenticateUser` passes the password straight through, the DrainEvents truncation message names sequences not values). net48 constraints are respected in every new type (plain ctors, get-only properties, `readonly struct`, no init/records — several carry the CS0518 rationale in their remarks). The IPC layer's failure-attribution story is now the best part of the worker: written-and-flushed-before-completion, per-frame vs stream-failure separation, boundary-flush completion, tombstone/claim interlock, and a fault observer on effectively every abandoned task, each with a regression test that asserts the invariant rather than the implementation. Comment quality across the new code is exceptional — ownership contracts, memory-model reasoning, and rejected alternatives are recorded at the exact seams where the next editor would otherwise re-litigate them.
+
+## Verification needed on Windows (windev)
+
+- This review is static: the x86 worker was not built and `Worker.Tests` were not run here. The tracking log and repo memory record the worker suite fully green on windev after the harness fixes (`7da52b6`/`462850a`/`aaeb86b`), and the WRK-21 cluster carries windev evidence (`c992568`); any fix for WRK-32..34 needs an x86 build plus filtered `MxAccessCommandExecutorTests`/`WorkerPipeSessionTests` runs there.
+- WRK-32's pre-baseline pump mitigation should get a windev test that records a stale completion into the fake's message path before the second write's baseline and asserts it does not ride the second reply.
+- WRK-33 is assertable with the existing `UnobservedTaskException`-arming test shape (`WorkerPipeSessionTests:2328-2434`): drive an exception through `TryWriteFaultAsync`'s filter gap with a throwing fake writer and assert nothing reaches the finalizer.
+- Live MXAccess only: the WRK-05 narrowing (failover absorbing a transient alarmmgr E_FAIL without a session fault) deserves a live-rig confirmation next time the opt-in alarm smoke runs; the rig's object-driven alarms make this drivable only from the simulator side.

archreview/2026-08-19/30-contracts-ipc.md

@@ -0,0 +1,143 @@
+# Contracts & IPC Protocol — Cycle-3 Review
+
+- **Date:** 2026-08-19
+- **Commit:** `8b3e3b3` (`main`, clean)
+- **Baseline:** `4f5371f` (2026-07-12 cycle-2 review; ~288 commits since, ~55 of them touching the contracts, frame-protocol, codegen, or IPC-doc paths in scope)
+- **Method:** static review on macOS; every cited file read at HEAD. No build, no test, no source change. `git diff 4f5371f..HEAD -- '*.proto'` walked line by line; generated trees grep-checked for every new symbol; the GalaxyRepository package proto compared against the sibling `scadaproj` checkout.
+- **Scope:** `src/ZB.MOM.WW.MxGateway.Contracts` (three protos, `Generated/` hygiene, `net10.0;net48` multi-targeting, `galaxy_repository.proto` retention vs the `ZB.MOM.WW.GalaxyRepository` package), the frame protocol on both sides (`Worker/Ipc/*`, `Server/Workers/WorkerFrame*`, `WorkerClient` envelope/sequence/size handling, hello/handshake, `max_frame_bytes` negotiation, DrainEvents byte bound), the codegen/descriptor pipeline (`scripts/check-codegen.ps1` Checks 1–4, `scripts/publish-client-proto-inputs.ps1`, `clients/proto/*`, vendored Rust protos, Go/Python/Java binding freshness guards, Gradle `checkGeneratedClean`), proto-evolution hygiene, and currency of `docs/WorkerFrameProtocol.md`, `gateway.md` (IPC envelope section), `docs/Grpc.md`, `docs/ClientProtoGeneration.md`, `docs/Contracts.md`. Worker-internal mechanics of the drain/write seam are owned by `20-worker.md` and are cross-referenced, not re-reviewed.
+- **Prior reports:** `archreview/2026-07-12/30-contracts-ipc.md` (cycle 2); `archreview/30-contracts-ipc.md` (cycle 1); remediation status sources `archreview/2026-07-12/remediation/00-tracking.md` + `30-contracts-ipc.md`, `90-candidate-findings-next-cycle.md`; first-cycle backlog `archreview/remediation/00-tracking.md`.
+
+## Proto evolution since baseline — verified additive-only
+
+`git diff 4f5371f..HEAD -- src/ZB.MOM.WW.MxGateway.Contracts/Protos/` contains exactly four wire-affecting lines, all additive with next-free tags:
+
+| Change | Location | Tag check |
+|---|---|---|
+| `bool snapshot_truncated = 2;` on `QueryActiveAlarmsReplyPayload` | `mxaccess_gateway.proto:735` | message previously ended at field 1 |
+| `bool from_truncated_snapshot = 16;` on `ActiveAlarmSnapshot` | `:951` | previous highest was `source_provider = 15` |
+| `AlarmSnapshotStatus snapshot_status = 5;` oneof arm on `AlarmFeedMessage` | `:1028` | previous highest arm was `provider_status = 4` |
+| new `message AlarmSnapshotStatus { bool truncated = 1; }` | `:1048-1054` | new type |
+
+Everything else in the diff is comment-only (write-completion `statuses` contract at `:538-551`, DrainEvents truncation at `:702-705`, `ReplayGap` empty-ring sentinel at `:797-801`, `AlarmFeedMessage` open sequence at `:1006-1012`, and the `GatewayHello.max_frame_bytes` byte-cap sentence at `mxaccess_worker.proto:50-52`). No renumbering, no repurposing; `reserved 1` / `reserved "session_id"` intact on the two retired fields (`mxaccess_gateway.proto:964-965`, `:978-979`); wire-compat policy headers present in all three files; `gatewayProtocolVersion: 3` / `workerProtocolVersion: 1` in `clients/proto/proto-inputs.json:4-5` match `GatewayContractInfo.cs:12,15` (additive changes correctly did not bump either). `galaxy_repository.proto` is unchanged and still wire-identical to the package source (see IPC-37).
+
+Generated-tree freshness for the new symbols, grep-verified at HEAD: `Contracts/Generated/MxaccessGateway.cs` (`AlarmSnapshotStatus` ×16, `from_truncated_snapshot` ×2), Go `mxaccess_gateway.pb.go` (×15 / ×3), Python `mxaccess_gateway_pb2.py` (descriptor bytes carry `\x66rom_truncated_snapshot`, `AlarmSnapshotStatus`), Java `MxaccessGateway.java` (×106 / ×7), the committed protoset (×2 / ×2), and the Rust vendored copies `cmp`-identical for all three files. Protos, protoset, and every binding set were last touched in the same commit (`c3c603f`, 2026-08-18) — the regen-everything-together discipline held, including for a comment-only change.
+
+## Prior-finding verification
+
+### Cycle-2 findings (IPC-23..32)
+
+Every tracked status was verified against code read at HEAD. **All eight Done claims hold; one carries a residual filed as a new finding (IPC-23 → IPC-33).**
+
+| ID | Claimed | Verified? | Evidence (HEAD) | Notes |
+|----|---------|-----------|-----------------|-------|
+| IPC-23 | Done (mechanics in WRK-21) | ✅ holds, **with caveat** | Contract half: `GatewayHello.max_frame_bytes` comment extended (`mxaccess_worker.proto:50-52`); `DrainEventsReply` truncation comment (`mxaccess_gateway.proto:702-705`); `docs/WorkerFrameProtocol.md:43-51` states the byte-cap rule and drain-until-empty contract; `gateway.md:565-572` documents both axes. Mechanics: byte-budgeted `Drain(uint, int)` leaves a non-fitting head queued and reports `OversizedHeadSequence` (`Worker/MxAccess/MxAccessEventQueue.cs:389-429`, per-event size memoized at enqueue `:493-509`); reply builder `CreateDrainEventsReply` consumes it with a truncation `DiagnosticMessage` (`Worker/Ipc/WorkerPipeSession.cs:978-1020`); budget = `max(Max − 64 KiB, Max/2)` (`:1034-1039`); shared count ceiling `GatewayContractInfo.MaxDrainEventsPerCommand` (`Contracts/GatewayContractInfo.cs:33`) consumed by both the gateway validator (`Grpc/MxAccessGrpcRequestValidator.cs:81-86`) and the worker clamp (`:992-994`); reply-too-large backstop answers `InvalidRequest` instead of dying (`:847-864`). R1–R3 satisfied. | Caveat: R1 bounds the reply against the **pipe** frame max; the same `MxCommandReply` then crosses the public gRPC boundary whose send cap is `MaxGrpcMessageBytes` = pipe max − 64 KiB, and the budget's own 64 KiB headroom consumes exactly that margin → **IPC-33**. The tracker's rationale for leaving `docs/Grpc.md` without a DrainEvents row ("a worker IPC diagnostic, not a public RPC") is inaccurate — `DrainEvents` is a public `MxCommandKind` (`mxaccess_gateway.proto:136`) reachable through `Invoke` → **IPC-38**. |
+| IPC-24 | Done | ✅ holds | Churn-revert step deleted; `java` job runs `gradle test` then `git diff --exit-code -- clients/java/src/main/generated` (`.gitea/workflows/ci.yml:135-142` (test step `:135-137`, diff gate `:138-142`), comment at `:113-119` records the rationale); toolchain pinned `grpcVersion 1.76.0` / `protobufVersion 4.33.1` (`clients/java/build.gradle:8,11`); `checkGeneratedClean` revived for Gradle 9 via `providers.exec` evaluated lazily in `doLast` (`clients/java/zb-mom-ww-mxgateway-client/build.gradle:71-92`, `df45cb4`); docs updated (`docs/ClientProtoGeneration.md:97,100-106`). Java aggregate at HEAD carries every new symbol (see above). | The lazy `ProviderFactory.exec` shape is correct: the provider is first realized in `doLast`, after the `generateProto` dependency ran, so the git probe sees the regenerated tree. |
+| IPC-25 | Done | ✅ holds | Go/Python bindings regenerated and fresh (grep above; `max_frame_bytes` now in `mxaccess_worker.pb.go` ×2 and `mxaccess_worker_pb2.py` ×1); Check 4 regenerates both via the per-client scripts and fails on any `git status --porcelain` diff, with tool-missing → fail not skip (`scripts/check-codegen.ps1:109-129`, header `:19-24`); Go script asserts `protoc-gen-go v1.36.11` / `protoc-gen-go-grpc 1.6.2` exactly and warns-only on protoc (`clients/go/generate-proto.ps1:13-15,63-78`), with the Windows `.exe` banner normalized (`:22-28`, `c94c4d4`); Python script asserts grpcio-tools 1.80.0 with PS5.1-safe quoting (`clients/python/generate-proto.ps1:36-49`, `8ae0c2f`); CI installs the pinned generators before the check (`ci.yml:63-72`); pins table in `docs/ClientProtoGeneration.md:90-98`. | Windows-side fixes landed after the cycle-2 design (Check 4 now runs on windev too). Backslash-joined paths in both scripts are normalized by pwsh on Unix (verified locally with `Test-Path`). |
+| IPC-26 | Done (mechanics in WRK-22) | ✅ holds | Contract requirement ("cancelled means never written") implemented by claim/tombstone interlock: `TombstoneIfUnclaimed` (`Worker/Ipc/WorkerFrameWriter.cs:181,308-338`), `DequeueNext` skips tombstones (`:588`), NEXT-04 fault observer on every abandoned frame (`:350`); documented with the claimed-mid-write residual at `docs/WorkerFrameProtocol.md:202-224`. Full mechanics verification in `20-worker.md` (WRK-22, NEXT-04/05). | Doc landed in the same batch as the code, as the plan required. |
+| IPC-27 | Done | ✅ holds | `ClientProtoInputTests.Descriptor_ContainsEveryContractSymbol` collects messages, fields, nested/top-level enums and values, services and methods from the protoset (`Tests/Contracts/ClientProtoInputTests.cs:37-62,108-146`) and walks all three reflection descriptors including `GalaxyRepositoryReflection.Descriptor` (`:65-99`); doc prose updated (`docs/ClientProtoGeneration.md:76-82`). | Presence-only (no field numbers / types), per the recorded design; the protoc-pinned script check remains the byte-level gate. |
+| IPC-28 | Done | ✅ holds | `docs/Grpc.md:285` lists `CommandTooLarge → ResourceExhausted`; Invoke-section paragraph at `:85` explains the per-correlation failure and the headroom rule; matches `Grpc/MxAccessGatewayService.cs:1056`. | |
+| IPC-29 | Done (discharged by WRK-26) | ✅ holds | `docs/WorkerFrameProtocol.md:79-224` "Write Scheduling And Sequencing": two-class scheduler, peek-stamp-commit write-time stamping (`:60-64`, `:98-107`), per-frame vs stream failure (`:109-120`), flush coalescing and boundary flush (`:122-172`), unparking (`:174-200`), cancellation tombstones (`:202-224`); plus a new "Pipe Buffers" section (`:226-235`, `da84635`). Each claim cross-checked against `Worker/Ipc/WorkerFrameWriter.cs` anchors cited in `20-worker.md`. | The doc now slightly *exceeds* the code in one place: `mxaccess_worker.proto:50-52` says "reply builders truncate to fit" immediately after listing events — events do not truncate (IPC-30 policy). Readable as intended; no action. |
+| IPC-30 | Done | ✅ holds | Batch-path `MessageTooLarge` is mapped to the first overshooting envelope (`Worker/Ipc/WorkerPipeSession.cs:633-648`, `FindOversizedEvent :648-660`), logged identity-only, written as a `ProtocolViolation` fault with `CommandMethod = "EventDrain"` (`:713`), then thrown; policy documented at `docs/WorkerFrameProtocol.md:53-58`. | Gateway side needed no change: `DispatchEnvelope` already routes `WorkerFault` to `SetFaulted(WorkerFaulted, …)` (`Server/Workers/WorkerClient.cs:614-618`). |
+| IPC-31 | N/A (accepted divergence) | **Superseded — resolved by GWC-28** | The gateway now stamps `Sequence` in `WriteLoopAsync` immediately before the write (`Server/Workers/WorkerClient.cs:465-471`), `CreateEnvelope` leaves it unset (`:1229-1236`); `gateway.md:425-431` rewritten to "both sides stamp at the point of writing". Regression test `WorkerClientTests.ConcurrentInvokesEmitStrictlyIncreasingSequencesOnTheWire`. | The two sides now implement the same semantics; flip the IPC-31 row from N/A to Resolved-by-GWC-28 in the tracker. See IPC-39 for the one interaction the change created with the IPC-03 pre-check. |
+| IPC-32 | Done | ✅ holds | Banners read `Check 1/4` … `Check 4/4` (`scripts/check-codegen.ps1:40,52,78,110`); header comment lists four checks (`:4-24`). | |
+
+**Done claims that do not fully hold:** none outright. IPC-23 holds for the defect as filed but leaves the gRPC-boundary residual (IPC-33).
+
+### Cycle-1 findings still open at cycle 2 — re-checked at HEAD
+
+| ID | Cycle-2 status | HEAD status | Evidence |
+|----|----------------|-------------|----------|
+| IPC-08 | Open (`WorkerCancel` defined, handled, never sent) | **Substantially resolved by `7c1ea12`/`a1a38b5` — recommend closing with a residual** | `InvokeAsync` forwards a `WorkerCancel` when the gateway's own command timeout fires and it owned the removal (`Server/Workers/WorkerClient.cs:269-291`); `TrySendCancelForTimedOutCommand` is structurally no-throw (`:835-870`, doc `:806-833`); worker routes it to `CancelCommand` (`Worker/Ipc/WorkerPipeSession.cs:755-756` → `MxAccess/MxAccessStaSession.cs:529-532` → `Sta/StaCommandDispatcher.CancelQueuedCommand :151`); test `WorkerClientTests.InvokeAsync_WhenCommandTimesOut_SendsWorkerCancelForThatCorrelation` (`:185`); `docs/GatewayProcessDesign.md:596-602` documents it. Residual: the caller-cancellation path sends nothing → **IPC-34**. |
+| IPC-10 | Open (softened) | **Still open, unchanged** | `Server/Workers/WorkerEnvelopeValidator.cs:15-39` and `Worker/Ipc/WorkerEnvelopeValidator.cs` check version/session/body only; `mxaccess_worker.proto:23` `uint64 sequence = 3;` still has no comment. `gateway.md:425-431` continues to state honestly that sequence is a diagnostic aid, and now that both sides stamp at write (GWC-28/WRK-04) the property an inbound check would rely on actually holds. Acceptable state. |
+| IPC-11 | Open | **Still open, unchanged** | Strict equality on both sides: `Worker/Ipc/WorkerPipeSession.cs:370-375` (`ProtocolVersionMismatch`), `Server/Workers/WorkerClient.cs:913-918`; no min/max-range note on `supported_protocol_version` (`mxaccess_worker.proto:43`). No action was ever required. |
+| IPC-12 | Open | **Still open, surface unchanged** | `Server/Workers/WorkerFrameWriter.cs:8-11` class doc still has no single-writer remark and the class has no lock; the invariant is now stated at the *call site* instead (`WorkerClient.cs:465-470`: "This loop is the channel's single consumer (SingleReader = true)"), and every outbound envelope — hello `:171`, commands `:251`, cancels `:848-858`, shutdown — funnels through `_outboundEnvelopes`. The gateway reader, by contrast, got its contract written into the class remark when GWC-30 made it stateful (`WorkerFrameReader.cs:11-16`); the writer deserves the same one-paragraph note. |
+| IPC-16 / IPC-22 | N/A | Unchanged | Correlation id still carried on envelope + inner reply (`gateway.md:432-434` documents envelope as authoritative); status-code-plus-prose error model unchanged. |
+| IPC-18 | Preserve | **Preserved** | See "Proto evolution" above — `Generated/`, all five client trees, the protoset, and the Rust vendored copies are in sync at HEAD; `galaxy_repository.proto` retention comment intact (`ZB.MOM.WW.MxGateway.Contracts.csproj:36-41`). |
+
+### NEXT rows touching this domain
+
+- **NEXT-04 / NEXT-05** (frame-writer abandonment observation; lazy tombstone purge as a documented decision) — both verified in `20-worker.md`; the contract-level statement lives at `docs/WorkerFrameProtocol.md:213-224`. Nothing further from this domain.
+- **NEXT-01** (pipe name length) — verified in `10-gateway-core.md`; the frame protocol is unaffected (the name is a transport address, not a wire field).
+
+### Also verified in passing
+
+- **TST-28** — the gateway half of the `max_frame_bytes` handshake is pinned on the CI-run suite: `WorkerClientTests.StartAsync_SendsGatewayHelloWithConfiguredMaxFrameBytes` (`:46-57`), hello built at `WorkerClient.cs:1172-1184` from `FrameOptions.MaxMessageBytes`.
+- **Negotiation floor/ceiling agreement** — worker accepts `[1024, 256 MiB]` (`Worker/Ipc/WorkerFrameProtocolOptions.cs:19,30,137-161`); the gateway validator bounds `Worker:MaxMessageBytes` to the identical range (`Configuration/GatewayOptionsValidator.cs:10-11,290-293`) and enforces pipe ≥ gRPC + 64 KiB with `long` arithmetic (`:614-628`). The worker can never reject a value the gateway's validator admits.
+- **Public gRPC caps are actually applied** — `MaxReceiveMessageSize` **and** `MaxSendMessageSize` are both set from `Protocol:MaxGrpcMessageBytes` (`Security/Authorization/GrpcAuthorizationServiceCollectionExtensions.cs:36-37`); default 16 MiB (`Configuration/ProtocolOptions.cs:16`). Load-bearing for IPC-33.
+- **Check 3 both directions** (`711793f`) — a canonical proto without a vendored Rust copy now fails too (`scripts/check-codegen.ps1:95-103`), closing the `build.rs`-prefers-canonical blind spot its comment describes.
+- **Contracts package version** bumped to 0.2.0 with the CLI-39 release train (`ZB.MOM.WW.MxGateway.Contracts.csproj:10`); `Grpc.Tools` 2.80.0 / `Google.Protobuf` 3.34.1 pins unchanged.
+
+## New findings
+
+IDs continue from the register (cycle 2 ended at IPC-32). No `IPC-33+` label exists anywhere in the repo or its history (grep), so there is no collision of the GWC-31 kind here.
+
+### IPC-33 — The DrainEvents byte budget is sized against the pipe frame maximum, not the public gRPC send cap; the reply can cross the pipe and then fail at the gRPC boundary with the drained events already consumed
+
+- **Severity:** Low
+- **Files:** `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs:31` (`DrainReplyFrameHeadroomBytes = 64 KiB`), `:1034-1039` (`ResolveDrainReplyByteBudget` = `max(MaxMessageBytes − 64 KiB, MaxMessageBytes / 2)`); `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs:389-429` (events dequeued as they are packed); `src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs:57-66` (pipe default = gRPC 16 MiB + 64 KiB reserve); `src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs:614-628` (pipe ≥ gRPC + reserve, no upper coupling); `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GrpcAuthorizationServiceCollectionExtensions.cs:37` (`MaxSendMessageSize = MaxGrpcMessageBytes`); `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto:20` (`Invoke` returns the bare `MxCommandReply`).
+- **Description:** WRK-21/IPC-23 made the reply fit the *pipe* (R1), but the same `MxCommandReply` is then returned verbatim through `Invoke`, where the gateway's gRPC `MaxSendMessageSize` applies. The two limits are coupled only from below: the validator requires pipe ≥ gRPC + 64 KiB, and the drain budget subtracts the same 64 KiB — so at default configuration the event bytes are budgeted to exactly `MaxGrpcMessageBytes` (16 MiB), leaving the `MxCommandReply` wrapper (34-byte correlation id, protocol status, kind, the `drain_events` length prefix, and — precisely in the truncated case — the `DiagnosticMessage`) with zero margin. The per-event `RepeatedFieldOverheadBytes` slack (+8 per event) absorbs this whenever the packed batch has more than ~30 events, so the default-config overshoot needs a backlog of ≥ 16 MiB composed of a few very large (≥ ~500 KiB) array events — narrow but real. The coupling is entirely absent for the configurations the validator explicitly permits: `Worker:MaxMessageBytes` may be anything up to 256 MiB while `Protocol:MaxGrpcMessageBytes` stays 16 MiB, and then every `DrainEvents` against a backlog above 16 MiB builds a reply of up to `pipeMax − 64 KiB`, which the worker writes, the pipe carries, the gateway parses — and the gRPC layer refuses to send (`ResourceExhausted: Sending message exceeds the maximum configured message size`). At that point the events are already dequeued from the worker queue (`Drain` removes what it packs), so up to 10 000 events are lost to both the diagnostic caller and the event stream, and the caller sees only a failed `Invoke`. This is the IPC-23 invariant ("no diagnostics command may kill a session") satisfied, and its sibling ("no event loss on truncation", R2) violated one boundary later. The same asymmetry exists for single events in the 64 KiB window between the gRPC cap and the pipe cap (an event the pipe accepts but `StreamEvents` cannot send), but a single ≥ 16 MiB event is pathological and already fails loudly on the stream; it is noted, not filed.
+- **Recommendation:** Bound the drain against the *smaller* of the two boundaries. Cheapest correct fix without a wire change: the gateway knows both caps, so either (a) add an upper coupling in `GatewayOptionsValidator` (`Worker:MaxMessageBytes ≤ MaxGrpcMessageBytes + 2 × reserve`, or simply document and validate the equality the default already has) and change the worker budget to `MaxMessageBytes − 2 × reserve` so the reply wrapper has a full reserve below the gRPC cap; or (b) convey `MaxGrpcMessageBytes` in `GatewayHello` (additive `uint32`, same zero-means-unknown convention as `max_frame_bytes`) and budget against `min(pipe − reserve, grpc − reserve)`. Add a gateway fake-worker test that drains a reply packed to the budget through the real gRPC pipeline (the TST-24 wire-test harness can host it) so the boundary is exercised end to end.
+
+### IPC-34 — `WorkerCancel` is forwarded only when the gateway's own command timeout fires; a caller-cancelled `Invoke` (client deadline, disconnect) leaves the command queued on the worker's STA
+
+- **Severity:** Low
+- **Files:** `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs:269-291` (timeout arm → `TrySendCancelForTimedOutCommand`), `:294-305` (`OperationCanceledException` arm: `RemovePendingCommandAsFailed(GatewayShutdown, "Command wait was canceled.")`, no cancel sent), `:307-318` (timer-won-but-token-cancelled arm, same); `src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs:138` (`context.CancellationToken` flows into `InvokeAsync`); `src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs:1100`; worker side `src/ZB.MOM.WW.MxGateway.Worker/Sta/StaCommandDispatcher.cs:151-185` (`CancelQueuedCommand` drops only not-yet-started entries); `docs/GatewayProcessDesign.md:596-602`; `gateway.md:913-919`.
+- **Description:** The IPC-08 closure's own rationale — "the gateway has stopped waiting, but the worker has not stopped working … a queued-but-not-started command is dropped instead of occupying the STA behind a caller that is already gone" (`WorkerClient.cs:279-284`) — applies identically when the caller's token fires, and that is the *common* abandonment path: gRPC clients routinely set per-call deadlines shorter than the gateway's `CommandTimeout`, and a client disconnect cancels `context.CancellationToken`. Both land in the `OperationCanceledException` arm, which removes the pending entry and returns, so the correlation stays on the single STA queue and executes when its turn comes. Under a burst of deadline-cancelled calls (a client retry storm against a slow provider) the STA works through dead commands while live callers queue behind them — exactly the backlog the cancel was added to shorten. The gateway-side design text still frames cancellation as "stop waiting" only (`gateway.md:915`), and `docs/GatewayProcessDesign.md:596` says "on timeout", so the asymmetry is documented-by-omission rather than decided. Secondary: the arm labels caller cancellation `WorkerClientErrorCode.GatewayShutdown` (pre-existing; maps to `Cancelled` at the gRPC layer per `docs/Grpc.md:285`, so the client sees the right status, but metrics/logs record a shutdown that did not happen).
+- **Recommendation:** Call the same no-throw `TrySendCancelForTimedOutCommand` (renamed to `TrySendCancelForAbandonedCommand`, reason "caller cancelled") from the caller-cancellation arms, gated on the same `removed` ownership check so a reply that won the race is never chased. Skip it only when `_stopCts` is the source of the cancellation (the channel is being completed anyway). Add a `WorkerClientTests` twin of the timeout test driven by the caller token. Separately, introduce a `CallerCancelled` error code (or reuse `CommandTimeout` semantics) so the metric label stops saying `GatewayShutdown`; update the `docs/Grpc.md:285` mapping line in the same change.
+
+### IPC-35 — A `WorkerFault` the worker emits during the startup handshake is flattened by the gateway into a generic `ProtocolViolation`, discarding the structured fault category and diagnostic message
+
+- **Severity:** Low
+- **Files:** `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs:894-907` (`ReadHandshakeEnvelopeAsync`: any body other than the expected one → `WorkerClientException(ProtocolViolation, "Worker handshake expected {expected} but received {actual}")`), `:164-185` (`StartAsync` expects `WorkerHello` then `WorkerReady`); worker side `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs:335-357` (`CompleteStartupHandshakeAsync` writes a `WorkerFault` for every `WorkerFrameProtocolException` and for `MxAccessCreationException` — COM creation failure, the most likely real-world startup fault — before rethrowing), `:1392-1447` (`TryWriteFaultAsync` overloads); `src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs:132-160` (the exception message is what reaches `MarkFaulted`, the warning log, and the `SessionManagerException` inner).
+- **Description:** The worker deliberately reports startup failures as structured frames — `MxAccessCreationException.From(...)` carries the COM HRESULT/ProgID context and `WorkerFaultCategory` — so the gateway can tell "MXAccess is not installed / not registered / access denied" apart from a pipe-level protocol problem. The gateway's handshake reader does not look inside: it sees `BodyCase == WorkerFault` where it expected `WorkerReady`, throws a `ProtocolViolation` whose message names only the body type, and `SessionManager.OpenSessionAsync` surfaces that text to the operator log and to the client's `OpenFailed`. The post-handshake path does this correctly (`DispatchEnvelope` → `SetFaulted(WorkerFaulted, CreateWorkerFaultMessage(fault))`, `WorkerClient.cs:614-618`), so the information loss is confined to the window in which it matters most — first session open on a freshly deployed host, where the answer is almost always an MXAccess installation or permission problem, and the operator is pointed at "protocol violation" instead. The `FakeWorkerHarness` only ever scripts well-formed handshakes (`Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs:140-200`), so nothing pins the behaviour either way.
+- **Recommendation:** In `ReadHandshakeEnvelopeAsync`, special-case `BodyCase == WorkerFault`: throw `WorkerClientException(WorkerFaulted, CreateWorkerFaultMessage(envelope.WorkerFault))` (the same formatter the steady-state path uses) so category and diagnostic reach the log and the `OpenFailed` detail. Keep `ProtocolViolation` for every other unexpected body. Add a `WorkerClientTests` case where the fake writes a `WorkerFault{Category = MxaccessCreationFailed}` in place of `WorkerReady` and asserts the category text surfaces.
+
+### IPC-36 — The worker launch environment has become a second, undeclared gateway↔worker contract: six variable names duplicated as independent literals on both sides, four of them new since baseline, two documented
+
+- **Severity:** Low
+- **Files:** gateway literals `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs:18-52` (`MXGATEWAY_WORKER_NONCE`, `…_PIPE_CONNECT_ATTEMPT_TIMEOUT_MS`, `…_WRITE_COMPLETION_WAIT_MS`, `MXGATEWAY_ALARM_POLL_INTERVAL_MS`, `MXGATEWAY_ALARM_MAX_ALARMS_PER_FETCH`, `MXGATEWAY_EVENT_QUEUE_CAPACITY`), set at `:207-217`; worker literals `src/ZB.MOM.WW.MxGateway.Worker/Bootstrap/WorkerOptions.cs`, `Ipc/WorkerPipeClient.cs`, `MxAccess/MxAccessStaSession.cs:14-24`, `MxAccess/MxAccessEventQueue.cs`, `MxAccess/WnWrapAlarmConsumer.cs` (one copy each); the one precedent that *is* shared: `src/ZB.MOM.WW.MxGateway.Contracts/GatewayContractInfo.cs:35-43` (`LiveMxAccessOptInVariableName`, shared "so any future opt-in tweak does not silently leave one project behind"); docs `docs/WorkerProcessLauncher.md:25-27,47-51` (nonce + connect timeout only), `docs/MxAccessWorkerInstanceDesign.md:93-98` ("Expected protected environment values": nonce + log context only); public proto comment `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto:540` names `MXGATEWAY_WORKER_WRITE_COMPLETION_WAIT_MS`.
+- **Description:** Since `4f5371f`, four runtime knobs were plumbed to the worker through `ProcessStartInfo.Environment` rather than through `GatewayHello` — write-completion wait, alarm poll cadence, alarm fetch cap, event-queue capacity. Environment conveyance is a reasonable choice (the values are needed before or independent of the handshake, and the worker's fallback-to-default on a missing variable keeps old/new pairs working), but it makes the launch environment a wire contract in everything but name, and it is treated with none of the discipline the proto contract gets: each name exists as two unrelated string literals in two projects (a typo on either side silently reverts that knob to its default — the worker's documented "missing or unparseable → default" fallback turns a contract break into a no-op), the Contracts project already demonstrates the right home for exactly this kind of name and was not used, and the documentation that should enumerate the contract lists two of the six. The public `mxaccess_gateway.proto` comment additionally exposes the internal env-var name to every language-client consumer, where the operator-facing key is `MxGateway:Worker:WriteCompletionWaitMilliseconds` (the `WorkerOptions.cs:28-34` and launcher `:25-27` doc comments are also stale — they still say "WriteSecured/WriteSecured2" although `b0e65d4` extended the wait to plain `Write`/`Write2`).
+- **Recommendation:** Move the six names into a `WorkerLaunchEnvironment` static class in `ZB.MOM.WW.MxGateway.Contracts` (net48-safe `const string`s, like `GatewayContractInfo`) and reference it from both sides; add a one-table "Launch environment" section to `docs/WorkerProcessLauncher.md` (variable, source config key, worker default on absence) and point `docs/MxAccessWorkerInstanceDesign.md:93-98` at it; replace the env-var name in the proto comment with the config key; refresh the two "WriteSecured-only" comments. A small test asserting the launcher sets every constant in the class keeps the table and the code from drifting.
+
+### IPC-37 — The `galaxy_repository.proto` wire-identity with the `ZB.MOM.WW.GalaxyRepository` package is still unguarded, although both descriptors are now loadable in one test process
+
+- **Severity:** Low
+- **Files:** `src/ZB.MOM.WW.MxGateway.Contracts/ZB.MOM.WW.MxGateway.Contracts.csproj:36-41` (retention rationale: the proto is the codegen source for all five clients and must stay wire-identical to the package); `src/ZB.MOM.WW.MxGateway.Server/ZB.MOM.WW.MxGateway.Server.csproj:23` (`ZB.MOM.WW.GalaxyRepository` 0.2.0 — the server serves the package's implementation); `src/ZB.MOM.WW.MxGateway.Tests/Contracts/ClientProtoInputTests.cs:65-70` (walks `GalaxyRepositoryReflection.Descriptor` from *Contracts* only); package proto `~/Desktop/scadaproj/ZB.MOM.WW.GalaxyRepository/src/ZB.MOM.WW.GalaxyRepository/Protos/galaxy_repository.proto` (last changed `5572edd`, 2026-06-23).
+- **Description:** Verified at HEAD: the two files differ only in `option csharp_namespace` (comment-stripped `diff`), so the invariant holds today. Nothing keeps it true. The package version moves with the whole `ZB.MOM.WW.*` family (repo memory: a version jump is not evidence the package changed), so a routine `PackageReference` bump can bring a wire change in `galaxy_repository.v1` — a new field, a renamed RPC, a different streaming shape — that the server starts serving while the five clients keep generating from the retained, now-stale copy. Check 1–4 and the descriptor test would all stay green, because they compare the clients against the *retained* proto, never the retained proto against the *served* one. Cycle 1 recorded this as an optional test (`archreview/remediation/30-contracts-ipc.md:340-344`) when it would have required a cross-repo path; that is no longer the case — the test project transitively loads the package through the server reference (`Tests/Configuration/GalaxyRepositoryOptionsValidatorTests.cs` and four other test files already `using ZB.MOM.WW.GalaxyRepository`), so `ZB.MOM.WW.GalaxyRepository.Grpc.GalaxyRepositoryReflection.Descriptor` and `ZB.MOM.WW.MxGateway.Contracts.Proto.Galaxy.GalaxyRepositoryReflection.Descriptor` are both in-process.
+- **Recommendation:** Add `GalaxyContractParityTests`: take `FileDescriptor.ToProto()` from both, clear `SourceCodeInfo` and `Options.CsharpNamespace` (and `Name` if the paths differ), and assert byte-equality of the serialized `FileDescriptorProto`. One test, no protoc, runs in the portable CI job, and turns a silent package-bump drift into a red build with the exact differing symbol named. Record the new guard in the csproj retention comment and `docs/Contracts.md`.
+
+### IPC-38 — `docs/Grpc.md` has no coverage of `DrainEvents` although it is a public `MxCommandKind` reachable through `Invoke`; the tracker's reason for omitting it is inaccurate
+
+- **Severity:** Info
+- **Files:** `docs/Grpc.md` (grep for `DrainEvents` → 0 hits); `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto:136` (`DrainEventsCommand drain_events = 103` in the public `MxCommand` oneof), `:520-523`, `:701-706`; `src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcRequestValidator.cs:74-88` (public-boundary validation of `max_events`); `archreview/2026-07-12/remediation/00-tracking.md:87` ("`docs/Grpc.md` intentionally carries no DrainEvents row since it scopes to the seven public gRPC RPCs and DrainEvents is a worker IPC diagnostic"); `gateway.md:565-572` (does document it).
+- **Description:** `DrainEvents` is not an IPC-only diagnostic: any client with `invoke:read` can issue it through `Invoke`, the public validator has a rule for it, and the cycle-2 plan (IPC-23 step 5) asked for a rules-table note because the reply is now truncated on two axes with a drain-until-empty contract that callers must know. The tracker closed that step on a premise the proto contradicts. `gateway.md` carries the prose, so the information exists; `docs/Grpc.md` is the document the client READMEs and the rules table point readers at for per-command behaviour, and it is silent.
+- **Recommendation:** Add a `DrainEvents` row to the rules table in `docs/Grpc.md` (count cap, byte cap, `DiagnosticMessage` truncation marker, drain-until-empty, and that drained events are removed from the session's event stream), cross-linking `gateway.md:565`. Correct the tracker note.
+
+### IPC-39 — The IPC-03 `CommandTooLarge` pre-check now measures the envelope before GWC-28 stamps the sequence, leaving an ≤ 11-byte window in which the writer's `MessageTooLarge` is still session-fatal; the explanatory comment has the direction of conservatism backwards for the pre-check itself
+
+- **Severity:** Info
+- **Files:** `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs:236-249` (pre-check: `CalculateSize()` on an envelope whose `Sequence` is still 0 → the `sequence` field is not encoded), `:465-471` (`WriteLoopAsync` stamps `Sequence` → up to 1 tag + 10 varint bytes added), `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameWriter.cs:42-55` (post-stamp `CalculateSize()` > max → `MessageTooLarge`), `:478-483` (write-loop exception → `SetFaulted(WriteFailed)`, session-fatal).
+- **Description:** The comment at `:236-241` correctly explains why the pre-check size cannot be reused as the writer's `knownSize` ("a lower bound … conservative in the right direction") — true for that purpose, but the same fact inverts for the pre-check's own purpose: a lower bound admits envelopes the writer will reject. An envelope in `(MaxMessageBytes − 11, MaxMessageBytes]` passes `:243` and faults the session at the write. Reachability is nil under the shipped validator: every caller-controlled command arrives through gRPC capped at `MaxGrpcMessageBytes`, which is ≥ 64 KiB below the pipe max, and gateway-authored commands are tiny — so this is a correctness-of-argument note, not a live defect. It is worth recording because the headroom rule is the only thing making it unreachable, and the comment currently reads as if the pre-check were exact.
+- **Recommendation:** Either compare against `MaxMessageBytes − MaxSequenceEncodingBytes` (11) in the pre-check, or reword the comment to say the pre-check is a lower bound that relies on the 64 KiB reserve for its safety margin. No test needed beyond the existing `CommandTooLarge` case.
+
+## Severity roll-up (new findings)
+
+| Severity | Count | IDs |
+|----------|:-:|-----|
+| Critical | 0 | — |
+| High | 0 | — |
+| Medium | 0 | — |
+| Low | 5 | IPC-33, IPC-34, IPC-35, IPC-36, IPC-37 |
+| Info | 2 | IPC-38, IPC-39 |
+
+Prior-finding verification: 8/8 cycle-2 Done claims hold (IPC-23 with the IPC-33 residual); IPC-31 superseded by GWC-28 (recommend flipping to Resolved); cycle-1 IPC-08 substantially resolved by `7c1ea12` (recommend closing, residual tracked as IPC-34); IPC-10/11/12 confirmed still open, none regressed.
+
+## Still good / preserve under change
+
+The contract boundary is in the best shape it has been across three cycles. Proto evolution since baseline was purely additive with correct next-free tags and reserved ranges intact, and — for the first time — every generated artifact (C# `Generated/`, Go, Python, Java, Rust vendored copies, the protoset) moved in the *same commit* as the proto, including for a comment-only edit; the freshness net that makes this checkable is now genuinely closed (Checks 1–4 + the reflection test + the Java `git diff` gate, all in CI, all tool-missing-fails-not-skips, all with their generator pins asserted rather than assumed). The frame protocol core is unchanged where it should be — length validated before allocation on both readers, exact-read loops with typed `EndOfStream`, pooled buffers with sound lifetimes on all four codec halves, single-serialization single-write framing — and materially stronger where it changed: both sides now stamp `sequence` at the point of writing so the documented "monotonic per sender" contract is finally true on both ends, the negotiated `max_frame_bytes` handshake has matching `[1024, 256 MiB]` bounds on both sides so a legal gateway configuration can never be refused by the worker, the DrainEvents reply is bounded on count *and* bytes with no event loss at the pipe, oversized events die structured and named, and cancelled writes are guaranteed never to reach the wire. `WorkerCancel` is no longer a dead arm. `docs/WorkerFrameProtocol.md` is now a faithful, claim-by-claim description of the shipped writer, and `gateway.md`'s envelope section points at the proto as the single source of truth instead of carrying a drifting copy. Preserve in particular: the per-client generator pins and the "tool missing = check fails" rule (a skipped guard is the silent-drift hole the whole P0 codegen batch existed to close); the pipe-over-gRPC 64 KiB reserve invariant in the validator (IPC-33 and IPC-39 both rest on it); the habit of landing proto-comment changes with the full regen wave; and the csproj retention comment on `galaxy_repository.proto` — until IPC-37's test exists, that comment is the only thing standing between a package bump and a five-client wire drift.
+
+## Verification needed on Windows (windev)
+
+- This review is static. The worker-side halves cited here (byte-budgeted drain, IPC-30 structured death, tombstones, the floor/ceiling handshake rejection) are exercised only by `Worker.Tests -p:Platform=x86`; the tracker and repo memory record the suite fully green on windev after the harness fixes, which this review did not re-run.
+- IPC-33's end-to-end shape (reply built at budget → `Invoke` send failure) cannot be reproduced with the fake worker alone on macOS if the fix lands worker-side; the validator/gateway-side variant (option a) is portable and should get a `GatewayOptionsTests` case plus a TST-24-style wire test.
+- IPC-35's regression test is portable (fake worker writes a handshake-phase `WorkerFault`); no windev run needed.

archreview/2026-08-19/40-security-dashboard.md

@@ -0,0 +1,110 @@
+# Security & Dashboard — Re-review (Cycle 3)
+
+**Date:** 2026-08-19 · **Commit:** `8b3e3b3` (`main`, clean) · **Baseline:** `4f5371f` (2026-07-12 cycle) · **Method:** static review on the macOS working tree (no builds, no runtime probes, no source modifications). Every claim cites a `path:line` read at HEAD. Prior findings are the `SEC-NN` IDs from `archreview/remediation/40-security-dashboard.md` (cycle 1) and `archreview/2026-07-12/40-security-dashboard.md` / `archreview/2026-07-12/remediation/40-security-dashboard.md` (cycle 2); new findings continue from SEC-37. This domain absorbed the most new machinery since the baseline (~55 of ~247 commits touched Security/Dashboard/Configuration): the SEC-31/32 failure-limiter re-partition, SEC-33/34/36 remediations, the SEC-25 tag-ACL epic (`DashboardTags` constraints, session tag inheritance, `GroupToTag`/`UntaggedSessionVisibility`, events-hub + in-process ACL), the `ShowTagValues` three-seam closure, the secrets-store adoption (`${secret:}` expansion, `/admin/secrets`), the bounded async audit writer with retention, and the snapshot-feed/alarm-page reliability rework.
+
+## Prior-finding verification
+
+### Cycle-2 findings (SEC-31…SEC-36)
+
+| ID | Claimed | Verified? | Evidence (file:line at HEAD) | Notes |
+|----|---------|-----------|------------------------------|-------|
+| SEC-31 | Done | **Yes** | Composite `(transport peer, key id)` partition + cross-peer aggregate + probe admission, exactly per the remediation design: `Security/Authorization/ApiKeyFailureLimiter.cs:14-27` (two-layer remarks), `:143-204` (`Check` with probe claim/compensation), `:206-235` (`RecordFailure` into both layers), `:237-276` (`Reset` reachable-while-throttled); interceptor threads the partition and reuses the check-time resolution (`GatewayGrpcAuthorizationInterceptor.cs:83-115`, `:133-137`); new knobs validated (`SecurityOptions.ApiKeyFailureAggregateLimit`/`ApiKeyFailureProbeIntervalSeconds`, wired at `ApiKeyFailureLimiter.cs:74-83`); throttle metric tagged by stage only (`Metrics/GatewayMetrics.cs:350-356`, with an explicit SEC-14/SEC-20 cardinality comment). `ApiKeyFailureLimiterTests.cs` exists. The lockout inversion is structurally closed: a correct secret always reaches the verifier via a probe slot and resets both layers. | Probe-claim compensation is version-stamped (`:341-383`), closing the same-tick stomp the follow-up commits (`5b681ee`, `acebe18`) addressed. Two new residual corners found in the rework — see **SEC-38** (aggregate-layer bypass via the per-peer key-id cap) and **SEC-39** (null==null fallback-partition reset). Neither reopens the SEC-31 lockout: they weaken bounds, they do not deny the legitimate holder. |
+| SEC-32 | Done | **Yes** | Token shape checked before a key-id partition is minted: `GatewayGrpcAuthorizationInterceptor.cs:149-193` (`TryResolveKeyId` — `mxgw` must be the entire first segment, key id ≤ 64 chars, non-empty third segment; internal for direct test pinning). Per-peer key-id cap `MaxKeyIdPartitionsPerPeer = 32` with overflow collapsing to the address's fallback partition (`ApiKeyFailureLimiter.cs:48-53`, `:393-444`). Eviction never removes load-bearing state below a 2× ceiling and prefers expired windows (`:479-548`). | The junk-token flush is closed as designed. The cap's overflow-collapse rule has a side effect the design did not anticipate — see **SEC-38**. |
+| SEC-33 | Done | **Yes** | `IsRootedForAnyPlatform` is gone; rooting is host-meaningful `Path.IsPathRooted` in the shared `Configuration/GatewayConfigPathRules.cs:14-39` (used by both validators, with the SEC-01/SEC-33 mechanism named in the doc-comment). `appsettings.json` no longer carries either Windows literal (Authentication section `:20-24` has no `SqlitePath`; Galaxy section `:79-84` has no `SnapshotCachePath`). Gateway-side Galaxy default seeded pre-bind (`GatewayApplication.cs:156-168` — the init-only-property deviation is documented in place) and validated by the new `Configuration/GalaxyRepositoryOptionsValidator.cs:44-72` registered with `ValidateOnStart` (`GatewayApplication.cs:172-176`). No stray `C:*` artifact under `src/` (`find`-verified). | Went beyond the finding: `882c7ca` added `AddIfUnderContentRoot` (`GatewayConfigPathRules.cs:93-124`) after a real 2026-08-09 production key-loss incident (store inside the renamed app directory), and the secrets store got the same absolute default treatment (`GatewayApplication.cs:201-239`). |
+| SEC-34 | Done | **Yes** | Invalidate race closed with the per-key generation counter: bump-before-evict (`Security/Authentication/CachingApiKeyVerifier.cs:181-199`), snapshot-before-inner + set-then-recheck with self-evict (`:128-141`, `:147-178`); `TryParseKeyId` correctness argument (full key id, `_` forbidden by both key-creation paths) documented at `:205-218`. Expiry window took the documented fallback per the recorded outcome: the library verification identity carries no `ExpiresUtc`, so the ≤ TTL expiry window is documented in the remarks (`:49-60`) with the donor-library ask. | Matches the remediation outcome exactly, including the deviation. Cache-key design unchanged (SHA-256 of the token, success-only). |
+| SEC-35 | N/A (doc-only) | **Yes** | `docs/GatewayConfiguration.md:282` — the "hard-stops key on the exact environment name" contract is documented (both guards fire only on `IHostEnvironment.IsProduction()`). Validator unchanged: `GatewayOptionsValidator.cs:38`, `:211-214` (LDAP), `:387-391` (DisableLogin). | Note the consequence is now live: windev runs as `Staging` (NEXT-07 relabel), so neither guard applies there — that is the posture gap promoted as **SEC-37**. |
+| SEC-36 | Done | **Yes (repo side + rotation executed)** | `appsettings.json:33` ships `"ServiceAccountPassword": "${secret:ldap/mxgateway/bind}"` — a fail-closed encrypted-store reference expanded pre-host (`GatewayApplication.cs:75-106`), not a literal. `git grep` for the retired value returns nothing in tracked files. `glauth.md` samples use placeholders; `docs/runbooks/SEC-36-ldap-credential-rotation.md` records the 2026-08-07 rotation (GLAuth `passsha256` replaced in `scadaproj` commit `aada53b`, instance recreated, dashboard `/login` verified through the real `DashboardAuthenticator` path post-NEXT-07). `<UserSecretsId>mxaccessgw-server</UserSecretsId>` present in the csproj; the blank-password validator message names both channels. | The load-bearing half (rotation) is done: the historically recoverable value no longer binds `dc=zb,dc=local`. Fully closed. |
+
+### Cycle-1 residuals (still-open SEC-13…SEC-27 era)
+
+| ID | Prior status | At HEAD | Evidence | Notes |
+|----|--------------|---------|----------|-------|
+| SEC-13 | Open | **Open, impact reduced** | `Diagnostics/GatewayLogRedactor.cs:11-17` still lists only `AuthenticateUser`/`WriteSecured`/`WriteSecured2` — the `WriteSecuredBulk`/`WriteSecured2Bulk` command kinds (`Grpc/MxAccessGatewayService.cs:364-381`) are still missing. However the redactor is no longer latent: `GatewayLogRedactorSeam` is registered as the telemetry `ILogRedactor` (`GatewayApplication.cs:139`) and masks **every** non-null `CommandValue` because the seam exposes no value-logging opt-in (`Diagnostics/GatewayLogRedactorSeam.cs:38-56`; honestly documented at `docs/Diagnostics.md:120`). The Bulk omission only bites if a future opt-in knob passes `valueLoggingEnabled: true`. | Fix the two missing names when (if) the opt-in knob lands; until then fail-closed. |
+| SEC-14 | Open | **Confirmed open** | `GatewayApplication.cs:347-348` — `MapZbHealth()`/`MapZbMetrics()` with no `RequireAuthorization`. | Mitigating discipline held: the new throttle counter deliberately carries no key/peer tags because "/metrics is not authenticated (open SEC-14)" (`GatewayMetrics.cs:352-354`). Note `/health/active` has off-repo consumers (family Overview dashboard), so any auth change needs cross-repo coordination. |
+| SEC-15 | Open (accepted) | **Confirmed open** | GET `/logout` still skips antiforgery, intentional (`Dashboard/DashboardEndpointRouteBuilderExtensions.cs:92-99`). | Accepted risk; unchanged. |
+| SEC-16 | Open | **Confirmed open** | `Program.cs:37-39` still maps `command.Pepper` → `MxGateway:ApiKeyPepper` (pepper on the CLI command line). | |
+| SEC-17 | Open | **Confirmed open** | `GatewayGrpcAuthorizationInterceptor.cs:27-54` overrides only unary + server-streaming. All current RPCs remain those shapes. | |
+| SEC-18 | Open | **Confirmed open** | `Dashboard/DashboardApiKeyManagementService.cs:22,289` — `"pepper unavailable"` message-substring match. | |
+| SEC-19 | Open | **Resolved** (this period) | `Security/Audit/SqliteCanonicalAuditStore.cs:23-31` — the `CREATE TABLE IF NOT EXISTS` is latched: `AuditDrainService` runs `EnsureInitializedAsync` once at startup and later operations skip the DDL round-trip (self-bootstrap kept for the CLI/DI-free paths; the latch is deliberately racy-idempotent). Landed with the `e2ac5d1`/`7b2d046` audit rework (bounded async writer, batched inserts). | |
+| SEC-21 | Open | **Resolved** (this period) | `Dashboard/DashboardSnapshotService.cs:25,38,42,287-310` — API-key summaries refresh at most every 15 s behind a zero-timeout gate with a 2 s store timeout, and the snapshot tick itself is idle-gated (`77c5731`, `DashboardSnapshotFeed` idle gate). The per-tick unconditional store read is gone. | |
+| SEC-23 | Open | **Confirmed open** | `GatewayOptionsValidator.ValidateDashboard` (`:380-455`) gained GroupToRole/GroupToTag/UntaggedSessionVisibility shape checks and the Production `DisableLogin` stop, but still ignores `AutoLoginUser`, `RequireHttpsCookie`, and `CookieName` — an explicit `CookieName` starting `__Host-` with `RequireHttpsCookie=false` is still applied unconditionally (`DashboardServiceCollectionExtensions.cs:154-158`) with no validator diagnosis. | The runtime guard against *deriving* `__Host-` without Secure is correct (`:159-162`); only the explicit-override consistency check is missing. |
+| SEC-24 | Open | **Confirmed open (narrowed)** | `Configuration/GatewayConfigurationProvider.cs:58-74` / `EffectiveDashboardConfiguration.cs:3-12` — the projection gained `ShowTagValues`, `GroupToRole`, `GroupToTag` (with comparer propagation, `ff16c69`), and `UntaggedSessionVisibility`, and the settings page renders them (`SettingsPage.razor:53-104`); it still omits `DisableLogin`/`AutoLoginUser`/`RequireHttpsCookie`/`CookieName` and the `MxGateway:Security` limiter keys the SEC-31 design asked to absorb here. Secrets stay redacted (`:20,30`). | |
+| SEC-26 | Open | **Resolved** (this period) | Retention sweep exists: `SqliteCanonicalAuditStore.cs:172` (`DELETE FROM audit_event` with a documented NULL-timestamp keep rule at `:251-253`), driven by `SecurityOptions.AuditRetentionDays` (default 90, `SecurityOptions.cs:96-99`) from the audit drain service. | |
+| SEC-27 | Open | **Confirmed open** | `DashboardSnapshotService.cs:17,114` — `GatewayStatus` is still the hardcoded `"Healthy"` constant. | |
+
+### SEC-25 epic verification (the tag-ACL machinery is real and fail-closed)
+
+Not a table row above because cycle 2 closed SEC-25 as "mitigation, ACL deferred"; the deferred ACL has now shipped and was reviewed as new code:
+
+- **Tags come from the owning key, never the request:** `Grpc/MxAccessGatewayService.cs:36-46` (owner identity from `identityAccessor`, `owner?.EffectiveConstraints.DashboardTags` into `OpenSessionAsync`); `Sessions/GatewaySession.cs:207-212,270` (immutable ordinal-ignore-case `FrozenSet`); `Security/Authentication/ApiKeyConstraints.cs:35-44` (defensive copy on init; rides the constraints blob to avoid a schema migration; explicitly visibility-only, never data-access).
+- **ACL decision table:** `Dashboard/DashboardSessionAcl.cs:50-84` — Admin (authenticated) bypass; phantom session denied; untagged session follows `UntaggedSessionVisibility` (default `AdminOnly`, fail-closed); otherwise ordinal-ignore-case tag intersection over `DashboardTagClaimType` claims; `null` principal denied outright, even under `AllViewers` (`IDashboardSessionAcl.cs:25-32`). Anonymous localhost is an empty-grant Viewer.
+- **Both subscribe seams gated identically:** `Dashboard/Hubs/EventsHub.cs:59-79` (ACL before group join, registry-before-group ordering, opaque `HubException`), and the session-details in-process seam `SessionDetailsPage.razor:321-361` (ACL before any field write or `Subscribe`). The `7ec0b35` re-entrancy fix is sound: the attach generation is claimed synchronously before the first await (`:217-233`), a stale continuation returns without attaching or detaching (`:340-343`), and the pump's `ReferenceEquals` batch guard drops cross-session batches on the dispatcher (`:383-407`).
+- **Claims plumbing:** tags stamped at login (`DashboardAuthenticator.cs:114-158`) and re-resolved from group claims at hub-token mint so a token bounds grant staleness to 5 minutes (`HubTokenService.cs:80-98`); token `Validate` still rejects identity-less payloads (`:118-126`) and rehydrates tag claims for the hub-path ACL (`:139-145`).
+- **Owner-scoped gRPC attach (the threat-model anchor) holds:** `Grpc/EventStreamService.cs:52-63` — reattach is `PermissionDenied` unless the caller's key id ordinal-equals the session owner; null owner matches only a null caller.
+- **Redaction (second, independent layer) covers all three value seams:** events-hub mirror deep-clone (`Dashboard/Hubs/DashboardEventBroadcaster.cs:91,247-259` — top-level `value` plus alarm `CurrentValue`/`LimitValue`, source never mutated), alarms hub deep-clone on both value-bearing arms (`AlarmsHubPublisher.cs:59,103-126` — the cycle-2 closeout's named "alarms-hub redaction gap" residual, closed by `eff17d1`/TST-16 on 2026-08-17), and `/browse` live values at the service boundary with the `'-'`-placeholder-on-failed-reads fix (`DashboardLiveDataService.cs:132-140`; `1ea6f60`). `docs/GatewayConfiguration.md:195` documents all three seams and the "redaction applies to successful reads only" rule accurately.
+- **Tests exist for the corners:** `Tests/Gateway/Dashboard/DashboardSessionAclTests.cs`, `EventsHubTests.cs`, `SessionDetailsPageEventAclTests.cs`, `EventsHubViewerRegistryTests.cs`, `HubTokenServiceTests.cs`, `AlarmsHubPublisherTests.cs`, `DashboardGroupTagMappingTests.cs`; live-LDAP ACL proof via `IntegrationTests/DashboardLdapLiveTests.cs:186-243` (real bind → real group claims → GroupToTag → ACL admit/deny, plus Admin bypass).
+- **Docs honesty:** the short-name `GroupToRole`/`GroupToTag` key constraint (full-DN keys can never match the pre-stripped groups) is stated at the mapping site (`DashboardGroupTagMapping.cs:8-13,41-47`) and in the class summaries (`db7b1db`, `4b6f907`).
+
+## New findings
+
+### SEC-37 · Medium (Security / deployment posture) — The shared GLAuth offers no TLS, so the Production plaintext-LDAP hard-stop is unsatisfiable for every host that authenticates against it (promotion of NEXT-08)
+
+**Files:** `src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayOptionsValidator.cs:208-214`; `docs/GatewayConfiguration.md:264-265`; `glauth.md:30,357-360`; `archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md` (NEXT-08).
+
+The SEC-06 guard refuses to start a `Production`-environment host with `Ldap:Transport=None`, and the config doc instructs "Deployed hosts must set `Ldaps` or `StartTls`" — but the only shared directory (`10.100.0.35:3893`) has LDAPS disabled, port 3894 closed, and answers StartTLS with `protocolError` (per the NEXT-07/08 record), and `glauth.md:30` documents "LDAPS | disabled in dev". Neither permitted value can work, so the instruction is unsatisfiable: every deployed gateway that binds this directory must either run plaintext (which `glauth.md:358-360` itself warns sends passwords cleartext) or avoid the `Production` label. That is exactly what happened — windev was relabelled `Staging` on 2026-08-07 to boot past the guard. The net posture is that the hard-stop protects no real deployment: hosts that would trip it are renamed around it, and the SEC-35 documentation note now formalizes the escape hatch. The dashboard admin surface (API-key CRUD, session Kill, secrets UI) sits behind credentials that transit this plaintext bind.
+
+**Recommendation:** resolve the posture, in one of two written directions: (a) enable LDAPS (or StartTLS) on the shared GLAuth — certificate issuance plus a trust story on each gateway host, source of truth `scadaproj/infra/glauth/` — and flip the deployed hosts' `Transport`; or (b) record an explicit decision that production gateways bind a different, TLS-capable directory and that the shared GLAuth is dev/staging-only, and say so in `docs/GatewayConfiguration.md`'s `Transport` row and `glauth.md`. Until one lands, every "must use Ldaps/StartTls" sentence is aspirational and should be labeled as such. (Cross-repo: coordinate with the family per the umbrella-index rule.)
+
+### SEC-38 · Low (Security) — The failure limiter's cross-peer aggregate layer can be sidestepped by pre-filling the per-peer key-id cap, restoring the `limit × addresses` guessing bound the layer was built to remove
+
+**Files:** `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs:225-231` (aggregate fed only when `effectiveKeyId` is non-null), `:393-444` (`ResolvePartitionKey` returns the `(peer, null)` fallback once the address has minted `MaxKeyIdPartitionsPerPeer = 32` key ids), `:153-166` (`Check` consults the aggregate only via `effectiveKeyId`).
+
+The SEC-32 cap collapses the 33rd-and-later key id from one address onto that address's fallback partition, and — per the comment's cardinality rationale — such failures deliberately do not feed the per-key-id aggregate. The side effect: an attacker who first mints 32 junk `mxgw`-shaped key-id partitions from an address (32 requests) makes every subsequent guess against the *victim's real key id* from that address resolve to `(peer, null)` — so those failures neither feed the victim's aggregate (`RecordFailure`, `:227`) nor consult it (`Check`, `:158-163`). A source-rotating attacker (the IPv6-/64 scenario the aggregate was designed for, per the SEC-31 remediation design's G1 analysis) thereby recovers `ApiKeyFailureLimit` guesses per window per address with the aggregate fully bypassed, at ~32 extra requests per address — instead of the designed bound of `ApiKeyFailureAggregateLimit` total followed by ~1 probe per interval globally. Severity is Low, not Medium: gateway secrets are library-generated high-entropy values, so online guessing remains computationally hopeless and the limiter is defense-in-depth plus store-read cost shielding (which still holds per-address); the finding is that the *documented* distributed-attack bound (`ApiKeyFailureLimiter.cs:17-19`, `docs/Authentication.md` limiter section) is not the actual bound.
+
+**Recommendation:** feed (and consult) the aggregate for any *validly shaped* key id regardless of whether it earned a composite partition — the cardinality concern the comment cites is already handled by `_aggregates`' own capped eviction (`:234`), whose policy protects over-limit entries and preferentially evicts single-failure junk. Alternatively, count cap-overflow failures into the aggregate of the presented key id only when that aggregate already exists (zero new cardinality, closes the consult-bypass for hot keys). Update the class remarks and `docs/Authentication.md` either way.
+
+### SEC-39 · Info (Security) — `Reset` treats "no key id" as owning the shared fallback partition: a verified token that fails the interceptor's key-id parse clears the address's fallback state the guard comment promises to preserve
+
+**Files:** `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs:252-276`; `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayGrpcAuthorizationInterceptor.cs:149-193`.
+
+`Reset`'s ownership check is `string.Equals(effectiveKeyId, partition.KeyId)` (`:264`), which is true when both are null — and a partition with `KeyId = null` resolves to the transport-peer fallback key (`:398-403`), so the reset then `RemovePartition(peer)`s the *shared* fallback bucket, the exact wipe the comment above it (`:259-263`) explains must not happen because that bucket holds junk-spray failures from the same address. A null `KeyId` on a *successful* verification requires a token the library verifier accepts but `TryResolveKeyId` rejects — the identifiable shapes are a key id longer than 64 chars (`:178`) and a secret whose first character is `_` (`:187`, matching the old `Split`-based rejection); whether the shared verifier accepts either depends on library parsing not readable in this repo. Practical impact is minimal (the attacker must hold a *valid* key of an unusual shape merely to clear their own address's junk-spray throttle), hence Info — but the null==null branch contradicts the stated invariant and costs one line to close.
+
+**Recommendation:** make the fallback non-resettable explicitly: `if (effectiveKeyId is not null && string.Equals(effectiveKeyId, partition.KeyId, StringComparison.Ordinal))`, and pin with a test (successful reset with a null-key-id partition leaves the fallback partition tracked).
+
+### SEC-40 · Low (Security) — Alarm visibility is not tag-scoped: the session-less alarms hub (and alarms page) broadcasts galaxy-wide alarm metadata to every Viewer, including empty-grant Viewers and anonymous localhost
+
+**Files:** `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHub.cs:12-25` (`[Authorize(HubClientsPolicy)]`, auto-join `__alarms__` on connect); `AlarmsHubPublisher.cs:48-63` (unfiltered `StreamAsync(alarmFilterPrefix: null)` re-broadcast to all clients); `Dashboard/DashboardAuthorizationHandler.cs:39-53` (loopback/disabled bypasses satisfy the Viewer requirement the hub policy uses); `docs/GatewayDashboardDesign.md:277` ("all clients receive every message").
+
+The SEC-25 ACL scopes *session event* visibility by tag intersection, and CLAUDE.md/docs describe dashboard event visibility as tag-scoped — but the alarm feed is a parallel, session-less surface with no per-caller scoping at all: any principal satisfying `HubClientsPolicy` (any Viewer role, the `Authentication:Mode=Disabled` bypass, or anonymous localhost under the default `AllowAnonymousLocalhost=true`) receives every `AlarmFeedMessage` the central monitor emits. Values are stripped by default (`ShowTagValues`, verified above), but the surviving metadata — alarm reference (a full tag path), state, severity, category, area, and acknowledging-operator usernames — discloses process structure and operator activity for the whole subscription expression to callers the tag model would deny every single session. A Viewer with an empty tag grant, whom the ACL was explicitly designed to show nothing, still sees the full plant alarm picture. The alarm monitor being gateway-central (not per-session) means tag scoping does not map mechanically, so this may well be a deliberate posture — but no design doc states it against the ACL: `GatewayDashboardDesign.md:277` describes the fan-out without a scoping rationale, and the ACL plan (`docs/plans/2026-07-10-dashboard-session-acl-tst15.md`) does not carve alarms out.
+
+**Recommendation:** decide and document. If deliberate, add one paragraph to the ACL section of `docs/GatewayDashboardDesign.md` (and the `GroupToTag` row of `docs/GatewayConfiguration.md`) stating that alarm metadata is intentionally visible to every dashboard Viewer and why (central monitor, no per-session provenance). If not, the cheap tightening is a dedicated policy on `AlarmsHub` (e.g. require a real role claim, excluding the anonymous-loopback bypass) or an area/tag-prefix filter derived from the caller's grant.
+
+### SEC-41 · Low (Security) — Tag grants revoked by a `GroupToTag` config change survive on cookie principals across the restart that applies the change: the ACL trusts login-stamped tag claims for up to the 8-hour sliding cookie life, while the hub-token path re-resolves at mint
+
+**Files:** `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs:145-149` (tags stamped as claims at login); `Dashboard/DashboardSessionAcl.cs:75-81` (ACL reads the *stamped* tag claims); `Dashboard/HubTokenService.cs:84-90` (token mint *re-resolves* tags from group claims against current options — the asymmetry); `Dashboard/DashboardServiceCollectionExtensions.cs:114-130` (`ZbCookieDefaults.Apply(..., idleTimeout: 8h)` — sliding, so an active session renews indefinitely).
+
+`GatewayOptions` binds once at startup (no reload — noted at `DashboardSnapshotService.cs:45-49`), so changing `GroupToTag` requires a gateway restart. A restart tears down circuits and hub connections, but **not** the auth cookie: data-protection keys persist, and a browser returning after the restart re-authenticates from the cookie, whose `DashboardTagClaimType` claims were resolved under the *old* map at login. `DashboardSessionAcl` consults exactly those claims, so a user whose group's tag grant was removed from config keeps observing the corresponding sessions' event streams — hub joins and the session-details in-process seam alike — until they log out or the sliding 8-hour window finally lapses, renewable indefinitely by activity. The hub-token seam does not have this gap for the *grant* (mint re-resolves from group claims, bounding staleness to 5 minutes, exactly as its comment at `:41-47` claims) — but that comment's promise that "revoking a GroupToTag entry takes effect ... within one lifetime" holds only for token-authenticated connections, and the cookie-authenticated in-process seam it shares an ACL with is the weaker path. LDAP group-membership revocations have the same 8-hour cookie bound on both paths (standard cookie-auth behavior, but worth stating for a *security grant*).
+
+**Recommendation:** have the ACL (or a claims transformation) re-resolve tags from the principal's `LdapGroupClaimType` claims against current options — the same one-call `DashboardGroupTagMapping.MapGroupsToTags` the token mint already uses — instead of trusting login-stamped tag claims; the group claims are already on the cookie. That makes a config-revoked grant take effect at the next subscribe after restart, symmetrically with the token path. At minimum, document the revocation bounds (restart + re-login for cookies, 5 minutes for tokens, never for an already-open subscription) in `docs/GatewayDashboardDesign.md`'s ACL section.
+
+## Severity roll-up (new findings)
+
+| Severity | Count | IDs |
+|----------|-------|-----|
+| Medium | 1 | SEC-37 |
+| Low | 3 | SEC-38, SEC-40, SEC-41 |
+| Info | 1 | SEC-39 |
+
+Prior-backlog movement this period: SEC-19, SEC-21, SEC-26 resolved; SEC-31…SEC-36 all verified Done (SEC-36 including the operator rotation); still open from cycle 1: SEC-13 (impact reduced — redactor now wired fail-closed), SEC-14, SEC-15 (accepted), SEC-16, SEC-17, SEC-18, SEC-23, SEC-24 (narrowed), SEC-27.
+
+## Still good / preserved
+
+The load-bearing controls all survive at HEAD, several strengthened:
+
+- **Fail-closed authorization core:** unrecognized request → `admin` scope (`GatewayGrpcScopeResolver`); opaque auth failures that never disclose the failing stage (`GatewayGrpcAuthorizationInterceptor.cs:96-109`); constant-time compare stays inside the shared verifier on every cache miss with only SHA-256 digests in memory (`CachingApiKeyVerifier.cs:36-39,284-286`); the throttle message reveals nothing about secret validity.
+- **The SEC-25 ACL is genuinely fail-closed on every branch** (unknown session, empty grant, untagged-session default `AdminOnly`, null principal), enforced identically on both subscribe seams, with the decision-table corners and the live-LDAP group-shape proof pinned by tests; session tags are immutable and inherited only from the owning key's constraints, never the client request; the gRPC event-stream attach stays owner-scoped (`EventStreamService.cs:52-63`).
+- **Read-only environment bypasses stay read-only:** the `Disabled`-mode and anonymous-loopback bypasses satisfy only requirements containing Viewer, never `AdminOnly` (`DashboardAuthorizationHandler.cs:37-53`); API-key CRUD and session Close/Kill keep their service-layer Admin re-checks (`DashboardApiKeyAuthorization.cs:10-18`, `DashboardSessionAdminService`) beneath the policy layer; the secrets UI mounts behind its own `secrets:manage` policy (`GatewayApplication.cs:147-150`), which the dashboard bypass handler cannot satisfy (different requirement type).
+- **Value redaction is now a complete, tested, three-seam system** (events mirror, alarms hub, `/browse`), clone-only so gRPC clients and the replay ring are untouched, with the config doc telling the truth about all three seams and the failed-read placeholder rule.
+- **Secret handling improved materially:** the committed LDAP literal is gone and rotated; config secrets ride fail-fast `${secret:}` references expanded pre-host; the effective-config projection redacts pepper name and LDAP password; hub tokens remain 5-minute, data-protected, identity-required; no new call site logging credentials, tag values, or grant contents was found in the changed files (the ACL logs decisions, never tag values — `DashboardAuthenticator.cs:145-146`).
+- **Credential-store paths are now defended in depth:** host-meaningful rooting, the under-content-root rule born of a real outage, and absolute defaults for all three stores (auth DB, secrets store, Galaxy snapshot) via one shared rule set (`GatewayConfigPathRules`).
+- **Dashboard hygiene held:** local Bootstrap only (no UI component libraries in any new page); antiforgery on POST login/logout; `SanitizeReturnUrl` still blocks open redirects (`DashboardEndpointRouteBuilderExtensions.cs:238-249`); the login limiter stays remote-IP-partitioned; the viewer registry / snapshot feed / broadcaster concurrency reworks (`EventsHubViewerRegistry.cs`, `DashboardSnapshotFeed.cs`, `DashboardEventBroadcaster.cs`) are careful, generation-guarded code with the subtle orderings documented in place and covered by dedicated tests.

archreview/2026-08-19/50-clients.md

@@ -0,0 +1,104 @@
+# Language Clients — Architecture Review (Cycle 3)
+
+**Date:** 2026-08-19 · **Commit:** `8b3e3b3` (main) · **Baseline:** `4f5371f` (2026-07-12 cycle) · **Method:** static review on macOS (no builds, no tests run)
+
+Scope: `clients/dotnet` (lib + CLI + tests), `clients/go` (mxgw-go), `clients/java`, `clients/python`, `clients/rust` (lib + CLI + build.rs + vendored protos), cross-client parity, `docs/CrossLanguageSmokeMatrix.md`, `docs/ClientLibrariesDesign.md`, `docs/ClientPackaging.md`, `docs/ClientBehaviorFixtures.md`, client READMEs, `scripts/pack-clients.ps1` / `scripts/tag-go-module.ps1`. Prior findings tracked as CLI-01..34 (`archreview/remediation/00-tracking.md`) and CLI-35..45 (`archreview/2026-07-12/remediation/50-clients.md` + `00-tracking.md`), plus NEXT-02 (`archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md`).
+
+Since 4f5371f the client family absorbed: the full CLI-35..45 remediation train, NEXT-02, the 0.2.0/0.2.1 family publish (2026-08-07, `5b153da`), TST-24 real-wire tests for .NET and Python (`a8f86b5`), alarm truncation-flag regen (`d9ea8a8`), the feed-level `snapshot_status` rollout (`c748361` + `4d4f443`), StreamAlarms open-sequence doc truth (`c3c603f`), and Gradle-9 survival for `checkGeneratedClean` (`df45cb4`).
+
+## Prior-finding verification
+
+### 2026-07-12 cycle (CLI-35..45) and NEXT-02
+
+| ID | Claimed status | Verified? | Evidence (file:line at HEAD) | Notes |
+|----|----------------|-----------|------------------------------|-------|
+| CLI-35 | Done | **Yes** | `clients/python/src/zb_mom_ww_mxgateway_cli/commands.py:24` (imports `ReplayGap`), `:1660-1666` (`_event_row` branches on `isinstance(item, ReplayGap)` and emits `{"replayGap": {"requestedAfterSequence": …, "oldestAvailableSequence": …}}`); regression `clients/python/tests/test_cli.py:935` (`test_stream_events_renders_replay_gap`, asserts exit 0, the gap row at index 0, the normal event row after, `:971-974`) | Exactly the designed fix, camelCase row matching the cross-CLI shape. |
+| CLI-36 | Done | **Yes** | `clients/go/cmd/mxgw-go/main.go:1012-1029` (`if result.IsReplayGap()` → JSON `replayGapCursors` struct `:70`, text `REPLAY_GAP requested_after=%d oldest_available=%d`); test `clients/go/cmd/mxgw-go/main_test.go:734` (`TestRunStreamEventsPrintsReplayGap` over a real fake gateway `:698-713`, both output modes) | Gap counted toward `-limit` as designed. |
+| NEXT-02 | Resolved 2026-08-10 | **Yes** | .NET `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:1425-1429` (stream loop branch), `:1845-1846` (`EventToJsonElement` inside `--json` aggregate), `:1858-1873` (`FormatReplayGapRow` — hand-built so cursors are JSON numbers, not proto-JSON uint64 strings); Java `clients/java/zb-mom-ww-mxgateway-cli/.../MxGatewayCli.java:1665-1674` (`replayGap` JSON row + `REPLAY_GAP` text line); tests `MxGatewayCliTests.java:909-937`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs` (grep `FormatReplayGapRow`/`replayGap` hits); `docs/CrossLanguageSmokeMatrix.md:43-67` now records one converged contract with a per-CLI table | All five CLIs emit the same two camelCase keys with numeric cursors; the matrix doc explains *why* the row is hand-typed (proto3 JSON stringifies uint64). Genuinely converged. |
+| CLI-37 | Done | **Yes — all five conformant on `category`** | .NET `MxStatusProxyExtensions.cs:15-20` (`Category is MxStatusCategory.Ok` only); Go `clients/go/mxgateway/status.go:15-17` (`nil ∥ category == OK`, doc `:7-14` states the diagnostics-only rule); Java `MxStatuses.java:33-35` (+ `success()` Javadoc `:55-57` marks it diagnostic); Python `errors.py:144-152`; Rust `error.rs:407-413` (`status.category != Ok as i32`, doc `:383-388` covers the unspecified-category-fails edge); shared fixtures `clients/proto/fixtures/behavior/command-replies/write.status-category-error-success-set.reply.json` + `write.status-category-ok-success-zero.reply.json`, registered in the manifest, documented in `docs/ClientBehaviorFixtures.md` (Reply Validation Conformance table), and consumed by all five suites (`MxCommandReplyExtensionsTests.cs`, `protofixtures_test.go`, `MxGatewayFixtureTests.java`, `test_errors.py`, `client_behavior.rs`) | The 4-vs-1 divergence is gone; the edge decisions (absent entry = success, present-unspecified = failure) are uniform and documented. |
+| CLI-38 | Done | **Yes — all five on `hresult < 0`** | .NET `MxCommandReplyExtensions.cs:38` (`HasHresult && Hresult < 0`); Go `errors.go:299` (`GetHresult() < 0`); Java `MxGatewayErrors.java:52` (`getHresult() < 0`); Python `errors.py:137`; Rust `error.rs:407`; fixtures `write.hresult-s-false.reply.json` (S_FALSE=1 → success) + `write.hresult-e-fail.reply.json` (E_FAIL → failure) wired into all five suites | Lands prior-cycle CLI-08 (cycle-1 tracker row updated to Done via CLI-38 — verified). `docs/ClientLibrariesDesign.md:112-125` is now *true*, and states both rules explicitly. |
+| CLI-39 | Done | **Yes (bump + both guards); pattern has already recurred → CLI-46** | Versions: `clients/rust/Cargo.toml:3`/`:28` = 0.2.0, `clients/python/pyproject.toml:9` + `version.py:3` = 0.2.0 (+ new drift test `test_cli.py:17` `test_version_matches_pyproject_toml`, closing the CLI-26 residual), `clients/go/mxgateway/version.go` = 0.2.0, `.../ZB.MOM.WW.MxGateway.Client.csproj:22` = 0.2.0, Contracts `.csproj:10` = 0.2.0 (`440e7cf`), Java `build.gradle` = 0.2.1. Guards: `scripts/tag-go-module.ps1:40-53` (tag fails on `ClientVersion` mismatch — the CLI-21 guard finally implemented), `scripts/pack-clients.ps1:91-156` (`Test-GiteaPackageExists` + hard abort "never force-overwrites"). Publish record: `docs/ClientPackaging.md:102-113` (2026-08-07: nuget/pypi/cargo 0.2.0, maven 0.2.1 with the rationale for the Java exception, Go tag `clients/go/v0.2.0` — tag exists in this repo) | Executed to the letter, including the doc's "bump before every publish" rule and the collision-guard rationale. However the *same hazard is live again at HEAD*: post-publish library changes (incl. a shipped Python `connect()` fix) sit at the already-published 0.2.0/0.2.1 — see **CLI-46**. |
+| CLI-40 | Done | **Yes** | Java `MxGatewaySecrets.java:62-78` (`redactExact`); .NET `MxGatewaySecretRedaction.cs` (`Redact`/`RedactReply:55-68`/`RedactStatus:81-89`/`RedactStatuses` — scrubs message *and* the structured reply accessors, the `0d874f9` follow-up); Rust `error.rs:208-231` (`MxAccessError.secrets` + `with_secrets`, hand-written `Debug` prints only the count `:202-204`); fixtures `authenticate-user.echoed-credential.reply.json` + `.echoed-credential-mxaccess-failure.reply.json` in the manifest, consumed by all five suites (`MxGatewaySessionReplyContractTests.cs`, `command_reply_fixtures_test.go`, `MxGatewayCredentialReplyTests.java`, `test_malformed_reply.py`/`test_errors.py`, `client_behavior.rs`) | The echoed-credential fixture is exactly the test the prior review said was missing ("the fakes don't echo"). |
+| CLI-41 | Done | **Yes** | Go `errors.go:164-176` (`MalformedReplyError`); Python `errors.py:56`; Java `MxGatewayMalformedReplyException.java` + throw sites `MxGatewaySession.java:889/:917/:948` (redaction rethrow preserves the type `:1143-1144`); .NET `MxGatewayMalformedReplyException.cs`; Rust `session.rs:1024-1042` (payload → present `return_value` → `Error::MalformedReply`, now uniform across its extractors); fixtures `authenticate-user.missing-payload.reply.json` + `authenticate-user.return-value-only.reply.json`; contract documented in `docs/ClientLibrariesDesign.md` ("Malformed-reply extraction" paragraph) | Silent-0 user ids and the .NET NRE are gone; the payload → return_value → typed-error ladder is uniform. |
+| CLI-42 | Done | **Yes** | `clients/rust/README.md:24-36` (repo-path-first / vendored-fallback resolution, same-commit refresh rule, both-direction drift guard); `docs/ClientPackaging.md:199-213` (same, plus why `cargo package` runs *with* verification); `711793f` extended check-codegen Check 3 to sweep both directions (canonical proto missing a vendored copy now fails) | Vendored copies verified byte-identical to Contracts at HEAD (all three protos, diff clean). |
+| CLI-43 | Done | **Yes** | `docs/style-guides/JavaStyleGuide.md:8` — "Target Java 17 (the Ignition 8.3 baseline; the client build enforces …)" | `grep "Java 21"` in style-guides: clean. |
+| CLI-44 | Done | **Yes** | `clients/go/mxgateway/session.go:1077` (Recv-error path uses `sendTerminalEventResult`), `:1096-1111` (bypasses the overflow substitution via the reserved slot) | Root cause of a genuine stream error is no longer relabeled `ErrSlowConsumer`. |
+| CLI-45 | Done | **Yes** | Canonical env `MXGATEWAY_VERIFY_PASSWORD` in all five: Go `main.go:452` (default) + `:483-491` (fail-fast before dialing, message names flag+var only); Java `MxGatewayCli.java:194` + `:1166-1179` (`ParameterException` on null/blank); Rust `main.rs:151` (clap default) + test `:2644-2660`; Python `commands.py:38` (`DEFAULT_VERIFY_PASSWORD_ENV`); .NET `MxGatewayClientCli.cs:353-359` (canonical default + `MXGATEWAY_VERIFY_USER_PASSWORD`/`--verify-user-password*` kept as deprecated aliases, resolution order documented `:385-386` and in help `:2455-2456`). Tests: Go `main_test.go:605` (`RejectsEmptyPassword`) + `:632` (`ReadsPasswordFromCanonicalEnv`). Contract documented at `docs/CrossLanguageSmokeMatrix.md:90-119`, incl. the parity rationale (libraries still transmit whatever they are given) and the .NET deprecation window | One exported variable now drives all five CLIs; empty credential is a usage error everywhere. |
+
+**Done claims that don't fully hold:** none. Every 2026-07-12 remediation claim checked out at HEAD with the designed shape (fixtures-first conformance, docs in the same commit, guards actually implemented — including the CLI-21 tag guard skipped last cycle). This is the cleanest verification pass of the three cycles.
+
+### Cycle-1 residue (CLI-05..33, previously Open)
+
+Re-verified briefly at HEAD; all unchanged unless noted:
+
+- **CLI-08 → Done** via CLI-38 (see above); cycle-1 tracker row correctly updated.
+- Still open, unchanged: **CLI-05** (`MxGatewaySession.cs:19` internal ctor, no .NET `AttachSession`), **CLI-06** (`:1501-1505` `DisposeAsync` → unbounded `CloseAsync`), **CLI-07** (`MxGatewayClient.cs:311`), **CLI-09** (no typed auth errors in Go `errors.go`), **CLI-10** (`client.go:64/:68` `WithBlock`/`DialContext`), **CLI-11** (library has `RequireCertificateValidation` (`client.go:239`) but `mxgw-go` still exposes no flag), **CLI-13** (`MxGatewayClient.java:248` hardcoded 16), **CLI-14** (no TOFU runtime warning in `options.py`), **CLI-17/20** (TLS posture divergence: .NET accept-all `MxGatewayClient.cs:369`, Go skip-verify, Java `InsecureTrustManagerFactory` `MxGatewayClient.java:391`, Python TOFU, Rust strict), **CLI-19** (duplicate `InternalsVisibleTo`), **CLI-22** (`session.go:1176-1181` empty correlation id on rand failure), **CLI-23** (nil-vs-empty asymmetry — now at least documented in the `WriteBulk` doc comment `session.go:431-432`), **CLI-25** (`close()` doesn't await termination `MxGatewayClient.java:347-351`), **CLI-27** (`session.py` close still lock-free), **CLI-28** (bottom-of-file import `session.py`), **CLI-31** (`mxgw-cli/src/main.rs` now **2,991** lines — grew again; the other CLIs sit at 1,668–2,478), **CLI-32** (no .NET/Java client-side bulk cap), **CLI-33** (no backpressure section in `docs/ClientBehaviorFixtures.md`).
+
+## New findings
+
+### CLI-46 — Version constants again equal the already-published 0.2.0/0.2.1 while post-publish changes shipped — including the fix for a published-broken Python `connect()`
+**Severity:** Medium · **Category:** packaging / release hygiene
+**Files:** `clients/python/pyproject.toml:9` + `clients/python/src/zb_mom_ww_mxgateway/version.py:3` (0.2.0 = the pypi artifact published 2026-08-07, `docs/ClientPackaging.md:102-113`); `clients/python/src/zb_mom_ww_mxgateway/client.py:61-72` (the fixed `connect()`); post-publish library commits `a8f86b5` (2026-08-10), `d9ea8a8`, `c748361` (2026-08-17/18); guard `scripts/pack-clients.ps1:145-156`
+The published pypi 0.2.0 Python client cannot open a real connection at all: its `connect()` built the `grpc.aio` channel inside `asyncio.to_thread`, and a `grpc.aio` channel binds to the event loop current on the constructing thread, so every non-stub connection raised "There is no current event loop in thread" (TST-24's first real-wire run caught it; fixed in-tree by `a8f86b5` — `resolve_channel_security` runs off-loop, `create_channel` on-loop). That fix has sat unpublished for nine days while the version constant still reads 0.2.0. Since the publish, all five libraries also gained wire-surface additions at the published numbers (alarm truncation fields `d9ea8a8`, feed-level `snapshot_status` bindings `c748361`; Java at its published 0.2.1). This is precisely the CLI-39 pattern one cycle later — now *mitigated* (the pack-script collision guard refuses a same-version publish, and the bump-together deferral is deliberate) but the live registry keeps serving a broken Python client, and nothing in the tree records that 0.2.0-as-published ≠ 0.2.0-as-source.
+**Recommendation:** cut the next release train soon (0.2.1 four clients / 0.2.2 Java, or 0.3.0 across the board), prioritizing the Python publish; until then, note the known-broken `connect()` against pypi 0.2.0 in `clients/python/README.md`/`docs/ClientPackaging.md` (or yank the pypi artifact). Consider bumping version constants in the same commit as the first post-publish library change rather than at publish time — the collision guard then enforces nothing because nothing needs catching.
+
+### CLI-47 — `snapshot-status`/`provider-status` rendering diverges: .NET text mode hides the flag on the frames that matter most; Rust's JSON is the lone always-explicit shape
+**Severity:** Low · **Category:** cross-client parity / CLI
+**Files:** .NET `MxGatewayClientCli.cs:15` (`JsonFormatter.Default` — omits default-valued proto3 fields) + `:1563-1566` (text arms format the payload through it); Go `main.go:1112-1114` (`snapshot-status truncated=%t`); Java `MxGatewayCli.java:2309-2312` (`truncated=%b`) + `protoJson` `:2362-2368` (default printer); Rust `main.rs:2262-2264` (summary) + `:2314-2318` (hand-built JSON, always includes `"truncated"`); Python CLI renders generic `MessageToDict`
+The c748361 rollout achieved surface parity (all five render the new arm) but not rendering parity. The `snapshot_status` clearing frame — `truncated = false`, emitted on monitor restart, explicitly called out by the commit and `docs/Grpc.md` as a frame consumers must see — renders in the .NET CLI's text mode as `snapshot-status {}` (proto3 JSON omits the false bool), while Go/Java/Rust print `truncated=false` explicitly. The same applies to a healthy `provider-status` (`degraded=false` invisible in .NET text). In JSON mode the split flips: Rust's hand-built row always carries `"truncated"`, while .NET/Go/Java/Python protobuf formatters emit `{"snapshotStatus": {}}` for the clearing frame. Semantically recoverable (absent = false), but the same class of divergence NEXT-02 just cleaned up for `ReplayGap`, where the cross-CLI row was deliberately hand-specified.
+**Recommendation:** hand-format the .NET text arms like the other three (`snapshot-status truncated={bool}`, `provider-status mode=… degraded=… reason=…`); decide one JSON shape (either Rust adopts formatter-omission or the matrix documents Rust's explicitness the way `CrossLanguageSmokeMatrix.md:54-67` documents the ReplayGap row's deliberate choices).
+
+### CLI-48 — The new alarm-feed renderer arms are untested in three of the four CLIs that changed; the Go CLI has no `stream-alarms` test at all
+**Severity:** Low · **Category:** testing coverage
+**Files:** `clients/go/cmd/mxgw-go/main_test.go` (test inventory `:16-734` — no stream-alarms/`formatAlarmFeedMessage` test of any kind); .NET `MxGatewayClientCliTests.cs:671-706` (`stream-alarms` test feeds only `ActiveAlarm` + `SnapshotComplete`); Java `MxGatewayCliTests.java:653-687` (asserts `active-alarm`/`snapshot-complete`/`transition` arms only); Rust `main.rs:2986` (the one snapshot-status render test, added by `4d4f443`)
+The review-nits commit covered only the Rust render; the .NET/Go/Java `provider-status` and `snapshot-status` arms shipped with zero assertions. The .NET `{}`-rendering divergence (CLI-47) is exactly what a text-mode assertion on a `truncated=false` frame would have caught — cycle 2's lesson ("each new epic achieves surface parity but re-introduces behavioral divergence") repeating at the test layer.
+**Recommendation:** one fake-fed `stream-alarms` test per CLI whose feed includes a `provider_status` and a `snapshot_status` frame (including the false/clearing shape), asserting the rendered row in both output modes; for Go this also creates the missing `stream-alarms` scaffold.
+
+### CLI-49 — Go library exposes no aliases for `AlarmProviderStatus`/`AlarmSnapshotStatus`, whose generated types live in an `internal` package; the `AlarmFeedMessage` alias doc names three of five payload arms
+**Severity:** Low · **Category:** code organization & conventions / documentation
+**Files:** `clients/go/mxgateway/types.go:129-137` (alias block: `AlarmFeedMessage`, `ActiveAlarmSnapshot`, `OnAlarmTransitionEvent` — but not the two status payloads; `ReplayGap` got its alias at `:38` when it shipped); generated types under `clients/go/internal/generated` (Go `internal` visibility)
+Because the generated package is `internal`, a Go consumer cannot name `*generated.AlarmSnapshotStatus`: they can call `message.GetSnapshotStatus().GetTruncated()` on an inferred value but cannot declare a variable, struct field, or function parameter of the type — unlike every other StreamAlarms payload, which `types.go` deliberately aliases for exactly this reason. (`AlarmProviderStatus` has had the same gap since provider-mode shipped.) The `AlarmFeedMessage` alias comment (`types.go:130-132`) still describes the oneof as "an active-alarm snapshot row, a snapshot-complete sentinel, or a transition" — stale by two arms.
+**Recommendation:** add `AlarmProviderStatus = pb.AlarmProviderStatus` and `AlarmSnapshotStatus = pb.AlarmSnapshotStatus` aliases and refresh the `AlarmFeedMessage` comment to name all five arms.
+
+### CLI-50 — Python CLI `stream-alarms` default (`--max-messages 1`, collect-then-print) now returns only the provider-status preamble — the README's own example fetches no alarm data
+**Severity:** Low · **Category:** cross-client parity / CLI usability
+**Files:** `clients/python/src/zb_mom_ww_mxgateway_cli/commands.py:514` (`--max-messages` default 1), `:1539-1551` (`_collect_alarm_messages` — bounded collect, printed once at the end), `:1122-1134`; `clients/python/README.md:451` (`mxgw-py stream-alarms --max-messages 1 --json`); feed order per `docs/Grpc.md`/`c3c603f`: `provider_status` → `snapshot_status` → cached `active_alarm` → `snapshot_complete`
+The other four CLIs stream rows as they arrive and default to unbounded (`--limit 0` / `--max-events 0`). Python is the outlier twice over: it buffers then prints (so it cannot tail a live feed at all), and its default of 1 — reasonable when the first frame was an alarm — now returns exactly one `provider_status` frame, since the feed gained a two-frame status preamble. The README example therefore demonstrates a command that yields no alarm rows. Pre-existing shape, but the preamble growth this cycle changed what the default actually returns.
+**Recommendation:** raise the default (or document that N status frames precede the data) and update the README example; longer term consider printing rows incrementally like the other four CLIs.
+
+## Severity roll-up (new findings)
+
+| Severity | Count | IDs |
+|---|---|---|
+| High | 0 | — |
+| Medium | 1 | CLI-46 |
+| Low | 4 | CLI-47, CLI-48, CLI-49, CLI-50 |
+
+## Cross-client parity note (updated)
+
+The 2026-07-12 divergence table has largely collapsed. Converged this cycle, with shared fixtures pinning the bytes: **HRESULT `< 0`** (CLI-38), **status verdict = `category != OK`** (CLI-37), **exact-secret scrub incl. echoed diagnostics** (CLI-40), **malformed-reply ladder** (CLI-41), **CLI ReplayGap row** (CLI-35/36 + NEXT-02, all five, same keys, numeric cursors), **credential env-var contract** (CLI-45). What remains divergent:
+
+| Behavior | .NET | Go | Java | Python | Rust |
+|---|---|---|---|---|---|
+| Version vs published artifact (CLI-46) | 0.2.0 = shipped | 0.2.0 = shipped | 0.2.1 = shipped | 0.2.0 = shipped (**broken `connect()` on pypi**) | 0.2.0 = shipped |
+| CLI text render of snapshot/provider status (CLI-47) | proto-JSON, flag invisible when false | explicit `truncated=…` | explicit | n/a (JSON only) | explicit ✓ |
+| CLI JSON shape of `snapshot_status` (CLI-47) | omits false | omits false | omits false | omits false | always explicit |
+| New renderer arms tested (CLI-48) | no | no (no stream-alarms test) | no | n/a (generic dump) | yes ✓ |
+| `stream-alarms` consumption model (CLI-50) | stream, unbounded default | stream, unbounded | stream, unbounded | **collect, default 1** | stream, unbounded |
+| Session re-attach (CLI-05) | **No** | Yes | Yes | Yes | Yes |
+| Client-side bulk cap (CLI-32) | No | Yes | No | Yes | Yes |
+| Event backpressure (CLI-13/33) | unbuffered | 16+1, loud, true root cause ✓ | 16, cancel+error | unbuffered | unbuffered |
+| TLS default (CLI-17) | accept-all | accept-all | accept-all | TOFU pin | strict pin |
+| Typed auth errors (CLI-09) | Yes | **No** | Yes | Yes | Yes |
+| Real-wire test harness | Kestrel h2c ✓ (TST-24) | bufconn ✓ | InProcessServer ✓ | grpc.aio loopback ✓ (TST-24) | tonic loopback ✓ |
+
+CLI subcommand coverage remains uneven by design and is now honestly specified (`docs/CrossLanguageSmokeMatrix.md:163-186`: .NET all nine, Rust + unregister/credential pair, Go/Python/Java credential pair only) — verified accurate against all five dispatch tables at HEAD, including the note that every gap is CLI-surface-only (all five libraries carry all typed helpers, confirmed for the Rust bulk family at `session.rs:605-654`).
+
+## What is good
+
+- **The remediation train executed at design fidelity.** All eleven 2026-07-12 findings landed exactly as their remediation designs specified — including the parts prior cycles skipped (the CLI-21 tag guard, the CLI-02/42 doc half, the CLI-26 pyproject-drift test). The conformance findings (CLI-37/38/40/41) all went fixtures-first: ten shared command-reply fixtures under `clients/proto/fixtures/behavior/command-replies/`, each consumed by all five suites, so the canonical rules are pinned in bytes, not prose.
+- **The publish pipeline is now guarded end to end**: version-vs-tag check in `tag-go-module.ps1`, registry-collision hard-stop in `pack-clients.ps1`, publish coordinates and the Java 0.2.1 exception recorded in `docs/ClientPackaging.md` with the reasoning ("these are the numbers a future release bumps off").
+- **TST-24 paid for itself immediately**: the first real-wire run of the Python client found `connect()` could never work outside stubs — a defect every mock-based test had actively asserted as correct. All five clients now have genuine-transport tests (framing, metadata, status codes, ReplayGap, PERMISSION_DENIED mapping), and CI runs all five suites plus a Java generated-tree drift check (`.gitea/workflows/ci.yml:84-142`).
+- **Generated-code hygiene is the best it has been**: vendored Rust protos byte-identical to Contracts (verified, all three files); check-codegen Check 3 sweeps both directions; the Java toolchain pin eliminated the spurious ~64k-line protobuf churn (per `build.gradle:61-70`, IPC-24) and `checkGeneratedClean` survived Gradle 9 (`df45cb4`); Python regen stays on the pinned grpcio-tools 1.80.0 / protobuf 6.31.1 baseline (`pyproject.toml:41-44`, generated header confirms 6.31.1); even a proto-comment-only change (`c3c603f`) regenerated every downstream tree in the same commit.
+- **Parity discipline held through the alarm epic**: no client wraps or reinterprets `AlarmFeedMessage`/`ActiveAlarmSnapshot` — the truncation and status flags pass through the generated types untouched in all five, and every README explains the `from_truncated_snapshot`/`snapshot_status` semantics with the same don't-reconcile-deletions guidance, explicitly disambiguating them from the per-record `degraded` flag.
+- **The smoke-matrix doc is a model of honest divergence-recording**: where the CLIs still differ (subcommand coverage, JSON formatting quirks per language), the doc specifies the delta instead of leaving it to be discovered — the exact practice this review series asked for two cycles ago.

archreview/2026-08-19/60-testing-docs-gaps.md

@@ -0,0 +1,104 @@
+# Testing, Documentation & Underdeveloped Areas — Re-Review (cycle 3)
+
+- **Date:** 2026-08-19
+- **Commit:** `8b3e3b3` (`main`, "Merge feat/sweep-reread-tag-summary…")
+- **Baseline:** prior report `archreview/2026-07-12/60-testing-docs-gaps.md` @ `4f5371f` (2026-07-12); ~288 commits since.
+- **Method:** macOS static review — reading only, no builds, no test runs, no remote hosts. One read-only script executed (`python3 code-reviews/regen-readme.py --check`, sanctioned by REVIEW-PROCESS.md). All paths relative to `/Users/dohertj2/Desktop/MxAccessGateway`.
+
+## Prior-finding verification
+
+### Cycle-2 findings (TST-25..30)
+
+| ID | Claimed status | Verified? | Evidence (file:line) | Notes |
+|----|----------------|-----------|----------------------|-------|
+| TST-25 | Done | **Yes** | `.gitea/workflows/ci.yml:144-165` (`windows-x86` per-push job, Linux runner → SSH to windev, exit-code propagation) and `:167-201` (`nightly-windev`, `if: github.event_name == 'schedule'`, `run-windev-ci.sh live`, on-failure Gitea-issue step); `scripts/ci/run-windev-ci.sh` (SHA-shape validation `:46-49`, base64 key decode `:70-78`, bootstrap holds the worktree lock `:91-119`); `scripts/ci/windev-worker-ci.ps1` (lock reuse via `MXGW_CI_LOCK_HELD` `:77-94`, re-checkout under lock + HEAD verification `:99-111`, cumulative build/test/live modes `:119-139`) | The whole design shipped as specified, including the acceptance-check hardening: the CI key is stored base64 single-line so Gitea's line-oriented masker redacts it (`ci.yml:156-159`), the bootstrap fetch/checkout races were serialized under the mkdir lock, and the nightly failure issue carries a browser-reachable link via `PUBLIC_SERVER_URL` (`ci.yml:174-181`, Check 6 closed 2026-08-10 per `docs/runbooks/TST-30-second-ci-runner.md`). Ground truth from the operator confirms the tier runs green. |
+| TST-26 | Done | **Yes** | `docs/GatewayTesting.md:624-657` describes exactly the four jobs that exist (`portable`, `java`, `windows-x86`, `nightly-windev`) incl. the "red can mean the tier is down" triage (`:649-652`); `scripts/check-codegen.ps1:26-27` now attributes the x86 guard to "the SSH-driven `windows-x86` CI job"; `ci.yml:14-18` cron comment matches what the schedule runs | Guard attribution is correct both ways: primary = Check 2 regeneration diff in `portable`, secondary = the windows-x86 net48 compile (`GatewayTesting.md:699-703`). No named job is missing and no phantom job survives. |
+| TST-27 | Done | **Yes** | `docs/GatewayConfiguration.md:195` — the `ShowTagValues` row now describes all three seams (events-hub mirror, alarms hub, `/browse` live values incl. the successful-read-only nuance from `e50ec49`), the clone-don't-mutate guarantee, and the ACL layering | "Reserved" is gone; the row was rewritten a second time when TST-16's residuals shipped (2026-08-17), so it documents strictly more than the finding asked. |
+| TST-28 | Done | **Yes** | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs:46` — `StartAsync_SendsGatewayHelloWithConfiguredMaxFrameBytes(int maxMessageBytes)` theory, in the CI-run portable suite | Exactly the designed test (default + override), pinning the gateway half of the IPC-02 negotiation on Linux CI. |
+| TST-29 | Done | **Yes** | `oldtasks.md` absent from the tree; the five `*-docs-*.md` root artifacts absent; `docs/DesignDecisions.md:264` "Session-Resilience Epic Scope" carries the Phase-5 deferred-not-planned record and `:295` the "`EnableOrphanReattach` does not exist and must not be referenced" rule; CLAUDE.md's invariant paragraph points at DesignDecisions | Governance record migrated intact before deletion, exactly per the design. Residual: `stillpending.md` (also a TST-14 root artifact) still contradicts the migrated record — see new finding TST-33. |
+| TST-30 | Done | **Yes, with one pending operator action** | `docs/runbooks/TST-30-second-ci-runner.md:1-13` (Executed 2026-08-07: `gitea-runner-2`, id 5, capacity 2, `container.network: traefik`, token from a `0600` file mount; concurrency verified across three simultaneous runs) plus the honest correction that the per-repo runners endpoint legitimately returns 0 (instance-level registration); `docs/GatewayTesting.md:659-685` documents the shared-capacity/no-cancel reality and the `run-windev-ci.sh` bypass | The runner exists and the docs match the executed state. The **follow-up it surfaced is still open**: the shared instance registration token (world-readable ~5 months, proven still live by a probe registration) needs a web-UI reset — promoted to new finding TST-32 so it stops living only inside a closed finding's runbook. |
+
+**Done claims that failed verification: none.** All six hold with file:line evidence; TST-30 carries the one explicitly-tracked operator residual (TST-32 below).
+
+### Cycle-1 leftovers still tracked open (spot-verified at HEAD)
+
+| ID | Tracker status | State at HEAD |
+|----|----------------|---------------|
+| TST-05 | Partially done | **Residual confirmed open.** The live suite is 8 `[LiveMxAccessFact]`s (`WorkerLiveMxAccessSmokeTests.cs:34-920`) with zero references to `Ping`/`GetSessionState`/`GetWorkerInfo`/`DrainEvents`/`ShutdownWorker`; the gap is honestly documented at `docs/GatewayTesting.md:90-95`. The scheduling half stays closed (nightly-windev runs the suite). The two control-command live facts specified in `archreview/remediation/60-testing-docs-gaps.md` remain unauthored (windev-only work). |
+| TST-06 | Not started | **Materially narrowed, tracker stale.** `Tests/Gateway/Dashboard/DashboardLiveDataServiceTests.cs`, `EventsHubTests.cs`, `EventsHubViewerRegistryTests.cs`, `AlarmsHubPublisherTests.cs`, `SessionDetailsPageEventAclTests.cs` all exist now (mostly landed with SEC-25/TST-16 closeouts and the in-process dashboard-feed work). The cycle-1 row deserves a re-audit-and-close pass rather than "Not started". |
+| TST-07 | Not started | Partially addressed: the `WorkerClientTests` 150 ms sleep-then-negative-assert is gone; `SessionManagerTests.cs:491` still has a real-clock `Task.Delay(50)`. |
+| TST-09 | Not started | Partially addressed: `GatewayApplication.cs:117-124` now registers `AuthStoreHealthCheck` **and** `SessionHealthCheck` (the `/health/active` probe with off-repo consumers). Still no Galaxy SQL / LDAP / worker-executable checks. |
+| TST-10 | Not started | Still no `docs/Deployment.md`; partial mitigation via `docs/runbooks/IdentifyingADeployedBuild.md` and the SEC-36 rotation runbook, but deploy/upgrade mechanics (dated `Server-YYYYMMDD` dirs, NSSM repoint, env-var config) still live in operator memory notes, and the 2026-08-09 credential-store-renamed-away outage (`docs/GatewayConfiguration.md:100`) shows the cost. |
+| TST-14 | Not started | Mostly discharged by TST-29 (oldtasks + the five untracked artifacts gone). Remaining root-file triage: `stillpending.md` (now the stale one — see TST-33) and `A2-galaxyrepository-adoption-handoff.md` (carries a current "ADOPTED" banner; acceptable as a handoff record). |
+| TST-17 | Not started | **Materially changed — row needs refresh by the worker domain.** `WnWrapAlarmConsumer.cs:309-327` now passes `szDomainName`/`szOprFullName` into the COM call (the `_ = ackOperatorDomain` discard is gone), and the 2026-08 probe cycle recorded the vendor 8-arg leg as "unavailable by configuration" with evidence (`docs/AlarmProbeFindings.md`, commits `d1ae43d`, `540a118`). Whether a caller-visible degrade signal exists is the worker reviewer's call, but "silent drop, unchanged" is no longer an accurate row. |
+| TST-18 | Not started | Confirmed open — no `*HostedService*` tests under `src/ZB.MOM.WW.MxGateway.Tests`. |
+| TST-19 | Not started | Confirmed open — `FakeWorkerHarness.cs` still has no lockstep note / `WorkerPipeSession` pointer. Mitigation since: the harness's canned control replies are no longer the *only* automation (windows-x86 runs the real worker suite per push), but the drift risk the finding names is intact. |
+| TST-20 | Not started | Confirmed open — `run-client-e2e-tests.ps1:48` `DrainEveryTags = 15` workaround intact. |
+| TST-21 | Not started | Confirmed open — `Server/appsettings.json:10` daily rolling file sink, no size cap / retained-count. |
+| TST-22 | Not started | Confirmed open — the shape block (`docs/GatewayConfiguration.md:12-88`) is maintained (gained `GroupToTag`, `UntaggedSessionVisibility`, `RecentFaultLimit`/`RecentSessionLimit`) but still omits documented keys (`DisableLogin`, `AutoLoginUser`, `CookieName`, `Tls`, `Ldap`, `Security`, `Alarms:Fallback`, `Worker:WriteCompletionWaitMilliseconds`, …). The option **tables** below it are complete and current — the omissions are only in the illustrative block. |
+
+## New-code / new-docs review since `4f5371f`
+
+**CI workflow evolution — verified sound.** `ci.yml` at HEAD is a four-job pipeline whose comments are accurate down to the failure semantics. Details worth recording: the portable job now runs the .NET client *tests* (TST-24 wire tests included, `ci.yml:83-90`), installs pwsh and the pinned Check-4 generators explicitly (`:58-72`), and pins protoc 34.1; the java job's `git diff --exit-code` is the true drift gate after IPC-24 deleted the churn-revert; `run-windev-ci.sh` validates `CI_SHA` shape before it reaches a remote shell (`:46-49`), never echoes the key, and pins host keys from the committed `windev.known_hosts`; `windev-worker-ci.ps1` re-checks-out under the lock, verifies `HEAD == Sha`, `git clean -fdx -e ci.log`, and is deliberately PS5.1-safe. The nightly's issue-on-failure path uses the runner-resolvable `github.server_url` for the API call but `PUBLIC_SERVER_URL` for the human link — the exact defect Check 6 caught, fixed in `30c92e8`.
+
+**check-codegen.ps1 —** now four checks with the Check 3 reverse sweep (`:98-103`, canonical proto missing a vendored copy fails — the drift class no in-repo build can surface, `711793f`) and Check 4 Windows support (`.exe` banner normalization in the per-client `generate-proto.ps1` probes, `c94c4d4`; PS5.1-safe native-arg quoting, `8ae0c2f`). The header comment (`:2-27`) is accurate.
+
+**fca978d marker sweep → f82dac1 restoration — assessed complete on sampled evidence.** The sweep (2026-07-07, 203 files) deleted ~1,100 prose-bearing comment lines alongside the tracking markers; the 2026-08-18 re-read narrowed those to 816 residual lines, judged each, and restored 17 collateral deletions across 10 files (`f82dac1`, merged `8b3e3b3`). Spot-checks of four substantive deletions **outside** the restored set all found surviving equivalents at HEAD: the `SessionEventDistributor` mode-vs-count FailFast race rationale survives in `docs/Sessions.md:206` (including the "fixed mode rather than a live subscriber-count snapshot" decision) and `docs/GatewayConfiguration.md:159`; the `MxAccessStaSession` off-STA-deadlock affinity-guard rationale survives in `AlarmCommandHandler.cs:68-80`; the subtag source's own-COM-object isolation rationale survives in `LmxSubtagAlarmSource.cs` class remarks; the watchdog `HeartbeatStuckCeiling` rationale survives in `WorkerPipeSessionOptions.cs:35+`. No residual collateral damage found. The companion `d3ac527` audit (re-verifying 20 doc-only `Server-0xx` resolutions, re-fixing the two the sweep had silently reverted — Server-040 among them) closes the loop on the sweep's other damage class. This is a model recovery process.
+
+**Doc currency at HEAD — high.** Sampled claims all match code: `docs/Sessions.md:17` pipe-name prose matches `SessionManager.cs:536` (`mxgw-{pid}-{uid}`, with the sun_path rationale); `docs/Sessions.md:248` owner-scoped attach matches `EventStreamService`; `docs/GatewayConfiguration.md` documents the new `WriteCompletionWaitMilliseconds` (`:121`, with the OtOpcUa 2 s budget interaction), `EventQueueCapacity` (`:122`), alarm poll/fetch ceilings incl. truncation semantics (`:435-436`), and the `Fallback` block (`:448-451`); `docs/GatewayTesting.md` is the standout — the windev sections (`:488-622`) record the 879 baseline, the two "environmental" failures that were test bugs, the load-sensitivity list, and a first-class post-mortem of the zero-buffer-pipe testhost hang with the structural fixes (`TestNamedPipe.CreateServer`, bounded `PipePair.WriteAsync`). `docs/Authentication.md` was made runnable-as-written (`2b2748b`); `docs/AlarmProbeFindings.md` states the probe questions and their evidence honestly (ack leg unavailable-by-configuration, GUID leg still assumed). `python3 code-reviews/regen-readme.py --check` passes at HEAD ("up to date and consistent").
+
+**New-feature test placement — correct layers.** Alarm truncation / `snapshot_status`: proto-level flag plumbed with direct tests (`b9fb0dd` atomic snapshot+truncation read), gateway feed frame (`AlarmTruncationSignalTests.cs`), dashboard banner (`AlarmsPageTruncationBannerTests.cs` incl. the mid-truncation attach case, `f57a6ae`), and all five clients regenerated with render coverage (`4d4f443`). SEC-25 tag ACL: `DashboardSessionAclTests`, both subscribe seams covered (`EventsHubTests`, `SessionDetailsPageEventAclTests`), decision-table corners pinned (`7ec0b35`), plus live-LDAP ACL coverage (`1d8a4a6`). Drain cluster (WRK-21/IPC-23/IPC-30): worker-side byte-budget tests run per-push via windows-x86; the gateway side keeps the drain-to-empty walk inside the pipe-harness envelope (`a256560`). Write-completion correlation: worker executor tests (`b0e2b8b`, `c867aca`) plus gateway-side config plumb tests (`GatewayOptionsTests`, `WorkerProcessLauncherTests`). In-process wire tests (TST-24) exist for all five clients and run in CI (`docs/GatewayTesting.md:280-321`).
+
+## New findings
+
+### TST-31 — `glauth.md`'s pre-provisioned-users table contradicts the directory, and its wrong credential propagated into the live-test surface — **Medium** · documentation currency / test fixtures (promotes NEXT-10)
+
+- **Files:** `glauth.md:55` (ReadOnly gid `5501` vs actual `5601`), `:65-78` (pre-provisioned users table: `readonly`/`readonly123` does not exist in `scadaproj/infra/glauth/config.toml`; `admin` carries the standard dev password, not `admin123`), `:124` (`Password: admin123` in a worked example); propagation: `docs/GatewayTesting.md:119` (`MXGATEWAY_LIVE_MXACCESS_WRITE_SECURED_PASSWORD` default `admin123`) and `src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs:1432` (`?? "admin123"`, with `:517` attributing the identity to the GLAuth-bridged ArchestrA `admin`/`admin123`).
+- **Description:** Carried over from `archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md` NEXT-10, which was deliberately deferred pending the OPC-UA group-taxonomy sweep and is still fully present at HEAD. The table was already the proven root cause of one defect (the NEXT-06 live-LDAP fixture drift, fixed 2026-08-07 — the *fixtures* were realigned but the table that misled them was not). The remaining propagation has a quiet coverage cost: the nightly live smoke's `WriteSecured` parity test authenticates with the defaulted `admin`/`admin123`, and because `AuthenticateUser` legitimately returns a real MXAccess *failure* reply on bad credentials (parity, not `InvalidRequest`), a wrong default does not fail the test — it silently degrades the WriteSecured leg from "authenticated round-trip" to "failure-reply shape check" every night. A reader also cannot tell which half of `glauth.md` to trust, since its dashboard section is correct.
+- **Recommendation:** Reconcile the table against `scadaproj/infra/glauth/config.toml` as one unit with the OPC-UA group taxonomy (the reason it was deferred), or — if that sweep stays parked — fix the two provably-wrong rows and the gid now and leave the taxonomy note. In the same change, re-derive the `MXGATEWAY_LIVE_MXACCESS_WRITE_SECURED_PASSWORD` default and the `WorkerLiveMxAccessSmokeTests` literal from whatever the rig's ArchestrA-bridged identity actually is, and make the live test log (not assert) whether `AuthenticateUser` succeeded, so a credential regression in the nightly is at least visible.
+
+### TST-32 — The shared Gitea runner registration token (world-readable for ~5 months, proven live) still awaits its web-UI reset — **Medium** · CI/ops security
+
+- **Files:** `docs/runbooks/TST-30-second-ci-runner.md:33-42` ("Follow-up 2026-08-07 — token hygiene"); `archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md:24` (the pending-operator-action row).
+- **Description:** Both CI runners share one **instance-scope** registration token that sat world-readable in `/opt/gitea/docker-compose.yml` for roughly five months and is **still valid** — a probe registered runner id 6 with it and then deleted it. Host hygiene is done (token now a `0600` file mount, compose + backups locked down), but hygiene does not retire the credential: anyone who read it can register a rogue runner that receives this repo's jobs, including the `WINDEV_SSH_KEY` secret that grants shell on windev. Gitea 1.26.4 exposes no rotation via CLI or API (both are get-or-create), so the reset must be done in the admin web UI, then `/opt/gitea/runner_token` refreshed and the two token-bearing compose backups shredded. This has been a documented "pending operator action" since 2026-08-07 with no tracked finding forcing it to a close — precisely the state in which such actions rot.
+- **Recommendation:** Execute the reset per the runbook (minutes of work; re-registration of the two runners with the new token is the only follow-on). Track it here rather than only in a closed finding's runbook, and after the reset append the dated "Executed" note the runbook's own convention expects.
+
+### TST-33 — `stillpending.md` has drifted into self-contradiction and now overstates what is pending — **Low** · repo hygiene / documentation currency
+
+- **Files:** `stillpending.md:54` (reconnect "not safe to rely on end-to-end yet" — Tasks 13–15 shipped and were verified as cycle-2 TST-01/TST-02 with all-five-client consumers); `:56` (Phase 5 orphan reattach "**planned** (epic Phase 5, not started)" — directly contradicts the governance record TST-29 migrated to `docs/DesignDecisions.md:264-295`, which says **deferred, not planned**, and CLAUDE.md's invariant); `:111` ("WorkerLiveMxAccessSmokeTests.cs (6 `[LiveMxAccessFact]`)" — there are 8); `:118` ("Cross-language client↔gateway wire behavior — no per-client integration unit tests… All client wire behavior is unverified in default unit runs" — false since TST-24 closed 2026-08-10: all five clients have real-transport wire tests running in CI); `:150` (§8: per-session-ACL "still scheduled… not yet started") vs `:65` (§2: the same item "RESOLVED (2026-08-17)") — the file contradicts itself.
+- **Description:** The file is not a frozen snapshot — it has been annotated through 2026-08-17 — so readers reasonably treat it as live, and it is one of the two remaining root-level triage artifacts from TST-14. Its most dangerous staleness is `:56`: it re-asserts the "planned Phase 5 / `EnableOrphanReattach`" framing that TST-04/TST-29 spent two cycles extinguishing, in the exact file a future contributor greps when asking "what's still pending".
+- **Recommendation:** One currency pass: fix the five spots above (§2 reconnect → shipped; §2 Phase 5 → deferred-not-planned, pointing at DesignDecisions; §5 counts and the wire-test bullet; §8 ACL row → resolved, matching §2). Then either commit to keeping it current or stamp it "historical snapshot — see archreview trackers for live status" at the top, which is the cheaper honest option.
+
+### TST-34 — The code-reviews README consistency check is "the intended hook for CI" but is wired into no CI job — **Low** · CI
+
+- **Files:** `REVIEW-PROCESS.md:124-131` ("`regen-readme.py --check` exits non-zero if README.md is stale… The PowerShell wrapper `scripts/check-code-reviews-readme.ps1` runs that check and is the intended hook for CI or a pre-commit step"); `.gitea/workflows/ci.yml` (no reference to either script in any job); also `REVIEW-PROCESS.md:130-131` (the "bare `python3` alias resolves to the Windows Store stub — use `python`" note is Windows-only advice presented unconditionally; on the macOS tree `python` may be absent and `python3` is correct).
+- **Description:** The check passes at HEAD (verified this review), and the repo's process doc names CI as its intended home, but 13 months of CI evolution never picked it up — so a stale `code-reviews/README.md` or an inconsistent open-findings count merges silently until the next human runs the script. The portable job already installs Python 3.12, so the cost is one step.
+- **Recommendation:** Add a `Code-reviews README consistency` step to the `portable` job (`python code-reviews/regen-readme.py --check`), and qualify the interpreter note in REVIEW-PROCESS.md as Windows-specific (macOS/Linux: `python3`).
+
+### TST-35 — The Windows-host run of the gateway (net10) suite is still manual-only, and its failure class recurred three times in August — **Low** · CI/coverage
+
+- **Files:** `docs/GatewayTesting.md:488-498` ("The gateway suite… is not part of the CI Windows tier — `windows-x86` and `nightly-windev` run only the x86 Worker build and `Worker.Tests`. It is still run on windev by hand…"); evidence the class is real: `e2352d1` (two Windows-only gateway test bugs long dismissed as environmental), `bfcf829`/`02baf0c` (zero-buffer Windows pipe wedged the whole suite's testhost), `25f07f8` (SQLite pool sharing-violation on Windows-only cleanup).
+- **Description:** TST-25 restored the Worker tier, but the *gateway* suite's Windows-specific divergences (platform crypto formatting, Windows pipe buffer semantics, file-sharing rules) surface only when someone remembers the manual windev run. Every one of the three August finds was a real defect in test code that CI could have caught nightly; the suite's idle wall time on windev is ~31 s, and the nightly job already builds the full slnx there — the marginal cost of running the suite is small. Severity Low, not Medium: the same 879 tests run on Linux per push, the manual procedure is well documented with a current baseline, and known load-sensitivity (the suite flakes when run concurrently with the x86 tier, `GatewayTesting.md:530-555`) means naive stacking into the nightly needs care.
+- **Recommendation:** Extend `windev-worker-ci.ps1`'s `live` mode with a `dotnet test src/ZB.MOM.WW.MxGateway.Tests` step **after** the Worker tier finishes (the lock already serializes the box, which addresses the documented load sensitivity), and record the 879 expectation in the script log. If nightly wall time becomes a concern, a weekly cadence still beats "when someone remembers".
+
+### TST-36 — Docs and CLAUDE.md refer to the "`event` scope"; the canonical scope string is `events:read` — **Low** · documentation
+
+- **Files:** `CLAUDE.md:160` (twice: "Possessing the `event` scope…", "an `event`-scoped key"); `docs/Sessions.md:248` (twice); `docs/DesignDecisions.md:93`; `docs/Grpc.md:103` (mitigated — it parenthesizes `GatewayScopes.EventsRead`); canonical constants in `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayScopes.cs:10` (`"events:read"`).
+- **Description:** `f2a422b` ("canonical scope strings in the two remaining stale lists") fixed the scope *lists* but left the `event`-scope shorthand in running prose. There is no `event` scope: an operator who takes the prose literally and runs `apikey create-key --scopes event` mints a key that cannot stream. CLAUDE.md even uses the correct `events:read` forty lines earlier (`:158`), so the same file teaches both spellings.
+- **Recommendation:** Mechanical sweep: replace "`event` scope" with "`events:read` scope" in the four prose sites (Grpc.md's parenthesized form is acceptable as-is). One grep verifies completion: `git grep -n '\`event\` scope'` over `CLAUDE.md docs/` returns nothing.
+
+## Severity roll-up (new findings)
+
+| Severity | Count | IDs |
+|---|---|---|
+| High | 0 | — |
+| Medium | 2 | TST-31, TST-32 |
+| Low | 4 | TST-33, TST-34, TST-35, TST-36 |
+
+## Still good / preserved
+
+- **The windev CI tier is the best piece of infrastructure this domain has reviewed in three cycles.** It shipped exactly per design and then got hardened by its own acceptance checks: the cleartext-key leak was found and fixed by storing the key base64 (with the masking rationale documented in three places), the bootstrap lock race was closed with an explicit `MXGW_CI_LOCK_HELD` handoff, the SHA is shape-validated before it reaches a remote shell, an unreachable windev fails red instead of wedging, and the nightly's failure path was itself tested to failure (Check 6) and fixed. `scripts/ci/README.md` records the bring-up, the acceptance checks with dated outcomes, and the degraded mode.
+- **The codegen guard now covers every generated surface in both directions:** descriptor set, `Contracts/Generated`, Rust vendored protos (including the reverse sweep for never-vendored canonical protos — drift no in-repo build can surface), Go/Python bindings with pinned generators that fail rather than skip when missing, and the Java aggregates via the java job's git-diff gate. Check 4 was additionally made Windows-runnable, so windev regen workflows use the same guard.
+- **The prior cycle's High (TST-25) is not just closed — its unlocks were followed through:** old TST-24 closed with real-transport wire tests in all five clients running in CI, old TST-05's scheduling half closed by the nightly, the EventBurst flake root-caused and deflaked (flush-edge sampling), the two "windev-environmental" failures exposed as test bugs, and the months-old full-suite testhost hang root-caused to a zero-buffer Windows pipe with structural fixes (`TestNamedPipe.CreateServer`, bounded `PipePair.WriteAsync`) plus a first-class post-mortem in `docs/GatewayTesting.md` — the windev baseline now equals macOS at 879.
+- **Documentation discipline visibly recovered from its one big self-inflicted wound.** The fca978d sweep's collateral damage was found by a systematic re-read (816 lines judged, 17 restorations), the doc-only `Server-0xx` resolutions were re-audited with the two regressions re-fixed (`d3ac527`), and every doc-currency spot-check this review ran (Sessions, GatewayConfiguration, GatewayTesting, Authentication, alarm probes, pipe naming, write-completion) matched code at HEAD. The two currency findings this cycle (TST-31, TST-33) are inherited debris, not new drift from the ~288-commit period.
+- **New features landed with tests at the right layers**: alarm truncation is asserted from the worker flag through the proto, the gateway feed frame, the dashboard banner (including mid-truncation attach), and all five client renders; the SEC-25 tag ACL is pinned at both subscribe seams with decision-table corners and live-LDAP coverage; `regen-readme.py --check` passes; `code-reviews/README.md` is consistent with the per-module findings at HEAD.