Commit Graph

25 Commits

Author SHA1 Message Date
Joseph Doherty fa9eb0c0b4 refactor(sessions): remove the dead ISessionManager.ReadEventsAsync chain
ISessionManager.ReadEventsAsync had zero production call sites: the worker
event channel is drained once by GatewaySession.MapWorkerEventsAsync (the
distributor pump), and every consumer — gRPC subscribers, the dashboard
mirror, the alarm monitor — attaches to the distributor. The interface
member, SessionManager's forwarder, and GatewaySession.ReadEventsAsync are
gone; IWorkerClient/WorkerClient.ReadEventsAsync is untouched, it is the
live worker-channel claim.

No test was removed or rewired: nothing invoked the member through the
interface. Nine ISessionManager test fakes carried a required-member stub
(seven threw NotSupportedException or yielded nothing; EventStreamServiceTests
and GatewaySessionDashboardMirrorTests forwarded to the session; the two
MxAccessGatewayService fakes yielded their Events list) — all nine stubs were
deleted. The MxAccessGatewayService suites' streaming tests already run
through FakeEventStreamService, which reads the same Events list, so their
coverage is unchanged; only the now-inaccurate doc comments on Events /
LastReadEventsSessionId were reworded.

The MapWorkerEventsAsync comment no longer describes a twin to keep in step;
it now states the single-reader claim directly. docs/Sessions.md drops
ReadEventsAsync from the SessionManager member list and from the Run-state
prose. The 2026-08-15 deferred-remediation as-built note records the removal.
2026-08-17 03:46:41 -04:00
Joseph Doherty 94dbe9f1af perf(sessions): fold the ReadEventsAsync pass-through into MapWorkerEventsAsync — one fewer iterator per event 2026-08-15 20:07:11 -04:00
Joseph Doherty 1a63fdd7db fix(GWC-27): gate AttachInternalEventSubscriber on session readiness
AttachInternalEventSubscriber ran EnsureDistributorCreated / Register /
StartPumpIfRequested with no state check, unlike AttachEventSubscriber. A
premature attach would start the pump against a not-yet-Ready worker; the pump
source throws SessionNotReady, PumpAsync completes every subscriber with that
error and latches the distributor, and _eventDistributorStarted is never reset —
so the session would reach Ready with permanently dead event streaming.

Mirror AttachEventSubscriber's gate: check _state/_workerClient.State under
_syncRoot and throw SessionManagerException(SessionNotReady) before the
distributor is created, keeping the distributor calls outside the lock.

Refs: archreview/2026-07-12/remediation/10-gateway-core.md GWC-27
2026-08-07 05:28:35 -04:00
Joseph Doherty 20392cf246 fix(archreview): gateway core P0 remediation (GWC-01/02/03, TST-02, TST-12)
Interlocking changes across the gateway server (shared GatewaySession.cs /
SessionManager.cs), committed together:

- GWC-01 (Critical): alarm monitor now attaches as an internal
  (non-counted) distributor subscriber instead of a second raw drain of the
  single worker event channel; WorkerClient._events -> SingleReader with a
  claimed-once guard so a future dual-consumer regression throws loudly.
- GWC-02 (High): faulted sessions are swept in CloseExpiredLeasesAsync
  (IsFaultedReapable + FaultedReason); new FaultedGraceSeconds (default 0).
- GWC-03 (High): configurable MaxSparseArrayLength (default 1_000_000)
  enforced before allocation.
- TST-02 (High, security): StreamEvents attach now enforces the opening key
  id -> PermissionDenied on owner mismatch.
- TST-12 (Medium): CLAUDE.md retention-defaults sentence corrected.

Verified: NonWindows build clean; targeted tests 135/135 on macOS, plus
WorkerClientTests 18/18 on the Windows host.
2026-07-09 05:51:57 -04:00
Joseph Doherty fca978de07 docs(src): add missing XML docs and strip tracking-ID comments
Sweep of 203 source files resolving CommentChecker findings: add
<summary>/<param>/<returns>/<inheritdoc> where missing, and remove
resolved task/issue tracking markers (Tests-NNN, Worker-NNN, Server-NNN,
Task N) from code comments. Comment/doc-only — no logic changes.
Server+Tests build clean under TreatWarningsAsErrors.
2026-07-07 14:09:49 -04:00
Joseph Doherty 2671639250 fix(gateway): resolve 2026-06-18 array-write review findings
- Server-057: extend []-suffix normalization to AddItemBulk/AddBufferedItem so bulk-added
  array tags bind write-capable handles (authz check, worker bind, and registration kept
  consistent); update gateway.md + client READMEs. Tests: AddItemBulk/AddBufferedItem wiring.
