mxaccessgw/archreview/2026-08-19/10-gateway-core.md

Gateway Server Core — Cycle-3 Review

• Date: 2026-08-19
• Commit: 8b3e3b3 (main, clean)
• Baseline: 4f5371f (2026-07-12 cycle-2 review; ~288 commits since, ~99 of them touching src/ZB.MOM.WW.MxGateway.Server)
• Method: static review on macOS; every cited file read at HEAD. No build, no test, no source change.
• Scope: gateway server process — Sessions/, Workers/, Grpc/ (MxAccessGatewayService, EventStreamService, mapper, validator), Alarms/ (monitor, watch-list resolver, subtag-fallback gateway half), the session/alarm-feed plumbing the dashboard consumes (DashboardLiveDataService session use, AlarmsHubPublisher feed consumption), and the ZB.MOM.WW.GalaxyRepository package adoption (AddZbGalaxyRepository/MapZbGalaxyRepository wiring, GatewayBrowseScopeProvider, DashboardGalaxySummaryProjector). Excludes Security/ internals and dashboard UI (security-dashboard reviewer), except the browse-scope provider assigned here.
• Prior reports: archreview/2026-07-12/10-gateway-core.md; remediation status source archreview/2026-07-12/remediation/00-tracking.md (+ 10-gateway-core.md); first-cycle backlog archreview/remediation/00-tracking.md.

Prior-finding verification

Cycle-2 findings (GWC-24..30, all tracked Done)

Every Done claim was verified against code read at HEAD. All 7 hold; 3 are verified with caveats — in each case the fix is present and sound for the reported defect, but a residual failure mode one layer out survives and is filed as a new finding.

