# Testing, Documentation & Underdeveloped Areas — Re-Review

- **Date:** 2026-07-12
- **Commit:** `4f5371f` (`main`, "Merge branch 'fix/archreview-p2' into main (P2 tier: completeness & polish)")
- **Baseline:** pre-remediation `59856b8`; prior report `archreview/60-testing-docs-gaps.md` (2026-07-08)
- **Method:** macOS static review — reading only, no builds, no test runs. All paths relative to `/Users/dohertj2/Desktop/MxAccessGateway`.

## Prior-finding verification

| ID | Claimed status | Verified? | Evidence (file:line) | Notes |
|----|----------------|-----------|----------------------|-------|
| TST-01 | Done | **Yes** | `src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayEndToEndReconnectReplayTests.cs:52` (no-gap resume), `:151` (stale-cursor `ReplayGap`) | Real e2e: drives the actual gRPC `StreamEvents` path via `FakeWorkerHarness`, gated deterministic event emission, full detach (cancel + await so the lease disposes), reconnect with `AfterWorkerSequence`, exact-sequence/order/no-dup assertions plus sentinel shape (`RequestedAfterSequence`/`OldestAvailableSequence`, empty body, family Unspecified) at `:214-231`. Client consumers exist in all five clients (e.g. `clients/dotnet/.../MxEventStreamItem.cs`, `clients/python/tests/test_replay_gap.py`, Java `MxEventStreamItem.java`). Not tautological — it exercises the replay ring across a detach, not a mock of it. |
| TST-02 | Done | **Yes** | `src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs:57-62` (ordinal `OwnerKeyId` vs caller-key check → `PermissionDenied` before attach); owner recorded at `Sessions/GatewaySession.cs:197,242` | Tested both ways: `Tests/Gateway/Grpc/EventStreamServiceTests.cs:50-57` (owner match streams) and `:73-89` (mismatch → `PermissionDenied`, asserted **before** any event yields). Documented: `docs/Sessions.md:220`, CLAUDE.md:124. |
| TST-03 | Done | **Yes, with a large residual gap** | `.gitea/workflows/ci.yml:17-121` (`portable` + `java` jobs); codegen guard wired at `:60-62` → `scripts/check-codegen.ps1` | CI exists and per the tracking log ran green on main. But the `windows` and `live-mxaccess` jobs were **removed** (`ci.yml:123-131`, commit `abb0930`) because act_runner host-mode on Windows is broken — see new finding TST-25. Codegen freshness is genuinely wired (descriptor set, `Contracts/Generated` diff, vendored Rust protos — `check-codegen.ps1:4-16`). |
| TST-04 | Done | **Yes** | `oldtasks.md:27-34` (Phase 4 scoped as TST-15, Phase 5 "DEFERRED, not planned"), `:65-93` (per-phase status incl. "`EnableOrphanReattach` (Task 26) does not exist — do not reference"); CLAUDE.md:79 reconnect paragraph states clients consume `ReplayGap` and reattach is deferred | Governance decision recorded in the tracking source and its mirror; the CLAUDE.md invariant ("gateway restart does not reattach") stands. |
| TST-08 | Done | **Yes** | CLAUDE.md:91 — filtered-run guidance reframed as speed-only: "the suite exits cleanly (verified on macOS and the Windows dev box — 0 surviving `testhost`/worker processes)" | De-staled as claimed; tracking log records the windev 3×-run evidence and the separately-fixed Windows temp-file-lock flakes (commit `11a716a`). |
| TST-11 | Done | **Yes, with caveat** | `src/Directory.Build.props:19` (`<Version>0.1.2</Version>`), `:24-33` (git short SHA → `SourceRevisionId`/InformationalVersion, guarded for git-less builds); Contracts `0.1.2` (`Contracts.csproj:10`) | .NET side single-sourced and SHA-stamped; Python/Rust/Go/.NET client constants aligned to 0.1.2 (CLI-18/21/26/29 per tracking). Residual: Java remains `0.2.0` (`clients/java/build.gradle:16`) — convergence explicitly deferred as a release decision, so the drift is now *documented*, not accidental. |
| TST-12 | Done | **Yes** | CLAUDE.md:79 — "detach-grace and replay retention are **on** by default (`DetachGraceSeconds=30`, `ReplayBufferCapacity=1024`, `ReplayRetentionSeconds=300`)" | Matches code: `Configuration/SessionOptions.cs:46`, `Configuration/EventOptions.cs:21,29`. |
| TST-13 | Done | **Yes** | `gateway.md:305-312` — envelope sketch replaced with proto-as-source-of-truth pointer ("do not hand-copy… an inlined copy drifts"), correct `string correlation_id`; `gateway.md:776` — "one active client event subscriber per session **by default**" | All three stale spots corrected. |
| TST-23 | Done | **Yes** | `gateway.md:365-372` — "### Future work: bidirectional `Session` (not implemented)" | "Best long-term shape" framing gone. |
| TST-05 | Not started | **Confirmed open — now worse** | `WorkerLiveMxAccessSmokeTests.cs` still 8 `[LiveMxAccessFact]`s, opt-in only; `ci.yml` contains no `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS` anywhere | The planned scheduled live job was removed with the Windows jobs (`ci.yml:123-131`), so live coverage regressed from "planned nightly" back to "run by memory". Folded into new finding TST-25. |
| TST-06 | Not started | **Confirmed open** | `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs` exists; no `DashboardLiveDataServiceTests` in `Tests/Gateway/Dashboard/` (directory listing has 18 classes, none for it) | `EventsHub`/`AlarmsHubPublisher` hub methods likewise still untested (new `DashboardEventBroadcasterTests` covers the mirror redaction, not the live-data session lifecycle). |
| TST-07 | Not started | **Confirmed open (lines moved)** | `Tests/Gateway/Workers/WorkerClientTests.cs:545` (`Task.Delay(150ms)` then negative assertion); `Tests/Gateway/Sessions/SessionManagerTests.cs:402` (`Task.Delay(50)` race) | Same latent flakes; now more relevant because they run on shared CI runners on every push. |
| TST-09 | Not started | **Confirmed open** | `Server/GatewayApplication.cs:75-76` — still exactly one `AuthStoreHealthCheck` | No Galaxy SQL / LDAP / worker-executable / alarm-monitor checks. |
| TST-10 | Not started | **Confirmed open** | No `docs/Deployment.md` (`ls docs/` — no deploy doc); no deploy script under `scripts/` | Deploy knowledge still lives only in operator memory notes. |
| TST-14 | Not started | **Confirmed open** | Repo root still holds `MxAccessGateway-docs-{issues,fixed,final}.md`, `MxGatewayClient-docs-{issues,fixed}.md`, `oldtasks.md`, `stillpending.md` (root `ls`) | `oldtasks.md` did gain a legitimate new role as the epic-governance record (TST-04), so "delete when epic closes" needs re-deciding; the five untracked `*-docs-*.md` files remain pure clutter. |
| TST-15 | Not started (design done) | **Confirmed open** | `Server/Dashboard/Hubs/EventsHub.cs:42` — `TODO(per-session-acl)` still present | Design now exists: `docs/plans/2026-07-10-dashboard-session-acl-tst15.md`. Mitigation shipped meanwhile: SEC-25 value redaction in the hub mirror (see TST-27). |
| TST-16 | Not started | **Partially resolved — see TST-27** | `Server/Dashboard/Hubs/DashboardEventBroadcaster.cs:16,29` — `ShowTagValues` now gates value redaction of the SignalR mirror (SEC-25) | The flag is no longer fully dead, but it still does not gate `/browse` live-value display, and the config doc still calls it "Reserved" — fresh drift (TST-27). |
| TST-17 | Not started | **Confirmed open** | `Worker/MxAccess/WnWrapAlarmConsumer.cs:~264-275` — `_ = ackOperatorDomain; _ = ackOperatorFullName;` then 6-arg call; no diagnostic surfaced in the ack reply | Silent drop unchanged (comment block explains the −55 stub, but callers still can't see the degrade). |
| TST-18 | Not started | **Confirmed open** | No `*HostedService*` test files under `Tests/Gateway/Sessions/` or `Tests/Gateway/Workers/` | Sweep cores remain covered indirectly (`SessionManagerTests.cs:985,1015` now also cover faulted-reap). |
| TST-19 | Not started | **Confirmed open** | `Tests/Gateway/Workers/Fakes/FakeWorkerHarness.cs` — no lockstep note / `WorkerPipeSession` pointer (grep empty); no README in `Fakes/` | Risk is higher now that the real-worker suite has no scheduled run (TST-25). |
| TST-20 | Not started | **Confirmed open** | `scripts/run-client-e2e-tests.ps1:46-48` — `DrainEveryTags = 15` workaround intact | Advise-without-consumer sharp edge still un-addressed as product feedback. |
| TST-21 | Not started | **Confirmed open** | `Server/appsettings.json:10` — daily rolling file sink, relative path, no size cap / retained-count | Unchanged. |
| TST-22 | Not started | **Confirmed open** | `docs/GatewayConfiguration.md` shape block (~:12-95) still lacks `DisableLogin`/`AutoLoginUser`/`CookieName`/`Tls`/`Alarms:Fallback` (grep of the block returns none; tables at `:181-188,:313+` document them) | The block did gain new keys (`FaultedGraceSeconds:41`, `ShowTagValues:60`) so it is maintained — the omissions are just still omitted. |
| TST-24 | Not started (deferred, CI-gated) | **Confirmed open** | Only Java has an in-process gateway harness (`clients/java/.../cli/InProcessGatewayHarness.java`); other four clients still unit-test against mocks (e.g. `clients/python/tests/test_replay_gap.py`) | Deferral on TST-03 no longer blocks it — CI is live. |

**Done claims that failed verification: none.** All nine Done claims hold with file:line evidence. Two carry material caveats: TST-03 (CI exists but the Windows/live half of the matrix was subsequently removed) and TST-11 (Java version convergence deferred).

## New findings

### TST-25 — The entire Windows-only test surface is now unguarded by any automation — **High** · CI/coverage

- **Files:** `.gitea/workflows/ci.yml:123-131` (removal note, commit `abb0930`); `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWritePriority.cs` (new); `src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs:353-395` (priority-ordering tests); `src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs` (8 live facts)
- **Description:** The `windows` and `live-mxaccess` CI jobs were removed because Gitea act_runner v0.6.1 host-mode is broken on Windows and a gated job with no runner wedges every run in "queued". The consequence is that the x86/net48 Worker build, all of `Worker.Tests`, and the live-MXAccess suite have **zero** automated execution — and this is not the pre-remediation status quo, because the remediation itself added new Windows-only mechanisms that are now tested only by hand: the `WorkerFrameWritePriority` control-vs-event write scheduling (WRK-01/WRK-07 machinery — its ordering tests exist and are meaningful, but they compile only on Windows), the worker-side `max_frame_bytes` adoption, the WnWrap/subtag alarm consumers, and STA runtime changes (`StaRuntime.cs` modified since baseline). The prior review's TST-05 concern ("live paths verified by memory") now also applies to a whole tier of *unit* tests. The portable job's `check-codegen.ps1` Generated-diff check does substitute for the net48 CS0246 guard (good), but nothing substitutes for running the worker tests.
- **Recommendation:** Restore Windows coverage by a route that doesn't depend on act_runner: a scheduled CI step (or standalone cron on the Mac/windev) that drives the existing windev worktree process over SSH — `dotnet build -p:Platform=x86` + `dotnet test Worker.Tests` — and reports failures loudly; the remote-PS `-EncodedCommand` recipe already exists in operator memory. Until then, record a mandatory manual cadence in `docs/GatewayTesting.md` (per-merge for worker-touching changes, weekly otherwise) instead of leaving it "when someone remembers".

### TST-26 — docs/GatewayTesting.md and check-codegen.ps1 describe CI jobs that no longer exist — **Medium** · documentation currency

- **Files:** `docs/GatewayTesting.md:426-431`; `scripts/check-codegen.ps1:17`; `.gitea/workflows/ci.yml:12-14`
- **Description:** `docs/GatewayTesting.md` still documents the **`windows`** job ("the only host that builds the x86/net48 Worker… the definitive guard for the 'regenerate and commit Generated/' rule") and the **`live-mxaccess`** scheduled job — both deleted from `ci.yml` in `abb0930`, which touched only the workflow file. This violates the repo's own docs-change-in-same-commit rule and actively misleads: a reader concludes the worker build is CI-guarded when it is not, and the "definitive guard" claim is doubly wrong (the surviving guard for the Generated-commit rule is the portable job's diff check, not a Windows compile). Same stale claim inside `check-codegen.ps1:17` ("guarded by the Windows CI job, not here") and in `ci.yml:12-14`, whose nightly cron comment still says "Nightly live-MXAccess smoke (windev)" though the schedule now only re-runs `portable`+`java`.
- **Recommendation:** One small commit: rewrite the GatewayTesting.md CI section to the two-job reality plus the manual windev process (and link the ci.yml:123-131 rationale), fix the check-codegen.ps1 comment, and either repurpose or delete the cron (a nightly portable re-run has some drift-detection value — if kept, say so).