- Server-058: assert []-fallback-resolved bare array names are still denied when out of
  read/write scope and that MaxWriteClassification is enforced on suffixed array registrations.
- Contracts-023/024/025: round-trip + field-19 descriptor pin for MxSparseArray; document
  MxSparseArray in docs/Contracts.md; enumerate it in the protocol-version-3 test summary.
- Tests-040: add wiring tests for the six uncovered sparse-write arms (WriteSecured, Write2,
  WriteSecured2, Write2Bulk, WriteSecuredBulk, WriteSecured2Bulk).

dotnet build + targeted tests green (184 passed).
2026-06-18 10:58:42 -04:00
Joseph Doherty f0ef7ea0a8 feat(gateway): normalize array AddItem suffix and expand sparse writes at the worker boundary 2026-06-18 03:10:13 -04:00
Joseph Doherty 6b5fe6aa82 fix: resolve code-review findings (locally verified)
Server-054/055/056, Contracts-020/021/022, Tests-036/038/039,
IntegrationTests-030/031/032 (+033 deferred to live rig),
Client.Dotnet-026/028/029 (+027 won't-fix), Client.Go-030..034,
Client.Python-032..036, Client.Rust-033..038.

Key fix: SessionEventDistributor orphaned a subscriber that registered after
the pump completed but before disposal (Server-056) -> register paths now
complete late registrants under _lifecycleLock; regression test added. The
racy dashboard-mirror gRPC test made deterministic (Tests-039).

Verified green locally: gateway Tests targeted classes (GatewaySession,
SessionEventDistributor, GatewayOptionsValidator, ProtobufContractRoundTrip,
GatewaySessionDashboardMirror) + dotnet/go/python/rust client suites.
2026-06-17 05:23:14 -04:00
Joseph Doherty 4ab3bd55e5 fix(server): single-clock poll loop + drop dead sync GetReadyWorkerClient (Task 8 review) 2026-06-16 17:04:18 -04:00
Joseph Doherty 4966ef3359 feat(server): bounded worker-ready wait in GatewaySession (default off) 2026-06-16 16:48:02 -04:00
Joseph Doherty 36ab8d15f1 feat(sessions): replay-on-reconnect with ReplayGap sentinel 2026-06-16 07:22:19 -04:00
Joseph Doherty 042f5e3d82 fix(sessions): expose DetachGraceSeconds in effective-config; single clock; close reconnect-vs-sweep race
- EffectiveSessionConfiguration: add DetachGraceSeconds field; GatewayConfigurationProvider
  forwards value.Sessions.DetachGraceSeconds (blocker fix).
- GatewaySession.InvokeAsync and ReadEventsAsync: switch TouchClientActivity calls from
  DateTimeOffset.UtcNow to _eventStreaming.TimeProvider.GetUtcNow() so Task 12 fake-clock
  control works end-to-end (split-clock fix).
- TOCTOU fix: add TryBeginCloseIfExpired(now, out alreadyClosing) to GatewaySession that
  re-checks IsLeaseExpiredCore/IsDetachGraceExpiredCore AND _activeEventSubscriberCount==0
  under _syncRoot before transitioning to Closing; CloseExpiredLeasesAsync calls it before
  CloseSessionCoreAsync so a reattach that wins the race leaves the session Ready/usable.
- Minors: lease-expiry-takes-precedence comment in CloseExpiredLeasesAsync; TOCTOU comment
  block; sweep-cycle latency note added to SessionOptions.DetachGraceSeconds XML doc and to
  GatewayConfiguration.md DetachGraceSeconds row.
- New tests: TryBeginCloseIfExpired_ReattachedSubscriberWinsRace_DeclinesClose (GatewaySession),
  CloseExpiredLeasesAsync_DoesNotCloseSessionThatReattachedBeforeSweepCloses (SessionManager),
  plus IsLeaseExpiredCore/IsDetachGraceExpiredCore private helpers used by the guard.
2026-06-16 07:11:59 -04:00
Joseph Doherty db95f8644f feat(sessions): detach-grace retention window for reconnect 2026-06-16 06:15:46 -04:00
Joseph Doherty 281e00b300 refactor(sessions): derive subscriber mode from session config; close Task 8 review nits
Remove the per-call allowMultipleSubscribers param from AttachEventSubscriber and
derive the mode internally from _eventStreaming.AllowMultipleEventSubscribers — the
same source SessionEventDistributor uses for singleSubscriberMode — so the two can
never structurally diverge. The maxSubscribers cap param is kept because
MaxEventSubscribersPerSession lives in SessionOptions, which the session does not hold
directly (only EventOptions flows through SessionEventStreaming).

Other nits:
- SubscriberCount XML doc clarifies it includes internal subscribers and differs from
  GatewaySession.ActiveEventSubscriberCount (external/gRPC only).
- SingleSubscriberMode_LoneExternalOverflow test: add Assert.Equal(1, observedSet) guard
  before the value assertion so the test cannot pass vacuously if the handler never fired.
- GatewayOptionsValidator.ValidateSessions: add explanatory code comment documenting why
  !AllowMultipleEventSubscribers && MaxEventSubscribersPerSession > 1 is NOT rejected as
  a hard error (the default config ships with this combination; the cap is simply unused
  in single-subscriber mode, not a behavior bug).
- GatewaySession.DetachEventSubscriber: add Debug.Assert before the clamp so a genuine
  double-decrement surfaces in debug builds.
2026-06-15 15:53:27 -04:00
Joseph Doherty ac42783e36 feat(sessions): multi-subscriber cap enforcement + mode-gated FailFast 2026-06-15 15:32:08 -04:00
Joseph Doherty 2ead9bc200 fix(dashboard): close StartDashboardMirror/DisposeAsync race; internal-overflow test + metric label
(1) GatewaySession.StartDashboardMirror: publish _dashboardMirrorLease and _dashboardMirrorTask
    atomically under one _syncRoot section; if the session is already Closing/Closed/Faulted,
    dispose the just-created lease and return without starting the mirror task so nothing is orphaned.
(2) WaitUntilAsync test helper: catch OperationCanceledException and call Assert.Fail with the
    timeout duration and predicate source text instead of letting the exception propagate raw.
(3) New SessionEventDistributorTests.InternalSubscriberOverflow_HandlerSeesIsOnlySubscriberFalse:
    verifies CountExternalSubscribers excludes the internal subscriber, so isOnlySubscriber==false
    even when the internal subscriber is the only registered subscriber.
(4) SubscriberOverflowHandler delegate gains isInternal parameter; overflow metric label is
    "dashboard-mirror" for internal subscribers and "grpc-event-stream" for external ones.
(5) DashboardEventBroadcaster.Publish: wrap SendAsync Task acquisition in try/catch so a
    synchronous throw cannot escape the never-throw Publish interface contract.
2026-06-15 15:02:36 -04:00
Joseph Doherty 1ea08c3b10 feat(dashboard): mirror events via SessionEventDistributor subscriber (fixes dark feed without gRPC client) 2026-06-15 14:42:32 -04:00
Joseph Doherty 039111ca05 feat(sessions): per-subscriber backpressure isolation in SessionEventDistributor 2026-06-15 13:39:25 -04:00
Joseph Doherty 61627fc5b0 fix(sessions): make EventSubscriberLease dispose atomic; dedupe lease dispose
Issue 1: replace plain bool _disposed in EventSubscriberLease with an
Interlocked.Exchange int (_leaseDisposed) matching the SubscriberLease
pattern in SessionEventDistributor. Concurrent stream-completion +
client-cancellation racing Dispose() now decrements _activeEventSubscriberCount
exactly once, never to -1.

Issue 5: remove the `using` declaration on the subscriber lease in
EventStreamService.StreamEventsAsync; the finally block already disposes it
alongside the reader, so the using was a redundant second dispose on the
same code path.

Issue 2: add an inline comment at the StartAsync().GetAwaiter().GetResult()
call documenting the sync-over-async invariant (StartAsync only schedules via
Task.Run and is synchronous; do not make it truly async without changing
this call site).

Issue 10: remove the redundant .WithCancellation(cancellationToken) chained
on ReadEventsAsync(cancellationToken) in MapWorkerEventsAsync; the
[EnumeratorCancellation] token already flows through the direct argument.

Issue 9: add EventSubscriberLease_ConcurrentDispose_DecrementsCountExactlyOnce
to GatewaySessionTests — 16 concurrent Dispose() calls on the same lease for
200 iterations; asserts count is exactly 0 after each race and a subsequent
single-subscriber AttachEventSubscriber succeeds.
2026-06-15 13:29:27 -04:00
Joseph Doherty 7f1018bac1 feat(sessions): route event streaming through SessionEventDistributor 2026-06-15 13:18:28 -04:00
Joseph Doherty a43b2ee6af test(sessions): cover OwnerKeyId service-layer forwarding; doc 11-param ctor
Add LastOwnerKeyId capture to FakeSessionManager and assert it equals
"operator01" in OpenSession_WithValidRequest_ReturnsSessionDetails, closing
the gap where OwnerKeyId threading through the service layer had no test
coverage. Add a <remarks> to the 11-param GatewaySession convenience ctor
documenting that OwnerKeyId is null there and authenticated call sites must
use the 12-param overload.
2026-06-15 12:29:16 -04:00
Joseph Doherty f5479f3ca3 feat(sessions): record OwnerKeyId on session creation
Add a nullable string? OwnerKeyId property to GatewaySession that captures
the API key identifier (KeyId) of the authenticated caller that opened the
session. Wire it through ISessionManager.OpenSessionAsync → SessionManager
→ GatewaySession constructor. The gRPC service passes identityAccessor
.Current?.KeyId; internal callers (GatewayAlarmMonitor, DashboardLiveDataService)
pass null. Covers the positive and null cases with two new TDD-first tests.
2026-06-15 12:24:29 -04:00
Joseph Doherty 615b487a77 docs+ui: backfill XML doc comments and finish dashboard layout pass
Adds missing <summary>/<param> XML docs across 99 server, worker, and test
files so CommentChecker reports zero issues (TreatWarningsAsErrors needs the
analyzer clean). Bundles in WIP dashboard work: NavSection extraction,
MainLayout/site.css/js styling alignment, and DashboardOptions/Auth tweaks.
2026-05-27 14:20:10 -04:00
Joseph Doherty 4d77279e7e Resolve Server-044..050: KillWorker accounting + admin service hardening
Server-044  KillWorkerAsync catch path now calls _metrics.SessionRemoved
            so the open-session gauge does not leak when KillWorker throws.
Server-045  KillWorkerAsync routes through a new
            GatewaySession.KillWorkerWithCloseGateAsync that takes the
            per-session close lock, so concurrent kills count SessionsClosed
            exactly once.
Server-046  CloseSessionCoreAsync's SessionCloseStartedException branch and
            ShutdownAsync's kill fallback both increment SessionsClosed (not
            just the gauge), so the counter and gauge stay consistent.