ID	Claimed	Verified?	Evidence (HEAD)	Notes
GWC-24	Done	✅ holds, with caveat	Workers/WorkerClient.cs:107-119 (staging channel bounded at checked(2 * EventChannelCapacity), Wait+TryWrite idiom documented), :646-676 (StageWorkerEvent faults ProtocolViolation on a rejected TryWrite, message names the bound, the depth, and the actionable fix), :653-658/:350-359 (single _eventQueueDepth counter incremented at staging, decremented at consumer read — total undelivered depth), :120-121 + Metrics/GatewayMetrics.cs:524 (pull-model gauge registration), :44-48 (field comment states bound + fault + gauge coverage); startup overflow guard Configuration/GatewayOptionsValidator.cs:343-357 (QueueCapacity ≤ int.MaxValue / 2 so the 2× derivation cannot overflow); regression tests Tests/Gateway/Workers/WorkerClientTests.cs:781 (StagingChannelOverflowFaultsWorkerWithoutWaitingForFullModeTimeout), :865 (WorkerEventQueueDepthGaugeCountsStagedEvents)	Implemented exactly per the remediation design, including the shutdown-drop guard (IsTerminalState()), and the design's extra validator guard. Caveat: the shutdown-drop guard does not cover DisposeAsync, which completes the channels without entering a terminal state — a dispose-race TryWrite failure is then misdiagnosed as the backpressure fault → GWC-32. A faulted-but-retained client also keeps contributing stale depth to the gauge until disposal → GWC-33 (Info).
GWC-25	Done	✅ holds	Sessions/SessionEventDistributor.cs:519-535 (empty-ring branch: oldestAvailableSequence = gap ? _highestSequenceSeen + 1 : 0, with a full explanatory comment citing the finding), out-param contract updated :430-440; emitted verbatim Grpc/EventStreamService.cs:139-145, :227-239; proto comment amended Contracts/Protos/mxaccess_gateway.proto:797-798 ("When nothing is retained … highest observed + 1 … resume formula remains valid"); docs/Sessions.md:262 (dedicated empty-ring paragraph); tests Tests/Gateway/Sessions/SessionEventDistributorTests.cs:935 (ResumeUsingSentinelFormulaAfterEmptyRingGapDeliversLiveEvents) and Tests/Gateway/GatewayEndToEndReconnectReplayTests.cs:246 (ReconnectAfterFullAgeEvictionResumesWithSentinelFormula)	Sound end to end. The afterSequence == ulong.MaxValue wrap guards in both branches (:521, :540) are preserved, so the old dead-stream input is also inert. Proto + docs + server + tests all landed as one story; the client halves (CLI-35/36, NEXT-02) are closed in the tracking record.
GWC-26	Done	✅ holds, with caveat	Alarms/GatewayAlarmMonitor.cs:246-258 (internal lease attached before SubscribeAlarmsAsync + first ReconcileAsync, with the window rationale in the comment), :272-281 (drain the same lease after reconcile; order-safety noted), ApplyReconcile acked-delta repair :674-684 (both-present, existing.CurrentState != incoming.CurrentState && incoming == ActiveAcked → Acknowledge feed transition); ISessionManager.ReadAlarmEventsAsync removed (grep: absent from Sessions/ISessionManager.cs and SessionManager.cs); repair-scope + at-least-once contract documented :607-633; regression tests Tests/Alarms/GatewayAlarmMonitorAttachOrderTests.cs	Implemented per design, including the method-removal half and the "never synthesize MxEvents" boundary note (:610-614: repairs are AlarmFeedMessages, never MxEvents). Caveat: the state-delta repair is one-directional — only unacked→acked is repaired; an acked alarm silently replaced by a new unacked instance is absorbed without a feed transition → GWC-35.
GWC-27	Done	✅ holds, with caveat	Sessions/GatewaySession.cs:590-617 (AttachInternalEventSubscriber readiness gate under _syncRoot before EnsureDistributorCreated, same exception code/message shape as AttachEventSubscriber:955-961; the poison mechanism is spelled out in the comment); test Tests/Gateway/Sessions/GatewaySessionTests.cs:683 (AttachInternalEventSubscriberBeforeReadyThrowsAndDoesNotPoisonDistributor)	The premature-attach poison path is closed. Caveat: the disposal-side twin remains — EnsureDistributorCreated recreates a never-started distributor after DisposeAsync nulls _eventDistributor, silently hanging a racing attach → GWC-34.
GWC-28	Done	✅ holds	Workers/WorkerClient.cs:463-472 (WriteLoopAsync stamps envelope.Sequence = unchecked(++_nextSequence) immediately before _writer.WriteAsync, single-consumer rationale in comment), :49-51 (field is a plain ulong touched only by the write loop, no interlock), CreateEnvelope:1225-1241 (sequence deliberately unset, comment cites GWC-28/WRK-04); test Tests/Gateway/Workers/WorkerClientTests.cs:228 (ConcurrentInvokesEmitStrictlyIncreasingSequencesOnTheWire)	Exactly the WRK-04-mirror design. All outbound envelopes flow through _outboundEnvelopes (hello :171, commands :251, cancel :849/:858, shutdown :383), so coverage is total. Note the InvokeAsync size pre-check comment (:236-241) correctly explains why the computed size cannot be reused as the writer's knownSize post-GWC-28 — the interaction was thought through.
GWC-29	Done	✅ holds	Grpc/MxAccessGrpcMapper.cs:43-55 (new MapCommand(MxCommand) overload; the one required clone kept with the aliasing rationale), :27-33 (request overload delegates); Grpc/MxAccessGatewayService.cs:131-136 (mapper.MapCommand(commandToInvoke) — the request clone and invokeRequest local are gone; TrackCommandReply still reads commandToInvoke at :147, which is why the inner clone must stay and does)	Sound. WorkerClient.CreateCommandEnvelope's no-aliasing invariant comment (:1194-1198) remains accurate.
GWC-30	Done	✅ holds	Workers/WorkerFrameReader.cs:22-24 (per-reader _lengthPrefix scratch buffer), :46 (used per frame), :11-16 (class remark documents the non-reentrancy contract)	As designed. (Unrelatedly, the label "GWC-30" has since been reused by an off-register perf pass for a different change — see GWC-31.)

Cycle-1 findings still open at cycle 2 — re-checked at HEAD

