From 9958f800264949a8f0d1f399413bb94ae3cb928b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 12:01:18 -0400 Subject: [PATCH] docs(plans): perf review remediation plan --- .../2026-08-15-perf-review-remediation.md | 624 ++++++++++++++++++ ...8-15-perf-review-remediation.md.tasks.json | 31 + 2 files changed, 655 insertions(+) create mode 100644 docs/plans/2026-08-15-perf-review-remediation.md create mode 100644 docs/plans/2026-08-15-perf-review-remediation.md.tasks.json diff --git a/docs/plans/2026-08-15-perf-review-remediation.md b/docs/plans/2026-08-15-perf-review-remediation.md new file mode 100644 index 0000000..71e8f89 --- /dev/null +++ b/docs/plans/2026-08-15-perf-review-remediation.md @@ -0,0 +1,624 @@ +# 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(): ` (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~" +``` + +--- + +# 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 3–4 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` ~202–247; gauge wiring ~91; snapshot ~461–492) +- 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 2–3× 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 depth)` returning an `IDisposable` handle, a `ConcurrentDictionary>` 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` (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` session→viewer count, `Increment(sessionId)`, `Decrement(sessionId)`, `HasViewers(sessionId)`. Track per-connection subscribed sessions in a `ConcurrentDictionary>` 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` (~69–83) +- 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, ~163–164 + ~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 ~226–270; 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` ~52–54, ~94, ~131–136) +- 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` (`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` ~256–296, `ShutdownAsync` ~301–329) +- 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 1–9, 11, 12, 13, 14 + +**Files:** +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs` (~61–70, `_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>` + `LinkedList` 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 1–10, 12, 13, 14 + +**Files:** +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Alarms/GatewayAlarmMonitor.cs` (~90–99 + 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? _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 1–11, 13, 14 + +**Files:** +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayRequestLoggingMiddlewareExtensions.cs` (~29–38) +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Diagnostics/GatewayLogRedactor.cs` (~54–77) +- 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__***`) 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 1–12, 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` (~244–269, ~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`/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 1–13 (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 ~466–troughs at 494/551/612/680; double session resolve ~104/126) +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs` (~214–215 LINQ; expose `HasReadConstraints`/`HasWriteConstraints` if not present) +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/GatewayApiKeyIdentityMapper.cs` (~39–42 cache cliff) +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SparseArrayExpander.cs` (~123–126 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 1–14) + +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`, ~345–372 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 ~25–29 and ~186–187 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` (~97–118 wait loop) +- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs` (~135–150 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:255–261`) — 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 ~14–26; `RemoveAdviceHandles` ~137–148; `UnregisterServerHandle` ~46–65) +- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessSession.cs` (`TryGetCachedReadFor` ~988–1000) +- 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>` 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>` 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` (~360–377 timestamp parse) +- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs` (~96–109 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>` (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` (~249–269 byte-budgeted `Drain`; enqueue path ~152–170) +- 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.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` (~402–437 parse; ~50 `DefaultMaxAlarmsPerFetch`; ~323–330 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. diff --git a/docs/plans/2026-08-15-perf-review-remediation.md.tasks.json b/docs/plans/2026-08-15-perf-review-remediation.md.tasks.json new file mode 100644 index 0000000..44e8522 --- /dev/null +++ b/docs/plans/2026-08-15-perf-review-remediation.md.tasks.json @@ -0,0 +1,31 @@ +{ + "planPath": "docs/plans/2026-08-15-perf-review-remediation.md", + "tasks": [ + {"id": 1, "subject": "Task 1: Named-pipe buffer sizes", "status": "pending"}, + {"id": 2, "subject": "Task 2: Metrics pull-gauge + Interlocked counters", "status": "pending"}, + {"id": 3, "subject": "Task 3: Distributor copy-on-write subscriber snapshot", "status": "pending"}, + {"id": 4, "subject": "Task 4: Dashboard event mirror viewer gating", "status": "pending"}, + {"id": 5, "subject": "Task 5: Snapshot pipeline idle gating + cached config", "status": "pending"}, + {"id": 6, "subject": "Task 6: Reply ownership transfer in MapCommandReply", "status": "pending"}, + {"id": 7, "subject": "Task 7: WorkerClient WaitAsync timeout + WorkerCancel", "status": "pending", "blockedBy": [2]}, + {"id": 8, "subject": "Task 8: Audit pipeline background writer + retention", "status": "pending"}, + {"id": 9, "subject": "Task 9: Parallel session teardown", "status": "pending"}, + {"id": 10, "subject": "Task 10: Dashboard live-data subscription cap", "status": "pending"}, + {"id": 11, "subject": "Task 11: Alarm monitor cached CurrentAlarms", "status": "pending"}, + {"id": 12, "subject": "Task 12: Logging middleware hoist + bearer redaction fix", "status": "pending"}, + {"id": 13, "subject": "Task 13: Auth-path span parsing + limiter keys", "status": "pending"}, + {"id": 14, "subject": "Task 14: Bulk constraint loops, caches, hygiene", "status": "pending"}, + {"id": 15, "subject": "Task 15: Phase A gate — full gateway suite", "status": "pending", "blockedBy": [1,2,3,4,5,6,7,8,9,10,11,12,13,14]}, + {"id": 16, "subject": "Task 16: Event drain wake signal", "status": "pending", "blockedBy": [15]}, + {"id": 17, "subject": "Task 17: Event queue capacity env plumbing", "status": "pending", "blockedBy": [16]}, + {"id": 18, "subject": "Task 18: STA message-driven completion waits", "status": "pending", "blockedBy": [15]}, + {"id": 19, "subject": "Task 19: Handle registry reverse index + O(1) removals", "status": "pending", "blockedBy": [15]}, + {"id": 20, "subject": "Task 20: Event conversion TryParseExact + compiled accessors", "status": "pending", "blockedBy": [15]}, + {"id": 21, "subject": "Task 21: Drain size memoized at enqueue", "status": "pending", "blockedBy": [16, 17]}, + {"id": 22, "subject": "Task 22: Worker frame writer/reader pooled buffers", "status": "pending", "blockedBy": [15]}, + {"id": 23, "subject": "Task 23: Alarm consumer parse + truncation + cadence", "status": "pending", "blockedBy": [15]}, + {"id": 24, "subject": "Task 24: Phase B verification on windev (psbridge)", "status": "pending", "blockedBy": [16,17,18,19,20,21,22,23]}, + {"id": 25, "subject": "Task 25: Wrap-up docs sweep + umbrella check", "status": "pending", "blockedBy": [24]} + ], + "lastUpdated": "2026-08-15T00:00:00Z" +}