fix(dashboard): ShowTagValues now gates the alarms hub and /browse live values

archreview/remediation/00-tracking.md

@@ -228,7 +228,7 @@ Full design + implementation for each row lives in the linked domain doc under i
 | TST-13 | Medium | P2 | S | — | Done | gateway.md carries stale design-era sketches |
 | TST-14 | Medium | P2 | S | — | Not started | Repo-root working artifacts need triage |
 | TST-15 | Medium | P2 | M | TST-04 | Done | Dashboard EventsHub has no per-session ACL |
-| TST-16 | Medium | — | S | — | Not started | `Dashboard:ShowTagValues` is a dead flag |
+| TST-16 | Medium | P1 | S | — | Done | `Dashboard:ShowTagValues` is a dead flag |
 | TST-17 | Medium | — | S | — | Not started | Vendor-gated alarm parity residuals silently lossy |
 | TST-18 | Low | — | S | — | Not started | Hosted-service wrappers untested |
 | TST-19 | Low | — | S | — | Not started | Keep FakeWorkerHarness canned replies in lockstep |
@@ -253,6 +253,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog
 
 | Date | Change |
 |---|---|
+| 2026-08-17 | **TST-16 → `Done`: `Dashboard:ShowTagValues` now gates every dashboard value surface** (branch `feat/followup-closeout`). The flag was live on one seam only (the events-hub mirror, shipped with SEC-25); the two recorded residuals are closed, so it is no longer a partly-dead flag. **Alarms hub:** `AlarmsHubPublisher` took `IOptions<GatewayOptions>` and now clears `current_value`/`limit_value` from a **deep clone** of each `AlarmFeedMessage` on both value-bearing payload arms (`transition`, `active_alarm`) before the `/hubs/alarms` broadcast. Clone-only, matching `DashboardEventBroadcaster.RedactValues`: the source message fans out from one feed to gRPC `StreamAlarms` subscribers and the alarms page, none of which this dashboard-display flag governs. The redaction switches on the value-bearing arms and passes everything else through untouched and uncloned, so a payload arm added later (the `snapshot_status` arm landing alongside this change) is forwarded unbroken rather than silently dropped. **`/browse`:** the gate lives at the service boundary per TST-16's own recommendation — `DashboardLiveDataService` substitutes `DashboardTagValue.RedactedValueText` (`[redacted]`) for the formatted value text, so the value never enters the render tree and `BrowsePage.razor` needed no change; data type, quality, source timestamp, and read errors stay real, keeping the panel diagnostic. A visible marker, not a blank, so a suppressed value is distinguishable from a tag that read back empty. Tests: new `AlarmsHubPublisherTests` (4 — both arms redacted with metadata intact, source not mutated, `true` passes values through as the same instance, valueless arms forwarded as the same instance) and two `DashboardLiveDataServiceTests` cases (`false` → `[redacted]`, `true` → formatted); `DashboardEventBroadcasterTests` re-run as regression. Docs: `docs/GatewayConfiguration.md`'s `ShowTagValues` row rewritten from "gates only the events hub mirror, two still-open residuals" to the three seams it now covers, with the "no gRPC client is affected" scope statement; `docs/GatewayDashboardDesign.md` gained the redaction note on the `AlarmsHub` row and had its Browse-panel paragraph corrected (it claimed the panel "always shows values regardless of `Dashboard:ShowTagValues`"). |
 | 2026-08-17 | **Alarm-snapshot truncation now has a structural degraded-status signal** (branch `feat/deferred-closeout`, commits `693a78d` + `b9fb0dd`). No review ID — this is branch work outside the 153-finding register, recorded here so the tracker is not silent on a shipped change to the alarm surface. Before it, a capped `GetXmlCurrentAlarms2` fetch suppressed absence-implies-Clear inference and said so only in a rate-limited worker stderr warning, so 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` and `ActiveAlarmSnapshot.from_truncated_snapshot = 16` (per record, because `QueryActiveAlarms` is a bare `stream ActiveAlarmSnapshot` with no envelope; the reply payload states it too, since a prefix filter can leave zero records and a capped fetch with nothing to report still has to say so). Flow: `WnWrapAlarmConsumer` → `AlarmDispatcher` / `IAlarmCommandHandler` → `MxAccessCommandExecutor` reply → `GatewayAlarmMonitor` → `IGatewayAlarmService.SnapshotTruncated` → `AlarmsPage` banner. `b9fb0dd` then made the pairing structural after review: `IMxAccessAlarmConsumer` and `IAlarmCommandHandler` expose one accessor (`SnapshotActiveAlarms(out bool truncated)` / `QueryActive(..., out bool snapshotTruncated)`) satisfied from a single lock acquisition, so the snapshot and its verdict can no longer be read across a poll; the separate `LastSnapshotTruncated` property is gone from every layer. Detection is deliberately unchanged (`fetchedRecordCount >= maxAlarmsPerFetch`); switching to `ALARM_RECORDS/@COUNT` stays blocked on probe evidence (`docs/AlarmProbeFindings.md`). Not latched, and dropped with the cache generation by `ClearCache`. Additive gateway metadata about our fetch mechanics, not MXAccess behaviour — no synthesized event, so not a parity deviation. Docs: `gateway.md` alarm surface, `docs/DesignDecisions.md`. |
 | 2026-08-17 | **TST-15 → `Done` (discharges the ACL half of SEC-25): per-session dashboard event ACL shipped** (branch `feat/deferred-closeout`, commits `693a78d` + `7ec0b35`). Implements `docs/plans/2026-07-10-dashboard-session-acl-tst15.md`, whose header is now `Implemented` with as-built notes in its §12. `IDashboardSessionAcl.CanViewSession` is the single decision **both** subscribe seams consult — `EventsHub.SubscribeSession` (denial is a `HubException`; the caller is neither joined to the group nor registered in `EventsHubViewerRegistry`, so the mirror stays off) and `SessionDetailsPage`'s in-process subscribe (inline denial, no subscription) — so neither path is the weaker one and the `TODO(per-session-acl)` is gone. Decision order, fail-closed on every branch: authenticated Administrator → allow (evaluated **before** the registry lookup, so Admin × unknown-session allows — pinned by a test because reordering the two checks is a plausible refactor); session not found → deny; untagged session → `Dashboard:UntaggedSessionVisibility` (`AdminOnly` default); else allow iff `session.Tags ∩ zb:dashboardtag` claims, ordinal-ignore-case. Session tags are inherited from the owning API key's `dashboard_tags` constraint (`apikey --dashboard-tags`, already in the `ApiKeyConstraints` JSON blob — no SQLite migration) and never from the client's wire request. Viewer grants come from `Dashboard:GroupToTag` over the user's LDAP groups, stamped at cookie login (`DashboardAuthenticator.CreatePrincipal`) and **re-resolved, not copied**, at hub-token mint (`HubTokenService.Issue`), so the 5-minute token lifetime bounds a stale grant. Anonymous localhost is an empty-grant Viewer; `Dashboard:DisableLogin` auto-login carries both roles and so takes the admin bypass unchanged. Tests: `DashboardSessionAclTests` (decision table, every branch asserted in its denying direction too), `EventsHubTests`, `DashboardAuthenticatorTests`, `HubTokenServiceTests`, a `GatewayOptionsTests` case proving `Dashboard:GroupToTag` keeps its ordinal-ignore-case lookup through configuration binding, and two `[LiveLdapFact]`s in `DashboardLdapLiveTests` that drive a real bind against the shared GLAuth (`gw-viewer` → `team-a` grant admits the `team-a` session and refuses the `team-b` one; `multi-role` bypasses on the sessions its own grant does not cover). The live pair needed **no GLAuth change** — the tag layer is config-side, keyed on the existing `GwAdmin`/`GwReader` groups (recorded in `glauth.md`). Docs: `docs/Sessions.md` (session-tag model), `gateway.md` + CLAUDE.md dashboard-auth paragraphs, `docs/GatewayDashboardDesign.md` (three passages that described the ACL as outstanding), `docs/GatewayConfiguration.md` (`ShowTagValues` row: redaction is now the second of two layers, not the only one), `docs/Authentication.md` (`--dashboard-tags` is the only *constraint* flag that splits on commas). |
 | 2026-08-10 | **TST-25 acceptance Check 6 (forced-failure nightly issue) → Done.** The 2026-07-13 record wrote this check off as "abandoned to shared-runner congestion"; that was wrong on both counts. The 2026-07-13 probe *did* land (issue #125, `[CHECK6 PROBE]`, run 375), and since 2026-07-17 the `nightly-windev` `if: failure()` step has filed an issue on **every** red nightly — #126–#139, all authored by the `gitea-actions` bot. Traced run 672 (schedule, main, red) line by line: main step fails → `exitcode '1': failure` → the `if: failure()` step runs → `POST /api/v1/repos/dohertj2/mxaccessgw/issues` with the built-in token masked to `***` → issue #139 created at the matching timestamp. Re-confirmed by a fresh forced-failure probe on the throwaway branch `test/tst25-check6-nightly-issue` (temporary `tst25-check6-probe.yml` reproducing the job shape with `exit 1` for the live step; run 677 → issue #140). Branch deleted, issues #125 and #140 closed with explanatory comments. **One real defect found and fixed** (`fix/tst25-nightly-issue-path`, not merged): `${{ github.server_url }}` is the runner-internal `http://gitea:3000`, so every filed issue's run link was unreachable from a browser. The API call must keep using it (the job container resolves `gitea` only on the docker network and has no LAN egress to the public origin), so the fix adds a `PUBLIC_SERVER_URL: https://gitea.dohertylan.com` job env used **only** for the browser-facing link in the issue body; the probe validated the fixed template (#140 carries a `https://gitea.dohertylan.com/...` link that returns 200). **Separately observed, not fixed:** the nightly has been red continuously since at least 2026-07-17 (run 672: `x86 Worker.Tests failed with exit code 1`, 1 failed / 398 passed / 11 skipped — the known `EventBurst_DrainLoopCoalescesFlushes` class of flake), and the step de-duplicates nothing, so 14 issues are open, seven of them (#132–#138) for the identical SHA `47c0b64`. Worth a follow-up: fix the red nightly, and consider having the step reuse an open issue with the same title instead of filing a new one. |