Server-047  ApiKeysPage.ConfirmPendingAsync holds PendingAction across the
            awaited action and clears it in finally, matching the sessions
            pages.
Server-048  Closed: the 044/045 regression tests cover the previously-
            untested kill paths.
Server-049  IDashboardSessionAdminService + DashboardSessionAdminService
            now carry XML docs that pin the Admin gate, missing-session
            return-Fail semantics, and the dashboard-admin-kill reason.
Server-050  CloseSessionAsync and KillWorkerAsync catch unexpected
            exceptions after the SessionManagerException catches and return
            a friendly Fail; OperationCanceledException tied to the caller
            token still propagates.

All resolved at 2026-05-24; 503/503 gateway tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-24 08:49:34 -04:00
Joseph Doherty dc9c0c950c rename: prefix gateway projects/namespaces with ZB.MOM.WW + sln→slnx
Apply the ZB.MOM.WW. prefix to all gateway-side projects, folders,
.csproj/.sln contents, C# namespaces, using directives, generated proto
C# (csharp_namespace + checked-in generated files), InternalsVisibleTo
attributes, project-name string literals (LoadProject, .sln lookups,
worker exe paths, staticwebassets manifest), and the install/script/doc
references that point at any of the above. Migrate the solution from
.sln to .slnx via `dotnet sln migrate` and delete the old file.