### TST-27 — `ShowTagValues` config doc still says "Reserved" after SEC-25 made the flag live — **Medium** · documentation currency

- **Files:** `docs/GatewayConfiguration.md:185` ("Reserved display control for tag values"); `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs:16-29` (flag read and enforced); `docs/GatewayDashboardDesign.md:170` (current, documents the redaction)
- **Description:** SEC-25 wired `Dashboard:ShowTagValues` into `DashboardEventBroadcaster`: when `false` (default), tag values are blanked from the deep-cloned event before it is mirrored to SignalR. The flag is therefore no longer dead — but the authoritative config reference still labels it "Reserved", so an operator consulting the options table concludes toggling it does nothing, when it actually controls whether tag values leak to every dashboard hub subscriber. This is remediation-created drift: the dashboard design doc was updated in the same change, the config doc was not. (Residual of TST-16 remains separately: the flag still gates nothing in the `/browse` live-value path.)
- **Recommendation:** Update the `:185` row to describe the mirror-redaction behavior (and its security relevance given the missing per-session ACL, TST-15); note the `/browse` gap or close it when TST-16 is decided.

### TST-28 — Gateway-side `max_frame_bytes` handshake field has no test in the portable suite — **Low** · test coverage

- **Files:** `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs:988` (`MaxFrameBytes = (uint)_connection.FrameOptions.MaxMessageBytes` in `GatewayHello`); no reference to `MaxFrameBytes` anywhere under `src/ZB.MOM.WW.MxGateway.Tests` (grep empty)
- **Description:** The frame-size negotiation added since baseline is asserted only on the worker side (adoption tests in `Worker.Tests`, Windows-only per TST-25). Nothing in the CI-run gateway suite asserts the gateway actually populates the field from its configured `MaxMessageBytes` — a regression to sending `0` (which the worker treats as "older gateway, use default") would pass every automated test while silently reverting the negotiated limit.
- **Recommendation:** One fake-worker assertion: capture the `GatewayHello` in `FakeWorkerHarness` and assert `MaxFrameBytes` equals the configured `Worker.MaxMessageBytes` (both default and overridden).

