The subtag fallback's coverage was fixed at SubscribeAlarms time and never
re-examined. AlarmWatchListResolver deliberately swallows a Galaxy Repository
outage and returns whatever configuration contributed, so a discovery failure at
startup left the monitor subscribed with an empty watch-list for the rest of its
life. In Auto mode the alarmmgr primary keeps working, so nothing looks wrong
until the failover that needs the standby finds it covering zero alarms — with a
single LogWarning emitted possibly weeks earlier as the only evidence.
ResolveAsync now returns an AlarmWatchListResolution (targets + whether discovery
was attempted and whether it failed, with the failure message), which is what the
caller needed to tell "discovery produced nothing" from "nothing was configured".
GatewayAlarmMonitor grades that resolution against the effective mode and folds
the verdict into AlarmProviderStatus, so it reaches the feed, the dashboard badge
and every late-joining subscriber:
- zero targets while the mode can need them (Auto/ForceSubtag) -> degraded with
SubtagWatchListEmpty; discovery enabled but failing adds SubtagDiscoveryFailed
and the message. ForceAlarmManager builds no subtag consumer and is not graded.
- degraded is now the OR of "subtag is serving" and "the watch-list covers
nothing", so the Auto case where alarmmgr is healthy and the standby is empty
is visible. DashboardAlarmProviderStatus renders that combination as the amber
"Alarm Manager (no subtag standby)" badge rather than the failover label, which
would have named the wrong live provider.
- on the failover edge (ApplyProviderModeChangeAsync, toMode == Subtag) the
monitor re-resolves BEFORE publishing the status and reports any difference as
SubtagWatchListStale with both counts.
The re-resolution is deliberately NOT re-sent. The worker rejects a second
SubscribeAlarms while subscribed (AlarmCommandHandler.Subscribe), and the only
expressible replacement — UnsubscribeAlarms then SubscribeAlarms — rebuilds
FailoverAlarmConsumer, which restarts on the primary that just failed and
discards the warm standby state, so it would flap the feed rather than repair it.
The finding's other alternative (treat empty coverage as a lifecycle failure) was
rejected too: it would crash-loop the monitor every 5 s on a persistent GR outage
and take the working alarmmgr feed down with it. A monitor restart re-resolves and
re-sends, and the status now says so.
Docs: docs/AlarmClientDiscovery.md gains the grading rules and corrects two stale
claims (the resolver method name, and "re-runs discovery on its reconcile cadence
and pushes an updated watch-list", which never happened); gateway.md gains the
standby-coverage visibility paragraph; docs/GatewayConfiguration.md records the
consequence on the UseGalaxyRepository and Mode rows.
Tests: SubtagCoverage_WhenDiscoveryFailsAtSubscribe_ProviderStatusIsDegradedWith-
Reason and SubtagCoverage_WhenDiscoveryRecoversByFailover_ReResolvesAndReportsDrift
drive the fake-worker monitor; two resolver cases pin the new provenance fields.
The pre-existing dashboard case for degraded+ALARMMGR now asserts the new label.
The worker reports startup failures as structured WorkerFault frames --
MxAccessCreationException carries the COM HRESULT/ProgID context and a
WorkerFaultCategory -- so the gateway can tell "MXAccess is not installed / not
registered / access denied" apart from a pipe-level protocol problem.
ReadHandshakeEnvelopeAsync did not look inside: any body other than the expected
one became a ProtocolViolation naming only the body type, and that text is what
SessionManager puts in the operator log and the client's OpenFailed detail. The
information was lost exactly in the window where it matters most -- first
session open on a freshly deployed host, where the answer is almost always an
MXAccess installation or permission problem.
Special-case BodyCase == WorkerFault and throw WorkerClientException(
WorkerFaulted, CreateWorkerFaultMessage(...)) -- the same formatter the
steady-state fault path already uses, so both windows read alike. Every other
unexpected body still gets ProtocolViolation.
Test: StartAsync_WhenWorkerFaultsDuringHandshake_SurfacesFaultCategoryAndDiagnostic
has the fake worker answer the hello and then write a WorkerFault
{MxaccessCreationFailed} in place of WorkerReady; the existing
FakeWorkerHarness.EmitFaultAsync covered it, so the harness needed no change.
IPC-08 closed with WorkerCancel sent from the gateway's own timeout arm only.
Its rationale -- the gateway has stopped waiting but the worker has not stopped
working, so a queued-but-not-started command should be dropped instead of
occupying the single STA behind a caller that is already gone -- applies exactly
as well to caller cancellation, which is the common abandonment path: gRPC
clients routinely set per-call deadlines shorter than the gateway's
CommandTimeout, and a client disconnect cancels the RPC's token. Both landed in
arms that removed the pending entry and returned silently, leaving the
correlation to execute in its turn. Under a retry storm against a slow provider
the STA works through dead commands while live callers queue behind them --
precisely the backlog the cancel exists to shorten.
- TrySendCancelForTimedOutCommand is now TrySendCancelForAbandonedCommand and
takes the reason string, since it serves both arms.
- Both caller-cancellation arms route through AbandonCommandOnCallerCancellation,
which sends the cancel under the same `removed` ownership gate as the timeout
arm (so a reply that won the race is never chased) and skips it once _stopCts
has fired, where the outbound channel is being completed and every pending
command is failed anyway.
- New WorkerClientErrorCode.CallerCancelled replaces the GatewayShutdown label
those arms used, so metrics and logs stop recording a shutdown that did not
happen. It maps to StatusCode.Cancelled, the same status GatewayShutdown maps
to, so nothing changes for clients.
Docs: docs/Grpc.md mapping line, gateway.md cancellation rules,
docs/GatewayProcessDesign.md pending-command section.
Tests: InvokeAsync_WhenCallerCancels_SendsWorkerCancelForThatCorrelation (twin
of the timeout test, driven by the caller token; verified failing without the
send) and InvokeAsync_WhenClientStopsUnderInFlightCommand_DoesNotSendWorkerCancel.
Committed together because the two changes share the same doc paragraphs (the ACL
section of GatewayDashboardDesign.md, the GroupToTag / AllowAnonymousLocalhost rows
of GatewayConfiguration.md, and CLAUDE.md's authentication paragraph), and splitting
them would leave one of the two describing behaviour that does not yet exist.
SEC-41 — tag grants no longer trusted from login-stamped claims
---------------------------------------------------------------
DashboardSessionAcl read the zb:dashboardtag claims stamped into the cookie at
login. GatewayOptions binds once at startup, so applying a GroupToTag edit needs a
restart — but the restart does not clear the auth cookie (data-protection keys
persist), so a browser returning afterwards re-authenticated with tag claims
resolved under the OLD map. A revoked grant therefore survived on the cookie seam
for the sliding 8-hour idle life, renewable indefinitely by activity, while the
hub-token seam had no such gap because its mint re-resolves.
CanViewSession now re-resolves the grant on every subscribe from the caller's
mxgateway:ldap_group claims through DashboardGroupTagMapping.MapGroupsToTags
against the currently bound map — the same one call HubTokenService.Issue makes.
Fallback (documented on the class and in the design doc): a principal carrying NO
group claims falls back to its stamped tag claims. That arm exists for the
hub-token principal — HubTokenService.Validate rehydrates roles and tags but not
groups, so re-resolution already happened at mint, one lifetime ago at worst. It
cannot re-open the cookie hole: DashboardAuthenticator stamps group and tag claims
from one group list, so a cookie principal holding a tag claim always holds the
groups it came from, and a login yielding no groups yields no tags. A principal
with neither kind of claim stays an empty-grant Viewer.
SEC-40 — the alarm feed requires a real authenticated dashboard role
--------------------------------------------------------------------
AlarmsHub sat behind HubClientsPolicy, which the read-only environment bypasses
satisfy, so an anonymous-localhost caller or a Mode=Disabled deployment received
galaxy-wide alarm metadata — alarm reference (a full tag path), state, severity,
category, area, acknowledging operator — for the whole subscription expression.
Decided: tighten the gate, keep the scope. New AuthenticatedDashboardRoleRequirement
+ its handler, and a MxGateway.Dashboard.Alarms policy built from it. Because
DashboardAuthorizationHandler is typed to DashboardAuthorizationRequirement, it is
never invoked for the new type, so neither Authentication:Mode=Disabled nor
anonymous localhost can satisfy the policy — the same structural device the mounted
Secrets UI already relies on. The policy keeps both schemes so external hub clients
can still authenticate with a bearer token, and DisableLogin auto-login is
unaffected (it mints a genuinely authenticated both-roles principal).
The /alarms page carries the same policy, not just the hub: it reads the same
central monitor in process, so gating only the hub would have left the disclosure
one navigation away. The side rail hides the Alarms link for callers the policy
denies, matching the Secrets link precedent. AlarmsHubPublisher is unchanged and
stays unfiltered by design — audience is decided at hub admission, and the monitor
carries no per-session provenance to narrow by.
Alarm metadata deliberately remains galaxy-wide for authenticated Viewers: an
AlarmFeedMessage has no session id or owning key for the SEC-25 tag intersection to
work against, and equating a visibility tag with an alarm-reference prefix would
turn GroupToTag into the data-access constraint it explicitly is not.
Tests
-----
DashboardSessionAclTests: the SEC-41 regression — a cookie-style principal whose
stale tag claim is no longer granted by any of its groups under the current map is
denied; plus the allowing direction (including with no stamped tag at all, which
proves the decision comes from the groups), the ignore-case corner, group claims
that map to nothing, and the hub-token fallback asserted against an EMPTY map so
dropping the fallback would fail loudly.
AlarmFeedAuthorizationTests (new): both handlers run together in the most permissive
environment (Mode=Disabled + loopback + AllowAnonymousLocalhost) do not satisfy the
alarm requirement, with the bypassable requirement as the control so the negative
cannot pass because the bypass was misconfigured; authenticated Viewer/Admin do
satisfy it; an unauthenticated role claim does not; the registered policy carries
the non-bypassable requirement and both schemes; and both readers — the hub type's
[Authorize] and the /alarms endpoint metadata — carry it.
SecretsNavRenderTests: registers the new policy (an unregistered policy name throws
rather than denying in an AuthorizeView) and gains the Alarms-link negative/control
pair.
Verified: dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx (zero warnings);
filtered run of the touched classes — 92 passed, 0 failed.
GatewayBrowseScopeProvider resolved `identityAccessor.Current?.EffectiveConstraints
?? ApiKeyConstraints.Empty`, so an absent ambient identity produced an empty
BrowseSubtrees list — which the shared GalaxyRepository library reads as "no
scoping". The control's fail state was therefore open: any future path reaching the
library's galaxy service without GatewayGrpcAuthorizationInterceptor having pushed
the identity would silently hand a browse-scoped key the full hierarchy, with
nothing failing and nothing logged. A constraint-narrowing control's absence is
invisible in normal operation, which is what made the honest "safe only because the
global interceptor runs first" comment insufficient on its own.
The provider now distinguishes the two states the old expression collapsed:
- authenticated key with no BrowseSubtrees constraint -> empty list, unscoped
(unchanged, and now asserted in its own test rather than implied);
- no ambient identity with Authentication:Mode=ApiKey -> log a warning naming the
broken ordering invariant and throw RpcException(PermissionDenied);
- no ambient identity with Authentication:Mode=Disabled -> unscoped, because
nothing authenticates there so no key and no constraints exist to scope by.
The registration-order comment at GatewayApplication.cs now says the ordering is
still required but no longer silently trusted.
Tests: GatewayBrowseScopeProviderTests gains the PermissionDenied case and the
Disabled-mode case alongside the unconstrained-identity one.
GalaxyRepositoryHostWiringTests phase 1 now pushes an identity carrying no
BrowseSubtrees instead of pushing none — that, not an absent identity, is
production's unscoped case.
Docs: docs/Authorization.md's constraint section states the two-state distinction
and both fail behaviours under browse_subtrees.
EnsureDistributorCreated read a null _eventDistributor as "not created yet",
which is true before MarkReady and false after DisposeAsync nulled it. An
attach that passed its readiness gate while the session was still Ready and
then lost the CPU across the whole close + dispose therefore built a
REPLACEMENT distributor: startNow came back false because the pump-start flag
is a one-way latch, so the subscriber registered on a distributor whose pump
never starts and whose channel is never completed, and EventStreamService
blocked in WaitToReadAsync until the client's own deadline — no error, no log,
no metric, the backlog gauge reporting 0 forever. This is the disposal-side
twin of the premature-attach poison GWC-27 closed.
DisposeAsync now sets _eventDistributorDisposed under _syncRoot in the same
section that nulls the distributor, and EnsureDistributorCreated throws
SessionManagerException(SessionNotReady) when it is set. The attach paths'
existing catch/rollback releases the subscriber slot, so a second attempt
still reports SessionNotReady rather than EventSubscriberAlreadyActive.
StartDashboardMirror is the one caller that is not an attach: it abandons the
mirror (releasing its loop token source) instead of propagating, so MarkReady
stays total. Losing that race previously left a started orphan distributor
behind, so this path strictly improves too.
Test: AttachAfterDisposeThrowsSessionNotReadyInsteadOfRegisteringOnANeverPumped-
Distributor covers all three attach entry points plus the rollback, and fails
on the pre-fix code. docs/Sessions.md records the disposal end of the
one-distributor-per-session rule.
Sweep code/doc comments citing unregistered GWC-30/GWC-31 (2026-08-15 perf
pass) to PERF-02/PERF-07/PERF-22, keyed to task numbers in
docs/plans/2026-08-15-perf-review-remediation.md; add a PERF-ID register
table to that plan. Genuine cycle-2 GWC-30 citations (gateway-side frame-
reader 4-byte prefix fix in Server/Workers/WorkerFrameReader.cs and its
test) are left untouched.
Add a line to REVIEW-PROCESS.md reserving the GWC/WRK/IPC/SEC/CLI/TST
prefixes for the archreview domain registers.
Tracker housekeeping per archreview/2026-08-19/00-overall.md:
- GWC-16 -> Won't fix (rationale: PERF(followup) comment documents the
double session lookup as measured-negligible, deliberately not collapsed)
- IPC-08 -> Done (closed by 7c1ea12/a1a38b5; residual tracked as IPC-34)
- IPC-31 -> Done (superseded by GWC-28's write-time sequence stamping)
- TST-06, TST-17 rows refreshed per cycle-3 60-testing-docs-gaps.md
- WRK-31 (and defensively WRK-29/30) registered as reserved/do-not-reuse
interim IDs burned by e913dab, in both the cycle-2 tracker and the
cycle-2 worker remediation doc
Flip Batch C to Done in archreview/2026-08-19/remediation/00-tracking.md.
Worker-side files (Worker/Ipc/WorkerFrameReader.cs, WorkerFrameWriter.cs)
are comment-only edits, unverified here (net48 x86, windev-only build).
DisposeAsync is the third path that completes the outbound, staging and event
channels, but unlike MarkClosed and SetFaulted it left _state untouched. A
client disposed while still Ready therefore kept reporting
IsTerminalState() == false, so a read loop already inside DispatchEnvelope when
the staging channel was completed took StageWorkerEvent's overflow arm: a
spurious ProtocolViolation fault, a worker-event-staging overflow metric, a
redundant kill and a WorkerStopped("ProtocolViolation") metric during an
intentional teardown — on exactly the path an operator inspects after a failed
SessionManager.OpenSessionAsync.
DisposeAsync now takes _syncRoot once, claims _disposed there, and transitions
to Closed only when the state is not already Closed or Faulted, so a
dispose-after-fault keeps Faulted and a double dispose is a no-op. Because the
transition is now real, the dispose path records the worker stop through the
existing RecordWorkerStoppedOnce latch, which also stops the running-worker
gauge leaking when a Ready client is disposed without a prior close or fault.
MarkClosed is deliberately not reused: it would overwrite Faulted with Closed
and would re-run a teardown sequence DisposeAsync performs itself.
The StageWorkerEvent XML doc now names all three completion paths.
Test: DisposeAsync_WhileWorkerIsEmittingEvents_ClosesWithoutFaultingOrOverflowing
disposes a Ready client mid-event-stream and asserts Closed state, zero faults,
zero queue overflows, zero kills and exactly one recorded worker exit. It fails
on the pre-fix code.
Targeted re-read of the 203-file fca978d sweep (docs(src): add missing
XML docs and strip tracking-ID comments): a mechanical pre-pass narrowed
1,383 deletions to 68 files / 816 residual prose lines, and a judged
review of every one found 17 collateral deletions across 10 files —
rationale prose deleted alongside resolved markers with no equivalent
surviving anywhere in the tree. Restored (markers stay stripped, per the
sweep's intent):
- SessionManager: the three metrics-accounting invariants (kill-path
gauge decrement safety, shutdown kill-fallback registry guard vs
double bookkeeping, SessionClosed-not-SessionRemoved on failed close)
- SessionManagerTests: the matching accounting expectation note and the
reason-string propagation pins (test summary + FakeWorkerClient.LastKillReason)
- MxAccessGatewayService.AcknowledgeAlarm: the routing remarks (GUID vs
Provider!Group.Tag vs InvalidRequest; session-less via IGatewayAlarmService)
— inheritdoc resolves to nothing (proto-generated base is undocumented)
- HubTokenService.Validate: why the hollow-token guard exists
(non-empty AuthenticationType alone satisfies IsAuthenticated)
- DashboardSessionAdminService: why both broad catches exist (keep raw
teardown exceptions out of Blazor's error boundary), Close + Kill paths
- WorkerPipeSession.RunAsync: why the factory result throws instead of
NREing (unambiguous failure; finally-block Dispose can't no-op)
- LmxSubtagAlarmSource: Advise idempotency; Write is always unsecured
(user id 0), never WriteSecured semantics
- WnWrapAlarmConsumer: the v1-prefix path is what WIN-911-style code uses
- DashboardSnapshotPublisherTests: what the 10ms slack absorbs
(Task.Delay's coarse Windows timer quantum)
- DashboardBrowseAndAlarmModelTests: why the label text is pinned, not
just the CSS class
Everything else flagged verified benign: inheritdoc replacements resolve
to equal-or-richer interface docs, or the substance survives relocated.
NonWindows slnx 0W/0E; touched gateway test classes 65/65.
The final integration review's non-blocker reservations, all documentation
or comment truth except one test arm.
The alarm feed opens provider_status -> snapshot_status -> cached
active_alarm -> snapshot_complete, which is what GatewayAlarmMonitor has
done since the snapshot_status frame landed. Two places still described
the old order: docs/Grpc.md said provider_status arrived *after* the
initial snapshot, contradicting its own snapshot_status section two
paragraphs down, and AlarmFeedMessage's leading proto comment named
neither status frame at all. Both now state the sequence the monitor
emits, so a client author reading either one gets the frame order right.
The proto comment change flows through the generated trees (Contracts,
Go, Java) and the client descriptor set; the Rust vendored copy stays
byte-identical to canonical. Python's generator does not carry proto
comments into its output, so it has no delta.
AlarmsHubPublisherTests' valueless-payload case covered snapshot_complete
and provider_status but not snapshot_status, leaving the newest arm
unpinned against the redaction switch that must ignore it. Added.
WnWrapAlarmConsumer's ack comment led with the 2026-05-01 reading that
-55 tracks the 8-arg overload, then refuted itself six lines later with
the 2026-08-18 probe. It now leads with the observation labelled as
narrower than it reads -- mirroring the correction already in
docs/AlarmClientDiscovery.md -- so the block argues one thing: the 6-arg
call site stays for parity, and rc semantics are per the probe. A
paragraph orphaned by an earlier splice is rewrapped. Comment interior
only; the file compiles on Windows.
TST-16 gets a dated closure note rather than a rewrite: the flag it
called dead was implemented 2026-08-18. GatewayDashboardDesign's /browse
paragraph gains the failed-read carve-out GatewayConfiguration already
documented, so the two agree that a failed read keeps its - placeholder.
Follow-up to the per-session event ACL. Part of that change rode into 693a78d
via a concurrent agent's pathspec-less commit; this commit carries the review
fixes and uses pathspecs on the commit itself so it cannot recur in either
direction.
Gating the page's subscribe seam made AttachEvents asynchronous — it awaits the
authentication state — and that await is a suspension point the synchronous
version did not have. On a rapid A -> B navigation the suspended A continuation
resumes after B's parameter set has run to completion, re-reads the live
SessionId (now B's), and attaches B a SECOND time. The ACL is not bypassed —
the newer attach already cleared that same session — but the fields holding B's
first subscription are overwritten in place, so nothing ever disposes it: its
EventsHubViewerRegistry entry is never released, which keeps the mirror cloning
events for a session the page is no longer watching through that handle, and
its pump is never cancelled. A resource leak the ACL work introduced.
OnParametersSetAsync now claims a monotonic _attachGeneration synchronously,
before its first await, and AttachEventsAsync re-checks it after the await and
before any field write or Subscribe call. A stale attach returns rather than
detaching: it owns nothing, and tearing down there would destroy the newer
attach's subscription. DetachEventsAsync needs no such guard — it captures and
nulls the live fields synchronously before it awaits, so a resumed detach only
unwinds what it already took ownership of. Same dispatcher-owned identity idea
as the existing ReferenceEquals guards in PumpEventsAsync and
MarkDisconnectedAsync, one level up.
The interleaving is not expressible with the static HtmlRenderer idiom the other
page tests use: it renders a root component once and exposes no parameter-update
seam. The new test therefore adds a minimal Renderer subclass whose only job is
to mount a component and drive a second SetParametersAsync into it while the
first is parked on a gated AuthenticationStateProvider. That subclass is the
lone reason for a narrowly scoped BL0006 suppression, justified in place: it is
test-only scaffolding that never ships, and the cost of the warning coming true
is a compile break in one test file on an SDK bump. Confirmed non-vacuous by
mutation — with the generation check disabled the test goes red on the doubled
subscription and the two passing ACL tests stay green.
Two decision-table corners are now pinned rather than implied. Admin x
nonexistent session id resolves to ALLOW, because the admin bypass is evaluated
before the registry lookup; a plausible "look the session up first, it reads
better" refactor would flip it, so a test documents the ordering. EventsHub's
remarks said "an unknown session id is denied" without qualification, which read
as universal; they now state that the bypass is checked first and every rule
below it is a non-Admin rule.
HubTokenServiceTests gains the truly-absent-field case: a hand-built payload
JSON with no Tags key at all, protected through the same purpose, which is the
shape every in-flight token has across the deploy that introduces the field. The
existing test covered present-but-empty, which does not exercise the null
coalesce that stands between a legacy token and a crash on the hub auth path.
ProtectorPurpose became internal so the test cannot drift from the real purpose
string.
Tag-count cardinality cap considered and recorded as a deliberate non-goal.
Build 0 warnings / 0 errors; 48 filtered (ACL/hub/token/page) and 257 dashboard
tests pass.
The truncation-cliff fix made alarm transitions truncation-safe but silent:
when GetXmlCurrentAlarms2 returns exactly maxAlmCnt records the worker
suppresses absence-implies-Clear inference and says so only in a rate-limited
stderr warning. No client and no operator could tell a complete active set
from a capped one.
Two additive proto3 booleans carry the verdict out:
- QueryActiveAlarmsReplyPayload.snapshot_truncated = 2 (worker IPC reply)
- ActiveAlarmSnapshot.from_truncated_snapshot = 16 (per record)
The per-record field is not an aesthetic choice. QueryActiveAlarms returns a
bare `stream ActiveAlarmSnapshot` with no envelope, header, or trailer, so a
per-record boolean is the only carrier that stays wire-compatible; an envelope
message would change every existing client's stream element type. The reply
payload states it too because a prefix filter can leave zero records and a
truncated fetch with nothing to report still has to say so. The flag means
"this set may be incomplete", never "this record is unreliable" — it is
independent of the subtag-fallback `degraded` field.
Detection is deliberately UNCHANGED: IsTruncatedFetch remains
`fetchedRecordCount >= maxAlarmsPerFetch`. The live probe (docs/AlarmProbeFindings.md,
ce5d8ae) could not verify whether ALARM_RECORDS/@COUNT reports the total active
count or only the records in the reply, so @COUNT is not parsed for detection;
switching to it stays blocked on probe evidence. The probe's comment
annotations in WnWrapAlarmConsumer.cs are preserved.
Reset semantics: not latched. WnWrapAlarmConsumer.FoldFetch replaces the
verdict on every poll under the same lock as the snapshot merge, so the first
sub-cap fetch clears it; GatewayAlarmMonitor.ClearCache drops it with the cache
generation it describes. A caveat that never turns off is one operators learn
to ignore.
Flow: WnWrapAlarmConsumer.LastSnapshotTruncated -> AlarmDispatcher (stamps every
record) / IAlarmCommandHandler (payload) -> MxAccessCommandExecutor reply ->
GatewayAlarmMonitor._snapshotTruncated -> IGatewayAlarmService.SnapshotTruncated
-> DashboardAlarmQueryResult -> AlarmsPage warning banner (render-side only; the
poll loop and DisposeAsync drain are untouched). The public QueryActiveAlarms
RPC forwards worker snapshots unmodified, so the per-record flag needed no
mapper change — a test pins that.
Parity: this describes OUR fetch mechanics — additive gateway metadata — not
MXAccess provider behavior. No event is synthesized and no MXAccess-observable
semantics change, so it is not a parity deviation.
Tests: worker LastSnapshotTruncated set/reset/consecutive-burst (windev-run);
gateway end-to-end truncated reply -> monitor -> public stream, with the
complete-reply control as the load-bearing assertion; AlarmsPage banner
present/absent. Docs: gateway.md alarm surface, docs/DesignDecisions.md entry.
Adds a dashboard event-visibility tag to ApiKeyConstraints, riding in the existing
constraints JSON blob so no auth-store schema migration is needed (design
docs/plans/2026-07-10-dashboard-session-acl-tst15.md sections 3/3.1, open call
settled per its own recommendation). The tag is visibility-only: no read, write,
browse, or subscribe path consults it, and HasRead/HasWriteConstraints ignore it.
GatewaySession gains an immutable, ordinal-ignore-case Tags set stamped at
construction from the owning API key, forwarded by MxAccessGatewayService.OpenSession
from the resolved ApiKeyIdentity — never from the wire request, so a client cannot
label its own session with another tenant's tag. ISessionManager gains a tag-carrying
OpenSessionAsync overload whose default implementation forwards to the tagless one, so
an implementation that does not model tags opens an untagged (least visible) session.
apikey create-key gains --dashboard-tags team-a,team-b (repeatable, trimmed,
de-duplicated; an empty segment is rejected rather than dropped) and list-keys prints
the tags column. No enforcement yet — the EventsHub ACL that consumes the tag is a
later change.
Groundwork for the per-session dashboard event ACL (docs/plans/2026-07-10-dashboard-session-acl-tst15.md 3.2): a dashboard group can now grant visibility tags, and untagged sessions default to AdminOnly. Enforcement lands with the EventsHub ACL; nothing consumes the grant yet.
GroupToTag is deliberately uncoupled from GroupToRole - a group may appear in either map, both, or neither - and is validated for shape only. Tags gate dashboard event visibility, never data access.
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.
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.
Decrement was decrement-first with a single non-retried repair CAS. From zero,
two unmatched decrements (SignalR calls OnDisconnectedAsync for a connection
whose OnConnectedAsync faulted) capture -1 and -2; a real Increment then makes
the count -1, and the first decrementer's stale CompareExchange(0, -1) matches
and resets to zero — erasing a live connection, so the idle gate freezes an open
dashboard. The same lost race also made Decrement report 0 when it had not
written 0.
Clamping now happens inside the compare-and-swap: read, clamp, publish, retry on
loss. A lost race re-reads the fresh value instead of repairing a stale one.
The counter moves to its own file per the one-public-type-per-file convention and
gains direct tests: the zero floor under concurrent unmatched decrements, matched
pairs settling at zero, and an interleaved connect/disconnect stress round. The
stress test asserts the observable invariants only — the specific interleaving
cannot be forced through the public API (verified: the previous implementation
passes it), which its remarks now state rather than implying a reproducer. A hub
wiring test is skipped for the EventsHub reason: driving Hub.OnConnectedAsync
needs caller-clients and connection-context fakes, and the overrides are two
lines of delegation to the tested type.
Also documents that the API-key refresh's pre-gate time check races benignly.
The snapshot publisher broadcast to Clients.All on every ~1s tick forever, with
zero viewers. Each tick cost a session-registry snapshot and sort, a metrics
snapshot that copies dictionaries under the global metrics lock, a full rebuild
of EffectiveGatewayConfiguration, and a SQLite read of the API key table.
DashboardSnapshotHub now counts live connections into the singleton
DashboardSnapshotHubConnectionCounter (clamped at zero, since SignalR can call
OnDisconnectedAsync for a connection whose OnConnectedAsync faulted). The
publisher drives the snapshot enumerator by hand instead of await foreach: with
no connections it does not call MoveNextAsync at all, so the producing iterator
stays suspended at its yield and no snapshot is built — the gate removes the
build, not just the broadcast. It re-checks once a second, so the first viewer
resumes the tick within about one interval; that viewer is seeded immediately by
DashboardPageBase's synchronous GetSnapshot() and by the hub's OnConnectedAsync.
Two per-tick costs are bounded independently of the gate: the effective
configuration is startup-static (options are bound once at boot and never
reloaded), so it is built once and cached; and the API key summaries refresh at
most every 15s, since the list only changes when an operator creates, rotates,
or revokes a key. Only a successful refresh restarts the interval, so a failed
or timed-out read is still retried on the next tick with the previous summaries
left on screen.
DashboardEventBroadcaster.Publish ran a deep protobuf Clone (redaction is on by
default) and a group SendAsync for every event of every session, before anything
checked whether a dashboard client was actually watching. In the steady state the
session:{id} group is empty, so that work was thrown away per event.
SignalR does not expose group membership, so EventsHub now mirrors its own
add/remove into a singleton EventsHubViewerRegistry, and OnDisconnectedAsync
releases everything a dropped connection held (SessionDetailsPage disposes the
connection rather than unsubscribing). Publish returns early when the session has
no viewers, before the redaction clone. Watched sessions behave exactly as before.
Lazy mirror-lease start/stop was deliberately not attempted — it entangles the
dashboard with SessionEventDistributor subscribe lifetime for no saving beyond
this gate; recorded in docs/GatewayDashboardDesign.md.
Family-wide nav sweep: the Secrets management page should be linked from
each app's UI, visible to Administrator-role users only.
The link already existed in MainLayout's Admin section. The gate did not:
the rail rendered every item for every visitor, including a Viewer and the
anonymous-localhost read-only identity. Not an access hole — the mounted
page carries [Authorize(Policy = "secrets:manage")], so a Viewer clicking
through was denied — but a dead link presented as a live one. There was
also no existing role-gated nav pattern to follow; the rail's only
AuthorizeView was the footer's signed-in/signed-out split.
Gated on the POLICY rather than a role literal, so nav visibility cannot
drift from what the page enforces. In this host the two are equivalent:
GatewayOptionsValidator constrains Dashboard:GroupToRole values to
Administrator or Viewer, so the shared library's other manage-granting
roles (secrets-manager, secrets-reveal) are unreachable. The policy form
stays correct if that ever relaxes, where a role literal would then hide
the link from users who can use the page.
API Keys is deliberately left ungated. It looks like the same case and is
not: ApiKeysPage renders for a Viewer with write affordances hidden, so
hiding its link would remove legitimate read access. The secrets page has
no read-only mode. The rule is "gate the link when the page denies the
role outright", not "gate everything under Admin".
Coverage: three tests pin the policy's verdict per principal
(Administrator admitted, Viewer refused, unauthenticated refused), and
/admin/secrets joins the canonical route list — it is the one nav
destination mounted from an RCL rather than declared here, so a routing
regression could remove it without touching this repo's pages. The
principal helper sets an authentication type deliberately: without one
the role assertions would pass vacuously for the wrong reason.
Not a rendering test — the suite has no component-testing harness, and
adding one to assert a single AuthorizeView would be a large dependency
for a small claim.
Build 0 warnings / 0 errors; suite 895/895.
xUnit v2 (2.9.3) never invokes IAsyncDisposable.DisposeAsync on a test
class, so the unobserved-fault safety net this class documents — awaiting
every scripted worker task after each test — has been dead code since it
was written. Found via dumpasync on the wedged-testhost investigation: the
NeverReadyWorkerProcessLauncher's Task.Delay(Infinite, _stop.Token) was
still pending (DelayPromiseWithCancellation, detached) minutes after its
test finished, which is only possible if DisposeAsync never ran.
Proven both ways with a throw-probe in DisposeAsync: under IAsyncDisposable
all 3 tests pass (never called); under IAsyncLifetime all 3 fail from the
probe (called after every test). Sweep confirmed this is the only test
class on IAsyncDisposable — all other implementers are helpers disposed
via await using.
The windev full-suite wedge — every test reported, then the x64 testhost sitting
at ~0 CPU forever while `dotnet test` never returns — was one test blocked on a
pipe write, not a leaked thread or an undisposed fixture.
`dotnet-stack report` on the wedged host showed no thread running test code:
xUnit's RunTestsInAssembly was parked on WaitHandle.WaitOne() waiting for the
assembly-finished event, so the wait lived in a suspended async state machine.
`dotnet-dump analyze -c dumpasync` named the frame — WorkerClientTests
.StagingChannelOverflowFaultsWorkerWithoutWaitingForFullModeTimeout awaiting
WorkerFrameWriter.WriteAsync on a 63-byte frame, with <extra>5__15 = 6, i.e. the
seventh of the twelve events the test pushes past the client's staging bound.
That test faults the worker client on purpose, and a faulted client stops its
read loop by design. The test-side pipe came from the NamedPipeServerStream
overload without buffer arguments, which passes inBufferSize: 0 / outBufferSize: 0
to CreateNamedPipe; on Windows that reserves no buffer at all, so a write
completes only once the peer reads it. Measured on windev, that pipe absorbed
0 bytes against a non-reading peer where the same pipe declared with 64 KiB
buffers absorbed 65 520. On macOS and Linux .NET backs named pipes with Unix
domain sockets whose socket buffer swallows the writes regardless, which is why
the identical test never hung there and the bug read as environmental.
Test-owned server pipes now go through TestSupport/TestNamedPipe.CreateServer in
both test projects, declaring explicit 64 KiB buffers so those tests exercise the
gateway's own staging/queue backpressure rather than the OS pipe's flow control.
Separately, every fake-worker write in WorkerClientTests now goes through
PipePair.WriteAsync, bounded by the class's five-second TestTimeout. That is
where the severity came from: a test method that never returns keeps xUnit from
raising ITestAssemblyFinished, so one unbounded await cost the whole suite its
result. A blocked write is now a named test failure instead of a silent wedge.
The fix also retires a wrong belief the wedge had created. windev reported 855
where macOS reported 879, and that gap was recorded in GatewayTesting.md as
Unix-gated test cases; it was really the results lost when the wedged host was
torn down. The same clone now reports 879 passed, matching macOS exactly.
Product code is unaffected. SessionWorkerClientFactory.CreatePipe keeps the
unbuffered declaration deliberately: both ends run continuous read loops and every
gateway write is bounded by the worker client's _stopCts, so a stalled peer
cancels the write rather than blocking on it.
Verified on windev at this SHA: gateway suite x64 three times (879 passed,
exit 0, no surviving testhost each time) and Worker.Tests x86 twice (400 passed,
11 skipped, exit 0, clean), plus the macOS gateway suite once (879 passed).
Docs: GatewayTesting.md replaces the --blame-hang workaround section with the
root cause and corrects the baseline to 879, CLAUDE.md's Source Update Workflow
no longer tells readers the windev suite wedges, and ToolchainLinks.md records
dotnet-stack and dotnet-dump as installed on windev.
Both failures in the windev baseline were test bugs that reproduce on any Windows
host, not anything missing or misconfigured on windev.
SelfSignedCertificateProviderTests.GenerateCertificate_HasExpectedSansEkuAndValidity
asserted SAN content by substring-matching X509Extension.Format(false). That string
comes from the platform crypto library: Windows' CryptFormatObject renders the IPv6
loopback fully expanded (0000:0000:...:0001) where the managed formatter renders
"::1", so the loopback assertion could never hold on Windows. Decode the extension
with X509SubjectAlternativeNameExtension and compare parsed IPAddress values and DNS
names instead, which removes the platform-dependent formatting from the assertion.
SessionManagerTests.OpenSessionAsync_PipeNameIsShortAndUniquePerPidAndSession guards
the 104-byte macOS sun_path budget NEXT-01 shortened the pipe name to fit. It padded
the measured name up to a five-digit pid but never substituted that worst case
downward, so Windows' routine six-digit pids over-counted by a character against a
budget that does not constrain the host running the test. Substitute the five-digit
macOS worst case for the running pid's digit count so the check measures the name
format rather than the current pid.
EventStreamServiceTests.WaitUntilAsync now reports the unmet condition on timeout
instead of letting a bare TaskCanceledException escape. Its five-second real-clock
deadline is genuinely load-sensitive on windev (36 logical CPUs, maxParallelThreads
-1), and an opaque cancellation there is exactly what got the previous failures
filed as "environmental" and left unexplained.
Documents the windev run in docs/GatewayTesting.md: the two fixed bugs and their root
causes, the real-pipe suites whose failures are evidence of machine load rather than
of the change under test, and the full-suite testhost that completes every test and
then never exits (filtered runs exit normally; macOS exits cleanly). Corrects the
CLAUDE.md claim that the suite exits cleanly on the Windows dev box.
The pipe name mxaccess-gateway-{pid}-session-{32hex} plus .NET's
CoreFxPipe_ prefix overflowed the 104-byte Unix-domain-socket path limit
under the default per-user macOS TMPDIR (~49 chars), so every test that
opened a real pipe threw ArgumentOutOfRangeException at pipe creation
unless TMPDIR=/tmp was exported. Rename to mxgw-{pid}-{sessionUid} (the
session guid hex without the session- prefix; worst-case 43 chars) and
shorten the three test-fixture names the same way. Uniqueness is
unchanged: gateway pid + full session guid. The worker receives the pipe
name via its launch command line, so mixed Server/Worker deploy SHAs are
unaffected. Docs updated in the same change (gateway.md,
GatewayProcessDesign, GatewayConfiguration, Sessions, CLAUDE.md); new
regression test pins the format and the length budget.
Verified: SessionManagerTests 39/39; SessionWorkerClientFactory,
GatewayEndToEndFakeWorkerSmoke, WorkerClient, and ReconnectReplay suites
33/33 under the default macOS TMPDIR — this also retires the
previously-misdiagnosed 'macOS pipe-timeout test failures': they were
this path-length throw, not a timeout-message defect.
Same-commit docs rule (were missed in the prior commit):
- docs/GalaxyRepository.md: SnapshotCachePath now documents the per-OS derived
default and the GalaxyRepositoryOptionsValidator rooting/validity enforcement.
- A2-galaxyrepository-adoption-handoff.md: correct the now-inaccurate NSSM caveat
(SnapshotCachePath override is optional, not required; blank seeds a rooted host
default, no silent no-op) and repoint the option-validation item at the new
GalaxyRepositoryOptionsValidator.
SEC-34 guard confirmed and documented: TryParseKeyId's '_' split cannot truncate a
key id because both — and the only — gateway key-creation paths
(ApiKeyAdminCommandLineParser.IsValidKeyId, DashboardApiKeyManagementService.ValidateKeyId)
restrict key ids to IsAsciiLetterOrDigit || '.' || '-', and key ids are never
library-generated. Added a citing comment; no behavior change.
Test consolidation: moved the three host-start SqlitePath overrides into
TestHostEnvironmentInitializer (per-process temp store, mirroring Secrets__SqlitePath)
so future host-start tests auto-cover.
SEC-33: make rooting host-meaningful and stop shipping foreign-platform literals.
- Delete IsRootedForAnyPlatform; AddIfNotRooted now uses Path.IsPathRooted (current OS).
- Promote AddIfNotRooted/AddIfInvalidPath to shared GatewayConfigPathRules so the new
Galaxy validator reuses them and the two validators cannot drift.
- Remove Authentication:SqlitePath and Galaxy:SnapshotCachePath Windows literals from
appsettings.json; the CommonApplicationData-derived code defaults take over. The
Galaxy default is seeded as a configuration value before AddZbGalaxyRepository
(SnapshotCachePath is init-only, so a PostConfigure mutation cannot compile).
- New GalaxyRepositoryOptionsValidator (ValidateOnStart) enforces a valid, host-rooted
SnapshotCachePath when PersistSnapshot is true.
- Root-cause the stray junk-named auth DB: host start eagerly builds
AuthSqliteConnectionFactory; under the non-rooted Windows literal on macOS SQLite
wrote it relative to the test bin CWD. The three real-host-start tests now pin
SqlitePath to a temp path.
SEC-34: verification cache Invalidate-vs-in-flight-repopulation race closed with a
per-key generation counter (bump-before-evict, snapshot-then-recheck). The expiry
cap (window 2) takes the documented fallback: the library verification identity
carries no ExpiresUtc, so the cache cannot cap at the key's expiry (donor-library ask).
GWC-24 rider: cap MxGateway:Events:QueueCapacity at int.MaxValue/2 so the derived
checked(2 * EventChannelCapacity) in WorkerClient cannot overflow at session creation.
SEC-35 (doc-only): note IsProduction() env-name semantics in GatewayConfiguration.md.
Docs updated same commit (GatewayConfiguration.md, Authentication.md) and tracking
registers/change-log flipped (00-tracking.md, 40-security-dashboard.md).
Joseph Doherty