fix(GWC-26): attach the alarm monitor's lease before SubscribeAlarms

RunMonitorAsync issued SubscribeAlarms and the first reconcile before the
internal distributor subscriber was attached (via ISessionManager
.ReadAlarmEventsAsync). The pump has been running since MarkReady started the
dashboard mirror and only fans to subscribers registered at fan-out time, so
every transition raised in that two-round-trip window bypassed the alarm feed —
and a missed Acknowledge was never repaired, because ApplyReconcile broadcast
presence deltas only.

- The monitor now takes the internal lease directly from its session BEFORE
  SubscribeAlarms and drains it after the first reconcile; window transitions
  buffer in the lease's bounded channel. Processing them after ApplyReconcile is
  order-safe (ApplyTransition handles alarms the snapshot already placed).
- ISessionManager.ReadAlarmEventsAsync removed — zero remaining callers.
- ApplyReconcile broadcasts an Acknowledge feed transition when a both-present
  alarm's state advanced to ActiveAcked. This is a feed-level repair on the
  AlarmFeedMessage/StreamAlarms surface rebuilt from the worker's own snapshot,
  not MxEvent emission, so the "never synthesize events" rule is untouched;
  the reasoning is recorded on ApplyReconcile.

The alarm-monitor test fakes now hand the monitor a real Ready GatewaySession
with a dashboard mirror, which is what makes the window reproducible.

Docs: docs/Sessions.md and gateway.md alarm-monitor ordering notes.

