The remediation reviews approved every task but left a tail of small notes. This lands the gateway-side half of them. Hardening (behavior changes, all narrow): - BuildFilteredWriteBulkCommand's unreachable default case failed OPEN: a fifth bulk-write kind added upstream without a filter case here would have shipped the DENIED entries to the worker while reporting them denied to the caller. It now throws UnreachableException. - SqliteCanonicalAuditStore.ListRecentAsync no longer throws on a row it cannot date. The retention sweep deliberately preserves such rows (SQLite's datetime() yields NULL, so the DELETE never matches), which guaranteed the dashboard's recent-audit view would meet one eventually and lose the whole page to it. The row is now reported at DateTimeOffset.MinValue with every other column intact, behind an optional logger. - The audit drain loop's finally now completes the channel writer alongside detaching the drain, so a producer that raced past the attached check takes the write-through branch instead of stranding its event in a buffer nobody reads until shutdown. TryComplete is idempotent, so StopAsync is unaffected. Tests: - MapCommandReply ownership (Assert.Same on the inner reply), mirroring the existing MapEvent ownership test. - Redactor key-id length boundary at exactly 64 and 65 characters, pinning which way it fails. Nothing validates key-id length at creation, so docs/Diagnostics.md's "which no issued key id does" is now stated as the heuristic it is. - ApiKeyFailureLimiter.Reset with a PartitionResolution whose partition was evicted between the Check and the Reset: inert, and clears nobody else's block. - Constraint-cache concurrency stress: the cap is enforced by the inserting thread, so overshoot must be transient and proportional to the in-flight inserters, and the cache must settle at or under the cap. - ListRecentAsync against a raw-SQL undateable row. Comment/doc accuracy: - EventsHubViewerRegistry.ReleaseConnection records that it relies on SignalR's default sequential per-connection dispatch (MaximumParallelInvocationsPerClient = 1). - A PERF(followup) note on Invoke's double session resolve and why removing it needs a SessionManager overload. - SessionEventDistributor: the volatile-field comment named the pump as the lock-free reader, but the pump's single capture point is inside _replayLock; the genuinely lock-free reader is SubscriberCount. OnSubscriberOverflow's "cannot be observed here" now excepts the DisposeAsync abandon path. The churn test names its ConcurrentDictionary bucket-order assumption and that a violation surfaces as a read timeout, not a silent pass. - The two "restores the sequential drain's behavior" claims (SessionManager, docs/Sessions.md) were wrong: the sequential drain leaked too, because KillWorkerAsync's entry ThrowIfCancellationRequested aborted the whole loop on the first session for zero kills. Reworded to "fixes a leak the sequential drain also had", with the sweep-bound/shutdown-unbound ParallelOptions asymmetry explained. - ISessionManager.ShutdownAsync's token doc: it degrades the drain to a kill sweep rather than cancelling it, with the bounded overrun stated. SessionShutdownHostedService.StopAsync records that its cancellation-logging branch is now unreachable.
48 KiB
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,sealedby default,Asyncsuffix, 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,
OperationCompletebehavior, 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=latestapplies, so modern syntax is fine, but only net48-era BCL APIs exist (noSpan-taking stream overloads, noArgumentNullException.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>(orfix(...)for the two correctness bugs).
Verification commands used throughout:
# 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
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<int> depth)returning anIDisposablehandle, aConcurrentDictionary<long, Func<int>>of sources, and makeGetWorkerEventQueueDepthsum the sources (clamp negatives). DeleteSetWorkerEventQueueDepthand the_workerEventQueueDepthfield. WorkerClient: at construction (or first use), register a source returning its staged+channel depth viaVolatile.Readof a field the stage/consume paths maintain withInterlocked— the hot path does no metrics call at all anymore. Dispose the registration inDisposeAsync.- Command counters:
_commandsStarted/_commandsSucceeded/_commandsFailedbecomelongupdated withInterlocked.Increment;_commandFailuresByMethodbecomesConcurrentDictionary<string, long>(follow the existingEventReceivedpattern in the same file). Snapshot reads withInterlocked.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 aConcurrentDictionary<string, ConcurrentDictionary<string,byte>>keyed by connection id soOnDisconnectedAsynccan decrement everything that connection held.EventsHub:SubscribeSession/UnsubscribeSessionupdate the registry alongside the group add/remove; overrideOnDisconnectedAsyncto release the connection's sessions. Keep the existing SEC-25 remark intact.DashboardEventBroadcaster.Publish: first line after the null-guards becomesif (!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
EffectiveGatewayConfigurationin a field on first build (it's startup-static; add a comment saying so). RefreshApiKeySummariesAsync: skip unlessRefreshInterval(new private constant, 15 s) has elapsed since the last successful refresh.DashboardSnapshotHub:OnConnectedAsync/OnDisconnectedAsyncmaintain anintconnection 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 fromIDashboardSnapshotServicedirectly 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 atry/catch (TimeoutException)/(OperationCanceledException)mapping to the exact sameWorkerClientErrorCodes and messages as today (tests depend on them). - On the timeout path, after
RemovePendingCommandAsFailed, best-effort enqueue aWorkerCancelenvelope for the correlation id (fire-and-forget with a swallow-and-log; never let cancel failure mask the timeout exception). The worker already handlesWorkerCancel(WorkerPipeSession→CancelCommand). - Thread the already-computed
envelopeSizeinto 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-opEnsureTableAsync~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 — readCanonicalAuditWriter.csfirst): boundedChannel<CanonicalAuditEvent>(BoundedChannelFullMode.DropWrite), aBackgroundServicedrain 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
EnsureTableAsynconce from the drain service'sStartAsync(and from the store's first list call via aLazy/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(newSecurityOptions/audit option, default 90 days, validated ≥1 inGatewayOptionsValidator); document indocs/GatewayConfiguration.md. - Wire DI so
ConstraintEnforcer.RecordDenialAsynctransparently goes through the channel writer — no signature changes at the enforcer/service layer. - Flush-on-shutdown: drain the channel in
StopAsyncwith 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<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 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<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 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_<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
ILoggeronce outside the per-request lambda (category-keyed, not request-keyed) via the app'sILoggerFactoryatUse...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(~153TryResolveKeyId) - Modify:
src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/CachingApiKeyVerifier.cs(~229TryParseKeyId) - 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<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 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; exposeHasReadConstraints/HasWriteConstraintsif 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 constraintscheck 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 withforloops 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/
useaSessionManageroverload accepting the already-resolvedGatewaySession(or have the service pass the session it resolved); keep the not-found exception behavior identical. SparseArrayExpander: setRepeatedField.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:
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;Enqueuesets it) - Modify:
src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs(~18EventDrainInterval, ~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 aSemaphoreSlim _signal = new(0, 1)(or anAsyncAutoResetEvent-shaped helper if the codebase has one — check first).Enqueuereleases it (cap at 1, swallowSemaphoreFullException). ExposeTask WaitForEventsAsync(TimeSpan timeout, CancellationToken ct).- Drain loop: when a drain returns empty,
await queue.WaitForEventsAsync(EventDrainInterval, ct)instead ofTask.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) andGatewayOptionsValidator.cs(≥1000, ≤1_000_000) - Modify:
src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs(new env var, mirror theWorkerWriteCompletionWaitEnvironmentVariableNamepattern 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 tonew 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).
As-built note (
1358332): shipped as silent default without logging, matching the alarm-resolver precedent — noILoggeris reachable from the static resolve site without new plumbing; the silent fallback is disclosed inGatewayConfiguration.md. The Bootstrap parser files listed above were correctly NOT touched — the established env-var pattern readsEnvironment.GetEnvironmentVariableat the resolve site.
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 (
OnWriteCompleterecording a completion /OnDataChangerecording a value) signals a Win32 auto-reset event (AutoResetEventis fine — it wraps one). - Replace
Thread.Sleep(pollIntervalMs)with a pump-integrated wait:MsgWaitForMultipleObjectsEx(1, [waitHandle], remainingMs-capped-at-50, QS_ALLINPUT, MWMO_INPUTAVAILABLE); onWAIT_OBJECT_0 + 1(message arrived) runpumpStep()and re-check; onWAIT_OBJECT_0(signaled) re-check the entry immediately. The existingStaMessagePump/StaRuntimealready 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-
Writecompletion-wait default in this task. The 1.5 s default is a documented OtOpcUa contract (MxGateway:Worker:WriteCompletionWaitMillisecondsis 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/AdviceHandlesproperties ~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<long, Dictionary<string, int>>server→(tagAddress→itemHandle) — or flatDictionary<(int,int-packed + tag)>— maintained on register/unregister.TryGetCachedReadForbecomes 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) soRemoveAdviceHandles/UnregisterServerHandlestop 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.TryParseExactagainst a small cached array of the observed MXAccess formats (M/d/yyyy h:mm:ss.fff ttand its zero-padded/24 h siblings — derive the list from the existing tests' fixture strings) first, falling back to the existing two-stageTryParsechain so behavior never regresses on an unexpected locale. Order: exact formats → current-culture → invariant (today's chain). - Status fields: replace the per-read
field.GetValuewith delegates compiled once per field viaExpression.Lambda<Func<object, T>>(net48-safe) cached alongside the existingFieldInfocache. 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-budgetedDrain; 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-framenew 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(~402–437 parse; ~50DefaultMaxAlarmsPerFetch; ~323–330 snapshot rebuild;ComputeTransitionsabsence 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):
- Parse cost: in the per-alarm extraction, replace the ~14
SelectSingleNode(child)XPath calls with one pass overalarmNode.ChildNodesswitching onName(same fields, same defaults for absent children). KeepXmlDocument(anXmlReaderrewrite 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. - Truncation cliff (correctness fix): when the fetch returns exactly
maxAlarmsPerFetchrecords, treat the snapshot as truncated: log a warning (rate-limited, identifiers only) and suppress the absence-implies-Clear inference inComputeTransitionsfor 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. - Cadence + cap configurable: plumb
MxGateway:Alarms:PollIntervalMilliseconds(default 500, min 100) andMaxAlarmsPerFetch(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:
- Invoke the
psbridgeskill and follow it (it covers exec/push/deploy against the Windows box). - Push/pull the branch to windev (whatever the skill's established flow is — the repo has a remote the Windows box shares;
git pullthe branch there). - On windev, run in order and capture output:
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 - 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.
- 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 (newMxGateway:Worker:EventQueueCapacity/ alarm options are config, not indexed facts; expected outcome: no umbrella change needed — verify, don't assume) - Check: no
.protodiffs (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. |
SessionEventDistributor._subscribers ConcurrentDictionary → plain Dictionary (Task 25 / Task 3 review) |
Every mutation is already inside _lifecycleLock, so the concurrent type buys nothing. Behavior-neutral refactor with no measurable win, proposed after the windev gate was already green — not worth re-running the verification matrix for. Comment cleanups from the same review landed; this swap did not. |
Execution notes for the orchestrator
- Branch:
git checkout -b perf/review-remediationbefore 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 withfields are the contract.) - Each implementer gets: its full task text, the ground rules block, and nothing else — the
Files:block is the scope contract.