From bfbfcdd1125cdf9c210a03b4422b1fdb82b112da Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 09:21:48 -0400 Subject: [PATCH] docs(archreview-p1): mark IPC-02/03/04 + WRK-04/07 Done (Wave 3, windev-verified) --- archreview/remediation/00-tracking.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/archreview/remediation/00-tracking.md b/archreview/remediation/00-tracking.md index 5c0d61a..3ad4355 100644 --- a/archreview/remediation/00-tracking.md +++ b/archreview/remediation/00-tracking.md @@ -90,10 +90,10 @@ Full design + implementation for each row lives in the linked domain doc under i | WRK-01 | High | P0 | M | — | Done | Long `ReadBulk` self-faults as `StaHung`; all replies then dropped | | WRK-02 | Medium | — | M | — | Not started | STA thread death after startup is silent; future work hangs forever | | WRK-03 | Medium | — | S | WRK-02 | Not started | Commands after shutdown starts are dropped with no reply | -| WRK-04 | Medium | — | M | — | Not started | Envelope `sequence` can appear out of order on the wire | +| WRK-04 | Medium | — | M | — | Done | Envelope `sequence` can appear out of order on the wire | | WRK-05 | Medium | — | M | — | Not started | One transient alarm-poll failure kills the whole session | | WRK-06 | Medium | P2 | S | — | Not started | `MXSTATUS_PROXY` conversion reflects per field, per event | -| WRK-07 | Medium | P1 | M | WRK-04 | Not started | Documented outbound write priority not implemented | +| WRK-07 | Medium | P1 | M | WRK-04 | Done | Documented outbound write priority not implemented | | WRK-08 | Low | — | S | — | Not started | Residual event queue discarded at graceful shutdown, undocumented | | WRK-09 | Low | — | S | — | Not started | No `AppDomain.UnhandledException` hook | | WRK-10 | Low | — | S | — | Not started | Top-level catch logs exception type but never message | @@ -113,9 +113,9 @@ Full design + implementation for each row lives in the linked domain doc under i | ID | Sev | Tier | Eff | Dep | Status | Title | |---|---|:-:|:-:|---|---|---| | IPC-01 | High | P1 | M | — | Done | Published client descriptor set 7 weeks stale; nothing enforces freshness | -| IPC-02 | Medium | P1 | M | IPC-03 | Not started | Worker max frame size hard-coded, cannot follow gateway config | -| IPC-03 | Medium | P1 | M | IPC-02 | Not started | gRPC max == pipe max with zero headroom; oversized write faults whole session | -| IPC-04 | Medium | P1 | S | IPC-03 | Not started | `DrainEvents max_events=0` packs entire queue into one reply frame | +| IPC-02 | Medium | P1 | M | IPC-03 | Done | Worker max frame size hard-coded, cannot follow gateway config | +| IPC-03 | Medium | P1 | M | IPC-02 | Done | gRPC max == pipe max with zero headroom; oversized write faults whole session | +| IPC-04 | Medium | P1 | S | IPC-03 | Done | `DrainEvents max_events=0` packs entire queue into one reply frame | | IPC-05 | Medium | P2 | S | — | Not started | Redundant deep copies on command and event hot paths | | IPC-06 | Medium | P2 | S | — | Not started | `gateway.md` Worker Envelope sketch no longer matches the contract | | IPC-07 | Medium | P2 | S | — | Not started | `docs/Grpc.md` says six RPCs; there are seven | @@ -257,6 +257,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog | 2026-07-09 | P0 tier executed via parallel agents. GWC-01/02/03, TST-02, TST-12, CLI-01, CLI-03 → `Done` (NonWindows build clean; gateway 135/135 targeted tests pass, Go `go test ./...` + `-race` pass, Rust `test`/`clippy -D warnings` pass). | | 2026-07-09 | Windows full-suite temp-file-lock flakiness (discovered via TST-08) → **fixed** (commit `11a716a`). Two shared-state parallel collisions that macOS never surfaces: (1) self-signed cert generation — `SelfSignedCertificateProvider` now stages the PFX in a unique `..tmp` (was a fixed name that concurrent/interrupted writers collided on); `TestHostEnvironmentInitializer` defaults the cert path to a per-process temp dir (so the suite never writes shared `ProgramData` or fights the deployed service); `GatewayTlsBootstrapTests` moved to a `DisableParallelization` collection so its process-global env-var mutation can't bleed into parallel tests. (2) `AuthStoreHealthCheckTests` — reuse the existing `TempDatabaseDirectory` helper (`SqliteConnection.ClearAllPools()` before delete) so the Microsoft.Data.Sqlite pool releases the temp `.db` handle. **windev-verified:** 3 consecutive full-suite runs all **793 passed / 2 failed, 0 new orphans** (was flaky 2–4 fails, count-varying). The 2 stable remaining failures are pre-existing windev-environmental (`SelfSignedCertificateProviderTests.GenerateCertificate_HasExpectedSansEkuAndValidity` machine-name/FQDN SAN assertion; `EventStreamServiceTests…TracksAggregateQueueDepth` timing) — both pass on macOS, unrelated to this change. | | 2026-07-09 | P1 TST-08 (orphaned testhost) → `Done` by evidence: the claimed leak does **not** reproduce. Full `ZB.MOM.WW.MxGateway.Tests` suite exits cleanly on macOS (0 survivors) and on the Windows dev box (windev, `origin/main` worktree: 0 new `testhost` and 0 new `ZB.MOM.WW.MxGateway.Worker` processes after a full run, isolated by StartTime). Static audit confirms the prime-suspect fixtures already dispose deterministically (WorkerClient cancels its `_stopCts` + awaits read/write/heartbeat tasks with a 5 s timeout; GatewaySession disposes the client; E2E/harness fixtures use `await using`). CLAUDE.md "Source Update Workflow" updated to drop the stale "leaves orphaned testhost processes" rationale (filtered runs kept as a speed guideline). **Discovered separately (NOT TST-08, still open):** Windows-only temp-file-lock flakiness in ~4 full-host test classes — `AuthStoreHealthCheckTests` (Microsoft.Data.Sqlite connection-pool holds the temp `.db` handle after `await using`, so the finally `File.Delete` throws "used by another process"), `GatewayTlsBootstrapTests`/`DashboardCookieOptionsTests`/`DashboardHubsRegistrationTests` (a `gw.pfx.tmp` handle from `SelfSignedCertificateProvider`'s fixed-name `path+".tmp"` atomic-write racing teardown). Count varies run-to-run (2–4 fails); macOS never sees it (Unix allows deleting open files). Fix = `SqliteConnection.ClearAllPools()` before temp delete + a unique/guarded `.tmp` name; verify on windev. | +| 2026-07-09 | P1 Wave 3 (size/backpressure topology + write ordering). IPC-02/03/04 + WRK-04/07 → `Done`. **Size negotiation (IPC-02):** added `GatewayHello.max_frame_bytes` (regen `Generated/` + `clients/proto` descriptor refresh with pinned protoc 34.1); the gateway sends its negotiated worker-frame max and the worker adopts it (`WorkerFrameProtocolOptions.AdoptNegotiatedMaxMessageBytes`, 0 = keep default, >256 MiB rejected) instead of a hard-coded default. **Headroom (IPC-03):** the pipe frame max now sits `EnvelopeOverheadReserveBytes` (64 KiB) above the public gRPC cap (default `Worker.MaxMessageBytes` 16 MiB→16 MiB+64 KiB), cross-validated at startup; `WorkerClient` pre-checks command envelope size and fails only the offending correlation (`ResourceExhausted`) instead of `SetFaulted`ing the session. **Drain bound (IPC-04):** gateway request validator rejects `DrainEvents max_events` above 10 000; the worker caps each reply at `MaxDrainEventsPerReply` (10 000) and treats `max_events = 0` as that cap, not "drain all". **Sequence (WRK-04):** `WorkerFrameWriter` stamps the envelope `Sequence` at the point of writing under the write lock, so wire order and stamped sequence always agree under concurrent producers. **Priority (WRK-07):** the worker writer is now a cooperative priority scheduler — control frames (reply/fault/heartbeat/shutdown-ack) drain ahead of event frames; per-frame validation/size rejections fail only that frame, a stream failure fails all queued. Docs same-change (GatewayConfiguration, WorkerFrameProtocol, gateway.md). **Verified:** macOS NonWindows build clean + validator/grpc tests green; **windev** x86 worker builds clean, `Worker.Tests` 352 passed / 0 failed / 11 skipped (incl. new monotonic-sequence, control-before-event priority, negotiated-max, drain-bound tests), gateway `Tests` 799 passed / 3 failed — all 3 pre-existing windev-environmental (SelfSigned SAN + 2 `EventStreamServiceTests` timing, both pass in isolation). Commits `c8b3a22` (gateway half), `ebe6aea` (worker half), `309296f` (descriptor + default-expectation refresh). GWC-04 (event-channel decoupling) is the remaining Wave 3 item. | | 2026-07-09 | P1 S-misc (dashboard/observability hardening). SEC-02/12/20 → `Done`. SEC-02: `DashboardAuthorizationHandler` restricts the loopback + `Authentication:Mode=Disabled` bypasses to read-only (they satisfy a Viewer-bearing requirement but never `AdminOnly`), closing the policy-layer gap where anonymous localhost was authorized for Admin surfaces. SEC-12: `DashboardSessionAdminService` now emits canonical `AuditEvent`s (`dashboard-close-session`/`dashboard-kill-worker`, category `SessionAdmin`) through `IAuditWriter` on Success/Failure/Denied, so Close/Kill land durable audit rows. SEC-20: dropped the unbounded `session_id` tag from the exported `mxgateway.heartbeats.failed` counter. Docs updated same-change (CLAUDE.md, GatewayDashboardDesign.md, Metrics.md). Server build clean (0 warnings); targeted classes 30/30 pass; broader Dashboard+Security+GatewayApplication+Metrics sweep 295/295 pass. | | 2026-07-09 | P1 Wave 2b (security authz+hub). SEC-05/07/08/11 → `Done` (hub-token lifetime; QueryActiveAlarms scope arm; gateway-side verification cache + last-used coalescing; login rate limit + per-peer gRPC failure limiter). Full-suite checkpoint caught + fixed regressions the earlier narrow SEC-01/04/06 filter missed: cross-platform path-rooting, an `IHostEnvironment` fallback for minimal DI containers, a test-assembly `ASPNETCORE_ENVIRONMENT=Development` default, and a platform-correct default-path assertion. Suite: 747 passed / 42 failed, all 42 pre-existing macOS named-pipe-harness env failures. | | 2026-07-09 | P1 Wave 2a (security). SEC-01/04/06 → `Done` (config path-rooting + production validator guards; Server build clean, validator+hygiene tests 53/53). SEC-10 → `Done`: the shared `ZB.MOM.WW.Auth.ApiKeys` gained optional `ExpiresUtc` (expired keys rejected, auth DB auto-migrates to schema v3) via a concurrent HistorianGateway-remediation session's "G-2"; this repo consumes it by bumping the four `Auth.*` refs 0.1.2→0.1.4 (commit 197731a). Remaining SEC-10 polish (`apikey create --expires` + dashboard staleness badge) tracked as a small follow-up. |