Refs: archreview/2026-07-12/remediation/10-gateway-core.md GWC-26
This commit is contained in:
Joseph Doherty
2026-08-07 05:40:33 -04:00
parent 1a63fdd7db
commit 3b6a239ed6
16 changed files with 674 additions and 127 deletions
@@ -61,8 +61,8 @@ Full design + implementation for each row lives in the linked domain doc under i
|---|---|:-:|:-:|---|---|---|
| GWC-24 | Medium | P1 | M | GWC-21 (coord, old tracker) | Not started | Unbounded event staging channel: sustained slow drain grows memory silently and invisibly |
| GWC-25 | Medium | P0 | S | CLI-35/36 (coord) | Not started | Empty-ring ReplayGap sentinel carries `oldest_available_sequence = 0` |
| GWC-26 | Low | P2 | M | GWC-27 | Not started | Alarm monitor attaches its subscriber after SubscribeAlarms; window transitions bypass the feed |
| GWC-27 | Low | P2 | S | GWC-26 | Not started | `AttachInternalEventSubscriber` bypasses the readiness gate; premature attach poisons the distributor |
| GWC-26 | Low | P2 | M | GWC-27 | Done | Alarm monitor attaches its subscriber after SubscribeAlarms; window transitions bypass the feed |
| GWC-27 | Low | P2 | S | GWC-26 | Done | `AttachInternalEventSubscriber` bypasses the readiness gate; premature attach poisons the distributor |
| GWC-28 | Low | P2 | S | GWC-10 (coord, old tracker) | Not started | Gateway→worker envelope `sequence` stamped at creation, not at write |
| GWC-29 | Low | — | S | — | Not started | `Invoke` deep-clones the entire request only to discard the cloned command |
| GWC-30 | Info | — | S | — | Not started | Frame reader allocates a fresh 4-byte length-prefix array per frame |
@@ -161,3 +161,4 @@ Sequence these together rather than piecemeal — several are one change set spa
| 2026-07-13 | Operator bring-up complete: dedicated CI ed25519 key installed in windev `administrators_authorized_keys` (authorized into `dohertj2`, which owns the working MXAccess/toolchain env — a fresh OS account would break the build; the key is independently revocable), Gitea secrets `WINDEV_SSH_KEY`/`WINDEV_SSH_KNOWN_HOSTS` + variable `WINDEV_SSH_USER=dohertj2` stored, runner→`10.100.0.48:22` egress verified on the `traefik` net, issue-write confirmed. **TST-25/TST-26 → `Done`:** credentialed `windows-x86` ran GREEN on `d769244` (Gitea run #37) — Linux runner SSHed windev, checked out the SHA in `C:\build\mxaccessgw-ci` under lock, ran the x86 Worker build + `Worker.Tests`, exit 0; `nightly-windev` correctly skipped on the push event. Branch merged to `main`. Follow-ups (old tracker): revisit **TST-05** (scheduled live smoke — now covered by `nightly-windev`) and **TST-24** (client wire tests) which this unlocks. |
| 2026-07-13 | Ran the TST-25 acceptance checks (scripts/ci/README.md) — they caught **two real CI defects, both fixed** on `fix/tst-25-ci-key-log-leak`: (1) **CI SSH key leaked in cleartext** in the `windows-x86` step env echo (Gitea's line-oriented masker missed the multiline PEM) — rotated the CI key on windev (old pubkey revoked), stored the key **base64-encoded** so the masker redacts it to `***` (confirmed on run #38), taught `run-windev-ci.sh` to decode, dropped the redundant public known-hosts secret from the job env; (2) **bootstrap lock race**`run-windev-ci.sh`'s pre-hand-off `git fetch`/`checkout` ran outside the worktree lock, so concurrent runs collided on `.git/index.lock`; the bootstrap now holds the lock (ps1 re-uses it via `MXGW_CI_LOCK_HELD`), retest confirmed clean serialization. Also **deflaked** `SessionManagerTests` fail-fast timing assertions (absolute `<100ms` wall-clock bound flaked under CI load; now anchored to the configured timeout / dropped for the zero-timeout case). Checks passed: unreachable-host fast-fail (exit 255/15s), deliberate-red propagation (Worker.Tests failure → exit 1), lock concurrency (2nd run waits), no-key-in-logs (masked). Merge target `df7e20d` verified GREEN via the local windev path (Worker build + 356 tests); merged to `main` `19cbf7b`. Check 6 (forced-failure nightly issue): issue endpoint+token proven live at bring-up (#124); in-CI forced-failure probe abandoned to shared-runner congestion (residual `if: failure()` gating is standard Actions). |
| 2026-07-13 | New finding **TST-30** (`Low`/`P2`) added — surfaced during TST-25 acceptance: CI runs on a single shared `gitea-runner` (`maxParallel=1`, co-located `10.100.0.35`) interleaved with `dohertj2/lmxopcua`, and Gitea 1.26 exposes no run cancel/delete, so queue latency is unbounded under cross-repo contention and the runner is a single point of failure. Design: add a second/labelled runner + document the no-cancel reality and the `run-windev-ci.sh` bypass. Roll-ups updated (Testing Low 2→3, total 47→48; P2 9→10). |
| 2026-08-07 | **GWC-27 → `Done`, GWC-26 → `Done`** (branch `fix/gwc-26-27-alarm-attach`). GWC-27: `GatewaySession.AttachInternalEventSubscriber` now mirrors `AttachEventSubscriber`'s readiness gate under `_syncRoot`, before `EnsureDistributorCreated`, so a premature attach can no longer latch a poisoned distributor. GWC-26: the alarm monitor takes its internal lease directly from the session **before** `SubscribeAlarms` and drains it after the first reconcile; `ISessionManager.ReadAlarmEventsAsync` removed (zero remaining callers); `ApplyReconcile` now broadcasts an `Acknowledge` feed transition for a both-present alarm whose state advanced to `ActiveAcked` (feed-level repair on `AlarmFeedMessage`, not `MxEvent` synthesis). New tests `GatewaySessionTests.AttachInternalEventSubscriberBeforeReadyThrowsAndDoesNotPoisonDistributor` and `GatewayAlarmMonitorAttachOrderTests` (`TransitionsDuringSubscribeWindow_StillReachTheAlarmFeed`, `ApplyReconcileBroadcastsAcknowledgeDelta`); the alarm-monitor fakes now hand the monitor a real Ready `GatewaySession` with a dashboard mirror so the window is actually reproducible. Verification: NonWindows build 0 warnings/0 errors; `GatewayAlarmMonitor` 16 passed, `SessionManagerTests` 38 passed, `GatewaySessionTests` 19 passed, `AlarmFailoverEndToEndTests` 2 passed. |