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

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 2026-07-08, cycle 2 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	Gateway server: sessions, worker lifecycle, IPC client, gRPC streaming, alarm monitor, GalaxyRepository package adoption
20-worker.md	Worker process: STA pump, COM lifetime, pipe session, event queue, write-completion correlation, subtag failover
30-contracts-ipc.md	Protos, frame protocol (both sides), codegen/descriptor pipeline, launch-environment contract
40-security-dashboard.md	API-key auth + failure limiter, LDAP dashboard, tag ACL, secrets store, SignalR hubs, redaction
50-clients.md	.NET, Go, Java, Python, Rust clients; publish pipeline; cross-client parity
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)
• 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)
• 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)
• 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)

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)
• 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)
• 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)
• 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)
• 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, 20)
• 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)
• 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)
• 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)
• 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)
• 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)
• 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)

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 WorkerFaults 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.