All were re-read at HEAD. None regressed; none silently fixed; two materially changed shape (GWC-16 is now an explicitly documented deferral; GWC-20's client-side half improved).

ID	Cycle-2 status	HEAD status	Evidence
GWC-05	Open	Still open	Sessions/SessionWorkerClientFactory.cs:175-185 — pipe created with explicit 128 KiB buffers (the zero-quota fix, da84635) but still no ACL and no PipeOptions.CurrentUserOnly. The local pipe-squatting exposure and the gateway.md pipe-security contract gap are unchanged.
GWC-09	Open	Still open	Workers/WorkerProcessStartedProbe.cs:5-19 (no-op HasExited probe throwing WorkerProcessLaunchException); Workers/WorkerProcessLauncher.cs:331-339 (ShouldRetryStartupProbe still excludes WorkerProcessLaunchException); Configuration/WorkerOptions.cs:19-22 (both dead retry options remain). The retry pipeline still can never retry.
GWC-10	Open	Still open	Workers/WorkerEnvelopeValidator.cs:15-39 — version/session/body only; no inbound sequence tracking anywhere in WorkerClient. (The outbound half was fixed by GWC-28; enforcement remains unimplemented on both sides, consistent with IPC-31's recorded decision.)
GWC-11	Open	Still open	Sessions/GatewaySession.cs:23 (_workerClient not volatile); lock-free reads at :308 (WorkerProcessId), :313 (WorkerClient), :1579 (CloseAsync), :1732, :1765, :1877 (DisposeAsync).
GWC-12	Open	Still open	Workers/WorkerClient.cs:419-426 — plain if (_disposed) return; _disposed = true;, no interlock. (Now compounds with new GWC-32: dispose also never enters a terminal state.)
GWC-13	Open	Still open	Sessions/GatewaySession.cs:1956-2024 — 25 ms poll loop, routed through the injected TimeProvider (unchanged since cycle 2).
GWC-16	Open	Still open — now a recorded deferral; recommend closing as Won't-fix	Grpc/MxAccessGatewayService.cs:111-115 — a PERF(followup) comment now documents the double session lookup as measured-negligible and deliberately not collapsed (the fix would duplicate InvokeAsync's fault mapping at a second entry point). The rationale is sound; the tracker row should be flipped to Won't fix citing this comment rather than left Not started.
GWC-17	Open	Still open	Sessions/SparseArrayExpander.cs:306 (RpcException factory below the gRPC layer), :75 (GWC-03's cap rejection still flows through the same leak); invocation comment at Sessions/GatewaySession.cs:1103-1111 still documents the layering leak explicitly.
GWC-18	Open	Still open	Sessions/GatewaySession.cs:14 — no IAsyncDisposable; DisposeAsync at :1787.
GWC-19	Open	Still open	Sessions/GatewaySession.cs:1730-1734 — KillWorker(string); grep across src/ + clients/ finds zero callers (one doc-comment mention in SessionManagerTests.cs:1433).
GWC-20	Open	Still open (client half improved)	Configuration/WorkerOptions.cs:52 — the config key is still HeartbeatIntervalSeconds ("interval … for worker heartbeats") while it is bound to the check interval at Sessions/SessionWorkerClientFactory.cs:98; the WorkerClientOptions field was renamed to HeartbeatCheckInterval (Workers/WorkerClientOptions.cs:40), which fixes the client-side naming but leaves the config-key conflation. The heartbeat loop still uses raw Task.Delay without TimeProvider (Workers/WorkerClient.cs:534).
GWC-21	Open	Still open	Sessions/SessionWorkerClientFactory.cs:95-101 populates 4 of 6 WorkerClientOptions; EventChannelFullModeTimeout / HeartbeatStuckCeiling remain fixed defaults (Workers/WorkerClientOptions.cs:13, :23). Post-GWC-24 the staging bound is derived from EventChannelCapacity (which is configurable), so the exposure gap is narrower than at cycle 2 but the two knobs are still hard-coded.
GWC-22	Open	Still open	Grpc/EventStreamService.cs:217 — metrics.StreamDisconnected("Detached") hardcoded in the finally; overflow/fault exits are not differentiated.
GWC-23	N/A	Unchanged	Decision stands; multi-subscriber mode remains config-gated and the validator comment survives.

NEXT rows touching this domain

• NEXT-01 (macOS sun_path pipe names) — verified resolved: Sessions/SessionManager.cs:532-536 builds mxgw-{Environment.ProcessId}-{sessionUid} with the length-budget rationale in the comment; CLAUDE.md/gateway.md describe the same format.
• NEXT-03 (reconcile/live duplicate broadcasts) — verified resolved as designed (best-effort, positive-match-only): Alarms/GatewayAlarmMonitor.cs:483-489 (IsDuplicateOfCachedState, timestamp+state marker), :496-507 (IsDuplicateOfReconcileClear, tombstone consumed on match), :645-664 (tombstones recorded per reconcile generation, cleared at :649); at-least-once contract restated at :616-625; race-driving regressions in Tests/Alarms/GatewayAlarmMonitorAttachOrderTests.cs:159, :218. The one-generation tombstone lifetime and the unset-timestamp passthrough match the recorded design; consumers must (and are documented to) treat transitions idempotently.
• NEXT-08 / NEXT-10 (GLAuth TLS posture, glauth.md table) are security/docs-domain rows, not promoted here.

Also verified in passing (other-domain fixes landing in gateway-core files)

• WRK-21/WRK-28/IPC-23 gateway half — Grpc/MxAccessGrpcRequestValidator.cs:74-88 rejects DrainEvents.max_events above the now-shared GatewayContractInfo.MaxDrainEventsPerCommand constant, with the byte-cap division of labor documented in the comment.
• Zero-quota pipe fix (da84635) — SessionWorkerClientFactory.cs:15-25, :175-185: explicit 128 KiB in/out buffers with the windev-wedge rationale; this was the root cause of the historical full-suite testhost hang.
• Dead ISessionManager.ReadEventsAsync chain removed (fa9eb0c) and StreamEventsAsync now consumes the subscriber channel directly (Grpc/EventStreamService.cs:110-204, 935f002) — the WaitToReadAsync/TryRead loop preserves the fault-surface semantics (:158-204) including the worker-fault → MarkFaulted + metric path.
• Reply ownership transfer (3ff073d) — Grpc/MxAccessGrpcMapper.cs:62-85: single-consumer argument for moving reply.Reply is documented and holds (one frame → one PendingCommand awaiter → one mapper call; MergeDeniedInto splices in place on the owned instance).
• GalaxyRepository adoption — GatewayApplication.cs:151-176: scope provider registered before AddZbGalaxyRepository so the library's TryAddSingleton null default cannot win (order-sensitive, correctly commented); snapshot-path default seeded pre-bind with GalaxyRepositoryOptionsValidator + ValidateOnStart (SEC-33); MapZbGalaxyRepository at :351. A2-galaxyrepository-adoption-handoff.md's caveats are all marked resolved and match the code. One hardening gap filed as GWC-38.
• Exception-total shutdown drain (95f8ba9/0bc13b5) — Sessions/SessionManager.cs:354-427: bounded-parallel (4) close with an exception-total body, kill fallback on CancellationToken.None, and the deliberate token asymmetry against CloseExpiredLeasesAsync documented at length; ISessionManager.ShutdownAsync's doc (ISessionManager.cs:100-110) states the bounded overrun honestly. Sweep selection stays sequential with the TOCTOU re-check inside TryBeginCloseIfExpired (GatewaySession.cs:1671-1695) — the cycle-2 close-gate invariants are preserved under the parallelization.

New findings

IDs continue from the register. Note on ID allocation: the codebase already contains comment references to "GWC-31" (and a re-used "GWC-30") coined by an off-register remediation pass — those labels are not register entries (see GWC-31 below, which claims the ID and records the collision). Register IDs assigned here are authoritative.

GWC-31 — The 2026-08-15 perf remediation stamped unregistered "GWC-3x" IDs into code comments, colliding with this register

• Severity: Low
• Category: Documentation & comments
• Location: Workers/WorkerClient.cs:44 ("GWC-30" = pull-model queue-depth gauge), :236 ("PERF(GWC-31)"), :253 ("GWC-31: one pooled timer…"); Metrics/GatewayMetrics.cs:46, :213, :284, :463, :524 ("GWC-30" = gauge pull model); Tests/Gateway/Workers/WorkerClientTests.cs:163, :179 ("GWC-31" = WorkerCancel forwarding); analogous worker-side citations (Worker/Ipc/WorkerFrameReader.cs/WorkerFrameWriter.cs, per docs/plans/2026-08-15-perf-review-remediation.md:544-548)
• Description: The 2026-08-15 performance review (whose findings live only in docs/plans/2026-08-15-*.md, not in archreview/) labeled its changes with GWC-series IDs that were never added to any register. "GWC-30" now means two different things — the archreview register says frame-reader 4-byte prefix allocation; the code comments say pull-model worker queue-depth gauge — and "GWC-31" exists only in comments. Finding IDs are the repo's cross-reference currency ("cite it in commits, branches, and PRs" — remediation tracker); a reader following a code comment to the register lands on the wrong finding, and future review cycles (this one included) must dodge already-burned numbers. REVIEW-PROCESS.md's "never reused" rule is violated in spirit.
• Recommendation: Sweep the perf-pass citations to a distinct prefix (e.g. PERF-nn keyed to docs/plans/2026-08-15-perf-review-remediation.md task numbers) or append the perf findings to the register under fresh, unclaimed IDs and fix the comments to match. Add a line to REVIEW-PROCESS.md stating that GWC/WRK/IPC/SEC/CLI/TST prefixes are reserved for the archreview registers.

GWC-32 — WorkerClient.DisposeAsync never enters a terminal state, so the GWC-24 staging-overflow fault can misfire during a dispose race and the "already terminal" shutdown-drop claim is not guaranteed

• Severity: Low
• Category: Concurrency & thread safety (residual of GWC-24)
• Location: Workers/WorkerClient.cs:419-434 (DisposeAsync sets _disposed, cancels, completes all three channels — but _state is untouched), :661-676 (StageWorkerEvent fallback: TryWrite false → if (IsTerminalState()) return; → otherwise ProtocolViolation fault), :632-644 (XML doc: a completed staging channel is "safely dropped because the client is already terminal")
• Description: MarkClosed and SetFaulted both flip _state to a terminal value before completing the channels, so the IsTerminalState() guard in StageWorkerEvent correctly classifies a completed-channel TryWrite miss as shutdown. DisposeAsync is the third channel-completing path and does not: a client disposed while still Ready keeps reporting IsTerminalState() == false. If the read loop is inside DispatchEnvelope when DisposeAsync completes _eventStaging (the _stopCts.Cancel() only interrupts the next ReadAsync), the in-flight StageWorkerEvent takes the overflow arm: a spurious ProtocolViolation session fault with the misleading "consumer is draining slower than the worker produces" message, a worker-event-staging overflow metric, a redundant KillOwnedProcess, and a WorkerStopped("ProtocolViolation") metric — all during an intentional teardown. Reachability is narrow: in the normal teardown order (CloseAsync/Kill → DisposeAsync) the state is already terminal; the exposed path is disposing a still-Ready client, which SessionManager.OpenSessionAsync's failure arm does (Sessions/SessionManager.cs:133-140 — MarkFaulted faults the session, not the worker client) and SessionWorkerClientFactory's catch avoids only because it calls Kill first (SessionWorkerClientFactory.cs:120-132). Impact is corrupted diagnostics/metrics, not data loss — but the fault fires on exactly the code path an operator investigates after a failed open, pointing them at backpressure that never happened. It also makes the StageWorkerEvent XML doc's "already terminal" claim false on one of the three completion paths.
• Recommendation: Treat disposal as terminal in the guard — if (_disposed || IsTerminalState()) return; — or have DisposeAsync transition _state to Closed under _syncRoot before completing the channels (which would also give GWC-12's double-dispose interlock a natural home). Update the StageWorkerEvent doc to name all three completion paths.

GWC-33 — A faulted-but-retained worker client keeps contributing its stale undelivered-event depth to the pull-model gauge until session disposal

• Severity: Info
• Category: Performance & resource management (observability)
• Location: Workers/WorkerClient.cs:120-121 (gauge source registered at construction), :426-429 (unregistered only in DisposeAsync), :1011-1039 (SetFaulted completes channels but leaves staged+queued events uncounted-down); retention window: Configuration/SessionOptions.cs FaultedGraceSeconds + Sessions/SessionManager.cs:283-289 (faulted sessions swept only when reapable)
• Description: The GWC-24 gauge counts staged + queued events and is decremented only on consumer read. When a client faults, its channels are completed with the fault and never drained, so _eventQueueDepth freezes at whatever was undelivered. The registration is removed only in DisposeAsync; with a positive FaultedGraceSeconds (deliberately configurable so operators can observe faults before the reap) the dead client's frozen depth inflates the aggregate worker event queue depth gauge for the whole grace window. With the default FaultedGraceSeconds = 0 the window is one sweep interval, so the default posture is fine — recorded so the next metrics pass knows it was seen.
• Recommendation: Optional: dispose the gauge registration inside SetFaulted/MarkClosed (the comment at :427-428 already articulates the principle: "the sum must not keep counting a client that is going away").

GWC-34 — EnsureDistributorCreated recreates a never-started distributor after DisposeAsync; an attach that races a session teardown registers on a pump-less distributor and hangs silently

• Severity: Low
• Category: Concurrency & thread safety (residual of GWC-27 — the disposal-side twin)
• Location: Sessions/GatewaySession.cs:1865-1875 (DisposeAsync nulls _eventDistributor; _eventDistributorStarted is never reset — :41, :554-559), :535-563 (EnsureDistributorCreated: _eventDistributor is null → constructs a fresh distributor; startNow is false because the started flag is already latched), :946-998 (AttachEventSubscriber: readiness gate under _syncRoot, then the distributor calls run outside the lock)
• Description: GWC-27 closed the premature-attach poison; the mirror-image hole remains open on the teardown side. An AttachEventSubscriber (or AttachEventSubscriberWithReplay / AttachInternalEventSubscriber) that passes its readiness gate while the session is still Ready, then loses the CPU until CloseAsync + DisposeAsync complete, reaches EnsureDistributorCreated after _eventDistributor was nulled: a new distributor is constructed, startNow comes back false (the latch is one-way), the subscriber registers on a distributor whose pump will never start and whose channel will never be completed — WaitToReadAsync in EventStreamService.cs:167 then blocks until the client's own deadline/cancel. No error, no log, no fault; the replacement distributor is also never disposed. Contrast the adjacent race that is handled: if disposal instead wins between EnsureDistributorCreated returning the old distributor and Register, the old distributor's disposed check throws ObjectDisposedException loudly and the attach rolls back (RegisterSubscriber, SessionEventDistributor.cs:361-386). Reachability is small (the attach must stall across an entire multi-second close + dispose between two adjacent statements), but the failure mode is the worst kind — silent, per-stream, and invisible to metrics (the backlog registration reports 0 forever). This is the same "one distributor per session, never reset" lifecycle assumption GWC-27 documented, violated from the other end.
• Recommendation: Make the latch symmetric with the null-out: in DisposeAsync, set a _eventDistributorDisposed flag (or reuse a session-terminal check) under _syncRoot when nulling _eventDistributor, and have EnsureDistributorCreated throw SessionManagerException(SessionNotReady) instead of re-creating when it is set. One flag, one throw — the attach paths' existing catch/rollback already handles the exception.

GWC-35 — ApplyReconcile's state-delta repair is one-directional: an acked alarm replaced by a new unacked instance (or any acked→active regression) is absorbed silently by the snapshot replace

• Severity: Low
• Category: Correctness & logic (alarm feed completeness; residual of GWC-26)
• Location: Alarms/GatewayAlarmMonitor.cs:666-691 (ApplyReconcile both-present loop: broadcasts only when incoming.Value.CurrentState == AlarmConditionState.ActiveAcked), snapshot replace at :687-691
• Description: The GWC-26 repair covers three legs — missing→present (Raise), present→missing (Clear), and unacked→acked (Acknowledge). The fourth leg is absent: an alarm the cache holds as ActiveAcked that the worker snapshot now reports as Active (the acked instance cleared and re-raised as a new unacked instance while the live Clear+Raise transitions were lost — e.g. the internal alarm lease overflowed and disconnected, or worker-side event loss) is absorbed by the _alarms replace with no feed transition. CurrentAlarms/QueryActiveAlarms are correct (they read the replaced cache), but live StreamAlarms/dashboard subscribers keep rendering the alarm as acknowledged until its next genuine transition — for a re-raised alarm, precisely the state an operator must not misread. Reachability is much lower than the pre-GWC-26 window (a lease overflow restarts the whole monitor lifecycle, which re-snapshots every subscriber), so this is a completeness gap in the defense-in-depth rather than a live bug.
• Recommendation: Generalize the both-present delta to any CurrentState change: broadcast Acknowledge for → ActiveAcked and Raise (with the incoming snapshot's new instance identity) for → Active. The existing TransitionFromSnapshot already carries the timestamps the NEXT-03 dedup needs to suppress the live duplicates.

GWC-36 — EventSubscriberReplayAttachment.OldestAvailableSequence doc still describes the pre-GWC-25 empty-ring semantics

• Severity: Info
• Category: Documentation & comments
• Location: Sessions/EventSubscriberReplayAttachment.cs:26-30 ("The oldest worker sequence still retained and replayable; 0 when nothing is retained.")
• Description: GWC-25 changed the empty-ring-with-gap value to highest observed + 1; the distributor's out-param doc (SessionEventDistributor.cs:430-440), the proto, and docs/Sessions.md were all updated, but this record's XML doc was missed. "0 when nothing is retained" is now exactly the value the fix exists to avoid — a reader implementing against this doc re-introduces the dead-stream formula.
• Recommendation: Mirror the distributor's out-param wording (retained-oldest, or highest observed + 1 when the ring is empty; 0 only when Gap is false).

GWC-37 — The alarm subtag watch-list is resolved once per monitor lifecycle: a Galaxy-Repository outage at startup silently empties the subtag failover's coverage for the session's entire lifetime

• Severity: Low
• Category: Error handling & resilience (alarm subtag fallback)
• Location: Alarms/GatewayAlarmMonitor.cs:330-357 (SubscribeAlarmsAsync resolves the watch list once and sends it in the SubscribeAlarms command); Alarms/AlarmWatchListResolver.cs:54-83 (GR discovery failure → warn and continue with a config-only, typically empty set), :141-145 (zero-target warning); no re-resolve anywhere — ApplyProviderModeChangeAsync (GatewayAlarmMonitor.cs:513-566) triggers only a reconcile on failover
• Description: The subtag fallback's coverage is fixed at SubscribeAlarms time: the resolved AlarmSubtagTarget list travels once to the worker and is never refreshed while the monitor session lives. Discovery failure is deliberately non-fatal (ResolveAsync catches, warns, and returns whatever config contributes) — reasonable for the resolver, but the monitor then subscribes with an empty watch list and runs indefinitely. In Auto mode the alarmmgr primary works fine, so nothing looks wrong until the moment a failover to subtag fires — at which point the degraded provider covers zero alarms, the feed goes quiet, and the only evidence is a single LogWarning emitted possibly weeks earlier at startup. The same staleness applies to Galaxy objects added after monitor start (a documented design property), but the outage-at-startup case differs in kind: the operator configured discovery, it silently produced nothing, and the safety net the feature exists to provide is empty. The monitor's supervisor restarts (and thus re-resolves) only when the session faults — a healthy alarmmgr session never does.
• Recommendation: Re-resolve and re-subscribe on the failover edge (ApplyProviderModeChangeAsync when toMode == Subtag), or treat "GR discovery failed AND the effective mode can need the watch list (Auto/ForceSubtag) AND zero targets resolved" as a lifecycle failure so the supervisor's backoff loop retries resolution instead of latching the empty set. At minimum, surface the empty-watch-list condition on the provider status (Degraded/Reason) rather than only in a startup log line.

GWC-38 — GatewayBrowseScopeProvider fails open to the full Galaxy hierarchy when the ambient identity is absent, resting the browse-scoping control on an undeclared interceptor-ordering invariant

• Severity: Low
• Category: Security (defense-in-depth; GalaxyRepository adoption)
• Location: Security/Authorization/GatewayBrowseScopeProvider.cs:14-25 (identityAccessor.Current?.EffectiveConstraints ?? ApiKeyConstraints.Empty → empty BrowseSubtrees → library treats as "no scoping"); registration-order dependency documented at GatewayApplication.cs:151-154
• Description: The per-key browse-subtree scoping — the one piece of authorization mxaccessgw adds on top of the shared GalaxyRepository package — resolves to unscoped (full hierarchy) whenever IGatewayRequestIdentityAccessor.Current is null. The code's own comment says this is "safe only because the global interceptor has already authenticated and authorized the request first. Do not reorder the interceptor without revisiting this." That is a correct description of today's wiring and an honest warning, but it makes the control's fail state open: any future path that reaches the library's galaxy service without the interceptor populating the ambient identity (a new endpoint mapping, an interceptor scoped per-service instead of globally, a library upgrade changing pipeline order) silently grants every scoped key the full hierarchy, with nothing failing and nothing logged. Scoping is a constraint-narrowing control, so its absence is invisible in normal operation. (Whether identity-absent is legitimately reachable under Authentication:Mode=Disabled is the security reviewer's domain; this finding is about the fail-open shape itself.)
• Recommendation: Distinguish "authenticated with no BrowseSubtrees constraint" (legitimately unscoped) from "no ambient identity" (invariant broken): when Current is null and API-key authentication is enabled, throw RpcException(PermissionDenied) (or log-and-deny) instead of returning unscoped. One branch converts a silent authorization bypass into a loud misconfiguration signal.

Severity roll-up (new findings)

Severity	Count	IDs
Critical	0	—
High	0	—
Medium	0	—
Low	6	GWC-31, GWC-32, GWC-34, GWC-35, GWC-37, GWC-38
Info	2	GWC-33, GWC-36

Prior-finding verification: 7/7 cycle-2 Done claims hold (3 with caveats filed as GWC-32/34/35); 13 still-open cycle-1 findings re-confirmed open with none regressed; GWC-16 recommended for closure as a recorded Won't-fix.

Still good / preserve under change

The cycle-2 remediation wave and the subsequent perf pass landed without breaking the core invariants, and several of them got structurally stronger. The backpressure topology is now bounded end to end again — worker queue → pipe → bounded staging (2×) → bounded consumer channel → bounded per-subscriber channels — with a loud fail-fast fault at every seam and a single observable depth gauge across the first two gateway stages; the GWC-24 implementation even added the startup overflow guard the design didn't ask for. The replay→live handoff argument is intact after the GWC-25 change, and the empty-ring sentinel arithmetic is documented in four places that all agree (distributor, proto, docs/Sessions.md, e2e test). The GWC-28 write-loop stamping is the textbook fix, and the InvokeAsync size-pre-check comment proves the interaction with it was reasoned about rather than stumbled past. The alarm monitor's attach-before-subscribe plus the NEXT-03 positive-match dedup give the alarm feed a defensible, stated delivery contract (at-least-once, idempotent-state), and the reconcile repairs are correctly quarantined to the feed layer — the "never synthesize events" rule for MxEvents is respected everywhere, with the ReplayGap sentinel still the sole, documented control signal. Session state remains single-lock disciplined through the new detach-grace/faulted-grace/close-gate arms; the parallelized sweep and shutdown preserved the TOCTOU re-check and made the shutdown body exception-total with an honestly documented token asymmetry. The GalaxyRepository adoption is clean consumer wiring: scope-provider-before-package registration order is commented, the snapshot path is validated at startup, and the host-side projector is a pure function memoizable on the cache sequence. Comment quality across the new code is exceptional — most of this cycle's findings were found via comments that honestly documented their own residual assumptions ("safe only because…", "already terminal") — preserve that habit; it is what keeps one-layer-out defects findable. GatewaySession has grown again (2,173 lines) and the long-standing split-out recommendation stands.