dohertj2/mxaccessgw
1
0
Fork 0
Code Issues Pull Requests 2 Actions 30 Packages Projects Releases Wiki Activity

fix(archreview): P0 tier remediation (8 findings) #120

Merged
dohertj2 merged 5 commits from fix/archreview-p0 into main 2026-07-09 09:58:57 -04:00
Conversation 0 Commits 5 Files Changed 42
+1106 -91
dohertj2 commented 2026-07-09 05:52:49 -04:00
Owner
Copy Link
Copy Source

Executes the P0 tier (8/8 findings) of the 2026-07-08 architecture review (archreview/remediation/00-tracking.md). Work was fanned out across parallel agents on disjoint file sets, then verified.

Findings

ID Sev Change
GWC-01 Critical Alarm monitor attaches as an internal (non-counted) distributor subscriber instead of a second raw drain of the single worker event channel; WorkerClient._events → SingleReader + claimed-once guard so any future dual-consumer regression throws.
GWC-02 High Faulted sessions swept in CloseExpiredLeasesAsync (IsFaultedReapable + FaultedReason); new FaultedGraceSeconds (default 0).
GWC-03 High Configurable MaxSparseArrayLength (default 1,000,000) enforced before allocation.
WRK-01 High STA PumpPendingMessages() refreshes activity, removing the false StaHung on long ReadBulk.
CLI-01 High Go Events() emits terminal ErrSlowConsumer on overflow instead of closing silently.
CLI-03 High Rust typed invoke validates hresult < 0 / MXSTATUS_PROXY via new Error::MxAccess.
TST-02 High (security) StreamEvents attach enforces the opening key id → PermissionDenied on owner mismatch.
TST-12 Medium CLAUDE.md retention-defaults sentence corrected.