### TST-29 — `oldtasks.md` retirement decision needs refresh now that it carries the epic-closure record — **Low** · repo hygiene

- **Files:** `oldtasks.md:27-34,65-93`; repo root (five untracked `*-docs-*.md` files still present)
- **Description:** TST-14's plan was "keep `oldtasks.md` only until the epic resumes, then delete", but TST-04 made it the durable governance record for Phase-4-scoped / Phase-5-deferred — deleting it now would orphan the only statement that `EnableOrphanReattach` must not be referenced. Meanwhile the mechanical half of TST-14 (deleting the user's five untracked, gitignored `*-docs-*.md` working files, 15k+ lines) remains undone and keeps tripping greps.
- **Recommendation:** Fold the Phase-5-deferred statement into `docs/DesignDecisions.md` (where invariant-adjacent decisions live), then retire `oldtasks.md`; delete the untracked docs-review artifacts (zero repo impact).

## Severity roll-up (new findings)

| Severity | Count | IDs |
|---|---|---|
| High | 1 | TST-25 |
| Medium | 2 | TST-26, TST-27 |
| Low | 2 | TST-28, TST-29 |

## Still good / preserved

- The reconnect/replay e2e test (TST-01) is the strongest new test in the tree: real gRPC path, deterministic gated emission, detach-survival of the ring, exact-sequence and sentinel-shape assertions — a model for future e2e work.
- The new remediation machinery is genuinely tested, not tautologically: `CachingApiKeyVerifierTests` (9 facts, incl. `CoalescingMarkApiKeyStore` window behavior at `:106,:125` with an injected clock), `ApiKeyFailureLimiter` exercised through `GatewayGrpcAuthorizationInterceptorTests:363-445` (asserts the verifier is **not** called once tripped, and that success resets), faulted-session reaping via `SessionManagerTests.cs:985` (default reap) and `:1015` (grace window), `DashboardEventBroadcasterTests` (redaction), `DashboardLoginRateLimitTests`, plus a new repo-hygiene guard (`Tests/ProjectStructure/GatewayTreeHygieneTests.cs`) that fails if a stray SQLite DB materializes under `src/`.
- Documentation currency of the load-bearing docs is high where the remediation touched them: `docs/GatewayConfiguration.md` fully documents the new `Security:*` block (`:278-284`) and `FaultedGraceSeconds` (`:141`) matching `SecurityOptions.cs`/`SessionOptions.cs`; `docs/Sessions.md:220` documents owner-scoped attach precisely as implemented; `gateway.md`'s envelope section now points at the proto instead of inlining it; `docs/WorkerFrameProtocol.md` covers the `max_frame_bytes` handshake. The two drift spots found (TST-26, TST-27) are the exceptions, not the pattern.
- The codegen-freshness guard (`scripts/check-codegen.ps1`) is real defense-in-depth: descriptor set, `Contracts/Generated` diff, and vendored Rust protos all checked on every push, directly covering this repo's most-repeated historical failure class — and the tracking log shows CI caught five latent defects on its first green run.