mxaccessgw/archreview/2026-08-19/20-worker.md

# Worker Process — Architecture Review (cycle 3)

## Scope & method

Date 2026-08-19 · commit `8b3e3b3` (`main`, clean) · macOS static review — the worker targets .NET Framework 4.8 x86 and was **not built or tested** here; every claim cites file:line evidence read directly from the working tree at HEAD. Scope: `src/ZB.MOM.WW.MxGateway.Worker` (STA runtime/pump, `MxAccessSession`/`MxAccessStaSession` COM lifetime, `Ipc/` pipe session + frame reader/writer, event queue and DrainEvents, value/write-completion caches, alarm consumers incl. the subtag failover, watchdog/heartbeat, program lifecycle/exit codes) and `src/ZB.MOM.WW.MxGateway.Worker.Tests`. This is a verification pass over the 2026-07-12 review (`archreview/2026-07-12/20-worker.md`, remediation IDs WRK-21..28 in `archreview/2026-07-12/remediation/20-worker.md` / `00-tracking.md`, plus NEXT-04/NEXT-05 from `remediation/90-candidate-findings-next-cycle.md`), a re-check of the still-open cycle-1 findings, and a deep review of everything landed since `4f5371f` (~50 worker commits, 53 files, +9,014/−427).

Changed-in-scope since `4f5371f`: the WRK-21/28/23/IPC-30 drain cluster (`33ba612`, `6bc3f9b`), the WRK-22/24/25/27/IPC-26 write-seam batch (`8df35cd`), the OnWriteComplete correlation feature (`fc23a65`..`b0e65d4`), the perf wave (pooled frame buffers `f56798a`, reverse tag index `afec56d`, message-driven completion waits `dc9424d`, signal-driven event drain `f4b065b`, exact-format timestamp parse `94fdc18`/`25cbe5c`, enqueue-time frame sizing `58d97ad`, value-cache borrowing `9871d47`, class-transition flush `aac7957`, drain-pass unparking `9130994`), the NEXT-04/NEXT-05 abandonment-observation batch (`84dbf20`, `3ef56be`, `b8b7b69`), session-owned transport disposal (`e913dab`, labeled WRK-31 in code), the alarm-side truncation/failover hardening (`f3e1de5`, `b5ea6bb`, `693a78d`, `b9fb0dd`), and the WorkerPipeSessionTests harness rework (`7da52b6`, `462850a`, `aaeb86b`, `753d070`).

**Finding-ID note:** the tracker instruction says new IDs continue at WRK-29, but `e913dab` already consumed **WRK-31** as an interim ID (cited in code at `Ipc/WorkerPipeSession.cs:45,177` and in `Ipc/WorkerPipeSessionTests.cs`); WRK-29/WRK-30 appear nowhere in the repo or its history but may have been burned in the same interim numbering. To guarantee no reuse, this review's new findings start at **WRK-32**.

## Executive summary

- All eight cycle-2 Done claims (WRK-21..28) are present, sound, and tested at HEAD; none is a false Done and none carries a functional caveat. The WRK-21 fix went beyond the design: the drain sizes events at enqueue time (`58d97ad`) so the byte-budget walk no longer holds the queue lock the STA needs, and `6bc3f9b` made the budget monotonic at the reserve boundary so drain-until-empty terminates even at the 1024-byte negotiated floor.
- NEXT-04 (fault-observing continuations on abandoned/tombstoned frames) and NEXT-05 (lazy tombstone purge as a documented decision) are both verified — the observer discipline is applied uniformly in the writer (`ObserveAbandonedFault`/`ObserveFault`), and `e913dab`/`3ef56be` extended it to the abandoned net48 pipe read and overrun background tasks, with a GC-forcing `UnobservedTaskException` regression test.
- The two riskiest perf changes checked out. The value-cache borrow (`9871d47`) is safe because the post-publish hook runs only after `Enqueue` and every consumer of the borrowed instances is serialize-only (verified through `MxAccessSession.SucceededRead` and the write-completion cache, which deliberately still clones); the class-transition flush (`aac7957`) and drain-pass unparking (`9130994`) preserve the "written and flushed before completion" contract, are lost-wakeup-free (a walked-away lock acquisition is detached, drains, and releases — never dropped), and cannot double-release the semaphore.
- One cycle-1 finding was fixed incidentally: **WRK-13** (unpooled writer buffers) is resolved by `f56798a` — the writer now rents one combined prefix+payload buffer from `ArrayPool` (`Ipc/WorkerFrameWriter.cs:675-687`). The old tracker should be updated. WRK-18's stale-doc half is also discharged (the design doc now describes the implemented fail-fast and honestly names the generic exit code); its dedicated-exit-code half stays open.
- The remaining cycle-1 opens (WRK-02/03/05/08/09/10/14/16/17/18/19 and the WRK-02/03 slice of WRK-20) are all still present. WRK-05 is materially narrowed: in auto/failover mode `FailoverAlarmConsumer.RunPrimary` absorbs primary COM failures (`MxAccess/FailoverAlarmConsumer.cs:284-291`), but the default alarmmgr-only path is still one-poll-exception-fatal (`MxAccess/MxAccessStaSession.cs:392-405`).
- Fresh sweep found no Medium or higher. Three new Lows: recency-based OnWriteComplete correlation can attribute a stale completion from an earlier write to the current write's reply (WRK-32); the discarded command-task observer is the one discarded task in the worker without a fault-observing continuation (WRK-33); the StaHung fault writes the in-flight correlation id into `WorkerFault.CommandMethod` (WRK-34, pre-existing since 2026-04-26).