External-runtime identifiers are intentionally NOT prefixed so external
configuration keeps working:
- GatewayMetrics.cs MeterName ("MxGateway.Server")
- DashboardAuthenticationDefaults Scheme/Policy ("MxGateway.Dashboard")
- GatewayRequestLoggingMiddleware logger category ("MxGateway.Request")
- StaRuntime thread name ("MxGateway.Worker.STA")
- appsettings.json root section "MxGateway" + env-var prefix
  MxGateway__... and secret-name MxGateway:ApiKeyPepper
- C:\ProgramData\MxGateway\ data dir paths

Also fixes two tests that were not rename-related but became visible
while validating the rename:

- WorkerLiveMxAccessSmokeTests.ShutDownAsync: cancellation that the
  gateway service correctly maps to RpcException(Cancelled) per gRPC
  convention was being misclassified as a stream fault. Added a sibling
  catch on RpcException with StatusCode.Cancelled.

- IntegrationTestEnvironment.ResolveRepositoryRoot: extracted IsRepositoryRoot
  and made it accept either a .git marker OR a .sln/.slnx next to src/
  so the worker-exe walker works in non-git working copies.

clients/proto/proto-inputs.json's protoRoot updated to point at
src/ZB.MOM.WW.MxGateway.Contracts/Protos.

Verified by `dotnet build` and a full `dotnet test` of the .slnx with
MXGATEWAY_RUN_LIVE_{MXACCESS,LDAP,GALAXY}_TESTS=1:
  Tests: 472/472 pass
  Worker.Tests: 280/280 pass (4 dev-rig [Fact(Skip=...)] skipped)
  IntegrationTests: 18/18 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-23 16:22:23 -04:00