From 7c5eb062e3682f1ca428e50d53c4bfee424f8355 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:14:40 -0400 Subject: [PATCH 1/2] docs(archreview): cycle-3 remediation plan + tracker (batches A-G, model assignments) --- .../2026-08-19/remediation/00-tracking.md | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 archreview/2026-08-19/remediation/00-tracking.md diff --git a/archreview/2026-08-19/remediation/00-tracking.md b/archreview/2026-08-19/remediation/00-tracking.md new file mode 100644 index 0000000..1257225 --- /dev/null +++ b/archreview/2026-08-19/remediation/00-tracking.md @@ -0,0 +1,77 @@ +# MxAccessGateway — Remediation Plan & Tracking (2026-08-19 review) + +Source review: [`../00-overall.md`](../00-overall.md) (HEAD `8b3e3b3`, committed as `f4919ba`). Generated 2026-08-20. +Tracks the 34 cycle-3 IDs (GWC-31+, WRK-32+, IPC-33+, SEC-37+, CLI-46+, TST-31+) plus the tracker-housekeeping actions the overall asked for. Cycle-1/2 trackers remain the record for their own IDs. + +**Status legend:** `Not started` · `In progress` · `Done` · `Blocked (external)` · `Deferred (P2)` · `N/A`. + +## Execution model + +This plan is executed as **seven parallel batches**, each by one subagent working in an isolated git worktree on its own `fix/c3-` branch, merged back into `main` by the coordinator in the order listed (lowest conflict surface first). Every batch must: build the affected component(s) on macOS (`NonWindows.slnx` — the x86 worker cannot build here), run only the filtered tests it touches, update docs in the same commit, and cite finding IDs in commit messages (`fix(IPC-34): …`). Worker-side changes (batches E, F) are verified afterwards on windev via `windev-worker-ci.ps1 -Mode test`. + +**Model assignment rationale:** `opus` for batches that change concurrency/authorization logic or require reasoning across several interacting files with regression tests; `sonnet` for contained, well-specified edits with a clear recipe (version bumps, doc reconciliation, a two-line worker fix); `haiku` for none — every batch either touches code invariants or needs judgement about stale prose, where the cost of a wrong mechanical sweep exceeds the saving. + +| Batch | Branch | Findings | Model | Why this model | Status | +|---|---|---|---|---|---| +| A | `fix/c3-client-versions` | CLI-46 | sonnet | Mechanical five-client bump + README note + one script guard; recipe is fully specified | Not started | +| B | `fix/c3-glauth-doc` | TST-31 | sonnet | Reconcile a table against one TOML source of truth, re-derive one default, change an assert to a log | Not started | +| C | `fix/c3-tracker-hygiene` | GWC-31 + housekeeping (close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31) | sonnet | Cross-file comment/ID sweep needs care about what each ID *means*, but no logic changes | Not started | +| D | `fix/c3-dashboard-acl` | SEC-40, SEC-41, GWC-38 | opus | Authorization semantics, fail-closed decisions, claims re-resolution; each needs a regression test | Not started | +| E | `fix/c3-ipc-boundaries` | IPC-33, IPC-39, IPC-34, IPC-35 | opus | Changes the WorkerClient command lifecycle, the options validator, and the worker drain budget; race-sensitive | Not started | +| F | `fix/c3-worker-residuals` | WRK-32, WRK-33 (+WRK-34 opportunistic) | sonnet | Small, precisely located worker edits + doc prose; cannot be built on macOS so the agent's job is a careful, minimal diff | Not started | +| G | `fix/c3-session-lifecycle` | GWC-32, GWC-34, GWC-37 | opus | Distributor/dispose lifecycle races and an alarm-monitor re-resolve path; needs lock-discipline reasoning | Not started | + +**Out of scope for this execution (recorded, not dropped):** + +| ID | Status | Why | +|---|---|---| +| TST-32 | Blocked (external) | Gitea runner registration-token reset is a web-UI action on the Gitea admin page; no repo change. Operator to-do. | +| SEC-37 | Blocked (external) | Family-wide: TLS on the shared GLAuth lives in `scadaproj/infra/glauth/` (separate repo + docker host). Plan separately; after it lands, move the deployed service to `Production`. | +| CLI-47/48/49/50, IPC-36, IPC-37, TST-33/34/35/36, GWC-33/35/36, SEC-38/39, IPC-38, WRK-34 | Deferred (P2) | Roadmap tier 10; WRK-34 may be picked up by batch F if trivial. | + +## Batch specifications + +### A — CLI-46 client release-train bump (sonnet) +- Bump .NET/Go/Python/Rust clients `0.2.0 → 0.2.1`, Java `0.2.1 → 0.2.2` in every version-bearing file (`pyproject.toml` + `version.py`, `Cargo.toml` workspace + any `version =` consumers, `Directory.Build.props`/csproj ``, Java `build.gradle`/`gradle.properties`, Go module is tag-driven — update any `Version` const if one exists). Follow `docs/ClientPackaging.md` for the authoritative file list. +- `clients/python/README.md` + `docs/ClientPackaging.md`: note that published pypi 0.2.0 has a broken `connect()` (event-loop binding, fixed by `a8f86b5`) and is superseded by 0.2.1. +- Add a PR-time check to `scripts/check-codegen.ps1` (or a new `scripts/check-client-versions.ps1` wired where the other checks run): fail if any `clients/` source differs from its last publish tag while the version constant still equals that tag's version. Document in `docs/ClientPackaging.md`. +- Verify: `dotnet build clients/dotnet/...slnx`; `cargo check --workspace`; `python -m pytest` (clients/python); `JAVA_HOME=/opt/homebrew/opt/openjdk@17 gradle test` then **revert** spurious generated-proto churn; `go build ./... && go test ./...`. + +### B — TST-31 glauth.md + live-test credential (sonnet) +- Reconcile `glauth.md` against `~/Desktop/scadaproj/infra/glauth/config.toml`: ReadOnly gid, the pre-provisioned users table (drop `readonly/readonly123`, fix `admin` password), the worked example at `:124`. Keep the OPC-UA taxonomy note. +- Re-derive `MXGATEWAY_LIVE_MXACCESS_WRITE_SECURED_PASSWORD` default in `docs/GatewayTesting.md` and the `?? "admin123"` literal in `WorkerLiveMxAccessSmokeTests.cs` from the config; make the test **log** (not assert) whether `AuthenticateUser` succeeded. +- Verify: `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` (IntegrationTests compiles on mac? if excluded, note it). + +### C — GWC-31 + tracker housekeeping (sonnet) +- Sweep code comments citing unregistered `GWC-30`/`GWC-31` (perf pass) in `WorkerClient.cs`, `GatewayMetrics.cs`, `WorkerClientTests.cs`, worker `WorkerFrameReader/Writer.cs` to `PERF-nn` keyed to task numbers in `docs/plans/2026-08-15-perf-review-remediation.md`; add a PERF-ID table to that plan. Do not touch the genuine GWC-30 (frame-reader prefix) citations if any exist. +- `archreview/REVIEW-PROCESS.md` (or wherever the ID rule lives): state that GWC/WRK/IPC/SEC/CLI/TST prefixes are reserved for archreview registers. +- Cycle-2 tracker (`archreview/2026-07-12/remediation/00-tracking.md` + domain docs): GWC-16 → Won't fix (rationale from cycle-3 10-gateway-core.md), IPC-08 → Done (`7c1ea12`, residual IPC-34), IPC-31 → Done (superseded by GWC-28), refresh TST-06/TST-17 rows, register WRK-31 (interim ID burned by `e913dab`) so it is never reused. +- Verify: `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx`; `dotnet test --filter FullyQualifiedName~WorkerClientTests`. + +### D — SEC-40, SEC-41, GWC-38 (opus) +- SEC-41: `DashboardSessionAcl` re-resolves tags from `LdapGroupClaimType` claims via `DashboardGroupTagMapping.MapGroupsToTags` against current options (as `HubTokenService` does) instead of trusting login-stamped tag claims; keep stamped claims only as fallback when no group claims exist (document). Test: principal with stale tag claim + group whose grant was removed → denied. +- SEC-40: decide **tighten**: `AlarmsHub` gets a dedicated policy requiring a real role claim (anonymous-loopback/Disabled bypass excluded) — and document in `docs/GatewayDashboardDesign.md` ACL section + `docs/GatewayConfiguration.md` GroupToTag row that alarm metadata remains galaxy-wide for authenticated Viewers (central monitor, no per-session provenance). Test for the policy. +- GWC-38: `GatewayBrowseScopeProvider` throws `RpcException(PermissionDenied)` (and logs) when `Current` is null **and** API-key auth is enabled; unchanged when auth mode is Disabled. Test both branches. +- Verify: build Server; `dotnet test --filter "FullyQualifiedName~DashboardSessionAcl|FullyQualifiedName~AlarmsHub|FullyQualifiedName~BrowseScope"` (adjust to real class names). + +### E — IPC-33, IPC-39, IPC-34, IPC-35 (opus) +- IPC-33 option (a): `GatewayOptionsValidator` adds the upper coupling `Worker:MaxMessageBytes ≤ Protocol:MaxGrpcMessageBytes + 2×reserve`; worker `ResolveDrainReplyByteBudget` becomes `MaxMessageBytes − 2×DrainReplyFrameHeadroomBytes` (floor `MaxMessageBytes/2`); update `docs/GatewayConfiguration.md`, `docs/WorkerFrameProtocol.md`. Test: `GatewayOptionsTests` case for the new bound. +- IPC-39: pre-check compares against `MaxMessageBytes − MaxSequenceEncodingBytes (11)`; fix the comment direction. +- IPC-34: caller-cancellation arms call `TrySendCancelForAbandonedCommand` (renamed) gated on `removed`, skipped when `_stopCts` is the source; new `WorkerClientErrorCode.CallerCancelled` replacing the `GatewayShutdown` mislabel, mapped to `Cancelled` in the gRPC layer; update `docs/Grpc.md` mapping line, `gateway.md:913-919`, `docs/GatewayProcessDesign.md:596-602`. Test: twin of the timeout test driven by the caller token. +- IPC-35: `ReadHandshakeEnvelopeAsync` special-cases `WorkerFault` → `WorkerClientException(WorkerFaulted, CreateWorkerFaultMessage(...))`. Test: fake writes `WorkerFault{MxaccessCreationFailed}` instead of `WorkerReady`, category text surfaces. +- Verify: build Server; `dotnet test --filter "FullyQualifiedName~WorkerClientTests|FullyQualifiedName~GatewayOptions"`. Worker half → windev afterwards. + +### F — WRK-32, WRK-33 (sonnet) +- WRK-33: `ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask));` at `WorkerPipeSession.cs:1235` (or catch-all-and-log in the observer). +- WRK-32: one `pumpStep()` before capturing the `CurrentVersion` baseline in `MxAccessCommandExecutor.cs`; rewrite the executor doc comment (`:996-1003`) to state the recency-based contract; add the same to `docs/MxAccessWorkerInstanceDesign.md` write-completion section and the reply-field comment in `mxaccess_gateway.proto` (**comment-only proto edit → regenerate all generated artifacts in the same commit**, per the repo rule; if regen tooling is unavailable on macOS, skip the proto comment and say so). +- WRK-34 if trivial: `CreateFault(StaHung, "Watchdog", …)` with the correlation id moved into the message. +- Cannot build on macOS: keep diff minimal, net48-safe (no init-only/positional records). Verified on windev afterwards. + +### G — GWC-32, GWC-34, GWC-37 (opus) +- GWC-32: `DisposeAsync` transitions `_state` to `Closed` under `_syncRoot` before completing channels (or `if (_disposed || IsTerminalState()) return;` in `StageWorkerEvent`); update the XML doc to name all three completion paths. Test: dispose a Ready client mid-dispatch → no `ProtocolViolation` fault/metric. +- GWC-34: `_eventDistributorDisposed` flag set under `_syncRoot` when nulling; `EnsureDistributorCreated` throws `SessionManagerException(SessionNotReady)` when set. Test: attach after dispose → throws, no silent hang. +- GWC-37: on `ApplyProviderModeChangeAsync(toMode == Subtag)` re-resolve the watch list and re-subscribe; additionally surface "zero targets resolved while mode can need them" on the provider status (`Degraded` + reason). Update `docs/` alarm-subtag section. Test with the fake worker: GR discovery fails at start, succeeds at failover → watch list non-empty after failover. +- Verify: build Server; `dotnet test --filter "FullyQualifiedName~WorkerClientTests|FullyQualifiedName~GatewaySession|FullyQualifiedName~AlarmMonitor|FullyQualifiedName~AlarmWatchList"`. + +## Merge order +C is merged **last** (its comment sweep touches the same files E and G edit). Order: A, B, F, D, G, E, C. After merge: full `NonWindows` build + one full gateway test run; then windev worker CI for E+F. From 3619152b9f0664d45bf565c98c5affe38ad9d6c6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:23:36 -0400 Subject: [PATCH 2/2] chore(GWC-31): relabel perf-pass GWC-3x comments to PERF-nn; tracker housekeeping Sweep code/doc comments citing unregistered GWC-30/GWC-31 (2026-08-15 perf pass) to PERF-02/PERF-07/PERF-22, keyed to task numbers in docs/plans/2026-08-15-perf-review-remediation.md; add a PERF-ID register table to that plan. Genuine cycle-2 GWC-30 citations (gateway-side frame- reader 4-byte prefix fix in Server/Workers/WorkerFrameReader.cs and its test) are left untouched. Add a line to REVIEW-PROCESS.md reserving the GWC/WRK/IPC/SEC/CLI/TST prefixes for the archreview domain registers. Tracker housekeeping per archreview/2026-08-19/00-overall.md: - GWC-16 -> Won't fix (rationale: PERF(followup) comment documents the double session lookup as measured-negligible, deliberately not collapsed) - IPC-08 -> Done (closed by 7c1ea12/a1a38b5; residual tracked as IPC-34) - IPC-31 -> Done (superseded by GWC-28's write-time sequence stamping) - TST-06, TST-17 rows refreshed per cycle-3 60-testing-docs-gaps.md - WRK-31 (and defensively WRK-29/30) registered as reserved/do-not-reuse interim IDs burned by e913dab, in both the cycle-2 tracker and the cycle-2 worker remediation doc Flip Batch C to Done in archreview/2026-08-19/remediation/00-tracking.md. Worker-side files (Worker/Ipc/WorkerFrameReader.cs, WorkerFrameWriter.cs) are comment-only edits, unverified here (net48 x86, windev-only build). --- REVIEW-PROCESS.md | 6 ++++++ .../2026-07-12/remediation/00-tracking.md | 5 ++++- archreview/2026-07-12/remediation/20-worker.md | 1 + .../2026-07-12/remediation/30-contracts-ipc.md | 2 ++ .../2026-08-19/remediation/00-tracking.md | 2 +- archreview/remediation/00-tracking.md | 8 ++++---- archreview/remediation/10-gateway-core.md | 2 ++ archreview/remediation/30-contracts-ipc.md | 2 ++ archreview/remediation/60-testing-docs-gaps.md | 4 ++++ docs/Metrics.md | 4 ++-- docs/plans/2026-08-15-deferred-remediation.md | 2 +- .../2026-08-15-perf-review-remediation.md | 18 ++++++++++++++++++ .../Metrics/GatewayMetrics.cs | 10 +++++----- .../Workers/WorkerClient.cs | 8 ++++---- .../Gateway/Workers/WorkerClientTests.cs | 4 ++-- .../Metrics/GatewayMetricsTests.cs | 4 ++-- .../Ipc/WorkerFrameReader.cs | 2 +- .../Ipc/WorkerFrameWriter.cs | 8 ++++---- 18 files changed, 65 insertions(+), 27 deletions(-) diff --git a/REVIEW-PROCESS.md b/REVIEW-PROCESS.md index f53825d..11540d4 100644 --- a/REVIEW-PROCESS.md +++ b/REVIEW-PROCESS.md @@ -79,6 +79,12 @@ Add one entry per finding to the `## Findings` section of the module's - **Finding ID** — `-NNN`, numbered sequentially within the module and never reused (e.g. `Worker-001`). IDs are permanent even after resolution. + The `GWC`/`WRK`/`IPC`/`SEC`/`CLI`/`TST` prefixes are reserved for the + `archreview/` domain registers (a separate, whole-repo review series with its + own tracking documents) — never assign one of these prefixes to a + `code-reviews/` finding, and any other work stream (perf passes, ad hoc + remediation plans, etc.) that wants a stable per-item ID must mint its own + distinct prefix rather than borrowing one of these six. - **Severity:** - **Critical** — data loss, security breach, crash/deadlock, or outage. - **High** — incorrect behaviour with significant impact; no safe workaround. diff --git a/archreview/2026-07-12/remediation/00-tracking.md b/archreview/2026-07-12/remediation/00-tracking.md index 169b4aa..c4c6d57 100644 --- a/archreview/2026-07-12/remediation/00-tracking.md +++ b/archreview/2026-07-12/remediation/00-tracking.md @@ -79,6 +79,9 @@ Full design + implementation for each row lives in the linked domain doc under i | WRK-26 | Low | P1 | S | WRK-23 (soft); discharges IPC-29 | Done | Write-priority and overflow doc drift from the WRK-07 change | | WRK-27 | Low | — | S | — | Done | Alarm poll bypasses the watchdog's in-flight suppression (15 s vs 75 s) | | WRK-28 | Low | — | S | WRK-21 (same batch) | Done | 10,000 drain cap is a duplicated magic constant | +| WRK-29 | — | — | — | — | Reserved (do not reuse) | Interim ID space possibly burned alongside WRK-31 by `e913dab`'s ad hoc numbering; not confirmed in the repo or its history, held back defensively | +| WRK-30 | — | — | — | — | Reserved (do not reuse) | Interim ID space possibly burned alongside WRK-31 by `e913dab`'s ad hoc numbering; not confirmed in the repo or its history, held back defensively | +| WRK-31 | — | — | — | — | Reserved (do not reuse) | Interim ID burned by `e913dab` (session-owned transport disposal), cited in code at `Ipc/WorkerPipeSession.cs:45,177` and `Ipc/WorkerPipeSessionTests.cs` outside the register; not a finding — registered per cycle-3 `20-worker.md` (~line 9) so it is never reassigned. Cycle-3 new findings start at WRK-32. | ### Contracts & IPC — [30-contracts-ipc.md](30-contracts-ipc.md) @@ -92,7 +95,7 @@ Full design + implementation for each row lives in the linked domain doc under i | IPC-28 | Low | — | S | — | Done | docs/Grpc.md missing CommandTooLarge → ResourceExhausted mapping | | IPC-29 | Low | — | S | WRK-26 (discharged by) | Done (discharged by WRK-26) | WorkerFrameProtocol.md missing write-scheduling/sequencing section | | IPC-30 | Low | P0 | M | WRK-21 (same batch) | Done | Oversized event frame: keep session-fatal, make the death structured | -| IPC-31 | Info | — | — | — | N/A | Gateway creation-time sequence stamping accepted; diagnostic-only, decision recorded | +| IPC-31 | Info | — | — | — | Done (superseded by GWC-28) | Gateway creation-time sequence stamping — resolved when GWC-28 moved gateway stamping to write time; the two sides now implement the same semantics | | IPC-32 | Info | — | S | IPC-25 (folded in) | Done | check-codegen banner relabel 1/4…4/4 | ### Security & dashboard — [40-security-dashboard.md](40-security-dashboard.md) diff --git a/archreview/2026-07-12/remediation/20-worker.md b/archreview/2026-07-12/remediation/20-worker.md index 93ab57f..c00e482 100644 --- a/archreview/2026-07-12/remediation/20-worker.md +++ b/archreview/2026-07-12/remediation/20-worker.md @@ -24,6 +24,7 @@ members, no positional records). The worker builds and tests only on the Windows | WRK-26 | Low | P1 | S | WRK-23 (soft — sequence prose); discharges IPC-29 | Done | Write-priority and overflow doc drift from the WRK-07 change | | WRK-27 | Low | — | S | — | Done | Alarm poll bypasses the watchdog's in-flight suppression (15 s vs 75 s) | | WRK-28 | Low | — | S | WRK-21 (land in the same commit cluster) | Done | 10,000 drain cap is a duplicated magic constant with a comment-only sync contract | +| WRK-31 | — | — | — | — | Reserved (do not reuse) | Not a finding. `e913dab` (session-owned transport disposal) coined "WRK-31" ad hoc in code comments (`Ipc/WorkerPipeSession.cs:45,177`) and in `Ipc/WorkerPipeSessionTests.cs`, before the register assigned it. WRK-29/WRK-30 appear nowhere in the repo or its history but may have been burned by the same interim numbering, so they are held back defensively too — see the tracker's WRK-29/30/31 rows. Registered here per cycle-3 `archreview/2026-08-19/20-worker.md` (~line 9, "Finding-ID note"); that review's new findings start at **WRK-32** to guarantee no collision. | --- diff --git a/archreview/2026-07-12/remediation/30-contracts-ipc.md b/archreview/2026-07-12/remediation/30-contracts-ipc.md index cb97d99..5c4216e 100644 --- a/archreview/2026-07-12/remediation/30-contracts-ipc.md +++ b/archreview/2026-07-12/remediation/30-contracts-ipc.md @@ -231,6 +231,8 @@ The published-side collection (`CollectPublishedSymbols`, `:61-79`) grows matchi **Disposition: N/A — no action.** `gateway.md:328-330` already declares sequence a per-sender diagnostic aid, never validated; both validators ignore it; the divergence is harmless and now explicitly documented by the IPC-29 doc section (which states the two sides' stamping points). If sequence ever becomes load-bearing, stamp in the gateway write loop (`WorkerClient.cs:390-397`) — recorded here so the next editor finds the decision. +**Resolution: Done — superseded by GWC-28 (2026-08-20, per cycle-3 verification).** GWC-28 moved the gateway-side stamp from creation to the point of writing: `WriteLoopAsync` now stamps `Sequence` immediately before `_writer.WriteAsync` (`Server/Workers/WorkerClient.cs:465-471`), and `CreateEnvelope` leaves it unset (`:1229-1236`); `gateway.md:425-431` was rewritten to "both sides stamp at the point of writing", closing the divergence this finding recorded. Regression test `WorkerClientTests.ConcurrentInvokesEmitStrictlyIncreasingSequencesOnTheWire` covers it. Per `archreview/2026-08-19/30-contracts-ipc.md` (IPC-31 row), tracker flipped from `N/A` to `Done (superseded by GWC-28)`. See cycle-3 **IPC-39** for the one interaction the change created with the IPC-03 pre-check. + --- ## IPC-32 — `check-codegen.ps1` check labels are miscounted `Info` · `—` (rides IPC-25) diff --git a/archreview/2026-08-19/remediation/00-tracking.md b/archreview/2026-08-19/remediation/00-tracking.md index 1257225..d360af0 100644 --- a/archreview/2026-08-19/remediation/00-tracking.md +++ b/archreview/2026-08-19/remediation/00-tracking.md @@ -15,7 +15,7 @@ This plan is executed as **seven parallel batches**, each by one subagent workin |---|---|---|---|---|---| | A | `fix/c3-client-versions` | CLI-46 | sonnet | Mechanical five-client bump + README note + one script guard; recipe is fully specified | Not started | | B | `fix/c3-glauth-doc` | TST-31 | sonnet | Reconcile a table against one TOML source of truth, re-derive one default, change an assert to a log | Not started | -| C | `fix/c3-tracker-hygiene` | GWC-31 + housekeeping (close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31) | sonnet | Cross-file comment/ID sweep needs care about what each ID *means*, but no logic changes | Not started | +| C | `fix/c3-tracker-hygiene` | GWC-31 + housekeeping (close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31) | sonnet | Cross-file comment/ID sweep needs care about what each ID *means*, but no logic changes | Done | | D | `fix/c3-dashboard-acl` | SEC-40, SEC-41, GWC-38 | opus | Authorization semantics, fail-closed decisions, claims re-resolution; each needs a regression test | Not started | | E | `fix/c3-ipc-boundaries` | IPC-33, IPC-39, IPC-34, IPC-35 | opus | Changes the WorkerClient command lifecycle, the options validator, and the worker drain budget; race-sensitive | Not started | | F | `fix/c3-worker-residuals` | WRK-32, WRK-33 (+WRK-34 opportunistic) | sonnet | Small, precisely located worker edits + doc prose; cannot be built on macOS so the agent's job is a careful, minimal diff | Not started | diff --git a/archreview/remediation/00-tracking.md b/archreview/remediation/00-tracking.md index 258549d..d49dfdb 100644 --- a/archreview/remediation/00-tracking.md +++ b/archreview/remediation/00-tracking.md @@ -74,7 +74,7 @@ Full design + implementation for each row lives in the linked domain doc under i | GWC-13 | Low | — | M | — | Not started | Worker-ready wait is a 25 ms poll loop | | GWC-14 | Low | P2 | M | — | Done | Replay ring is a `LinkedList` with a node alloc per event | | GWC-15 | Low | P2 | S | — | Done | Per-event gauge reads `ChannelReader.Count` every event | -| GWC-16 | Low | — | S | — | Not started | `Invoke` resolves the session twice per command | +| GWC-16 | Low | — | S | — | Won't fix | `Invoke` resolves the session twice per command — measured negligible, deliberately not collapsed (see domain doc) | | GWC-17 | Low | — | M | — | Not started | Sessions layer throws `Grpc.Core.RpcException` (layering leak) | | GWC-18 | Low | — | S | — | Not started | `GatewaySession` implements `DisposeAsync` without `IAsyncDisposable` | | GWC-19 | Low | — | S | — | Not started | Dead `GatewaySession.KillWorker(string)` + stale doc | @@ -119,7 +119,7 @@ Full design + implementation for each row lives in the linked domain doc under i | IPC-05 | Medium | P2 | S | — | Done | Redundant deep copies on command and event hot paths | | IPC-06 | Medium | P2 | S | — | Done | `gateway.md` Worker Envelope sketch no longer matches the contract | | IPC-07 | Medium | P2 | S | — | Done | `docs/Grpc.md` says six RPCs; there are seven | -| IPC-08 | Medium | — | M | — | Not started | `WorkerCancel` defined and handled but never sent | +| IPC-08 | Medium | — | M | — | Done | `WorkerCancel` defined and handled but never sent — resolved by `7c1ea12`/`a1a38b5`; residual (caller-cancellation leg) tracked as cycle-3 IPC-34 | | IPC-09 | Medium | P1 | M | IPC-01 | Done | Known codegen fragilities have no in-repo guards | | IPC-10 | Low | — | S | — | Not started | Envelope `sequence` is write-only; monotonicity never validated | | IPC-11 | Low | — | S | — | Not started | No protocol version negotiation despite `supported_protocol_version` name | @@ -218,7 +218,7 @@ Full design + implementation for each row lives in the linked domain doc under i | TST-03 | High | P1 | M | — | Done | No CI exists | | TST-04 | High | P2 | L | — | Done | Session-resilience epic 16/28 tasks unfinished | | TST-05 | Medium | P1 | S | TST-03 | Partially done | Real-worker control/COM paths verified opt-in only. **Scheduling half closed 2026-08-10** by the TST-25 `nightly-windev` job (`.gitea/workflows/ci.yml`, cron `0 6 * * *` → `scripts/ci/run-windev-ci.sh live`, which sets `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`, runs `WorkerLiveMxAccessSmokeTests`, and opens a Gitea issue on failure) — "opt-in, run by memory" is now "runs nightly, reports failures". **Residual: the coverage-audit half.** The live suite's 8 facts cover all six late-added COM commands but none of the five control commands (`Ping`, `GetSessionState`, `GetWorkerInfo`, `DrainEvents`, `ShutdownWorker`), which real workers answer in `Worker/Ipc/WorkerPipeSession.cs` yet are still only exercised through `FakeWorkerHarness` canned replies — precisely the masking the finding named | -| TST-06 | Medium | — | M | — | Not started | Dashboard live-data path untested | +| TST-06 | Medium | — | M | — | Materially narrowed | Dashboard live-data path untested — `DashboardLiveDataServiceTests`, `EventsHubTests`, `EventsHubViewerRegistryTests`, `AlarmsHubPublisherTests`, `SessionDetailsPageEventAclTests` now exist (landed with SEC-25/TST-16); needs a re-audit-and-close pass, not re-implementation | | TST-07 | Medium | — | S | — | Not started | Real-clock sleeps with negative assertions are latent flakes | | TST-08 | Medium | P1 | M | — | Done | Full-suite orphaned testhost processes (does not reproduce; doc de-stale) | | TST-09 | Medium | — | M | — | Not started | Health checks cover only the auth store | @@ -229,7 +229,7 @@ Full design + implementation for each row lives in the linked domain doc under i | TST-14 | Medium | P2 | S | — | Not started | Repo-root working artifacts need triage | | TST-15 | Medium | P2 | M | TST-04 | Done | Dashboard EventsHub has no per-session ACL | | TST-16 | Medium | P1 | S | — | Done | `Dashboard:ShowTagValues` is a dead flag | -| TST-17 | Medium | — | S | — | Not started | Vendor-gated alarm parity residuals silently lossy | +| TST-17 | Medium | — | S | — | Materially changed | Vendor-gated alarm parity residuals silently lossy — `WnWrapAlarmConsumer.cs:309-327` now passes `szDomainName`/`szOprFullName`; the vendor 8-arg leg is recorded "unavailable by configuration" with evidence (`docs/AlarmProbeFindings.md`, `d1ae43d`, `540a118`); "silent drop, unchanged" is no longer accurate — needs a worker-domain refresh pass | | TST-18 | Low | — | S | — | Not started | Hosted-service wrappers untested | | TST-19 | Low | — | S | — | Not started | Keep FakeWorkerHarness canned replies in lockstep | | TST-20 | Low | — | S | — | Not started | E2E script papers over a real advise-without-consumer sharp edge | diff --git a/archreview/remediation/10-gateway-core.md b/archreview/remediation/10-gateway-core.md index 8121ba3..042fb0a 100644 --- a/archreview/remediation/10-gateway-core.md +++ b/archreview/remediation/10-gateway-core.md @@ -330,6 +330,8 @@ Open question the review cannot settle: whether a future probe (COM-init readine **Verification.** `dotnet build src/ZB.MOM.WW.MxGateway.Server`; `dotnet test src/ZB.MOM.WW.MxGateway.Tests --filter "FullyQualifiedName~SessionManager"`. +**Resolution: Won't fix (2026-08-20, closed by cycle-3 verification).** `Grpc/MxAccessGatewayService.cs:111-115` carries a `PERF(followup)` comment documenting the double session lookup as measured-negligible and deliberately not collapsed — the fix would duplicate `InvokeAsync`'s fault mapping at a second entry point, trading a negligible per-command cost for a maintenance liability. Per the cycle-3 gateway-core review (`archreview/2026-08-19/10-gateway-core.md`, GWC-16 row), the rationale is sound; tracker flipped from `Not started` to `Won't fix`. + --- ## GWC-17 — Sessions layer throws `Grpc.Core.RpcException` `Low` · `—` diff --git a/archreview/remediation/30-contracts-ipc.md b/archreview/remediation/30-contracts-ipc.md index f8115f7..1c5f224 100644 --- a/archreview/remediation/30-contracts-ipc.md +++ b/archreview/remediation/30-contracts-ipc.md @@ -175,6 +175,8 @@ The frame protocol and proto evolution hygiene are sound; the remediable risk cl **Verification.** `dotnet build src/ZB.MOM.WW.MxGateway.Server` + `dotnet test ...Tests --filter FullyQualifiedName~WorkerClient`; worker side `-p:Platform=x86`. +**Resolution: Done (2026-08-20, closed by cycle-3 verification, residual filed).** Wired up substantially as designed: `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`), the worker routes it to `CancelCommand` (`Worker/Ipc/WorkerPipeSession.cs:755-756` → `MxAccess/MxAccessStaSession.cs:529-532` → `Sta/StaCommandDispatcher.CancelQueuedCommand:151`), regression test `WorkerClientTests.InvokeAsync_WhenCommandTimesOut_SendsWorkerCancelForThatCorrelation` exists, and `docs/GatewayProcessDesign.md:596-602` documents it. Landed in `7c1ea12`/`a1a38b5`. **Residual, not covered by this closure:** the caller-cancellation leg (context deadline / client disconnect, as opposed to the gateway's own `CommandTimeout`) sends nothing — filed as cycle-3 **IPC-34**. Per `archreview/2026-08-19/30-contracts-ipc.md` (IPC-08 row), tracker flipped from `Not started` to `Done` with the residual cross-referenced. + --- ## IPC-09 — Known codegen fragilities have no in-repo guards `Medium` · `P1` diff --git a/archreview/remediation/60-testing-docs-gaps.md b/archreview/remediation/60-testing-docs-gaps.md index 6ebf9cd..e585765 100644 --- a/archreview/remediation/60-testing-docs-gaps.md +++ b/archreview/remediation/60-testing-docs-gaps.md @@ -191,6 +191,8 @@ This finding is the umbrella; TST-01/02/15 are its actionable slices. The remedi **Verification.** `dotnet test ... --filter FullyQualifiedName~DashboardLiveDataServiceTests`. +**Status refresh (2026-08-20, per cycle-3 verification).** Materially narrowed — the tracker row was stale. `Tests/Gateway/Dashboard/DashboardLiveDataServiceTests.cs`, `EventsHubTests.cs`, `EventsHubViewerRegistryTests.cs`, `AlarmsHubPublisherTests.cs`, and `SessionDetailsPageEventAclTests.cs` all exist now (mostly landed with the SEC-25/TST-16 closeouts and the in-process dashboard-feed work). Per `archreview/2026-08-19/60-testing-docs-gaps.md` (TST-06 row), this needs a re-audit-and-close pass against the current coverage rather than fresh implementation — left open pending that audit, tracker status changed from `Not started` to `Materially narrowed` to reflect the actual state. + --- ## TST-07 — Real-clock sleeps with negative assertions are latent flakes `Medium` · `—` @@ -371,6 +373,8 @@ If TST-02's interim mitigation (flip retention off) is chosen instead of impleme **Verification.** `dotnet build src/ZB.MOM.WW.MxGateway.Worker -p:Platform=x86`; `dotnet test src/ZB.MOM.WW.MxGateway.Worker.Tests/... -p:Platform=x86` (Windows). +**Status refresh (2026-08-20, per cycle-3 verification).** Materially changed — the tracker row needs a refresh from 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`). Per `archreview/2026-08-19/60-testing-docs-gaps.md` (TST-17 row), "silent drop, unchanged" is no longer an accurate description of the row; whether a caller-visible degrade signal now exists is the worker reviewer's call to close out. Tracker status changed from `Not started` to `Materially changed`. + --- ## TST-18 — Hosted-service wrappers untested `Low` · `—` diff --git a/docs/Metrics.md b/docs/Metrics.md index c13ea91..e3b9bf6 100644 --- a/docs/Metrics.md +++ b/docs/Metrics.md @@ -72,7 +72,7 @@ Observable gauges are pull-based; the `Meter` invokes the supplied callback when |------------|--------------|-------------| | `mxgateway.sessions.open` | `_openSessions` | Currently open sessions tracked by `SessionManager`. | | `mxgateway.workers.running` | `_workersRunning` | Worker clients in a running state. | -| `mxgateway.events.worker_queue.depth` | `_workerEventQueueDepthSources` (summed on demand) | Undelivered worker events held by `WorkerClient` — staged *and* queued (GWC-24) — summed across every live client at collection time (GWC-30). Each client owns an interlocked counter incremented when the read loop stages an event and decremented when the consumer reads it, and registers it as a gauge source for its lifetime, so a backlog stuck in a staging channel is visible and concurrent sessions add up instead of overwriting one another. | +| `mxgateway.events.worker_queue.depth` | `_workerEventQueueDepthSources` (summed on demand) | Undelivered worker events held by `WorkerClient` — staged *and* queued (GWC-24) — summed across every live client at collection time (PERF-02). Each client owns an interlocked counter incremented when the read loop stages an event and decremented when the consumer reads it, and registers it as a gauge source for its lifetime, so a backlog stuck in a staging channel is visible and concurrent sessions add up instead of overwriting one another. | | `mxgateway.events.grpc_stream_queue.depth` | `_eventStreamBacklogSources` (summed on demand) | Live backlog buffered across every active `EventStreamService` subscriber, summed from the subscribers' channel `Count` at collection time. | ## Snapshot Shape @@ -111,7 +111,7 @@ The scalar fields mirror the counters and gauges. The four dictionaries provide - `EventsBySession` keys by `sessionId`; entries are removed via `RemoveSessionEvents` when a session closes so the map does not grow without bound. - `RetryAttemptsByArea` keys by the resilience `area` tag, e.g. `worker_startup`. -`EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (GWC-30). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients. +`EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (PERF-02). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients. ## Recording Sites diff --git a/docs/plans/2026-08-15-deferred-remediation.md b/docs/plans/2026-08-15-deferred-remediation.md index a01c5a2..4058674 100644 --- a/docs/plans/2026-08-15-deferred-remediation.md +++ b/docs/plans/2026-08-15-deferred-remediation.md @@ -241,7 +241,7 @@ Run `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` (expect 0 warnings) a - Modify: `src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs` - Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs` (comment only) -**Why:** On net48, `NamedPipeClientStream.ReadAsync` ignores its `CancellationToken` (`WorkerFrameReader.cs:109-111`). Fault-path exits (event-drain fault, oversized event, watchdog, heartbeat write failure) leave `readTask` pending; it is unblocked only when `WorkerPipeClient`'s `using` disposes the pipe, at which point it faults with `ObjectDisposedException`/`IOException` on a Task nobody observes (the finally at `:303-309` awaits only heartbeat and drain). The frame-pooling change (GWC-30) makes this sharper: the abandoned read owns the per-instance `_lengthPrefix` buffer and possibly a rented ArrayPool payload — the reader's single-consumer invariant holds today only because nothing ever reads again after abandonment. +**Why:** On net48, `NamedPipeClientStream.ReadAsync` ignores its `CancellationToken` (`WorkerFrameReader.cs:109-111`). Fault-path exits (event-drain fault, oversized event, watchdog, heartbeat write failure) leave `readTask` pending; it is unblocked only when `WorkerPipeClient`'s `using` disposes the pipe, at which point it faults with `ObjectDisposedException`/`IOException` on a Task nobody observes (the finally at `:303-309` awaits only heartbeat and drain). The frame-pooling change (PERF-22) makes this sharper: the abandoned read owns the per-instance `_lengthPrefix` buffer and possibly a rented ArrayPool payload — the reader's single-consumer invariant holds today only because nothing ever reads again after abandonment. **Spec — constraints, implementer designs within them:** 1. **No unobserved faulted Task.** After the stream is disposed, `readTask`'s fault must be awaited/observed (reuse `ObserveBackgroundTaskStopAsync`'s timeout-and-log shape, `:312-348`) before `WorkerPipeClient.RunAsync` returns. diff --git a/docs/plans/2026-08-15-perf-review-remediation.md b/docs/plans/2026-08-15-perf-review-remediation.md index 83a975e..21ca92a 100644 --- a/docs/plans/2026-08-15-perf-review-remediation.md +++ b/docs/plans/2026-08-15-perf-review-remediation.md @@ -608,6 +608,24 @@ Commit anything found: `docs: remediation plan doc sweep` --- +## PERF-ID register + +This plan's tasks were cited in code comments and test names as `GWC-30`/`GWC-31` before +those labels were recognized as colliding with the `archreview` finding register — see +cycle-3 finding `GWC-31` (`archreview/2026-08-19/10-gateway-core.md`). The comments were +swept to the `PERF-nn` prefix below (`nn` = this doc's task number); `GWC-30`/`GWC-31` no +longer appear in perf-pass comments. Citations of the *genuine* archreview `GWC-30` +("Frame reader still allocates a fresh 4-byte prefix array per frame", +`archreview/2026-07-12/10-gateway-core.md`) — the gateway-side +`Server/Workers/WorkerFrameReader.cs` fix and its test — were left untouched; they refer +to a real register entry, not this plan. + +| PERF-ID | Task | Files | +|---|---|---| +| PERF-02 | Task 2 — Metrics: pull-gauge for worker queue depth, lock-free command counters | `Server/Workers/WorkerClient.cs` (`:44`, `:355`), `Server/Metrics/GatewayMetrics.cs` (`:46`, `:213`, `:284`, `:463`, `:524`), `Tests/Metrics/GatewayMetricsTests.cs` (`:22`, `:61`), `docs/Metrics.md` | +| PERF-07 | Task 7 — WorkerClient: pooled-timer timeout, single sizing pass, `WorkerCancel` on timeout | `Server/Workers/WorkerClient.cs` (`:236`, `:253`), `Tests/Gateway/Workers/WorkerClientTests.cs` (`:163`, `:179`) | +| PERF-22 | Task 22 — Worker frame writer/reader: pooled buffers | `Worker/Ipc/WorkerFrameReader.cs` (`:23`), `Worker/Ipc/WorkerFrameWriter.cs` (`:671`), `docs/plans/2026-08-15-deferred-remediation.md` (Task cross-reference) | + ## Explicitly deferred (decided, not forgotten) | Finding | Why deferred | diff --git a/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs b/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs index 36e8938..3f1a7dc 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs @@ -43,7 +43,7 @@ public sealed class GatewayMetrics : IDisposable private readonly ConcurrentDictionary> _eventStreamBacklogSources = new(); private long _nextEventStreamBacklogSourceId; - // GWC-30: the same pull model for the worker event queue depth. It replaces a pushed scalar that + // PERF-02: the same pull model for the worker event queue depth. It replaces a pushed scalar that // every WorkerClient wrote twice per event (staged, consumed) under _syncRoot — a process-wide // lock on the hottest path, and last-writer-wins across sessions, so the gauge reported one // arbitrary session's backlog instead of the gateway's. Each client registers a source returning @@ -210,7 +210,7 @@ public sealed class GatewayMetrics : IDisposable /// Name of the command method. public void CommandStarted(string method) { - // GWC-30: the three command counters run two-to-three times per gRPC call, so they use + // PERF-02: the three command counters run two-to-three times per gRPC call, so they use // Interlocked rather than _syncRoot — the same idiom as EventReceived. Nothing here needs a // consistent multi-field view; GetSnapshot reads each with Interlocked.Read. Interlocked.Increment(ref _commandsStarted); @@ -281,7 +281,7 @@ public sealed class GatewayMetrics : IDisposable /// Registers a live depth source for the worker event queue-depth gauge and returns a handle /// that removes it when disposed. Each WorkerClient registers once and reports its own /// undelivered (staged + queued) event count, so the gauge is the gateway-wide sum instead of - /// the last value any one session happened to push (GWC-30). + /// the last value any one session happened to push (PERF-02). /// /// /// Returns this worker client's current undelivered event count. Invoked only at collection @@ -460,7 +460,7 @@ public sealed class GatewayMetrics : IDisposable { // Compute the live queue depths outside _syncRoot: the sources are the subscriber channels' // Count (their own locks) and the worker clients' interlocked counters, neither of which may - // run under this lock. GWC-15, GWC-30. + // run under this lock. GWC-15, PERF-02. int workerEventQueueDepth = GetWorkerEventQueueDepth(); int grpcEventStreamQueueDepth = GetGrpcEventStreamQueueDepth(); lock (_syncRoot) @@ -521,7 +521,7 @@ public sealed class GatewayMetrics : IDisposable } } - // Sums the undelivered event backlog across every live worker client (GWC-30). + // Sums the undelivered event backlog across every live worker client (PERF-02). private int GetWorkerEventQueueDepth() => SumSources(_workerEventQueueDepthSources); // Sums the live backlog across every registered event-stream subscriber. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs index bff47b1..1e9276f 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs @@ -41,7 +41,7 @@ public sealed class WorkerClient : IWorkerClient private readonly SemaphoreSlim _pendingCommandSlots; private readonly CancellationTokenSource _stopCts = new(); - // GWC-30: this client's contribution to the gateway-wide worker queue-depth gauge. Registered + // PERF-02: this client's contribution to the gateway-wide worker queue-depth gauge. Registered // once here and read only when the gauge is scraped, so staging and consuming an event cost an // Interlocked on _eventQueueDepth and nothing else — previously each of those two hot-path steps // called into GatewayMetrics and took its process-wide lock. Null when metrics are disabled. @@ -233,7 +233,7 @@ public sealed class WorkerClient : IWorkerClient // size the caller controls; checking here keeps a MessageTooLarge in the write loop a // genuine desync signal. // - // PERF(GWC-31): this size cannot be handed to WorkerFrameWriter to spare its own + // PERF-07: this size cannot be handed to WorkerFrameWriter to spare its own // CalculateSize. WriteLoopAsync stamps envelope.Sequence immediately before the write // (GWC-28), and a non-zero varint field grows the encoding — so the number computed here // is a lower bound on the frame the writer actually emits, never the frame length. Passing @@ -250,7 +250,7 @@ public sealed class WorkerClient : IWorkerClient await EnqueueAsync(commandEnvelope, cancellationToken).ConfigureAwait(false); - // GWC-31: one pooled timer instead of a linked CTS + Task.Delay + WhenAny per command. + // PERF-07: one pooled timer instead of a linked CTS + Task.Delay + WhenAny per command. // Task.WaitAsync arms a TimerQueueTimer on the shared timer queue and cancels it when the // reply lands, so the steady-state cost of a command that replies in time is a single // continuation — the old shape allocated a linked CancellationTokenSource, its @@ -352,7 +352,7 @@ public sealed class WorkerClient : IWorkerClient { await foreach (WorkerEvent workerEvent in _events.Reader.ReadAllAsync(cancellationToken).ConfigureAwait(false)) { - // No metrics call on the hot path: the gauge pulls _eventQueueDepth when scraped (GWC-30). + // No metrics call on the hot path: the gauge pulls _eventQueueDepth when scraped (PERF-02). Interlocked.Decrement(ref _eventQueueDepth); yield return workerEvent; } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs index 807249d..38e38d6 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs @@ -160,7 +160,7 @@ public sealed class WorkerClientTests TestTimeout, CancellationToken.None); - // The timeout also emits a WorkerCancel for the abandoned correlation (GWC-31), which sits + // The timeout also emits a WorkerCancel for the abandoned correlation (PERF-07), which sits // ahead of the second command on the FIFO pipe; skip it rather than mistaking it for the // command this assertion is about. WorkerEnvelope secondCommand = await ReadNextCommandAsync(pipePair, timedOutCommand.CorrelationId); @@ -176,7 +176,7 @@ public sealed class WorkerClientTests /// /// A command timeout abandons the gateway-side wait, but the worker keeps the correlation on /// its single STA queue and would still run it — so the gateway forwards a WorkerCancel - /// for the abandoned correlation id (GWC-31). Without it, a client that retries after a + /// for the abandoned correlation id (PERF-07). Without it, a client that retries after a /// timeout stacks work the worker still intends to execute. Asserted on the wire because the /// cancel is protocol behavior the worker depends on, not an internal detail. /// diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs index 5d67920..4101600 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs @@ -19,7 +19,7 @@ public sealed class GatewayMetricsTests metrics.CommandFailed("WriteSecured", "AuthorizationFailed", TimeSpan.FromMilliseconds(12)); metrics.EventReceived("session-1", "OnDataChange"); metrics.EventReceived("session-1", "OnDataChange"); - // GWC-30: the worker queue-depth gauge sums one live source per worker client, so the two + // PERF-02: the worker queue-depth gauge sums one live source per worker client, so the two // registrations below stand in for two concurrent sessions holding 3 and 4 events. using IDisposable workerDepthSourceA = metrics.RegisterWorkerEventQueueDepthSource(static () => 3); using IDisposable workerDepthSourceB = metrics.RegisterWorkerEventQueueDepthSource(static () => 4); @@ -58,7 +58,7 @@ public sealed class GatewayMetricsTests } /// - /// GWC-30: the worker queue-depth gauge sums every registered source rather than holding a + /// PERF-02: the worker queue-depth gauge sums every registered source rather than holding a /// single pushed scalar, so concurrent sessions add up instead of overwriting one another, /// and a disposed registration (a worker client going away) drops out of the sum. Disposal /// is idempotent because a client's dispose path can run twice. diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs index 8ae6601..21c2afe 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs @@ -20,7 +20,7 @@ public sealed class WorkerFrameReader private readonly WorkerFrameProtocolOptions _options; private readonly Stream _stream; - // Reused across frames rather than allocated per read (GWC-30). Safe because ReadAsync is + // Reused across frames rather than allocated per read (PERF-22). Safe because ReadAsync is // single-consumer by construction; the prefix is fully overwritten by every read. // // The single-consumer invariant has to survive teardown as well as steady state, because a read diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs index 1c1c90d..0593a15 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs @@ -668,10 +668,10 @@ public sealed class WorkerFrameWriter // Serialize once into a single pooled buffer that carries the 4-byte length prefix followed // by the payload, then issue one stream write. This avoids a second serialization pass, a // separate prefix array, a separate prefix write, and any per-frame heap allocation — the - // gateway-side writer's GWC-30 shape, now matched on the net48 side. The rented buffer may - // be larger than requested, so only the first frameLength bytes are ever written. The buffer - // is returned only after this frame's write has completed; the batch flush deferred to the - // end of the drain (see DrainQueuedFramesAsync) does not read from it. + // gateway-side writer's pooled-buffer shape (PERF-22), now matched on the net48 side. The + // rented buffer may be larger than requested, so only the first frameLength bytes are ever + // written. The buffer is returned only after this frame's write has completed; the batch + // flush deferred to the end of the drain (see DrainQueuedFramesAsync) does not read from it. int frameLength = sizeof(uint) + payloadLength; byte[] frame = ArrayPool.Shared.Rent(frameLength); try