## Prior-finding verification — cycle 2 (all tracked Done) and NEXT items

| ID | Claimed status | Verified? | Evidence (file:line at HEAD) | Notes |
|----|----------------|-----------|------------------------------|-------|
| WRK-21 (= IPC-23) | Done | **Yes** | Byte-budgeted `Drain(uint, int)` dequeues only events known to fit, leaves a non-fitting head queued, reports `OversizedHeadSequence` (`MxAccess/MxAccessEventQueue.cs:389-429`); per-event size memoized at enqueue outside the drain's lock hold (`:248-281`, `:502-509`, slack constants `:73-81`); reply builder consumes it with truncation `DiagnosticMessage` (`Ipc/WorkerPipeSession.cs:978-1020`); budget = `max(MaxMessageBytes − 64 KiB, MaxMessageBytes/2)` so it stays monotonic at the reserve boundary (`:1034-1039`, `6bc3f9b`); shared result type is net48-safe (`MxAccess/WorkerEventDrainResult.cs`). Backstops: control-reply write catches `MessageTooLarge` and answers with a small `InvalidRequest` reply (`Ipc/WorkerPipeSession.cs:847-864`), STA command replies get the same catch (`:1136-1147`), and the fallback write itself is size-guarded so a degenerate negotiated max cannot make the backstop fatal (`:876-900`). Tests: `WorkerPipeSessionTests.DrainEvents_ByteHeavyQueue_ReplyIsBoundedAndSessionSurvives` (:571), `DrainEvents_RepeatedCalls_RecoverAllEventsWithoutLoss` (:630), `ControlReplyTooLarge_WritesErrorReplyInsteadOfDying` (:770), `CommandReplyTooLarge_WritesErrorReplyInsteadOfFaulting` (:815), plus `MxAccessEventQueueTests` byte-budget cases. | Sound, and stronger than designed (no sizing walk under the STA-shared lock). No diagnostics command is session-fatal on size anymore. |
| WRK-22 (= IPC-26) | Done | **Yes** | `PendingFrame.Claimed` interlock documented and mutated only under `_gate` (`Ipc/WorkerFrameWriter.cs:54-62`); cancelled waiter tombstones only unclaimed frames (`:169-183`, `:308-338`); `DequeueNext` skips cancelled frames and claims atomically in the same critical section (`:584-618`); contract documented in the method doc (`:105-112`) and `docs/WorkerFrameProtocol.md:203-211`. Tests `WorkerFrameProtocolTests.WriteAsync_CancelledWhileWaitingForLock_FrameIsNeverWritten` (:695), `WriteAsync_CancelledEventFrame_DoesNotTrailShutdownAck` (:800). | Sound. The claimed-mid-write residual window is documented as by-design. |
| WRK-23 | Done | **Yes** | Peek-stamp-commit: candidate stamped before the size checks (sequence participates in `CalculateSize`), counter committed only once the frame is certain to be written, so a per-frame rejection burns no number (`Ipc/WorkerFrameWriter.cs:648-666`; comment `:72-76`). | Wire sequence stays contiguous across rejections; the gap-free concurrent-sequence test's guarantee is now genuinely true. |
| WRK-24 | Done | **Yes** | `MinNegotiableFrameBytes = 1024` matching the gateway validation floor, rejected at the handshake with `InvalidConfiguration` (`Ipc/WorkerFrameProtocolOptions.cs:30`, `:144-150`); range documented in the property doc and `docs/WorkerFrameProtocol.md`. | Also closes the degenerate config that could have made WRK-21's fallback swallow reachable (`Ipc/WorkerPipeSession.cs:866-875` documents that chain). |
| WRK-25 | Done | **Yes** | `WriteBatchAsync` enqueues the whole batch under one `_gate` acquisition, drains under one lock, awaits every completion so no per-frame rejection faults unobserved, rethrows the first failure in batch order (`Ipc/WorkerFrameWriter.cs:236-306`); the event drain loop submits drained batches through it (`Ipc/WorkerPipeSession.cs:615-630`). Tests `WriteBatchAsync_FlushesOnceAndPreservesOrder` (:839), `WriteBatchAsync_ControlFrameQueuedDuringBatch_JumpsRemainingEvents` (:876), `WorkerPipeSessionTests.EventBurst_DrainLoopCoalescesFlushes` (deflaked `50322ba`). | The WRK-12 coalescing now engages on the exact path it was built for: an uninterrupted N-event burst pays one flush. `aac7957` later refined the mixed-class case (see deep review). |
| WRK-26 (discharges IPC-29) | Done | **Yes** | `docs/MxAccessWorkerInstanceDesign.md:717-751` — Outbound Queues rewritten to the two-class scheduler, explicitly records the accepted collapse of the five-level order and why, and rewrites the overflow paragraph to the implemented fail-fast (naming the still-generic `UnexpectedFailure` exit code honestly — WRK-18's exit-code half remains tracked); `docs/WorkerFrameProtocol.md:79+` "Write Scheduling And Sequencing" covers priority classes, write-time stamping, per-frame vs stream-failure semantics, flush coalescing, and cancellation. | No stale five-level prose remains. |
| WRK-27 | Done | **Yes** | `volatile bool staAlarmPollInProgress` flipped on the STA exactly around `PollOnce` (`MxAccess/MxAccessStaSession.cs:64-70`, `:352-365`); surfaced via `WorkerRuntimeHeartbeatSnapshot.StaCallInProgress` (`MxAccess/WorkerRuntimeHeartbeatSnapshot.cs`, ctor default keeps net48-safe plain ctor); watchdog suppression honors it alongside the in-flight correlation id up to the ceiling (`Ipc/WorkerPipeSession.cs:1359-1370`). Test `WorkerPipeSessionTests.Watchdog_StaCallInProgress_SuppressedUntilCeiling` (:1260). | A slow-but-healthy poll now gets the 75 s ceiling, not the 15 s grace; a poll blocking past the ceiling still faults, per the ceiling's contract. |
| WRK-28 | Done | **Yes** | `GatewayContractInfo.MaxDrainEventsPerCommand = 10_000` in Contracts (`src/ZB.MOM.WW.MxGateway.Contracts/GatewayContractInfo.cs:33`), consumed by the gateway validator (`src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcRequestValidator.cs:82-86`) and the worker clamp (`Ipc/WorkerPipeSession.cs:992-994`). | Single shared constant; the desync failure mode is removed, not merely detected. |
| NEXT-04 | Resolved 2026-08-10 | **Yes** | Unconditional fault-observing continuation on every frame a cancelled caller abandons — `TombstoneIfUnclaimed`/`TombstoneUnclaimed` → `ObserveAbandonedFault` → `ObserveFault` (`Ipc/WorkerFrameWriter.cs:308-372`); a cancelled task never fires `OnlyOnFaulted`, so the unconditional attach is safe, and it runs outside `_gate` because an already-faulted task continues inline. Also applied to the detached lock wait (`:389-426`) and, session-side, to overrun background tasks and the abandoned pipe read (`Ipc/WorkerPipeSession.cs:521-576`). Regression test drives the exact abandonment and asserts a marker exception never reaches `TaskScheduler.UnobservedTaskException` (`WorkerFrameProtocolTests:733-785`). | Verified; the discipline is now near-uniform (the one residual gap is WRK-33). |
| NEXT-05 | Resolved as documented decision | **Yes** | `DequeueNext` still lazily skips tombstones (`Ipc/WorkerFrameWriter.cs:588-618`); the rationale — any subsequent write drains both queues, the heartbeat bounds tombstone residency to one interval, and eager `Queue<T>` rebuilds would add ordering-invariant surface next to the WRK-22 interlock — is recorded in `docs/WorkerFrameProtocol.md:218-221`. | Decision recorded where the review asked for it. |

Done claims that failed verification: **none**. Regressions in previously-verified fixes: **none** — WRK-01 (pump refresh, `Sta/StaRuntime.cs:95-100`), WRK-04/GWC-28 (write-time sequence stamping, now peek-stamp-commit), WRK-06 (converter cache, extended by `94fdc18`'s compiled accessors), WRK-07 (two-class scheduler), WRK-11 (ownership transfer, now load-bearing for both the value-cache borrow and enqueue-time sizing), WRK-12 (flush coalescing, finally effective via WRK-25), and WRK-15 (doc names) all remain intact at HEAD.

## Prior-finding verification — still-open cycle-1 items

| ID | Status at HEAD | Evidence | Notes |
|----|----------------|----------|-------|
| WRK-02 | **Still open** | `Sta/StaRuntime.cs:265-269` (loop exception stored in write-only `startupException` after startup; only `startedEvent.Set()`); `InvokeAsync` (`:161-189`) checks disposal and shutdown but not thread death — work enqueued after `ThreadMain` unwinds is consumed by nobody and its task hangs forever (`CancelQueuedCommands` in the `finally` at `:272` only drains what was queued at death). | Highest-value remaining worker item, unchanged three cycles running. |
| WRK-03 | **Still open** | `Ipc/WorkerPipeSession.cs:1226-1229` — `TryStartCommandTask` silently `return`s when `_acceptingCommands` is false: no reply, no log. | The gateway's correlation wait times out blind. |
| WRK-05 | **Still open, narrowed** | `MxAccess/MxAccessStaSession.cs:392-405` — any exception from one poll records a fault and stops the loop; the drain loop turns it into session termination (`Ipc/WorkerPipeSession.cs:589-598`). Narrowed: in auto mode the `FailoverAlarmConsumer` absorbs primary COM failures inside `PollOnce` (`MxAccess/FailoverAlarmConsumer.cs:284-291`, catches all but OOM and counts toward failover), so a transient alarmmgr E_FAIL no longer escapes. The default alarmmgr-only mode (`Ipc/WorkerPipeSession.cs:78`, and subtag-source failures in standby mode) is still one-failure-fatal. | Worth re-scoping to "non-failover alarm modes" if kept. |
| WRK-08 | **Still open** | `Ipc/WorkerPipeSession.cs:487-491` — the message-loop `finally` cancels the drain loop after the shutdown ack; events already drained from the queue (`:600`) whose batch write is cancelled are tombstoned and lost, and the residual queue is never shipped. `docs/MxAccessWorkerInstanceDesign.md` still documents no discard (`grep residual/unshipped` finds nothing). | Same scale as before the batch change (the old loop also drained 128 ahead of writing). Documentation half remains the cheap part. |
| WRK-09 | **Still open** | No `AppDomain.UnhandledException`/`TaskScheduler.UnobservedTaskException` handler anywhere under `src/ZB.MOM.WW.MxGateway.Worker/` (grep at HEAD; the only mentions are comments noting the absence). | The NEXT-04 observer network reduces the practical exposure but is not a process-level backstop. |
| WRK-10 | **Still open** | `WorkerApplication.cs:112-139` — all three catch blocks log `exception_type` only; the redactor (`Bootstrap/WorkerLogRedactor.cs`) is available and unused here. | Still a cheap diagnostics win. |
| WRK-13 | **Resolved (side effect of `f56798a`)** | `Ipc/WorkerFrameWriter.cs:675-687` — single `ArrayPool<byte>.Shared.Rent` for prefix+payload, one stream write, returned in `finally`; matches the reader's pooling. | Flip to Resolved in `archreview/remediation/00-tracking.md` with commit `f56798a`. |
| WRK-14 | **Still open** | `Ipc/WorkerPipeSession.cs:33-62` `_camelCase` vs `MxAccess/MxAccessStaSession.cs:50-71` and `Sta/StaRuntime.cs:10-24` bare `camelCase`. | Cosmetic; new files followed their host file's convention, so the split persists. |
| WRK-16 | **Still open** | `Ipc/WorkerPipeSession.cs:1449-1544` — seven `CreateEnvelope`/`CreateBaseEnvelope` pairs; `Ipc/WorkerPipeClient.cs` still has 7 public constructors. | |
| WRK-17 | **Still open** | `WorkerApplication.cs:110-119` maps every `WorkerFrameProtocolException` (including `EndOfStream`) to exit 6 `ProtocolViolation`. | |
| WRK-18 | **Half discharged** | Doc half done: `docs/MxAccessWorkerInstanceDesign.md:743-751` now describes the implemented fail-fast and names the generic `UnexpectedFailure` exit honestly. Code half open: a drain-loop fault still exits 1 via the generic catch (`WorkerApplication.cs:131-139`); no dedicated overflow/event-fault exit code. | |
| WRK-19 | **Still open** | `Sta/StaCommandDispatcher.cs` — no logger field, no start/end/failure log anywhere in the file. | |
| WRK-20 | **Residual only** | The WRK-21..28 batch added its full test matrix (byte-budget drain, size backstops, cancellation/tombstoning, batch flush, floor handshake, watchdog suppression), plus teardown `UnobservedTaskException` tests (`WorkerPipeSessionTests:2328-2434`). Still missing: STA-thread-death (WRK-02) and command-refused-during-shutdown (WRK-03) tests, because their fixes are open. | |

## Deep review of the new code

**Frame-writer unparking (`9130994`) and class-transition flush (`aac7957`).** `WriteAsync` now races its frame's completion against the lock acquisition (`Ipc/WorkerFrameWriter.cs:148-198`): a caller whose frame is delivered by another drainer returns at the boundary flush instead of at the winner's release, and the acquisition it walks away from is handed to `DetachLockWait` (`:389-399`), whose continuation either observes a cancelled wait (no count taken, nothing to release) or drains-and-releases (`:409-452`). I probed this seam hard: acquisition and cancellation are mutually exclusive in `SemaphoreSlim`, so no count can leak and no double-release path exists (`DrainDetachedAsync` releases in a `finally` that runs exactly once per successful acquisition); a frame enqueued between a drainer's last `DequeueNext` and its release is picked up by the detached pass, so no lost wakeup; every discarded task in the chain carries the NEXT-04 observer. The drain flushes and completes at each control→event boundary (`:484-511`), so a heartbeat/reply/fault's delivery point is never charged for the event backlog behind it; frames completed at a boundary leave `written`, so a later flush failure in the same pass cannot reach back and fail an already-delivered frame (`:543-557`) — the failure-attribution edges are all correct. Cost claims in the comments are accurate: a pure-event pass still pays one flush; only a genuinely mixed pass pays one extra per class transition. Residual (accepted, documented at `Ipc/WorkerPipeSession.cs:186-191`): a detached acquisition can schedule an empty drain pass after session teardown — harmless because the queues are empty and an empty pass writes nothing.

**Value-cache borrowing (`9871d47`).** The hunt for aliasing bugs came up clean. The borrow is sound on three legs, each verified: (1) ordering — `MxAccessBaseEventSink.EnqueueEvent` runs the `postPublish` cache write only after `eventQueue.Enqueue` returns (`MxAccess/MxAccessBaseEventSink.cs:248-295`), so the event is already write-once (WRK-11 ownership plus the sequence/timestamp stamps under the queue lock); (2) consumers — `MxAccessSession.SucceededRead` copies the borrowed `Statuses` element references into a fresh `RepeatedField` on the reply and reference-shares `Value`/`SourceTimestamp` (`MxAccess/MxAccessSession.cs:1066-1097`), and everything downstream only serializes (the worker↔gateway process boundary prevents any other consumer from aliasing); (3) the size-memoization coupling — a post-enqueue mutation would now also invalidate `QueuedEvent.Size`, and the ownership contract text in both classes names that hazard explicitly (`MxAccess/MxAccessValueCache.cs:76-108`, `MxAccess/MxAccessEventQueue.cs:15-32`). The write-completion cache deliberately still clones and documents why the asymmetry is not an oversight (`MxAccess/MxAccessWriteCompletionCache.cs:77-89`). Protobuf reads (`CalculateSize`/`WriteTo`) from multiple threads on a frozen message are safe, and every cross-thread hand-off passes through a lock.

**Enqueue-time frame sizing (`58d97ad`).** `CalculateWorkerEventSize` reconstructs the generated `CalculateSize` arithmetic for exactly the two stamped fields (`MxAccess/MxAccessEventQueue.cs:493-509`); I checked the tag math — `worker_sequence` (field 9, varint) and `worker_timestamp` (field 10, length-delimited) are both single-byte tags, the `WorkerEvent.event` wrapper (field 1) matches the `DrainEventsReply.events` packing shape exactly, and the +8 `RepeatedFieldOverheadBytes` slack absorbs the documented renumbering hazard (`:66-81`). Clearing the two fields before measurement makes the pre-lock measurement exact even against a caller that pre-filled them. The wake signal (`SignalWake`, `:471-491`) is a level, not a count; I walked the check-then-release race against a concurrent consumer and every interleaving either sees the event in its re-drain or leaves a pending permit — no lost wake, and the 25 ms fallback tick bounds any hypothetical future unsignalled mutation.

**Session-owned transport disposal and abandonment observation (`e913dab`, `3ef56be`, WRK-31).** The session now owns the pipe stream and disposes it as the last teardown step — the only thing that unblocks a net48 pipe read parked in the kernel — then awaits the abandoned read's resulting fault (`Ipc/WorkerPipeSession.cs:143-290`). The `_pendingReadTask` bookkeeping preserves the reader's single-consumer invariant through teardown (set at issue, cleared before the loop's own await, read only after the loop's task completes — the await supplies the happens-before), and two `Debug.Assert`s pin it (`:447-449`, `:470-472`). `ObserveBackgroundTaskStopAsync` correctly separates the logging budget from observation: an overrun task gets `ObserveFaultWhenever` before the method returns (`:521-558`), so the 1 s budget is a diagnostics decision, not a correctness one. `DisposeTransportStream`'s total catch is well argued (nothing thrown while closing a handle may displace the session's terminal exception, `:212-250`), and `WorkerPipeClient` keeps its `using` as an explicitly-documented idempotent backstop (`Ipc/WorkerPipeClient.cs:153-165`).

**Oversized-event death (IPC-30) through the batch path.** A `MessageTooLarge` surfacing from the batch's awaited completions is mapped back to the first envelope in batch order that overshoots the negotiated max, logged with identity-but-never-value fields, written as a structured `ProtocolViolation` fault, and then thrown (`Ipc/WorkerPipeSession.cs:626-725`). `FindOversizedEvent`'s recomputation is consistent with what the writer rejected (the rejected envelope retains its tentative sequence stamp, so `CalculateSize` agrees), and the unreachable-fallback comment is honest. Other per-frame rejection codes still unwind the session as worker bugs — the deliberate IPC-30 scope.

**Write-completion correlation (`fc23a65`..`b0e65d4`).** Parity-clean in the load-bearing respects: no synthesized events (the completion rides the unary reply's `statuses` only), acceptance-only `Hresult`/`ProtocolStatus` either way, expiry yields empty statuses ("unconfirmed, not failed"), zero timeout restores pure fire-and-forget, and the pump-wait keeps the STA dispatching for the whole wait via the shared `MsgWaitForMultipleObjectsEx` declaration (`Sta/StaNativeMethods.cs`, single-copy rationale documented). `StaWaitHelper` handles the failed-wait degrade (one-shot trace + blind sleep, `Sta/StaWaitHelper.cs:104-112`) and documents the `MWMO_INPUTAVAILABLE` drain-to-empty contract that would otherwise busy-spin. The one soft spot is attribution across successive writes to the same handle — WRK-32 below.

**Alarm subtag failover and truncation.** The production wiring works despite the `standbyFactory: null` default: the `comFactory` threaded through `MxAccessStaSession.StartAsync` lets `AlarmCommandHandler.BuildStandby` construct the `SubtagAlarmConsumer` over `LmxSubtagAlarmSource` (`MxAccess/AlarmCommandHandler.cs:206-248`); mode selection (alarmmgr / subtag / auto-failover) matches the documented matrix, provider-mode changes are surfaced as queue events with handler detach on unsubscribe/dispose (`:159-183`, `:258-267`). WnWrap truncation handling is careful where it matters: a truncated fetch merges into the retained snapshot instead of clearing it (`ApplySnapshotUpdate`), so Clear transitions are never synthesized from records that merely fell off a truncated fetch, and the truncation verdict rides out of the same locked fold that produced the transitions (`FoldFetch`), with a rate-limited warning. STA affinity for the wnwrap COM object is asserted at every consumer entry (`MxAccess/MxAccessStaSession.cs:410-440`), and the affinity-violation exception is deliberately routed to the fault path rather than the graceful-stop path (`:378-405`).

**Handle-registry reverse index (`afec56d`).** Every mutation maintains all four secondary indexes and the three memoized views; I walked the consistency edges — re-registration with a changed definition drops the stale index entry first, per-server teardown removes exactly the owned sets, advice de-indexing trims empty containers, and the sorted-insertion keeps `GetItemHandlesForDefinition` in the same order the old linear scan visited (`MxAccess/MxAccessHandleRegistry.cs`). The registry is STA-confined by documented contract, so the lock-free indexes are sound. The returned live-view caveat is documented at the API.

**Timestamp fast path (`94fdc18`, `25cbe5c`).** The per-culture derived formats are validated by round-tripping a probe instant before they ever reach the event path, so a pathological culture pattern (quoted `ss`, existing fraction) degrades to no-fast-path instead of a throwing `TryParseExact`; the fast path is provably a shortcut, never a different answer, because the formats are derived from the same culture the general parse would use (`MxAccess/MxAccessEventMapper.cs:373-509`).

**Test harness rework.** `FakeRuntimeSession` now mirrors production's coalesced wake signal (never-disposed `SemaphoreSlim`, same rationale comment), models STA-pump-at-heartbeat (`refreshStaActivityOnCapture`), and its 30 s dispatch safety net is decoupled from scenario timing with the reasoning written down (`TestSupport/FakeRuntimeSession.cs:23-51`). The teardown tests arm `TaskScheduler.UnobservedTaskException` and force GC — the strongest possible assertion shape for the NEXT-04/WRK-31 class of fix. Test counts in scope: 40 facts/theories in `WorkerPipeSessionTests`, 31 in `WorkerFrameProtocolTests`, 23 in `MxAccessEventQueueTests`, plus new suites for the completion cache, wait helper, registry, and WnWrap XML parsing.

## New findings

**WRK-32 — Low (error handling / documentation).** Write-completion correlation is recency-based per handle; a late `OnWriteComplete` from an earlier write can be reported as the current write's outcome.
Evidence: the baseline is `CurrentVersion` captured before the COM call (`MxAccess/MxAccessCommandExecutor.cs:447-450`, `:522-528`), and `AwaitWriteCompletion` copies onto the reply *any* completion whose version exceeds it (`:1005-1027`; version test only, `MxAccess/MxAccessWriteCompletionCache.cs:162-169`). MXAccess's `OnWriteComplete` carries no correlation token, only the handle pair — so if write A to a tag times out unconfirmed (its callback still undelivered) and the client immediately writes B to the same tag, A's stale callback dispatching during B's pump-wait bumps the version past B's baseline and A's status rows ride B's reply. The executor doc comment claims "the OnWriteComplete row *matching* a WriteSecured/WriteSecured2 call" (`:996-1003`), and neither `docs/MxAccessWorkerInstanceDesign.md` nor the proto comments mention the ambiguity (grep for misattribution/stale/earlier-write finds nothing). Impact is bounded — the statuses are an enrichment field, `Hresult`/`ProtocolStatus` stay acceptance-only, and back-to-back same-tag writes with a timed-out predecessor is the only exposed sequence — and the ambiguity is inherent to the COM surface (a native client correlating by handle has the same problem, so mechanical parity is arguably preserved).
Recommendation: document the recency-based contract at the executor, in the design doc's write-completion section, and in the reply-field proto comment; additionally, run one `pumpStep()` immediately before capturing the baseline so an already-queued stale callback is folded into the baseline instead of the wait window. Full closure is impossible without an MXAccess-side token — say so where consumers will read it.

**WRK-33 — Low (concurrency / NEXT-04 discipline gap).** The discarded command-task observer is the one discarded task in the worker without a fault-observing continuation.
Evidence: `TryStartCommandTask` discards its wrapper — `_ = ObserveCommandTaskAsync(commandTask);` (`Ipc/WorkerPipeSession.cs:1235`) — and `ObserveCommandTaskAsync` catches only `OperationCanceledException` (`:1238-1254`). `ProcessCommandAsync` handles its own failures, but its catch path ends in `TryWriteFaultAsync`, whose filter is exactly `IOException`/`ObjectDisposedException`/`WorkerFrameProtocolException` (`:1440-1447`); any other exception escaping that write — or a future edit anywhere in the catch path — propagates out of `ProcessCommandAsync`, faults the discarded wrapper, and surfaces only as a finalizer-time unobserved-task exception in a process that installs no handler (WRK-09). Every other discarded task in this codebase now carries an observer (`ObserveFaultWhenever` lives 660 lines up in this same file); the gap is unlikely to fire but contradicts the discipline the NEXT-04/WRK-31 batch established and tested.
Recommendation: `ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask));`, or broaden `ObserveCommandTaskAsync` to catch-all-and-log (the redaction-safe fields are already at hand in `LogCommandResultDropped`'s shape).

**WRK-34 — Low (documentation / diagnostics).** The StaHung fault writes the in-flight correlation id into `WorkerFault.CommandMethod`.
Evidence: `ReportWatchdogFaultIfNeededAsync` builds its fault with `CreateFault(WorkerFaultCategory.StaHung, snapshot.CurrentCommandCorrelationId, $"STA activity is stale by {staleFor}.")` (`Ipc/WorkerPipeSession.cs:1384-1389`), whose second parameter is `commandMethod` (`:1712-1728`) — so the wire fault's `command_method` field carries a correlation id (or empty), a correlation id never lands in a correlation-shaped field, and a gateway/dashboard consumer reading `CommandMethod` as a method name gets an opaque id string. Pre-existing since `4a3560c` (2026-04-26), never previously flagged; noted now because the WRK-27 work touched this exact branch and the field's semantics matter more once polls also suppress here.
Recommendation: set `CommandMethod` to a stable label (empty, or `"Watchdog"`), and carry the stuck command's correlation id in the fault's diagnostic message or the envelope's `CorrelationId` where the gateway already correlates.

## Severity roll-up (new findings)

| Severity | Count | IDs |
|----------|-------|-----|
| High | 0 | — |
| Medium | 0 | — |
| Low | 3 | WRK-32, WRK-33, WRK-34 |

## Still good / preserved

Every structural strength recorded in the prior two reviews is intact at HEAD, and several were deepened. The STA core still runs the canonical `MsgWaitForMultipleObjectsEx` + `PeekMessage`/`DispatchMessage` loop with the wake event and 50 ms idle pump (`Sta/StaRuntime.cs:255-261`), and the subtle Win32 wait contract now has a single shared declaration with the drift hazard written down (`Sta/StaNativeMethods.cs`). All MXAccess COM calls remain on the dedicated STA (commands via the dispatcher, alarm polls via `InvokeAsync` with an affinity assertion, the consumer factory on-thread at `MxAccess/MxAccessStaSession.cs:282-296`); COM lifetime discipline (`FinalReleaseComObject`, sink-detach-before-release, UnAdvise → RemoveItem → Unregister, alarm handler disposed on the STA before data-side teardown) is unchanged. Nothing synthesizes events — the completion-correlation feature explicitly kept `OperationComplete` untouchable and the truncation work explicitly refuses to fabricate Clear transitions. Fail-fast backpressure stands (no drop policy; overflow self-records the first fault; the dispatcher's 128-entry bound rejects loudly). Credentials and values stay out of logs (the oversized-event fault logs sizes and identity only, `AuthenticateUser` passes the password straight through, the DrainEvents truncation message names sequences not values). net48 constraints are respected in every new type (plain ctors, get-only properties, `readonly struct`, no init/records — several carry the CS0518 rationale in their remarks). The IPC layer's failure-attribution story is now the best part of the worker: written-and-flushed-before-completion, per-frame vs stream-failure separation, boundary-flush completion, tombstone/claim interlock, and a fault observer on effectively every abandoned task, each with a regression test that asserts the invariant rather than the implementation. Comment quality across the new code is exceptional — ownership contracts, memory-model reasoning, and rejected alternatives are recorded at the exact seams where the next editor would otherwise re-litigate them.

## Verification needed on Windows (windev)

- This review is static: the x86 worker was not built and `Worker.Tests` were not run here. The tracking log and repo memory record the worker suite fully green on windev after the harness fixes (`7da52b6`/`462850a`/`aaeb86b`), and the WRK-21 cluster carries windev evidence (`c992568`); any fix for WRK-32..34 needs an x86 build plus filtered `MxAccessCommandExecutorTests`/`WorkerPipeSessionTests` runs there.
- WRK-32's pre-baseline pump mitigation should get a windev test that records a stale completion into the fake's message path before the second write's baseline and asserts it does not ride the second reply.
- WRK-33 is assertable with the existing `UnobservedTaskException`-arming test shape (`WorkerPipeSessionTests:2328-2434`): drive an exception through `TryWriteFaultAsync`'s filter gap with a throwing fake writer and assert nothing reaches the finalizer.
- Live MXAccess only: the WRK-05 narrowing (failover absorbing a transient alarmmgr E_FAIL without a session fault) deserves a live-rig confirmation next time the opt-in alarm smoke runs; the rig's object-driven alarms make this drivable only from the simulator side.