Files
mxaccessgw/docs/plans/2026-08-15-perf-review-remediation.md
T
2026-08-15 12:01:18 -04:00

625 lines
47 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Performance Review Remediation Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task (or superpowers-extended-cc:subagent-driven-development when executing in-session).
**Goal:** Resolve every actionable finding from the 2026-08-15 architectural performance review — six High findings, the Medium tier, and the worthwhile Low/hygiene items — without changing any MXAccess parity behavior or public contract.
**Architecture:** Two phases. Phase A is gateway-side (.NET 10, builds and tests locally on macOS via `NonWindows.slnx`); Phase B is worker-side (.NET Framework 4.8 x86, which does **not** compile on this Mac — Phase B tasks are edited here and verified in one consolidated pass on the windev box via the `psbridge` skill, Task 24). No `.proto` changes anywhere in this plan, so no client regeneration is needed. All work happens on branch `perf/review-remediation`.
**Tech Stack:** ASP.NET Core gRPC, System.Threading.Channels, SignalR, Microsoft.Data.Sqlite, .NET Framework 4.8 STA/COM interop, protobuf (Google.Protobuf).
---
## Ground rules for every implementer (read before your task)
- **Build gate:** `TreatWarningsAsErrors=true`, `Nullable=enable`, analyzers at latest. New warnings fail the build — fix them, never suppress.
- **Style:** follow `docs/style-guides/CSharpStyleGuide.md` — file-scoped namespaces, `sealed` by default, `Async` suffix, MXAccess-aligned names. Match the comment density and idiom of the file you're editing.
- **Parity is sacred:** do not change MXAccess-visible semantics (event ordering, `OperationComplete` behavior, write-completion reply shape, per-tag ReadBulk timeout meaning). These tasks change *mechanics* (waits, locks, allocations), never observable protocol behavior, except where a task explicitly says otherwise.
- **Never synthesize events.** Nothing in this plan may fabricate an `MxEvent`.
- **Docs in the same commit:** when a task changes configuration, event mechanics, security behavior, or lifecycle rules, the named docs must be updated in that task's commit.
- **Worker code (Phase B) does not compile on this machine.** `LangVersion=latest` applies, so modern syntax is fine, but only net48-era BCL APIs exist (no `Span`-taking stream overloads, no `ArgumentNullException.ThrowIfNull` — check what the file already uses). Match the existing worker idioms exactly. Verification is Task 24.
- **Tests:** gateway tests use the FakeWorkerHarness (`src/ZB.MOM.WW.MxGateway.Tests`), no MXAccess needed. Run only your task's filter, not the full suite (full suite runs once per phase).
- **Commit after every task**, message style: `perf(<area>): <what>` (or `fix(...)` for the two correctness bugs).
Verification commands used throughout:
```bash
# Gateway build (macOS-safe)
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
# Targeted gateway tests
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~<TestClass>"
```
---
# Phase A — Gateway (local verification)
### Task 1: Named-pipe buffer sizes
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Tasks 2, 3, 4, 5, 6, 8, 10, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionWorkerClientFactory.cs` (`CreatePipe`, ~line 157)
- Modify: `docs/WorkerFrameProtocol.md` (add a short "Pipe buffers" note)
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/` (existing factory/e2e tests must stay green; no new test — buffer size isn't observable through the .NET API)
**Why:** the current 5-arg `NamedPipeServerStream` overload passes `inBufferSize: 0, outBufferSize: 0`. A zero-quota byte-mode pipe forces every write to rendezvous with a pending read — lock-step IPC, and the exact failure class behind the historical windev suite wedge.
**Step 1: Change the overload**
```csharp
private const int PipeBufferSizeBytes = 128 * 1024;
private static NamedPipeServerStream CreatePipe(string pipeName)
{
return new NamedPipeServerStream(
pipeName,
PipeDirection.InOut,
maxNumberOfServerInstances: 1,
PipeTransmissionMode.Byte,
PipeOptions.Asynchronous,
inBufferSize: PipeBufferSizeBytes,
outBufferSize: PipeBufferSizeBytes);
}
```
Add a comment stating *why* (zero-quota rendezvous behavior; reference the windev wedge). Note: on Unix these sizes are advisory (Unix domain socket), which is fine — the fix targets Windows production.
**Step 2:** `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` → 0 errors.
**Step 3:** `dotnet test ... --filter "FullyQualifiedName~GatewayEndToEndFakeWorkerSmokeTests"` → PASS.
**Step 4:** Update `docs/WorkerFrameProtocol.md` with a 34 line "Pipe buffers" paragraph. Commit: `perf(ipc): give worker pipes real OS buffers instead of zero-quota rendezvous`
---
### Task 2: Metrics — pull-gauge for worker queue depth, lock-free command counters
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 1, 3, 4, 5, 6, 8, 10, 11, 12, 13, 14 (NOT Task 7 — both edit `WorkerClient.cs`)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs` (`SetWorkerEventQueueDepth` ~290; `CommandStarted/Succeeded/Failed` ~202247; gauge wiring ~91; snapshot ~461492)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs` (call sites ~303 and ~602)
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Metrics/` (extend the existing GatewayMetrics test class)
**Why:** `SetWorkerEventQueueDepth` takes the process-wide `_syncRoot` twice per event for every session, and the single scalar makes the gauge last-writer-wins across sessions (a correctness bug). The command counters take the same global lock 23× per RPC.
**Step 1 (failing test):** add a test that registers two worker-queue-depth sources reporting 3 and 4 and asserts the snapshot/gauge reports 7; add a test that `CommandStarted`×N from parallel tasks yields exactly N with no lock (behavioral: just correctness of count).
**Step 2 (implement):**
- Mirror the existing GWC-15 pattern verbatim: add `RegisterWorkerEventQueueDepthSource(Func<int> depth)` returning an `IDisposable` handle, a `ConcurrentDictionary<long, Func<int>>` of sources, and make `GetWorkerEventQueueDepth` sum the sources (clamp negatives). Delete `SetWorkerEventQueueDepth` and the `_workerEventQueueDepth` field.
- `WorkerClient`: at construction (or first use), register a source returning its staged+channel depth via `Volatile.Read` of a field the stage/consume paths maintain with `Interlocked` — the hot path does **no** metrics call at all anymore. Dispose the registration in `DisposeAsync`.
- Command counters: `_commandsStarted/_commandsSucceeded/_commandsFailed` become `long` updated with `Interlocked.Increment`; `_commandFailuresByMethod` becomes `ConcurrentDictionary<string, long>` (follow the existing `EventReceived` pattern in the same file). Snapshot reads with `Interlocked.Read`.
**Step 3:** run the Metrics test filter → PASS. **Step 4:** grep the repo for `SetWorkerEventQueueDepth` — zero hits outside tests you updated.
**Step 5:** Commit: `perf(metrics): pull-model worker queue gauge (fixes last-writer-wins), Interlocked command counters`
---
### Task 3: Distributor — copy-on-write subscriber snapshot
**Classification:** high-risk (core event fan-out concurrency)
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 1, 2, 4, 5, 6, 8, 10, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs` (pump loop ~600; register/unregister paths; the "snapshot-free enumerator" remark ~71)
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/` (existing SessionEventDistributor tests must stay green; add one test if a register-during-pump race test doesn't already exist)
**Why:** `_subscribers.Values` (the property) locks the whole `ConcurrentDictionary` and materializes a snapshot list **per event**, contradicting the adjacent comment.
**Step 1 (implement):** maintain a `volatile Subscriber[] _subscriberSnapshot` rebuilt inside the existing registration lock on every register/unregister (the set is tiny and mutates rarely). The pump iterates the array. Keep the dictionary if other paths use keyed lookup; the array is purely the fan-out view. Update the ~71 remark to describe the actual mechanism. Semantics to preserve exactly: a subscriber registered mid-iteration may miss the in-flight event ("late subscribers see events after they register") — the array snapshot preserves this naturally.
**Step 2:** run the distributor/replay test filters (`FullyQualifiedName~SessionEventDistributor`, `~Replay`) → PASS. The replay-handoff atomicity tests are the critical gate here.
**Step 3:** Commit: `perf(events): copy-on-write subscriber snapshot in fan-out pump`
---
### Task 4: Dashboard event mirror — viewer gating
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 1, 2, 3, 5, 6, 8, 10, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs`
- Create: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs` (Publish, ~39)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs` (register the registry)
- Modify: `docs/GatewayDashboardDesign.md` (mirror gating paragraph)
- Test: create `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/EventsHubViewerRegistryTests.cs` + extend the existing DashboardEventBroadcaster tests
**Why:** with `ShowTagValues=false` (default), `Publish` deep-clones every event and dispatches to a SignalR group that is empty in the steady state. No viewer gate exists anywhere on the path.
**Step 1 (failing test):** broadcaster with zero registered viewers for the session performs **no clone and no send** (assert via a counting fake hub-clients/`IHubContext` seam, matching however the existing broadcaster tests fake SignalR); with one viewer, behavior is unchanged (redacted clone sent).
**Step 2 (implement):**
- `EventsHubViewerRegistry` (singleton): `ConcurrentDictionary<string, int>` session→viewer count, `Increment(sessionId)`, `Decrement(sessionId)`, `HasViewers(sessionId)`. Track per-connection subscribed sessions in a `ConcurrentDictionary<string, ConcurrentDictionary<string,byte>>` keyed by connection id so `OnDisconnectedAsync` can decrement everything that connection held.
- `EventsHub`: `SubscribeSession`/`UnsubscribeSession` update the registry alongside the group add/remove; override `OnDisconnectedAsync` to release the connection's sessions. Keep the existing SEC-25 remark intact.
- `DashboardEventBroadcaster.Publish`: first line after the null-guards becomes `if (!viewerRegistry.HasViewers(sessionId)) { return; }` — before the redact/clone.
- Do **not** attempt lazy mirror-lease start in this task (it interacts with distributor lifecycle); the gate above removes ~all of the waste already. Note this decision in the doc paragraph.
**Step 3:** run Dashboard test filter → PASS. **Step 4:** update `docs/GatewayDashboardDesign.md`. Commit: `perf(dashboard): gate event mirror on live viewers — no clone, no send for unwatched sessions`
---
### Task 5: Snapshot pipeline — idle gating, cached config, keyed refresh cadence
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 1, 2, 3, 4, 6, 8, 10, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotPublisher.cs` (~6983)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardSnapshotHub.cs` (connection counting)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs` (~103 config rebuild, ~163164 + ~267 API-key refresh)
- Modify: `docs/GatewayDashboardDesign.md`
- Test: extend existing snapshot service/publisher tests under `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/`
**Why:** the 1 Hz tick runs an API-key SQLite read, a registry sort, a metrics snapshot, and a rebuild of the *static* effective-configuration record, broadcast to `Clients.All`, forever, with zero viewers.
**Step 1 (failing tests):** (a) effective configuration object is reference-identical across two snapshot builds; (b) API-key summaries refresh at most once per configured interval (inject `TimeProvider`, follow the file's existing time idiom); (c) publisher with zero connections does not enumerate the snapshot source (fake the hub context; count pulls).
**Step 2 (implement):**
- Cache `EffectiveGatewayConfiguration` in a field on first build (it's startup-static; add a comment saying so).
- `RefreshApiKeySummariesAsync`: skip unless `RefreshInterval` (new private constant, 15 s) has elapsed since the last successful refresh.
- `DashboardSnapshotHub`: `OnConnectedAsync`/`OnDisconnectedAsync` maintain an `int` connection count on a small singleton (or reuse the Task 4 registry class with a well-known key — implementer's choice, keep it simple). Publisher checks the count each tick: zero connections → `await Task.Delay(interval)` and skip both the snapshot build and the broadcast. First connection after idle gets a fresh snapshot on its next tick (≤1 interval of staleness — acceptable; pages also seed from `IDashboardSnapshotService` directly on load).
**Step 3:** dashboard test filter → PASS. Docs paragraph. Commit: `perf(dashboard): idle-gate the snapshot tick; cache static config; bound key-list refresh`
---
### Task 6: Reply ownership transfer in `MapCommandReply`
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 8, 10, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGrpcMapper.cs` (~74)
- Test: existing mapper/service tests under `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/`
**Why:** every `WorkerCommandReply` is parsed fresh from one pipe frame and completed to exactly one awaiter; the gRPC handler is its only consumer. Events already got this treatment under GWC-07 — replies still deep-copy, which doubles the largest hot-path message on bulk reads.
**Step 1 (verify precondition, in-code):** confirm (grep) that no caller of `WorkerClient.InvokeAsync` retains `reply.Reply` after mapping — the review found the Invoke path clean; `GatewayAlarmMonitor` and `DashboardLiveDataService` own their separate replies. If you find a second consumer, STOP and surface it — that's a plan defect.
**Step 2 (implement):** `return reply.Reply.Clone();``return reply.Reply;` with a GWC-07-style ownership comment: the worker reply object is single-consumer by construction (one frame → one `PendingCommand` completion → one mapper call); the mapper transfers ownership to the gRPC response.
**Step 3:** run `FullyQualifiedName~MxAccessGrpcMapper` + the fake-worker smoke filter → PASS. Commit: `perf(grpc): transfer reply ownership instead of deep-cloning every worker reply`
---
### Task 7: WorkerClient — pooled-timer timeout, single sizing pass, `WorkerCancel` on timeout
**Classification:** high-risk (IPC concurrency + protocol behavior)
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 3, 4, 5, 8, 10, 11, 12, 13, 14 (NOT Task 2 — both edit `WorkerClient.cs`; run after Task 2)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs` (InvokeAsync ~226270; timeout path)
- Modify: `docs/GatewayProcessDesign.md` (command timeout → cancel-forwarding note)
- Test: extend `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/` worker-client tests (the fakes there already exercise timeout paths)
**Why:** each Invoke churns a linked CTS + `Task.Delay` timer + `WhenAny`; `CalculateSize` runs twice (protobuf doesn't memoize); and on timeout the gateway never tells the worker, so a timed-out COM call keeps occupying the STA and an envelope still queued gets written anyway.
**Step 1 (failing test):** on command timeout, the client enqueues a `WorkerCancel` envelope carrying the timed-out correlation id (assert via the fake connection's written-frame log).
**Step 2 (implement):**
- Replace the CTS/Delay/WhenAny block with `await pendingCommand.Task.WaitAsync(timeout, cancellationToken)` wrapped in a `try/catch (TimeoutException)` / `(OperationCanceledException)` mapping to the exact same `WorkerClientErrorCode`s and messages as today (tests depend on them).
- On the timeout path, after `RemovePendingCommandAsFailed`, best-effort enqueue a `WorkerCancel` envelope for the correlation id (fire-and-forget with a swallow-and-log; never let cancel failure mask the timeout exception). The worker already handles `WorkerCancel` (`WorkerPipeSession``CancelCommand`).
- Thread the already-computed `envelopeSize` into the frame write path if the writer API allows passing a known size; if the writer's public surface would have to change more than trivially, skip this sub-item and leave a `// PERF:` note — the timer and cancel fixes carry the task.
**Step 3:** worker-client test filter → PASS, including existing timeout tests unchanged. Docs note. Commit: `perf(ipc): WaitAsync command timeouts + forward WorkerCancel so a timed-out COM call frees the STA`
---
### Task 8: Audit pipeline — startup bootstrap, background writer, retention
**Classification:** high-risk (security/audit semantics)
**Estimated implement time:** ~5 min (split if it runs long: 8a writer, 8b retention)
**Parallelizable with:** Tasks 1, 2, 3, 4, 5, 6, 7, 10, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Audit/SqliteCanonicalAuditStore.cs` (per-op `EnsureTableAsync` ~5254, ~94, ~131136)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Audit/CanonicalAuditWriter.cs` (~35)
- Create: `src/ZB.MOM.WW.MxGateway.Server/Security/Audit/ChannelAuditWriter.cs` (bounded channel + hosted drain)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/AuthStoreServiceCollectionExtensions.cs` (DI wiring + hosted service)
- Modify: `docs/DesignDecisions.md` (audit is asynchronous best-effort, bounded, with retention)
- Test: create `src/ZB.MOM.WW.MxGateway.Tests/Security/Audit/ChannelAuditWriterTests.cs`
**Why:** constraint denials await a SQLite insert inline per denied tag inside bulk RPC loops — sequential round-trips into the same DB file the auth store uses, each with a redundant `CREATE TABLE IF NOT EXISTS`, into a table with no retention.
**Step 1 (failing tests):** (a) `WriteAsync` returns without touching the store (enqueue-only) and the event lands in the store shortly after (drain); (b) when the bounded channel (capacity 4096) is full, `WriteAsync` drops (oldest or newest — pick drop-write/newest for simplicity) and increments a counter, never blocks; (c) retention sweep deletes rows older than the configured window.
**Step 2 (implement):**
- `ChannelAuditWriter : ICanonicalAuditWriter` (or whatever the current writer interface is named — read `CanonicalAuditWriter.cs` first): bounded `Channel<CanonicalAuditEvent>` (`BoundedChannelFullMode.DropWrite`), a `BackgroundService` drain that batches up to 64 events into one transaction per drain pass. The audit contract is already documented best-effort — say so in the class doc.
- Table bootstrap: run `EnsureTableAsync` once from the drain service's `StartAsync` (and from the store's first list call via a `Lazy`/latch); remove the per-insert and per-list calls.
- Retention: in the same drain service, once per hour, `DELETE FROM audit_event WHERE timestamp < now - RetentionDays` (new `SecurityOptions`/audit option, default 90 days, validated ≥1 in `GatewayOptionsValidator`); document in `docs/GatewayConfiguration.md`.
- Wire DI so `ConstraintEnforcer.RecordDenialAsync` transparently goes through the channel writer — **no signature changes** at the enforcer/service layer.
- Flush-on-shutdown: drain the channel in `StopAsync` with a 2 s cap.
**Step 3:** audit test filter + `FullyQualifiedName~ConstraintEnforcer` → PASS. Docs (`DesignDecisions.md`, `GatewayConfiguration.md`). Commit: `perf(audit): bounded async audit writer with batched inserts, one-time bootstrap, retention sweep`
---
### Task 9: Parallel session teardown in sweep and shutdown
**Classification:** high-risk (lifecycle concurrency)
**Estimated implement time:** ~4 min
**Parallelizable with:** Tasks 10, 11, 12, 13, 14 (edits only `SessionManager.cs` + docs; run any time)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionManager.cs` (`CloseExpiredLeasesAsync` ~256296, `ShutdownAsync` ~301329)
- Modify: `docs/Sessions.md` (teardown parallelism note)
- Test: extend `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/` session-manager tests
**Why:** both loops `await CloseSessionCoreAsync` strictly sequentially, each bounded by the 10 s worker-shutdown timeout — a mass expiry with hung workers stalls slot reclamation, and 50-session shutdown exceeds any host stop-timeout.
**Step 1 (failing test):** two sessions whose fake worker shutdowns each take T complete a sweep in ~T, not ~2T (the fake harness supports delayed shutdown; if not, add a delay knob to the fake).
**Step 2 (implement):** wrap both loops in `Parallel.ForEachAsync` with `MaxDegreeOfParallelism = 4` (named constant, comment why: bounded so a mass expiry can't stampede worker teardown). `TryBeginCloseIfExpired` already makes per-session close idempotent/exclusive — state that in a comment; that's the invariant making this safe. Preserve the existing sweep precedence (lease-expiry → faulted → detach-grace) by keeping the *selection* phase sequential and parallelizing only the close calls on the selected set.
**Step 3:** session-manager filter → PASS. Docs. Commit: `perf(sessions): bounded-parallel teardown in lease sweep and shutdown`
---
### Task 10: Dashboard live-data subscription cap
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** Tasks 19, 11, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs` (~6170, `_subscribed`)
- Modify: `docs/GatewayDashboardDesign.md`
- Test: extend existing live-data tests under `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/`
**Why:** every tag any viewer ever inspected stays advised on the shared worker session forever.
**Step 1 (failing test):** subscribing tag #257 when the cap is 256 unsubscribes the least-recently-read tag first (assert the fake session sees an `UnsubscribeBulk`/equivalent for the evicted tag).
**Step 2 (implement):** replace `_subscribed` (set) with an LRU: `Dictionary<string, LinkedListNode<string>>` + `LinkedList<string>` under the existing `_gate` (already serialized — no new locking). Cap at 256 (named constant; comment the sizing rationale: one browse page of tags plus headroom). On read of an already-subscribed tag, move to front. On insert past cap, evict from the back and call the session's unsubscribe for the evicted batch. On `InvalidateSession`, clear both structures (existing behavior).
**Step 3:** dashboard filter → PASS. Docs. Commit: `perf(dashboard): LRU cap on the shared live-read session's advised set`
---
### Task 11: Alarm monitor — cached `CurrentAlarms` projection
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Tasks 110, 12, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Alarms/GatewayAlarmMonitor.cs` (~9099 + every mutation site under `_sync`)
- Test: extend `src/ZB.MOM.WW.MxGateway.Tests/Alarms/` monitor tests
**Why:** `CurrentAlarms` clones the full alarm set under the broadcast lock on every call.
**Step 1 (failing test):** two consecutive `CurrentAlarms` calls with no intervening transition return the same cached array instance; a transition invalidates it.
**Step 2 (implement):** add `private IReadOnlyList<ActiveAlarmSnapshot>? _currentAlarmsCache;``CurrentAlarms` builds it (still cloning, still under `_sync`) only when null; every mutation path that touches the alarm dictionary (`ApplyTransition`, reconcile apply, clear) nulls it under `_sync`. Callers already treat the result as read-only.
**Step 3:** alarms filter → PASS. Commit: `perf(alarms): memoize CurrentAlarms projection, invalidate on mutation`
---
### Task 12: Request-logging middleware — hoisted logger, bearer redaction fix
**Classification:** small (contains a security fix)
**Estimated implement time:** ~4 min
**Parallelizable with:** Tasks 111, 13, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayRequestLoggingMiddlewareExtensions.cs` (~2938)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayLogRedactor.cs` (~5477)
- Test: extend `src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/` redactor tests
**Why:** `CreateLogger` (factory lock + DI resolve) per request; and — the security half — `RedactClientIdentity` passes any bearer credential that doesn't contain `mxgw_` through **unredacted** into log scope, violating the "never log secrets" convention.
**Step 1 (failing test):** `RedactClientIdentity("Bearer eyJhbGciOi...")` (a non-mxgw token) returns a redacted form (e.g. `Bearer [redacted]`), never the raw token. Keep the existing mxgw-shaped redaction (`mxgw_<id>_***`) intact — those tests must still pass.
**Step 2 (implement):**
- Redactor: any `authorization`-style value that is not recognized as an mxgw key redacts to a fixed `"[redacted]"` (preserve scheme word only). This is fail-closed.
- Middleware: resolve the `ILogger` once outside the per-request lambda (category-keyed, not request-keyed) via the app's `ILoggerFactory` at `Use...` registration time; keep the scope construction as-is (it carries per-request fields the log pipeline consumes — do not conditionalize it on log level in this task; note as considered-and-skipped since scope consumers may be added at runtime).
**Step 3:** diagnostics filter → PASS. Commit: `fix(logging): fail-closed bearer redaction; hoist per-request logger creation`
---
### Task 13: Auth-path hygiene — span token parse, limiter partition keys
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** Tasks 112, 14
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/GatewayGrpcAuthorizationInterceptor.cs` (~153 `TryResolveKeyId`)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/CachingApiKeyVerifier.cs` (~229 `TryParseKeyId`)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs` (~244269, ~398)
- Test: existing auth tests under `src/ZB.MOM.WW.MxGateway.Tests/Security/` must stay green; add parse-equivalence cases
**Step 1 (failing test):** parse-equivalence table test: for a set of tokens (well-formed, missing `_`, empty, extra `_`), the new span parser returns exactly what `Split('_')` logic returned.
**Step 2 (implement):** replace `Split('_')` in both parsers with `IndexOf('_')` twice over a `ReadOnlySpan<char>`/string (no arrays, no substrings until the final key-id slice). In the limiter, compute the composite partition key once per RPC and pass it to both `Check` and `Reset` (or add an overload taking the precomputed key) instead of concatenating twice.
**Step 3:** security filter → PASS. Commit: `perf(auth): allocation-free token parsing; single partition-key build per RPC`
---
### Task 14: Bulk constraint loops, caches, and per-call hygiene
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 113 (NOT Task 6 if the mapper edit collides — it doesn't; different files)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs` (bulk loops ~466troughs at 494/551/612/680; double session resolve ~104/126)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs` (~214215 LINQ; expose `HasReadConstraints`/`HasWriteConstraints` if not present)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/GatewayApiKeyIdentityMapper.cs` (~3942 cache cliff)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SparseArrayExpander.cs` (~123126 capacity hints)
- Test: existing constraint/service tests under `src/ZB.MOM.WW.MxGateway.Tests/` + one new eviction test
**Step 1 (failing test):** constraint-blob cache: inserting entry `MaxCachedConstraintBlobs + 1` evicts the oldest instead of refusing to cache (FIFO like `GalaxyGlobMatcher` — copy its idiom).
**Step 2 (implement):**
- Bulk loops: hoist a single `identity has no read/write constraints` check before each per-item loop → unconstrained keys take an O(1) fast path (no per-item async interface dispatch, no denial bookkeeping allocation).
- Glob matching: replace the two `.Any(lambda)` calls with `for` loops over the glob lists.
- Denied-path double clone: build the filtered command directly (new message, copy allowed entries in) instead of `command.Clone()` then clear-and-refill; `MapCommand`'s own clone stays (that one is the load-bearing no-aliasing copy).
- Session double-resolve: add/`use` a `SessionManager` overload accepting the already-resolved `GatewaySession` (or have the service pass the session it resolved); keep the not-found exception behavior identical.
- `SparseArrayExpander`: set `RepeatedField.Capacity = length` (per element type) before the fill loops.
**Step 3:** run `FullyQualifiedName~ConstraintEnforcer`, `~MxAccessGatewayService`, `~SparseArray` filters → PASS. Commit: `perf(grpc): O(1) unconstrained bulk fast path, direct filtered-command build, cache eviction, capacity hints`
---
### Task 15: Phase A gate — full gateway suite
**Classification:** trivial (verification only)
**Estimated implement time:** ~5 min wall (suite runtime)
**Parallelizable with:** none (runs after Tasks 114)
Run, in order:
```bash
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj
```
Expected: 0 build errors, full suite green, clean process exit (0 surviving testhost). Fix anything red before Phase B. Commit only if fixes were needed.
---
# Phase B — Worker (.NET Framework 4.8; verified on windev in Task 24)
> Phase B implementers: you cannot compile. Be conservative — minimal diffs, match file idioms, net48 BCL only. Every task here lands as an unverified commit that Task 24 builds and tests remotely; keep commits clean so a failure bisects trivially.
### Task 16: Event drain loop — wake signal instead of 25 ms poll
**Classification:** high-risk (event path liveness)
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 18, 19, 21, 22, 23
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs` (add wake handle; `Enqueue` sets it)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs` (~18 `EventDrainInterval`, ~345372 drain loop)
- Modify: `docs/MxAccessWorkerInstanceDesign.md` (drain-loop paragraph ~381)
- Test: extend `src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/` event-queue tests + `Ipc/` pipe-session tests (they run on windev)
**Why:** the drain loop polls at 25 ms with no wake from `Enqueue` — a 25 ms latency floor on every burst from idle, 40 wakeups/s per idle worker, and less burst absorption before the 10k queue faults the session.
**Implement:**
- `MxAccessEventQueue`: add a `SemaphoreSlim _signal = new(0, 1)` (or an `AsyncAutoResetEvent`-shaped helper if the codebase has one — check first). `Enqueue` releases it (cap at 1, swallow `SemaphoreFullException`). Expose `Task WaitForEventsAsync(TimeSpan timeout, CancellationToken ct)`.
- Drain loop: when a drain returns empty, `await queue.WaitForEventsAsync(EventDrainInterval, ct)` instead of `Task.Delay` — the 25 ms becomes a *fallback* ceiling, not the floor; a signaled wait returns immediately. Loop structure otherwise unchanged (fault handling, batch size).
- Doc paragraph: drain is signal-driven with a 25 ms fallback tick.
- Tests: enqueue-after-idle results in a drain without waiting for the fallback interval (windev-run; write it now).
Commit: `perf(worker): signal-driven event drain — removes the 25 ms latency floor and idle wakeups`
---
### Task 17: Event queue capacity — launcher-configurable
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 18, 19, 21, 22, 23 (NOT Task 16 — both edit `MxAccessEventQueue.cs`/`WorkerPipeSession.cs`; run after 16)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Configuration/WorkerOptions.cs` (+`EventQueueCapacity`, default 10000) and `GatewayOptionsValidator.cs` (≥1000, ≤1_000_000)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs` (new env var, mirror the `WorkerWriteCompletionWaitEnvironmentVariableName` pattern at ~2529 and ~186187 exactly)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Bootstrap/WorkerOptionsParser.cs` / `WorkerOptions.cs` / `EnvironmentVariableWorkerEnvironment.cs` (read it, following the write-completion variable's path)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessStaSession.cs` (~52: pass capacity to `new MxAccessEventQueue(...)`)
- Modify: `docs/GatewayConfiguration.md` (+`MxGateway:Worker:EventQueueCapacity`), `docs/MxAccessWorkerInstanceDesign.md` (capacity paragraph ~371)
- Test: gateway side — validator test + launcher env-var test (these run locally); worker side — parser test (windev)
**Why:** the 10,000 default is headroom-critical (overflow faults the session) but not configurable without a rebuild.
**Implement:** copy the `WriteCompletionWaitMilliseconds` plumbing end to end under a new name (`MXGW_EVENT_QUEUE_CAPACITY` shaped like the existing variable's naming). Absent/invalid env value → default 10000 (never crash the worker on a bad value; log and default).
Note the gateway-side files here don't overlap Phase A tasks — safe after Task 15.
Commit: `perf(worker): launcher-configurable event queue capacity`
---
### Task 18: STA completion waits — message-driven, not sleep-polled
**Classification:** high-risk (STA/pump semantics)
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 16, 17, 19, 21, 22, 23
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessWriteCompletionCache.cs` (~97118 wait loop)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs` (~135150 wait loop)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Sta/StaMessagePump.cs` (if it doesn't already expose a bounded "pump until signaled or timeout" primitive)
- Modify: `docs/MxAccessWorkerInstanceDesign.md`
- Test: extend `src/ZB.MOM.WW.MxGateway.Worker.Tests/Sta/` + `MxAccess/` cache tests (windev)
**Why:** both waits run `pumpStep(); ...; Thread.Sleep(5)` on the STA — during each 5 ms sleep no messages pump, so COM event dispatch stalls in 5 ms bites for up to 1.5 s (writes) / 1 s per tag (ReadBulk).
**Implement:**
- Add a wake to both caches: the update path (`OnWriteComplete` recording a completion / `OnDataChange` recording a value) signals a Win32 auto-reset event (`AutoResetEvent` is fine — it wraps one).
- Replace `Thread.Sleep(pollIntervalMs)` with a pump-integrated wait: `MsgWaitForMultipleObjectsEx(1, [waitHandle], remainingMs-capped-at-50, QS_ALLINPUT, MWMO_INPUTAVAILABLE)`; on `WAIT_OBJECT_0 + 1` (message arrived) run `pumpStep()` and re-check; on `WAIT_OBJECT_0` (signaled) re-check the entry immediately. The existing `StaMessagePump`/`StaRuntime` already use exactly this Win32 pattern (~`StaRuntime.cs:255261`) — reuse/extract their P/Invoke declarations, do not duplicate.
- **Semantics unchanged:** timeouts, deadline math, return values, and the unconfirmed-empty-statuses reply shape stay byte-identical. Only the *waiting mechanism* changes: latency to observe a completion drops from ≤5 ms granularity to immediate, and the pump keeps running throughout the wait.
- **Do not** change the plain-`Write` completion-wait default in this task. The 1.5 s default is a documented OtOpcUa contract (`MxGateway:Worker:WriteCompletionWaitMilliseconds` is already configurable). Leave a doc note that operators with pure fire-and-forget write workloads can lower it.
Commit: `perf(worker): message-driven completion waits — the STA pumps continuously while waiting`
---
### Task 19: Handle registry — reverse index, cached views, O(1) removals
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 16, 17, 18, 21, 22, 23
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessHandleRegistry.cs` (`ItemHandles`/`ServerHandles`/`AdviceHandles` properties ~1426; `RemoveAdviceHandles` ~137148; `UnregisterServerHandle` ~4665)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs` (`TryGetCachedReadFor` ~9881000)
- Test: extend `src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/` registry tests (windev)
**Why:** the sorted list properties re-sort and copy the whole table on **every access**, `TryGetCachedReadFor` reads `ItemHandles` once per ReadBulk tag (O(tags × items·log items)), and advice/server removals do full LINQ scans (O(n²) bulk teardown).
**Implement:**
- Reverse index: `Dictionary<long, Dictionary<string, int>>` server→(tagAddress→itemHandle) — or flat `Dictionary<(int,int-packed + tag)>` — maintained on register/unregister. `TryGetCachedReadFor` becomes two dictionary probes (the file's own comment already asks for this).
- Cached materialization: memoize each sorted array with a version stamp bumped on any mutation; property returns the cached array when the version matches. Registry is STA-confined (verify: no locking in the file today ⇒ single-threaded by contract — state it in a comment), so no locking needed.
- Removals: secondary index advice-by-item (`Dictionary<long, List<advice>>` keyed on the packed `(serverHandle, itemHandle)` the item table already uses) so `RemoveAdviceHandles`/`UnregisterServerHandle` stop scanning.
Commit: `perf(worker): reverse tag index + memoized views + indexed removals in the handle registry`
---
### Task 20: Event conversion — exact-format timestamps, compiled status accessors
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 16, 17, 18, 19, 21, 22, 23 (different files)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs` (~360377 timestamp parse)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs` (~96109 reflection reads)
- Test: extend `src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/` (windev) — these files have solid existing tests; add exact-format cases
**Implement:**
- Timestamps: try `DateTime.TryParseExact` against a small cached array of the observed MXAccess formats (`M/d/yyyy h:mm:ss.fff tt` and its zero-padded/24 h siblings — derive the list from the existing tests' fixture strings) **first**, falling back to the existing two-stage `TryParse` chain so behavior never regresses on an unexpected locale. Order: exact formats → current-culture → invariant (today's chain).
- Status fields: replace the per-read `field.GetValue` with delegates compiled once per field via `Expression.Lambda<Func<object, T>>` (net48-safe) cached alongside the existing `FieldInfo` cache. Same values out, no boxing per event.
Commit: `perf(worker): exact-format timestamp parse and compiled status-field accessors on the event path`
---
### Task 21: Event queue drain — size memoized at enqueue
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** Tasks 18, 19, 20, 22, 23 (NOT 16/17 — same file; run after them)
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventQueue.cs` (~249269 byte-budgeted `Drain`; enqueue path ~152170)
- Test: extend the windev event-queue tests: budget math unchanged for a mixed-size batch
**Why:** `Drain(maxEvents, maxTotalBytes)` calls `CalculateSize()` per event **inside** the queue lock the STA needs to enqueue — a large drain stalls COM callbacks.
**Implement:** compute `CalculateSize()` once at enqueue time (outside any lock — the caller owns the event exclusively there) and store it on the queue's node/wrapper alongside the event; `Drain` uses the memoized size. The WRK-21 never-strand-the-head guarantee is untouched (same comparisons, precomputed operand). Events are never mutated after enqueue (WRK-11 no-clone contract) so the memoized size cannot go stale — say so in a comment.
Commit: `perf(worker): memoize event frame size at enqueue; drain stops sizing under the STA's lock`
---
### Task 22: Worker frame writer/reader — pooled buffers
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Tasks 16, 17, 18, 19, 20, 21, 23
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs` (~467 per-frame `new byte[]`)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs` (~33 per-frame prefix buffer)
- Test: windev `Ipc/` frame tests must stay green (they're thorough — rely on them)
**Why:** the worker side allocates a fresh frame buffer + prefix buffer per frame while the gateway side already pools (`ArrayPool`, GWC-30) — the fix was applied on one side only. `System.Buffers` is already referenced by the worker (its reader uses `ArrayPool.Shared`).
**Implement:** mirror the gateway codec: rent the frame buffer from `ArrayPool<byte>.Shared`, write prefix+payload into it, return in a `finally`; hoist the 4-byte prefix buffer to an instance field on the reader (single-reader by contract — copy the gateway reader's comment). Exact same wire bytes.
Commit: `perf(worker): pooled frame buffers — brings the net48 codec up to the gateway side's GWC-30 pattern`
---
### Task 23: Alarm consumer — cheap parse, truncation detection, configurable cadence
**Classification:** high-risk (alarm correctness)
**Estimated implement time:** ~5 min (split 23a parse / 23b truncation+config if long)
**Parallelizable with:** Tasks 16, 17, 19, 20, 21, 22
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/WnWrapAlarmConsumer.cs` (~402437 parse; ~50 `DefaultMaxAlarmsPerFetch`; ~323330 snapshot rebuild; `ComputeTransitions` absence rule ~356)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessStaSession.cs` (~22 hard-coded 500 ms)
- Modify: `docs/GatewayConfiguration.md`, `docs/DesignDecisions.md` (alarm sections)
- Test: extend windev `MxAccess/` alarm-consumer tests — the truncation test is the important one
**Implement (three independent sub-changes):**
1. **Parse cost:** in the per-alarm extraction, replace the ~14 `SelectSingleNode(child)` XPath calls with one pass over `alarmNode.ChildNodes` switching on `Name` (same fields, same defaults for absent children). Keep `XmlDocument` (an `XmlReader` rewrite is a bigger change than the win justifies once XPath is gone). Reuse the snapshot dictionary across polls (clear-and-refill → swap two dictionaries) only if trivially safe; otherwise skip — the XPath removal is the payload.
2. **Truncation cliff (correctness fix):** when the fetch returns exactly `maxAlarmsPerFetch` records, treat the snapshot as **truncated**: log a warning (rate-limited, identifiers only) and suppress the absence-implies-Clear inference in `ComputeTransitions` for that poll (present alarms still update; nothing is cleared on the evidence of a capped fetch). Add the test: 1024-record fetch + a known alarm missing from it → no Clear transition emitted, warning logged.
3. **Cadence + cap configurable:** plumb `MxGateway:Alarms:PollIntervalMilliseconds` (default 500, min 100) and `MaxAlarmsPerFetch` (default 1024) through the existing env-var pattern (as in Task 17). Gateway-side option + validator + launcher env, worker-side parse.
Commit: `fix(alarms): truncation-safe transitions; perf: single-pass alarm parse; configurable poll cadence`
---
### Task 24: Phase B verification on windev (psbridge)
**Classification:** high-risk (this is the gate for every Phase B commit)
**Estimated implement time:** ~10 min wall
**Parallelizable with:** none (after all Phase B tasks)
**Steps:**
1. Invoke the `psbridge` skill and follow it (it covers exec/push/deploy against the Windows box).
2. Push/pull the branch to windev (whatever the skill's established flow is — the repo has a remote the Windows box shares; `git pull` the branch there).
3. On windev, run in order and capture output:
```powershell
dotnet build src/ZB.MOM.WW.MxGateway.slnx
dotnet build src/ZB.MOM.WW.MxGateway.Worker/ZB.MOM.WW.MxGateway.Worker.csproj -p:Platform=x86
dotnet test src/ZB.MOM.WW.MxGateway.Worker.Tests/ZB.MOM.WW.MxGateway.Worker.Tests.csproj -p:Platform=x86
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj
```
4. Any failure: fix on the Mac, commit, re-run the failed leg. Bisect by commit if the failure isn't obvious — Phase B commits are deliberately one-task-each.
5. If psbridge is unreachable: STOP and report — Phase B remains "edited, unverified"; do not merge.
Live MXAccess smoke (`MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`, `WorkerLiveMxAccessSmokeTests`) if provider state is available on windev; otherwise record why skipped, per `docs/GatewayTesting.md`.
---
### Task 25: Wrap-up — docs sweep, umbrella index, review deltas
**Classification:** small
**Estimated implement time:** ~5 min
**Parallelizable with:** none (last)
**Files:**
- Verify each task's doc edits landed (`gateway.md`, `docs/Sessions.md`, `docs/GatewayConfiguration.md`, `docs/GatewayDashboardDesign.md`, `docs/DesignDecisions.md`, `docs/MxAccessWorkerInstanceDesign.md`, `docs/WorkerFrameProtocol.md`)
- Modify: `../scadaproj/CLAUDE.md` — **only if** a fact the umbrella index records changed (new `MxGateway:Worker:EventQueueCapacity` / alarm options are config, not indexed facts; expected outcome: no umbrella change needed — verify, don't assume)
- Check: no `.proto` diffs (`git diff main -- '*.proto'` must be empty)
Commit anything found: `docs: remediation plan doc sweep`
---
## Explicitly deferred (decided, not forgotten)
| Finding | Why deferred |
|---|---|
| Value-cache triple clone per `OnDataChange` | Removing the defensive copies needs a GWC-07-style aliasing audit across cache consumers; risk outweighs the win until profiled. |
| net48 pipe-read cancellation | Benign in practice (worker exits after shutdown); a correct fix means restructuring stream teardown for a path that only fires at exit. |
| Control-frame completion coupled to event batch drain | Documented, bounded (≤128 frames) behavior of the two-class writer design; revisit only if heartbeat latency shows up in metrics. |
| Blazor pages' loopback SignalR hop | Works correctly; in-process `WatchSnapshotsAsync` consumption is a dashboard refactor with payoff only at viewer counts the product doesn't target. |
| Event-path triple async-iterator flattening | LOW-rated; touches the most invariant-dense code in the gateway for two `MoveNextAsync` hops per event. Reconsider after Tasks 3/4 land and if profiling still shows it. |
## Execution notes for the orchestrator
- Branch: `git checkout -b perf/review-remediation` before Task 1.
- Implementer subagents run on **Opus** per the user's instruction; reviewer chain per each task's Classification.
- Parallel dispatch waves (no file overlap): **Wave 1:** 1, 3, 4, 5, 6, 8 · **Wave 2:** 2, 9, 10, 11, 12, 13, 14 · then 7 (after 2) · then 15 · **Wave 3 (Phase B):** 16, 18, 19, 20, 22, 23 · then 17, 21 (after 16) · then 24 · then 25. (Waves are a suggestion; the per-task `Parallelizable with` fields are the contract.)
- Each implementer gets: its full task text, the ground rules block, and nothing else — the `Files:` block is the scope contract.