Verification

  • Gateway (macOS, NonWindows build clean): targeted suites 135/135 (SparseArrayExpander/validator 61, SessionManager/validator 106, EventStreamServiceTests 16 + auth 53, GatewaySessionDashboardMirror 20).
  • Worker (Windows host, x86): builds clean, StaRuntimeTests + WorkerPipeSessionTests 33/33.
  • WorkerClientTests (Windows host, net10 — can't run on macOS due to the Unix-socket path limit): 18/18, incl. GWC-01's ReadEventsAsync_SecondEnumerator_Throws.
  • Go: gofmt clean, go build/go test ./... (+ -race) pass.
  • Rust: cargo fmt/check/test (28) and clippy --all-targets -D warnings clean.

Windows verification also caught (and this PR fixes) an xUnit1030 build error — the new worker test used .ConfigureAwait(false) in a [Fact] body, which the macOS tree structurally cannot surface.

Notes

  • Commits are grouped by component; the four gateway findings share GatewaySession.cs/SessionManager.cs and are committed together with all IDs in the body.
  • MXAccess parity preserved throughout — delivery/safety fixes only, no synthesized events.
Executes the **P0 tier (8/8 findings)** of the 2026-07-08 architecture review (`archreview/remediation/00-tracking.md`). Work was fanned out across parallel agents on disjoint file sets, then verified. ## Findings | ID | Sev | Change | |----|-----|--------| | GWC-01 | Critical | Alarm monitor attaches as an internal (non-counted) distributor subscriber instead of a second raw drain of the single worker event channel; `WorkerClient._events` → `SingleReader` + claimed-once guard so any future dual-consumer regression throws. | | GWC-02 | High | Faulted sessions swept in `CloseExpiredLeasesAsync` (`IsFaultedReapable` + `FaultedReason`); new `FaultedGraceSeconds` (default 0). | | GWC-03 | High | Configurable `MaxSparseArrayLength` (default 1,000,000) enforced before allocation. | | WRK-01 | High | STA `PumpPendingMessages()` refreshes activity, removing the false `StaHung` on long `ReadBulk`. | | CLI-01 | High | Go `Events()` emits terminal `ErrSlowConsumer` on overflow instead of closing silently. | | CLI-03 | High | Rust typed `invoke` validates `hresult < 0` / `MXSTATUS_PROXY` via new `Error::MxAccess`. | | TST-02 | High (security) | `StreamEvents` attach enforces the opening key id → `PermissionDenied` on owner mismatch. | | TST-12 | Medium | `CLAUDE.md` retention-defaults sentence corrected. | ## Verification - **Gateway** (macOS, NonWindows build clean): targeted suites 135/135 (`SparseArrayExpander`/validator 61, `SessionManager`/validator 106, `EventStreamServiceTests` 16 + auth 53, `GatewaySessionDashboardMirror` 20). - **Worker** (Windows host, x86): builds clean, `StaRuntimeTests` + `WorkerPipeSessionTests` 33/33. - **`WorkerClientTests`** (Windows host, net10 — can't run on macOS due to the Unix-socket path limit): 18/18, incl. GWC-01's `ReadEventsAsync_SecondEnumerator_Throws`. - **Go**: `gofmt` clean, `go build`/`go test ./...` (+ `-race`) pass. - **Rust**: `cargo fmt`/`check`/`test` (28) and `clippy --all-targets -D warnings` clean. Windows verification also caught (and this PR fixes) an `xUnit1030` build error — the new worker test used `.ConfigureAwait(false)` in a `[Fact]` body, which the macOS tree structurally cannot surface. ## Notes - Commits are grouped by component; the four gateway findings share `GatewaySession.cs`/`SessionManager.cs` and are committed together with all IDs in the body. - MXAccess parity preserved throughout — delivery/safety fixes only, no synthesized events.
dohertj2 added 5 commits 2026-07-09 05:52:50 -04:00
fix(CLI-01): Go Events() emits terminal ErrSlowConsumer on overflow 1eb00276bc 
Events()/EventsAfter() previously closed the channel silently when the
16-slot buffer overflowed, indistinguishable from a graceful server end.
Reserve one slot and emit a terminal EventResult wrapping the new exported
ErrSlowConsumer so overflow is always observable. The blocking
SubscribeEvents path (gRPC flow-controlled) is unchanged.

archreview: CLI-01 (P0). Verified: gofmt clean, go build ./..., go test ./... (+ -race) pass.
fix(CLI-03): Rust typed invoke validates HRESULT/MXSTATUS_PROXY 5faef6c012 
Typed write/command wrappers now call ensure_mxaccess_success after
ensure_command_success, failing on hresult < 0 (COM-correct, matching
CLI-08/Python) or any MxStatusProxy.success == 0, via a new boxed
Error::MxAccess variant with credential-safe formatting. The raw invoke
escape hatch stays unvalidated.

archreview: CLI-03 (P0). Verified: cargo fmt/check/test (28) and clippy --all-targets -D warnings clean.
fix(WRK-01): STA pump step refreshes activity to prevent false StaHung 31eec41456 
A long legitimate ReadBulk pumped Windows messages without refreshing
LastStaActivityUtc, so the watchdog false-positived StaHung past
HeartbeatStuckCeiling and then silently dropped every reply. PumpPendingMessages()
now calls MarkActivity() after pumping; a genuinely stuck STA (no pumping)
still accrues staleness and faults correctly. No MXAccess parity change.

archreview: WRK-01 (P0). Verified on the Windows host (x86): worker builds
clean, StaRuntimeTests + WorkerPipeSessionTests 33/33 pass.
fix(archreview): gateway core P0 remediation (GWC-01/02/03, TST-02, TST-12) 20392cf246 
Interlocking changes across the gateway server (shared GatewaySession.cs /
SessionManager.cs), committed together:

- GWC-01 (Critical): alarm monitor now attaches as an internal
  (non-counted) distributor subscriber instead of a second raw drain of the
  single worker event channel; WorkerClient._events -> SingleReader with a
  claimed-once guard so a future dual-consumer regression throws loudly.
- GWC-02 (High): faulted sessions are swept in CloseExpiredLeasesAsync
  (IsFaultedReapable + FaultedReason); new FaultedGraceSeconds (default 0).
- GWC-03 (High): configurable MaxSparseArrayLength (default 1_000_000)
  enforced before allocation.
- TST-02 (High, security): StreamEvents attach now enforces the opening key
  id -> PermissionDenied on owner mismatch.
- TST-12 (Medium): CLAUDE.md retention-defaults sentence corrected.

Verified: NonWindows build clean; targeted tests 135/135 on macOS, plus
WorkerClientTests 18/18 on the Windows host.
docs(archreview): mark P0 tier Done in remediation tracker f726458086 
All 8 P0 findings (GWC-01/02/03, WRK-01, CLI-01, CLI-03, TST-02, TST-12)
recorded as Done with verification notes and change-log entries.
dohertj2 merged commit b75bfbe8f4 into main 2026-07-09 09:58:57 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
area:auth
Authentication and authorization
 area:client-dotnet
.NET client library
 area:client-go
Go client library
 area:client-java
Java client library
 area:client-python
Python client library
 area:client-rust
Rust client library
 area:contracts
Protocol and generated contract work
 area:dashboard
Blazor dashboard work
 area:docs
Documentation
 area:gateway
Gateway server work
 area:tests
Automated and live tests
 area:worker
MXAccess worker process work
 blocked
Blocked by dependency
 priority:p0
Must-have / blocking
 priority:p1
High priority
 priority:p2
Normal priority
 type:docs
Documentation work
 type:feature
Feature implementation
 type:infra
Infrastructure and scaffolding
 type:test
Test work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/mxaccessgw#120
Delete Branch "fix/archreview-p0"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?