From ddfd41b06d6972089d6baae250ab8b4ef5a40f2c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:14:40 -0400 Subject: [PATCH 1/5] 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 93edb1b74d2e76b6b3312dfe501f8352d1f216e8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:17:21 -0400 Subject: [PATCH 2/5] fix(WRK-33): observe the discarded command-task wrapper's fault TryStartCommandTask discarded ObserveCommandTaskAsync's task without a fault-observing continuation -- the one gap in the NEXT-04 discipline applied everywhere else in this file. ObserveCommandTaskAsync only swallows OperationCanceledException, so any other exception escaping ProcessCommandAsync's own catch path would fault the discarded wrapper and surface only as a finalizer-time unobserved-task exception. Wrap it with the existing ObserveFaultWhenever helper, same as every other discarded task in this file. --- src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs index a92fa0b..d61287c 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs @@ -1232,7 +1232,14 @@ public sealed class WorkerPipeSession _activeCommandTasks.Add(commandTask); } - _ = ObserveCommandTaskAsync(commandTask); + // WRK-33: every other discarded task in this file is wrapped with ObserveFaultWhenever + // (NEXT-04 discipline) so a fault reaching the finalizer unobserved is impossible. This + // wrapper task was the one exception — ObserveCommandTaskAsync only swallows + // OperationCanceledException, so any other exception escaping ProcessCommandAsync's own + // catch path (e.g. a future edit, or a failure past TryWriteFaultAsync's narrow filter) + // would fault the discarded task with no observer. Close the gap the same way the rest of + // the file does. + ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask)); } private async Task ObserveCommandTaskAsync(Task commandTask) From 20a3eefb1e461ba4b4e4e69249cc61e55822c34a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:20:28 -0400 Subject: [PATCH 3/5] fix(WRK-32): pre-baseline pump for write-completion correlation + honest contract docs Write-completion correlation is recency-based per (server_handle, item_handle): OnWriteComplete carries no per-call token, so the executor attributes the first completion whose version exceeds a pre-call baseline to that write. A stale completion from an earlier timed-out write to the same handle could ride the next write's reply if it was already queued when the baseline was captured. Add CaptureCompletionBaseline (Write, Write2, WriteSecured, WriteSecured2) to run one pumpStep() immediately before snapshotting CurrentVersion, so an already-queued stale completion folds into the baseline instead of being available to the next wait. This narrows, but per the finding cannot close, the correlation gap -- a completion queued after the pump step and before the COM call returns (or during the wait) is still indistinguishable from the new write's own completion; closing it needs an MXAccess-side correlation token that does not exist. Rewrite the AwaitWriteCompletion doc comment to state the recency-based contract honestly instead of implying exact per-call matching, and add the same contract to docs/MxAccessWorkerInstanceDesign.md's write-completion section. Deferred: the reply-field comment in src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto is not edited here. A comment-only proto change still requires regenerating every generated artifact (C#, Go, Python, Java, Rust, protoset) in the same commit per repo convention, and this macOS box cannot verify all five client generator pins together for a Low-severity doc-only change. The contract is documented in MxAccessWorkerInstanceDesign.md instead; the existing proto comment ("concurrent writes... can swap rows") is not wrong, just less precise than the new doc section. --- docs/MxAccessWorkerInstanceDesign.md | 24 +++++++++ .../MxAccess/MxAccessCommandExecutor.cs | 53 +++++++++++++++++-- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/docs/MxAccessWorkerInstanceDesign.md b/docs/MxAccessWorkerInstanceDesign.md index 28f49a1..fab6491 100644 --- a/docs/MxAccessWorkerInstanceDesign.md +++ b/docs/MxAccessWorkerInstanceDesign.md @@ -317,6 +317,30 @@ whose write workload is effectively fire-and-forget — no consumer reads the reply's `statuses` — can lower it, or set `0` to skip the wait entirely and reply on acceptance alone. +**Write-completion correlation is recency-based, not per-call.** MXAccess's +`OnWriteComplete` callback carries no correlation token — only the server/item +handle pair — so `MxAccessCommandExecutor` correlates a completion to a write +by *version*: it snapshots `MxAccessWriteCompletionCache.CurrentVersion` for +the handle as a baseline before the COM call, then reports the first +completion whose version exceeds that baseline as the write's outcome +(`AwaitWriteCompletion`). This is exact for the common case, but if an earlier +write to the same handle times out unconfirmed — its `OnWriteComplete` still +undelivered when the wait gives up — and the caller immediately issues another +write to the same tag, the stale callback can dispatch during the second +write's pump-wait and be reported as *its* outcome. The executor mitigates the +most common instance of this (`CaptureCompletionBaseline`) by running one +`pumpStep()` immediately before capturing the baseline, so a completion already +queued for the handle folds into the baseline instead of being available to +ride the next write's reply; this closes the gap for a callback that arrived +before the new write started, but a callback that MXAccess queues after that +pump step and before the COM call returns (or during the wait itself) is still +indistinguishable from the new write's own completion. Full closure is not +possible without an MXAccess-side correlation token, which does not exist — +this is a property of the COM surface, not a gap this worker introduced (a +native client correlating by handle has the identical ambiguity). The exposure +is bounded: `statuses` is an enrichment field, and `Hresult`/`ProtocolStatus` +stay acceptance-only regardless of which completion (if any) was attributed. + ## COM Creation The MXAccess analysis source at `C:\Users\dohertj2\Desktop\mxaccess` identifies diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessCommandExecutor.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessCommandExecutor.cs index 62b4e94..46cbc7e 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessCommandExecutor.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessCommandExecutor.cs @@ -445,7 +445,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor // also fire-and-forget in MXAccess, and its commit outcome only exists // in the later OnWriteComplete callback. MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache; - ulong completionBaseline = completionCache.CurrentVersion( + ulong completionBaseline = CaptureCompletionBaseline( + completionCache, writeCommand.ServerHandle, writeCommand.ItemHandle); @@ -485,7 +486,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor // Same pre-call baseline rule as ExecuteWriteSecured. MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache; - ulong completionBaseline = completionCache.CurrentVersion( + ulong completionBaseline = CaptureCompletionBaseline( + completionCache, write2Command.ServerHandle, write2Command.ItemHandle); @@ -523,7 +525,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor // immediately after WriteSecured bumps the version past this snapshot, // so a fast commit still correlates (no missed-callback window). MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache; - ulong completionBaseline = completionCache.CurrentVersion( + ulong completionBaseline = CaptureCompletionBaseline( + completionCache, writeSecuredCommand.ServerHandle, writeSecuredCommand.ItemHandle); @@ -564,7 +567,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor // Same pre-call baseline rule as ExecuteWriteSecured. MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache; - ulong completionBaseline = completionCache.CurrentVersion( + ulong completionBaseline = CaptureCompletionBaseline( + completionCache, writeSecured2Command.ServerHandle, writeSecured2Command.ItemHandle); @@ -994,7 +998,31 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor } /// - /// Bounded pump-wait for the OnWriteComplete row matching a + /// Captures the write-completion baseline for / + /// immediately before a Write/Write2/WriteSecured/ + /// WriteSecured2 COM call. Pumps once first (WRK-32) so an OnWriteComplete + /// already queued for this handle — most commonly the still-undelivered callback of an + /// earlier write to the same tag that this worker gave up waiting on — folds into the + /// baseline instead of being available to ride this write's reply. + /// + /// + /// This narrows, but does not close, the correlation gap documented on + /// : a completion that MXAccess queues after this pump + /// step but before the COM call returns is still indistinguishable from this write's own + /// completion, because OnWriteComplete carries no per-call token — only the + /// server/item handle pair. + /// + private ulong CaptureCompletionBaseline( + MxAccessWriteCompletionCache completionCache, + int serverHandle, + int itemHandle) + { + pumpStep(); + return completionCache.CurrentVersion(serverHandle, itemHandle); + } + + /// + /// Bounded pump-wait for the OnWriteComplete row matching a Write/Write2/ /// WriteSecured/WriteSecured2 call, copied onto the reply when it /// arrives in time. The executor holds the STA thread but pumps /// Windows messages each poll (ReadBulk precedent) so the COM callback @@ -1002,6 +1030,21 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor /// statuses — the consumer's unconfirmed path, never a synthesized /// failure. Protocol status/hresult stay acceptance-only either way. /// + /// + /// Correlation is recency-based per handle, not per call: MXAccess's + /// OnWriteComplete callback carries no correlation token, only the server/item + /// handle pair, so this method reports the first completion whose version exceeds + /// as this write's outcome. If an earlier write to + /// the same handle times out unconfirmed and its callback is still undelivered, that + /// stale completion can dispatch during this write's pump-wait and ride this reply + /// instead — 's pre-call pump only folds in a + /// completion already queued before this write started; it cannot fold in one queued + /// during the COM call or the wait itself. The Hresult/ProtocolStatus + /// fields stay acceptance-only regardless, so the exposure is limited to the enrichment + /// statuses field, and it is inherent to the COM surface — a native client + /// correlating by handle has the identical ambiguity. Full closure needs an + /// MXAccess-side correlation token, which does not exist. + /// private void AwaitWriteCompletion( MxCommandReply reply, MxAccessWriteCompletionCache completionCache, From 3679e59eed9116451ab5a13c80bdd22ed5ec0b85 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:20:34 -0400 Subject: [PATCH 4/5] fix(WRK-34): stop writing the watchdog correlation id into CommandMethod ReportWatchdogFaultIfNeededAsync built the StaHung fault with CommandMethod = snapshot.CurrentCommandCorrelationId, so the wire fault's command_method field carried an opaque correlation id (or empty) instead of a method name -- a consumer reading CommandMethod as a method name got a confusing value. Use a stable "Watchdog" label and fold the stuck command's correlation id (when present) into the diagnostic message instead. No test asserts CommandMethod for the StaHung fault; the existing assertions only check Category and that DiagnosticMessage contains "STA activity is stale", both unchanged. --- .../Ipc/WorkerPipeSession.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs index d61287c..d52f8c4 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs @@ -1388,11 +1388,18 @@ public sealed class WorkerPipeSession // the StaHung fault just sent. Without this the heartbeat loop keeps // advertising a non-faulted state that contradicts the fault. _state = WorkerState.Faulted; + // WRK-34: CommandMethod is a method-name field, not a correlation-id field, so a + // consumer reading it (dashboard, gateway logs) shouldn't see a bare correlation id + // there. Use a stable label and carry the stuck command's correlation id (if any) in + // the diagnostic message instead. + string correlationSuffix = string.IsNullOrEmpty(snapshot.CurrentCommandCorrelationId) + ? string.Empty + : $" Stuck command correlation id: {snapshot.CurrentCommandCorrelationId}."; await TryWriteFaultAsync( CreateFault( WorkerFaultCategory.StaHung, - snapshot.CurrentCommandCorrelationId, - $"STA activity is stale by {staleFor}."), + "Watchdog", + $"STA activity is stale by {staleFor}.{correlationSuffix}"), cancellationToken).ConfigureAwait(false); } From f1353dda866a4f9fbaee1e3407fe8a4c7dc301bf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:21:16 -0400 Subject: [PATCH 5/5] chore(plans): mark batch F (WRK-32, WRK-33, WRK-34) done-pending-windev --- archreview/2026-08-19/remediation/00-tracking.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/archreview/2026-08-19/remediation/00-tracking.md b/archreview/2026-08-19/remediation/00-tracking.md index 1257225..5f8429e 100644 --- a/archreview/2026-08-19/remediation/00-tracking.md +++ b/archreview/2026-08-19/remediation/00-tracking.md @@ -18,7 +18,7 @@ This plan is executed as **seven parallel batches**, each by one subagent workin | 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 | +| 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 | Done-pending-windev | | 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):** @@ -66,6 +66,7 @@ This plan is executed as **seven parallel batches**, each by one subagent workin - 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. +- **Done-pending-windev** (`93edb1b` WRK-33, `20a3eef` WRK-32, `3679e59` WRK-34) on `fix/c3-worker-residuals`. WRK-32 applied `CaptureCompletionBaseline` (pump-then-snapshot) uniformly to all four write call sites (Write/Write2/WriteSecured/WriteSecured2), not just the two the finding cited as evidence, since the correlation gap is identical at all four. **Deferred**: the `mxaccess_gateway.proto` reply-field comment was not edited — a comment-only proto change still requires regenerating every generated artifact (C#, Go, Python, Java, Rust, protoset) per the repo rule, and this macOS box cannot verify all five client generator pins for a Low-severity doc-only change; the contract is documented in `docs/MxAccessWorkerInstanceDesign.md` instead. `NonWindows.slnx` builds clean (0 warnings/errors); worker/Worker.Tests unverified until windev runs `MxAccessCommandExecutorTests`/`WorkerPipeSessionTests`. ### 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.