From fd941c249e5802f77bb82e54a75f13d9647525b2 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:36:47 -0400 Subject: [PATCH 01/21] =?UTF-8?q?docs(plan):=20followups-and-tickets=20?= =?UTF-8?q?=E2=80=94=2010=20tasks=20closing=20every=20recorded=20follow-up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../plans/2026-08-18-followups-and-tickets.md | 461 ++++++++++++++++++ ...-08-18-followups-and-tickets.md.tasks.json | 16 + 2 files changed, 477 insertions(+) create mode 100644 docs/plans/2026-08-18-followups-and-tickets.md create mode 100644 docs/plans/2026-08-18-followups-and-tickets.md.tasks.json diff --git a/docs/plans/2026-08-18-followups-and-tickets.md b/docs/plans/2026-08-18-followups-and-tickets.md new file mode 100644 index 0000000..76a8e8d --- /dev/null +++ b/docs/plans/2026-08-18-followups-and-tickets.md @@ -0,0 +1,461 @@ +# Follow-ups and Tickets Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development +> to execute this plan task-by-task in this session. + +**Goal:** Close every follow-up recorded in `docs/plans/2026-08-17-followup-closeout.md` +(as-built notes, "Follow-ups recorded, not started") plus the two ticket-worthy windev +findings — leaving no recorded open item anywhere. + +**Architecture:** No new components. Ten independent closures: two toolchain/script fixes +(Check 4 Windows pin, Gradle 9 `checkGeneratedClean`), two dashboard display gaps +(Settings recent-limit rows, push-driven Alarms truncation banner), two documentation +audits (Server-0xx resolution regression sweep, `Authentication.md` runnable-examples +pass), one worker-test investigation (the deterministic `WorkerPipeSessionTests` failure +on windev), one bounded rig probe (ack-leg unblock paths), a windev verification pass +(including the `protoc-gen-go-grpc` 1.6.2 pin install), and bookkeeping. + +**Tech Stack:** PowerShell (codegen scripts), Gradle/Groovy (Java client), Blazor +server-side Razor + xUnit/bUnit-style render tests (dashboard), .NET Framework 4.8 x86 +xUnit (worker tests, windev-only), wnwrap probe harness (windev rig), Markdown. + +**Branch:** `feat/followups-tickets` off `main` (`45058d5`). + +--- + +## Ground rules for every implementer subagent + +- **Git discipline (Mac tree):** NEVER `git stash`, `git reset`, `git clean`, or + `git checkout `. Commit with **pathspecs on the commit**: + `git commit -m "..." -- `. If `index.lock` blocks you, wait 5 s and retry. +- **Build/test lock:** before any `dotnet build`/`dotnet test`/`cargo`/`gradle`/`go test` + on the Mac, acquire the lock: + `mkdir /private/tmp/claude-501/-Users-dohertj2-Desktop-MxAccessGateway/f36938ae-bbca-4245-b5c9-fac512d69e22/scratchpad/buildlock` + (retry with backoff until it succeeds); `rmdir` it on ALL exit paths, including failure. +- **Quality gates:** `TreatWarningsAsErrors=true`, `Nullable=enable`. Follow + `docs/style-guides/CSharpStyleGuide.md` (file-scoped namespaces, `sealed` by default, + `Async` suffix). Update affected docs in the same commit as source. +- **MXAccess parity:** never synthesize events; the dashboard/gateway forwards only what + the worker/monitor emits. The `snapshot_status` frame is a gateway-status frame (like + `provider_status`), NOT a synthesized MXAccess event. +- **Never log or echo secrets, API keys, credentials, or tag values.** +- **Scope contract:** the task's `Files:` block is the `files_to_edit` contract. If the + task needs other files, that is a plan defect — surface it in your report, don't + silently expand scope. (Exception: Tasks 7 and 8 are investigations; their Files list + is starting points, and they must report every file they end up touching.) +- **Windev access (Tasks 7, 8, 9 only):** `ssh windev` lands in PowerShell. The CI clone + is `C:\build\mxaccessgw-ci`. Quirks: the first `slnx` build after a pull often fails on + stale Contracts obj artifacts (CS2001/CS0016) — clear the Contracts `obj`/`bin` and + rebuild, it is not a regression; the gateway suite is load-sensitive — rerun a filtered + subset before believing a flake. To get branch code there: + `git -C C:\build\mxaccessgw-ci fetch origin && git -C C:\build\mxaccessgw-ci checkout feat/followups-tickets && git -C C:\build\mxaccessgw-ci pull` — + which requires the Mac side to have pushed the branch first (the controller pushes; + ask if the branch tip you need is not on origin). + +--- + +### Task 1: check-codegen Check 4 — tolerate the Windows `.exe` version banner + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 2, Task 3, Task 4, Task 5, Task 7 + +**Files:** +- Modify: `clients/go/generate-proto.ps1` (pin compare at `:52-67`, pin constants at `:10-11`) +- Modify: any doc that states Check 4 is unrunnable on Windows (grep `docs/` and + `code-reviews/` for "Check 4"; `docs/GatewayTesting.md` is the likely holder) + +**Problem.** On Windows the plugins report their invoked name with an `.exe` suffix +(`protoc-gen-go.exe v1.36.11`), so the exact-string compare against +`'protoc-gen-go v1.36.11'` throws and Check 4 (which shells this script) is unrunnable on +any Windows host — including windev, the box where regeneration actually happens. + +**Step 1: Normalize the banner before comparing.** Add a small helper and use it for both +plugin compares (protoc stays warn-only and gets the same normalization for a fair warn): + +```powershell +function Get-NormalizedToolVersion { + # On Windows a plugin reports its argv[0] name, so the banner carries an `.exe` + # suffix ("protoc-gen-go.exe v1.36.11"). Strip it so the pin compare is + # host-independent; the version part must still match exactly. + param([string]$RawBanner) + return ($RawBanner -replace '\.exe(?=\s)', '') +} +``` + +Apply to `$protocGenGoVersion`, `$protocGenGoGrpcVersion`, `$protocVersion` at the point +each is read (`:54`, `:59`, `:64`), keeping the pinned constants unchanged. Update the +comment block at `:4-9` to note the normalization. + +**Step 2: Verify on macOS.** Acquire the build lock, then run +`pwsh scripts/check-codegen.ps1` — all 4 checks must pass (banner has no `.exe` here, so +this proves no regression). Also assert the helper logic directly: +`pwsh -c "& { ; Get-NormalizedToolVersion 'protoc-gen-go.exe v1.36.11' }"` +must print `protoc-gen-go v1.36.11`, and a name-only match check for +`protoc-gen-go-grpc.exe 1.6.2` → `protoc-gen-go-grpc 1.6.2`. + +**Step 3: Fix the stale prose.** Wherever docs state Check 4 cannot run on Windows, +rewrite to: Check 4 runs on Windows; the plugin pins must be installed +(`go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.11`, +`go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.2`). +Do NOT edit `docs/plans/2026-08-17-followup-closeout.md` — Task 10 owns plan bookkeeping. + +**Step 4: Commit** with pathspecs on the touched files: +`fix(codegen): normalize .exe off plugin version banners so Check 4 runs on Windows`. + +Windows-side proof is deferred to Task 9 (windev runs check-codegen 1–4 after installing +the 1.6.2 pin). + +--- + +### Task 2: Gradle 9 — revive `checkGeneratedClean` without `Project.exec` + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 3, Task 4, Task 5, Task 7 + +**Files:** +- Modify: `clients/java/zb-mom-ww-mxgateway-client/build.gradle` (`checkGeneratedClean`, `:71-92`) +- Modify: `docs/GatewayTesting.md` only if it describes the task's mechanism (grep + `checkGeneratedClean`) + +**Problem.** The task's `doLast` calls the script-level `exec {}` (i.e. `Project.exec`), +removed in Gradle 9 — the task is dead on any Gradle 9 host. + +**Step 1: Replace with `ProviderFactory.exec`** (available since Gradle 7.5, so it works +on both the current toolchain and Gradle 9, and is configuration-cache safe). Capture the +provider reference at configuration time, call `.get()` in `doLast` so the git status +runs at execution time: + +```groovy +tasks.register('checkGeneratedClean') { + group = 'verification' + description = 'Fails if the committed generated Java tree differs from a fresh regeneration.' + dependsOn 'generateProto' + def generatedDir = 'clients/java/src/main/generated' + def repoRoot = rootProject.projectDir.parentFile.parentFile + // Project.exec was removed in Gradle 9; ProviderFactory.exec runs the probe lazily + // at doLast time and works on Gradle 7.5+. + def gitStatus = providers.exec { + workingDir = repoRoot + commandLine 'git', 'status', '--porcelain', '--', generatedDir + ignoreExitValue = true + } + doLast { + def dirty = gitStatus.standardOutput.asText.get().trim() + if (!dirty.isEmpty()) { + throw new GradleException( + "Generated Java is stale:\n${dirty}\n" + + "Regenerate and commit the Java client after a .proto change " + + "(gradle :zb-mom-ww-mxgateway-client:generateProto).") + } + } +} +``` + +Preserve the explanatory comment block above the task (`:61-70`) — amend it, don't delete. + +**Step 2: Verify.** Acquire the build lock. From `clients/java`: +`gradle :zb-mom-ww-mxgateway-client:checkGeneratedClean` must pass (tree is clean), and +`gradle --version` must be recorded in your report. Then prove the failure path: touch a +scratch edit inside `clients/java/src/main/generated/` (append a comment line to one +generated file), rerun the task, confirm it fails with the stale message, then **revert +that file with `git checkout -- `** (this narrow file-level checkout is the +one permitted use; the file is committed generated output). + +**Step 3: Commit:** +`fix(java-client): checkGeneratedClean via ProviderFactory.exec — Project.exec is gone in Gradle 9`. + +--- + +### Task 3: SettingsPage — RecentFaultLimit / RecentSessionLimit rows + +**Classification:** small +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 2, Task 4, Task 5, Task 7 + +**Files:** +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SettingsPage.razor` + (dashboard rows around `:98-102`) +- Test: `src/ZB.MOM.WW.MxGateway.Tests/` — extend the existing settings render coverage + (`SettingsPageTagVisibilityRenderTests` or its sibling settings render test class; find + with `grep -rl "SettingsPage" src/ZB.MOM.WW.MxGateway.Tests`) +- Modify: `docs/GatewayDashboardDesign.md` only if it enumerates settings rows (grep + `RecentFaultLimit` / "Snapshot interval" there first) + +**Problem.** `EffectiveDashboardConfiguration` carries `RecentFaultLimit` and +`RecentSessionLimit` (already projected by `GatewayConfigurationProvider.cs:62-63`), but +`SettingsPage` renders every member except these two. + +**Step 1: Write the failing test.** Extend the settings render test: rendered page +contains `Recent fault limit` with the configured value and `Recent session limit` with +the configured value (use non-default values in the arranged options so the assertion +proves plumbing, not defaults). + +**Step 2: Run it, expect FAIL** (`dotnet test --filter "FullyQualifiedName~SettingsPage"` +under the build lock). + +**Step 3: Add the two rows** next to the other Dashboard rows (after "Snapshot interval", +matching the existing `……` idiom): + +```razor +Recent fault limit@Snapshot.Configuration.Dashboard.RecentFaultLimit +Recent session limit@Snapshot.Configuration.Dashboard.RecentSessionLimit +``` + +**Step 4: Run the filtered test, expect PASS.** + +**Step 5: Commit:** +`feat(dashboard): settings page shows RecentFaultLimit and RecentSessionLimit`. + +--- + +### Task 4: Authentication.md — runnable-as-written examples pass + +**Classification:** small +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 5, Task 7 + +**Files:** +- Modify: `docs/Authentication.md` + +**Problem.** The doc's CLI/scope examples were flagged during the previous branch as not +runnable as written — notably samples that grant only invoke/event scopes with no +`session:open`, which since Server-004's validation would produce keys that cannot open a +session (or, for unknown scope strings, be rejected at create time). The canonical scope +catalog is: `session:open`, `session:close`, `invoke:read`, `invoke:write`, +`invoke:secure`, `events:read`, `metadata:read`, `admin`; the verb is `apikey create-key` +with `--key-id` required. + +**Step 1: Sweep every example** (` ```-fenced blocks and inline commands) in the doc. For +each, either (a) make it runnable as written — canonical verb, required flags, only +canonical scopes, scope sets that support what the surrounding prose says the key is for +(a key described as opening sessions needs `session:open`) — or (b) if it is deliberately +a fragment, mark it explicitly (e.g. "illustrative — not a complete command"). Prefer (a); +use (b) only where completeness would obscure the point being made. + +**Step 2: Cross-check** each corrected scope list against +`src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/GatewayScopes.cs` (read-only) — +do not invent scope strings. + +**Step 3: Commit:** `docs(auth): make Authentication.md examples runnable as written`. + +--- + +### Task 5: AlarmsPage — push-driven truncation banner from the snapshot_status frame + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 4, Task 7 + +**Files:** +- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor` + (in-process feed loop around `:199-230`, `_snapshotTruncated` at `:170`/`:401`) +- Test: `src/ZB.MOM.WW.MxGateway.Tests/` — the AlarmsPage/dashboard alarm test class + (find with `grep -rl "AlarmsPage" src/ZB.MOM.WW.MxGateway.Tests`); if no AlarmsPage + render/behavior test exists, add coverage at the level the existing dashboard tests use +- Modify: `docs/GatewayDashboardDesign.md` (truncation-banner description, if it says the + banner is poll-driven) + +**Problem.** The truncation banner (`_snapshotTruncated`, rendered at `:39`) updates only +from the 3-second poll (`:401`), even though the page already holds an in-process +subscription to the alarm feed (the loop at `:211-213` that feeds the provider-status +badge). The feed now carries an edge-triggered `snapshot_status` frame +(`AlarmFeedMessage.PayloadCase.SnapshotStatus`, shipped `fccf753`) — consume it so the +banner flips on the edge instead of up to 3 s late. + +**Spec:** +- In the existing in-process feed loop, add a case for + `AlarmFeedMessage.PayloadCase.SnapshotStatus`: set `_snapshotTruncated = + message.SnapshotStatus.Truncated` and re-render (same invoke/StateHasChanged pattern the + provider-status case uses). +- Keep the poll's assignment at `:401` — the poll is the reconcile baseline and both + sources derive from the same `GatewayAlarmMonitor` verdict, so they cannot disagree + except transiently. Do not remove or restructure the poll. +- Remember `StreamAsync` primes every subscriber with an unconditional `snapshot_status` + baseline frame after `provider_status` — so on attach the page gets the current verdict + push-side too. No extra priming logic needed in the page. +- Do NOT touch `GatewayAlarmMonitor` or the proto — gateway emission is done and shipped. + +**Steps:** failing test first (a `SnapshotStatus` frame delivered through the in-process +subscription flips the banner state without a poll tick; a `false` frame clears it), then +implement, then filtered dashboard/alarm tests green under the build lock, then commit: +`feat(dashboard): alarms page consumes snapshot_status feed frame for the truncation banner`. + +--- + +### Task 6: Server-0xx resolution audit — doc-only "Resolved" entries re-verified + +**Classification:** standard +**Estimated implement time:** ~5 min (audit) + fixes as found +**Parallelizable with:** none (runs after Task 4 lands — both may touch `docs/Authentication.md`) + +**Files:** +- Modify: `code-reviews/Server/findings.md` (annotations only — do not rewrite history) +- Modify: any file where a claimed correction is found absent (expected candidates: + `CLAUDE.md`, `docs/*.md`, XML doc comments named in findings — report each) + +**Problem.** Server-012 was recorded *Resolved 2026-05-18* claiming two scope-list +corrections that were absent from the tree when `feat/followup-closeout` looked — a +resolution that regressed or was never applied. Findings that read Resolved are never +re-examined, so every *documentation/comment-only* resolution needs the same spot-check. + +**Procedure:** +1. Enumerate every `Server-0xx` entry in `code-reviews/Server/findings.md` whose + Resolution describes documentation-only or comment-only changes (no test named, prose + like "Pure documentation change" — at minimum Server-011, Server-012, Server-013/014 + remarks rewrites; sweep all entries, don't assume). +2. For each, verify the specific claimed text exists in today's tree (grep the exact + phrases/identifiers the resolution names). +3. Where present: append one line to that finding's Resolution: + `Re-verified present 2026-08-18 (feat/followups-tickets).` +4. Where absent: re-apply the correction in the target file, and append: + `Regressed or never applied; re-fixed 2026-08-18 in (feat/followups-tickets).` + (Server-012 itself was already re-fixed on the previous branch — annotate it as such, + citing commits `a5f843c`/`f2a422b`, rather than re-fixing.) +5. Report a table: finding id → verified/regressed → action. + +**Commit:** `docs(reviews): re-verify doc-only Server-0xx resolutions; re-fix regressions`. + +--- + +### Task 7: WorkerPipeSessionTests deterministic failure — investigate and fix (windev) + +**Classification:** high-risk +**Estimated implement time:** investigation timeboxed ~10 min; fix ≤5 min +**Parallelizable with:** Task 1–5 (only windev user in wave 1) + +**Files (starting points — investigation task, report everything touched):** +- Test: `src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs` + (`RunAsync_LongInFlightCommandThatKeepsPumping_DoesNotFaultAndDeliversReply`) +- Suspect: `src/ZB.MOM.WW.MxGateway.Worker/` pipe-session / frame-protocol sources the + test exercises (follow the test's references) + +**Problem.** The test fails deterministically on windev, and reproduces on `main` — +pre-existing, not introduced by any recent branch. Everything else in the worker suite +passes (523/523 otherwise). + +**Procedure (all building/testing on windev over ssh; edits in the Mac tree, pushed by +the controller, pulled on the CI clone — coordinate via your report if you need a push):** +1. Repro on the CI clone at this branch: + `dotnet test src/ZB.MOM.WW.MxGateway.Worker.Tests/ZB.MOM.WW.MxGateway.Worker.Tests.csproj -p:Platform=x86 --filter "FullyQualifiedName~RunAsync_LongInFlightCommandThatKeepsPumping"`. + Capture the full failure output (assertion text, timeout, stack). +2. Read the test and the code under test. Classify: (a) test defect (bad timing + assumption, races in the harness), (b) product defect in the pipe session under a + long in-flight command, or (c) environment-specific (windev timing/load). + **Timebox: if the cause is not isolated after ~10 minutes of investigation, stop and + report your best hypothesis with evidence — do not churn.** +3. Fix minimally per the classification. A product fix in the frame/pipe layer is + high-risk territory: preserve the frame protocol (`docs/WorkerFrameProtocol.md`), the + STA pumping rules, and MXAccess parity. A test fix must keep the scenario's intent — + a long in-flight command that keeps pumping must not fault the session and must + deliver its reply; don't weaken it into a sleep-and-hope. +4. Verify: the fixed test passes 3 consecutive runs on windev; then the full worker suite + (`-p:Platform=x86`) is green. If you touched product code, also build the full `slnx` + on windev and run the gateway suite filtered to any shared-surface tests. +5. Docs in the same commit if behavior/rules changed. + +**Commit:** `fix(worker): — WorkerPipeSessionTests long-in-flight repro` (adjust +`fix(worker-tests)` if the defect is in the test). + +--- + +### Task 8: Ack-leg probe — bounded unblock attempt (windev rig) + +**Classification:** standard +**Estimated implement time:** timeboxed ~15 min of probing +**Parallelizable with:** Task 6 (runs after Task 7 — serialize windev use) + +**Files (starting points — investigation task):** +- Modify: `docs/AlarmProbeFindings.md` (append a third-attempt section, whatever the outcome) +- Reference (read-only): the probe harness locations named in that doc's earlier attempts; + the mxaccess analysis project at `C:\Users\dohertj2\Desktop\mxaccess` (windev) + +**Problem.** The acknowledge leg remains unobserved: every wnwrap ack surface is +accepted-but-inert (`rc=0`, state stays `UNACK_ALM`), and `.Acked` is write-rejected +(OperationalError 1007). Two recorded unblock paths remain +(`docs/AlarmProbeFindings.md`, "Remaining unblock paths for the acknowledge leg"). + +**Procedure — attempt path 2 first (it is inspectable), path 1 only if non-interactive:** +1. **Path 2 — ack-security configuration:** inspect whether the rig's galaxy/`alarmmgr` + is configured with an alarm-acknowledgement security requirement the wnwrap consumer + (operator *name* string, no authenticated identity) cannot meet. Look in galaxy + configuration (the `ZB` SQL Galaxy Repository — read-only queries only), area/object + security settings for the `TestMachine_00x` objects, and any alarm-security docs in + the mxaccess analysis project. **Read-only: do not change galaxy security config.** +2. **Path 1 — platform-side ack:** only if a *scriptable, non-interactive* path exists on + the rig as-installed (e.g. an existing harness or automation entry point). Do NOT + install software, do NOT drive GUI automation, do NOT change rig state beyond the + established raise/clear pattern on `TestMachine_001.TestAlarm001`. If only interactive + IDE/InTouch paths exist, record that and stop. +3. Whatever the outcome, append the third-attempt section: what was inspected, evidence, + and the leg's final status (observed / unavailable-by-configuration / still assumed + with the paths requiring a human). If the ack was actually observed, record whether + `STATE` reached `ACK_ALM` and whether the GUID survived — that answers the original + question and should update the findings table at the top of the doc. +4. **Stop condition:** at ~15 minutes of probing without a decisive result, write up what + was learned and conclude "still assumed" — the doc itself notes this is a + documentation gap, not a correctness one. + +**Commit:** `docs(probe): ack-leg third attempt — `. + +--- + +### Task 9: Windev toolchain pin + full verification + +**Classification:** verification (no review chain) +**Estimated implement time:** ~15 min wall (mostly build/test wait) +**Parallelizable with:** none (after all code tasks land and are pushed) + +**Files:** none in-repo except possibly `docs/ToolchainLinks.md` (update the windev +`protoc-gen-go-grpc` entry if it records 1.6.1). + +**Procedure (all over `ssh windev`, CI clone `C:\build\mxaccessgw-ci` at this branch tip):** +1. `go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.2` — then + `protoc-gen-go-grpc --version` must report 1.6.2. Update `docs/ToolchainLinks.md` if + it pins the old version (commit from the Mac tree). +2. Full `slnx` build — 0 warnings / 0 errors (clear Contracts obj/bin on CS2001/CS0016). +3. Worker tests `-p:Platform=x86` — expect the Task 7 outcome (fully green if fixed; + otherwise the documented known-failure only). +4. Gateway tests — full suite; rerun filtered on load flakes before believing a failure. +5. `powershell scripts/check-codegen.ps1` — **all 4 checks**, proving Task 1 on the + platform that had the bug. +6. `gradle --version` + `gradle :zb-mom-ww-mxgateway-client:checkGeneratedClean` from + `clients\java` if Gradle is installed there — record version and result either way. +7. Live MXAccess smoke: `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`, filter + `WorkerLiveMxAccessSmokeTests` — 8/8. +8. Report every result verbatim (counts, not "green"). + +--- + +### Task 10: Bookkeeping — close the follow-ups record + +**Classification:** trivial +**Estimated implement time:** ~3 min +**Parallelizable with:** none (last) + +**Files:** +- Modify: `docs/plans/2026-08-17-followup-closeout.md` (follow-ups block `:483-507`): + annotate each bullet closed with this branch's closing commit (or precisely narrowed, + e.g. ack leg if still assumed) +- Modify: `docs/plans/2026-08-18-followups-and-tickets.md` (this file): append as-built + notes — per-task commits, review outcomes, verification results, anything learned +- Modify: `docs/plans/2026-08-18-followups-and-tickets.md.tasks.json`: final statuses + +**Commit:** `chore(plan): followups-and-tickets as-built record; prior follow-ups closed`. + +--- + +## Execution notes for the controller + +- **Model/review chains** (subagent-driven-development skill): Tasks 1–4 small → Sonnet + implementer + Haiku code review (diffs <100 LOC). Task 5 standard → Opus implementer, + spec (Haiku) ∥ code (Sonnet). Task 6 standard → Opus implementer, spec ∥ code. Task 7 + high-risk → Opus implementer, serial spec (Haiku) → code (Sonnet). Task 8 standard → + Opus implementer, spec ∥ code. Task 9 verification → Opus, no review. Task 10 trivial → + Sonnet, no review. +- **Waves:** Wave 1 = Tasks 1, 2, 3, 4, 5, 7 (disjoint files; Task 7 alone on windev). + Wave 2 = Task 6 (after 4) and Task 8 (after 7). Wave 3 = push branch, Task 9. + Wave 4 = Task 10, final integration review, then hold for the user's merge decision. +- **Push cadence:** controller pushes the branch before any windev task needs its code + there (Task 7 investigates on-branch; Tasks 8–9 need the wave-1/2 tips). diff --git a/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json b/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json new file mode 100644 index 0000000..6a82449 --- /dev/null +++ b/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json @@ -0,0 +1,16 @@ +{ + "planPath": "docs/plans/2026-08-18-followups-and-tickets.md", + "tasks": [ + {"id": 1, "subject": "Task 1: check-codegen Check 4 — Windows .exe banner normalization", "status": "pending"}, + {"id": 2, "subject": "Task 2: Gradle 9 checkGeneratedClean via ProviderFactory.exec", "status": "pending"}, + {"id": 3, "subject": "Task 3: SettingsPage RecentFaultLimit/RecentSessionLimit rows", "status": "pending"}, + {"id": 4, "subject": "Task 4: Authentication.md runnable-as-written examples pass", "status": "pending"}, + {"id": 5, "subject": "Task 5: AlarmsPage push-driven truncation banner (snapshot_status)", "status": "pending"}, + {"id": 6, "subject": "Task 6: Server-0xx doc-only resolution audit", "status": "pending", "blockedBy": [4]}, + {"id": 7, "subject": "Task 7: WorkerPipeSessionTests deterministic failure — investigate + fix (windev)", "status": "pending"}, + {"id": 8, "subject": "Task 8: Ack-leg probe bounded unblock attempt (windev rig)", "status": "pending", "blockedBy": [7]}, + {"id": 9, "subject": "Task 9: Windev toolchain pin + full verification", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8]}, + {"id": 10, "subject": "Task 10: Bookkeeping — close the follow-ups record", "status": "pending", "blockedBy": [9]} + ], + "lastUpdated": "2026-08-18" +} From df45cb4a3739975f4701912d05dd9cbb30c851b8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:38:35 -0400 Subject: [PATCH 02/21] =?UTF-8?q?fix(java-client):=20checkGeneratedClean?= =?UTF-8?q?=20via=20ProviderFactory.exec=20=E2=80=94=20Project.exec=20is?= =?UTF-8?q?=20gone=20in=20Gradle=209?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../zb-mom-ww-mxgateway-client/build.gradle | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/clients/java/zb-mom-ww-mxgateway-client/build.gradle b/clients/java/zb-mom-ww-mxgateway-client/build.gradle index 4fe4351..e13861f 100644 --- a/clients/java/zb-mom-ww-mxgateway-client/build.gradle +++ b/clients/java/zb-mom-ww-mxgateway-client/build.gradle @@ -72,16 +72,18 @@ tasks.register('checkGeneratedClean') { group = 'verification' description = 'Fails if the committed generated Java tree differs from a fresh regeneration.' dependsOn 'generateProto' + def generatedDir = 'clients/java/src/main/generated' + def repoRoot = rootProject.projectDir.parentFile.parentFile + // Project.exec was removed in Gradle 9; ProviderFactory.exec runs the git-status probe + // lazily (captured at configuration time, evaluated in doLast) and works on both the + // installed Gradle and Gradle 9. + def gitStatus = providers.exec { + workingDir(repoRoot) + commandLine('git', 'status', '--porcelain', '--', generatedDir) + ignoreExitValue = true + } doLast { - def generatedDir = 'clients/java/src/main/generated' - def stdout = new ByteArrayOutputStream() - def result = exec { - workingDir = rootProject.projectDir.parentFile.parentFile - commandLine 'git', 'status', '--porcelain', '--', generatedDir - standardOutput = stdout - ignoreExitValue = true - } - def dirty = stdout.toString().trim() + def dirty = gitStatus.standardOutput.asText.get().trim() if (!dirty.isEmpty()) { throw new GradleException( "Generated Java is stale:\n${dirty}\n" + From c94c4d43c42e8a21aa4cae44bb55f70c96e2f2b7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:39:00 -0400 Subject: [PATCH 03/21] fix(codegen): normalize .exe off plugin version banners so Check 4 runs on Windows --- clients/go/generate-proto.ps1 | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/clients/go/generate-proto.ps1 b/clients/go/generate-proto.ps1 index 85489b0..938f480 100644 --- a/clients/go/generate-proto.ps1 +++ b/clients/go/generate-proto.ps1 @@ -6,7 +6,10 @@ $ErrorActionPreference = 'Stop' # those header stamps, so a regeneration on an off-pin machine would churn the tree and make # check-codegen Check 4 false-fail (or mask real drift under churn). Assert the exact versions # so a regen is deterministic. protoc itself is warn-only (source_code_info is normalized out of -# the committed bindings), matching publish-client-proto-inputs.ps1. +# the committed bindings), matching publish-client-proto-inputs.ps1. On Windows a plugin reports +# its argv[0] name, so the banner carries a trailing `.exe` (e.g. "protoc-gen-go.exe v1.36.11"); +# Get-NormalizedToolVersion strips that suffix before the pin compare so Check 4 runs the same on +# Windows as everywhere else. $PinnedProtocGenGoVersion = 'protoc-gen-go v1.36.11' $PinnedProtocGenGoGrpcVersion = 'protoc-gen-go-grpc 1.6.2' $PinnedProtocVersion = 'libprotoc 34.1' @@ -16,6 +19,14 @@ $protoRoot = Join-Path $repoRoot 'src\ZB.MOM.WW.MxGateway.Contracts\Protos' $outputRoot = Join-Path $PSScriptRoot 'internal\generated' $modulePath = 'gitea.dohertylan.com/dohertj2/mxaccessgw/clients/go/internal/generated' +function Get-NormalizedToolVersion { + # On Windows a plugin reports its argv[0] name, so the banner carries an `.exe` + # suffix ("protoc-gen-go.exe v1.36.11"). Strip it so the pin compare is + # host-independent; the version part must still match exactly. + param([string]$RawBanner) + return ($RawBanner -replace '\.exe(?=\s)', '') +} + function Resolve-Tool { # Resolve a codegen tool from PATH first (portable), then the documented Windows fallbacks, # instead of the previous hard-coded per-machine paths. See docs/ToolchainLinks.md. @@ -51,17 +62,17 @@ $protocGenGoGrpc = Resolve-Tool -Names @('protoc-gen-go-grpc', 'protoc-gen-go-gr # Assert the pinned plugin versions before generating so Check 4 cannot false-fail (or mask drift) # on an off-pin machine. protoc is warn-only. -$protocGenGoVersion = (& $protocGenGo --version 2>&1 | Out-String).Trim() +$protocGenGoVersion = Get-NormalizedToolVersion (& $protocGenGo --version 2>&1 | Out-String).Trim() if ($protocGenGoVersion -ne $PinnedProtocGenGoVersion) { throw "protoc-gen-go reports '$protocGenGoVersion', but regeneration is pinned to '$PinnedProtocGenGoVersion'. " + "Install the pin: go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.11" } -$protocGenGoGrpcVersion = (& $protocGenGoGrpc --version 2>&1 | Out-String).Trim() +$protocGenGoGrpcVersion = Get-NormalizedToolVersion (& $protocGenGoGrpc --version 2>&1 | Out-String).Trim() if ($protocGenGoGrpcVersion -ne $PinnedProtocGenGoGrpcVersion) { throw "protoc-gen-go-grpc reports '$protocGenGoGrpcVersion', but regeneration is pinned to '$PinnedProtocGenGoGrpcVersion'. " + "Install the pin: go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.2" } -$protocVersion = (& $protoc --version 2>&1 | Out-String).Trim() +$protocVersion = Get-NormalizedToolVersion (& $protoc --version 2>&1 | Out-String).Trim() if ($protocVersion -ne $PinnedProtocVersion) { Write-Warning "protoc reports '$protocVersion', pin is '$PinnedProtocVersion'. Descriptor comments are normalized out of the committed Go bindings, so patch drift is tolerated; keep CI on the pin." } From a390fe16e2ffbb2affb2749f1268f88bd0cd3a5c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:39:32 -0400 Subject: [PATCH 04/21] feat(dashboard): settings page shows RecentFaultLimit and RecentSessionLimit --- .../Components/Pages/SettingsPage.razor | 2 ++ .../SettingsPageTagVisibilityRenderTests.cs | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SettingsPage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SettingsPage.razor index 5600287..b064d93 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SettingsPage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SettingsPage.razor @@ -98,6 +98,8 @@ else Dashboard enabled@Snapshot.Configuration.Dashboard.Enabled Anonymous localhost@Snapshot.Configuration.Dashboard.AllowAnonymousLocalhost Snapshot interval@Snapshot.Configuration.Dashboard.SnapshotIntervalMilliseconds ms + Recent fault limit@Snapshot.Configuration.Dashboard.RecentFaultLimit + Recent session limit@Snapshot.Configuration.Dashboard.RecentSessionLimit Show tag values@Snapshot.Configuration.Dashboard.ShowTagValues Untagged session visibility@Snapshot.Configuration.Dashboard.UntaggedSessionVisibility Worker protocol@Snapshot.Configuration.Protocol.WorkerProtocolVersion diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs index 5de301a..0c8f7e9 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs @@ -73,6 +73,26 @@ public sealed class SettingsPageTagVisibilityRenderTests Assert.Contains("Dashboard role mapping", html, StringComparison.Ordinal); } + /// + /// RecentFaultLimit and RecentSessionLimit reach the page. Non-default values + /// prove the projection is plumbed through, not just that the defaults happen to render. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task SettingsPage_RendersRecentFaultAndSessionLimits() + { + string html = await RenderAsync(new DashboardOptions + { + RecentFaultLimit = 123, + RecentSessionLimit = 456, + }); + + Assert.Contains("Recent fault limit", html, StringComparison.Ordinal); + Assert.Contains("123", html, StringComparison.Ordinal); + Assert.Contains("Recent session limit", html, StringComparison.Ordinal); + Assert.Contains("456", html, StringComparison.Ordinal); + } + private static async Task RenderAsync(DashboardOptions dashboard) { EffectiveGatewayConfiguration configuration = From 2b1efb5e508697c8e11755154cba111a433aa424 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:40:11 -0400 Subject: [PATCH 05/21] docs(auth): make Authentication.md examples runnable as written --- docs/Authentication.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Authentication.md b/docs/Authentication.md index 02c1206..7544c71 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -260,9 +260,9 @@ Examples: ```bash mxgateway apikey init-db -mxgateway apikey create-key --key-id ops.alice --display-name "Alice (ops)" --scopes invoke:read,invoke:write -mxgateway apikey create-key --key-id area1.reader --display-name "Area 1 reader" --scopes invoke:read,metadata:read --read-subtree "Area1/*" --browse-subtree "Area1/*" -mxgateway apikey create-key --key-id ops.temp --display-name "Temp contractor" --scopes invoke:read --expires 90d +mxgateway apikey create-key --key-id ops.alice --display-name "Alice (ops)" --scopes session:open,invoke:read,invoke:write +mxgateway apikey create-key --key-id area1.reader --display-name "Area 1 reader" --scopes session:open,invoke:read,metadata:read --read-subtree "Area1/*" --browse-subtree "Area1/*" +mxgateway apikey create-key --key-id ops.temp --display-name "Temp contractor" --scopes session:open,invoke:read --expires 90d mxgateway apikey create-key --key-id team-a.svc --display-name "Team A service" --scopes session:open,invoke:read --dashboard-tags team-a mxgateway apikey create-key --key-id ops.audit --display-name "Audit window" --scopes metadata:read --expires 2027-01-01T00:00:00Z mxgateway apikey list-keys --json From 7b6dfba6545f88d617013266a38cc0ba7a32dc4c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:43:23 -0400 Subject: [PATCH 06/21] feat(dashboard): alarms page consumes snapshot_status feed frame for the truncation banner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The truncated-snapshot caveat moved from poll-only to push-driven. The page already held an in-process alarm-feed subscription for the provider badge; it now also handles the feed's snapshot_status frame, so a capped provider fetch is caveated when the monitor decides it rather than up to three seconds later. The poll's assignment stays as the reconcile baseline — both sources read the same monitor verdict, and the frame is consumed, never synthesized page-side. StreamAsync primes every subscriber with a snapshot_status frame at open, so a page attaching mid-truncation needs no priming logic of its own; the loop is renamed StatusFeedLoopAsync because it now feeds two indicators, not one. --- docs/GatewayDashboardDesign.md | 25 ++- .../Components/Pages/AlarmsPage.razor | 54 +++-- .../AlarmsPageTruncationBannerTests.cs | 209 ++++++++++++++++++ 3 files changed, 266 insertions(+), 22 deletions(-) diff --git a/docs/GatewayDashboardDesign.md b/docs/GatewayDashboardDesign.md index f69b8c0..5bd7241 100644 --- a/docs/GatewayDashboardDesign.md +++ b/docs/GatewayDashboardDesign.md @@ -245,8 +245,9 @@ cancelling, a detach-driven exit leaves the pill to the incoming subscription; w the pill therefore reports is the case it exists for — the channel completing under a page that is still watching. -`AlarmsPage` owns two loops of its own (the 3 s alarm poll and the provider-status -badge) and bounds their drain at 5 seconds on dispose, for the same reason +`AlarmsPage` owns two loops of its own (the 3 s alarm poll and the status feed that +drives the provider badge and the truncated-snapshot banner) and bounds their drain +at 5 seconds on dispose, for the same reason `DashboardPageBase` bounds its watch drain: both loops render through the renderer's dispatcher, and disposal can run on it. The two are drained concurrently, so the bound on disposal is 5 seconds in total rather than per loop — a wedged dispatcher @@ -285,11 +286,18 @@ Both seams consume the same producing services, so they share these cadences: - alarm publisher emits on each transition observed by the central monitor; - event publisher emits per event fanned by the session's `SessionEventDistributor` to its internal dashboard-mirror subscriber (independent of any gRPC `StreamEvents`); -- the alarms page's provider-status badge resubscribes one second after its +- the alarms page's status feed resubscribes one second after its `IGatewayAlarmService.StreamAsync` enumeration ends — the monitor completes a subscriber's stream when it falls behind and again when it restarts, both - recoverable by resubscribing — and holds its last value in between. The page's - alarm rows are independent of that stream and refresh on the 3 s poll. + recoverable by resubscribing — and the badge and banner hold their last values in + between. That feed carries both gateway-status frames: `provider_status` drives the + badge, and `snapshot_status` drives the truncated-snapshot banner, so the caveat + appears on the monitor's verdict change rather than up to three seconds later. Every + subscriber is primed with a `snapshot_status` frame at open, so a page attaching + mid-truncation needs no priming logic of its own. The page's alarm rows are + independent of that stream and refresh on the 3 s poll, which also re-asserts the + truncation verdict as its reconcile baseline — both sources read the same monitor + verdict, and neither is synthesized page-side. ### Idle gating and snapshot cost @@ -540,8 +548,11 @@ alarm-history store, so the page reflects only the live active set. The page is read-only; it does not acknowledge alarms. A provider-status badge tracks the central monitor's health from `IGatewayAlarmService.StreamAsync` in process — the alarm service is already a multi-subscriber fan-out, so the badge needs no SignalR -client, no loopback socket, and no hub token — while the alarm rows themselves -still come from the three-second poll. If `MxGateway:Alarms:Enabled` is +client, no loopback socket, and no hub token — and the same subscription carries the +`snapshot_status` frame behind the truncated-snapshot banner ("Alarm snapshot may be +incomplete"), so a capped provider fetch is caveated the moment the monitor decides +it. The alarm rows themselves still come from the three-second poll, which also +re-asserts the truncation verdict as the reconcile baseline. If `MxGateway:Alarms:Enabled` is false the central monitor never starts, and the page says so instead of showing an empty list with no explanation. diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor index 0cabe58..8c3ad85 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor @@ -191,13 +191,13 @@ private Task? _pollTask; private DashboardAlarmProviderStatus _providerStatus = DashboardAlarmProviderStatus.Healthy; - private Task? _providerStatusTask; + private Task? _statusFeedTask; /// protected override void OnInitialized() { _pollTask = PollLoopAsync(); - _providerStatusTask = ProviderStatusLoopAsync(); + _statusFeedTask = StatusFeedLoopAsync(); } private string? ProviderStatusTitle() @@ -210,8 +210,13 @@ // The badge tracks the central monitor directly rather than looping back through // /hubs/alarms: the alarm service is an in-process multi-subscriber fan-out, so a // server-rendered page needs no SignalR client, no loopback socket and no auth token. - // Alarm rows still come from the 3-second poll below — this loop only feeds the badge. - private async Task ProviderStatusLoopAsync() + // This loop feeds the two gateway-status indicators — the provider badge and the + // truncation banner — from the feed's own status frames, so both move as soon as the + // monitor's verdict changes instead of on the next 3-second tick. Alarm rows still come + // from the poll below, which also re-asserts the truncation verdict as its reconcile + // baseline: both sources read the same monitor verdict, so they cannot disagree for + // longer than one tick, and neither one is synthesized here. + private async Task StatusFeedLoopAsync() { while (!_cts.IsCancellationRequested) { @@ -221,16 +226,30 @@ .StreamAsync(alarmFilterPrefix: null, _cts.Token) .ConfigureAwait(false)) { - if (message.PayloadCase != AlarmFeedMessage.PayloadOneofCase.ProviderStatus) + switch (message.PayloadCase) { - continue; - } + case AlarmFeedMessage.PayloadOneofCase.ProviderStatus: + await InvokeAsync(() => + { + _providerStatus = DashboardAlarmProviderStatus.FromFeed(message); + StateHasChanged(); + }).ConfigureAwait(false); + break; - await InvokeAsync(() => - { - _providerStatus = DashboardAlarmProviderStatus.FromFeed(message); - StateHasChanged(); - }).ConfigureAwait(false); + // Every subscriber is primed with this frame at open, so a page that + // attaches mid-truncation gets the caveat without waiting for an edge — + // no page-side priming needed. + case AlarmFeedMessage.PayloadOneofCase.SnapshotStatus: + await InvokeAsync(() => + { + _snapshotTruncated = message.SnapshotStatus.Truncated; + StateHasChanged(); + }).ConfigureAwait(false); + break; + + default: + break; + } } } catch (OperationCanceledException) @@ -241,7 +260,8 @@ { // The monitor completes a subscriber's stream when it falls behind, and // again when the monitor restarts. Both are recoverable by resubscribing; - // the badge holds its last value in the meantime. + // the badge and banner hold their last values in the meantime, and the + // resubscribe is primed with the current ones. } try @@ -321,7 +341,7 @@ }; } - // Fault handling sits inside the loop, matching ProviderStatusLoopAsync: a query or render + // Fault handling sits inside the loop, matching StatusFeedLoopAsync: a query or render // fault on one tick is transient (a provider blip, a momentarily unavailable session), so it // is surfaced on the page and retried on the next tick rather than ending polling for the // life of the page. Cancellation is the only exit. The loop method itself therefore cannot @@ -398,6 +418,10 @@ { DashboardAlarmQueryResult result = await LiveData.QueryAlarmsAsync(_cts.Token); _queryError = result.Error; + // Kept alongside the feed's snapshot_status frame rather than replaced by it: this is + // the reconcile baseline. Both read the same monitor verdict, so the poll can only + // confirm what the frame already showed — but it also re-establishes the banner for a + // page whose feed subscription is mid-resubscribe after the monitor dropped it. _snapshotTruncated = result.SnapshotTruncated; _workerPid = result.WorkerProcessId; _lastRefresh = DateTimeOffset.UtcNow; @@ -416,7 +440,7 @@ // Drained together, not one after the other: the wedged dispatcher this bound exists // for blocks both loops at once, so sequential drains would time out twice and make // the real bound 10 seconds. DrainAsync tolerates a null task. - await Task.WhenAll(DrainAsync(_pollTask), DrainAsync(_providerStatusTask)) + await Task.WhenAll(DrainAsync(_pollTask), DrainAsync(_statusFeedTask)) .ConfigureAwait(false); _cts.Dispose(); diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs index ebb4e54..8a7e7fc 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs @@ -1,7 +1,11 @@ +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Threading.Channels; using Microsoft.AspNetCore.Components.Web.HtmlRendering; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Server.Alarms; using ZB.MOM.WW.MxGateway.Server.Configuration; using ZB.MOM.WW.MxGateway.Server.Dashboard; @@ -36,6 +40,8 @@ public sealed class AlarmsPageTruncationBannerTests { private const string BannerMarker = "Alarm snapshot may be incomplete"; + private static readonly TimeSpan RenderWaitTimeout = TimeSpan.FromSeconds(10); + /// A capped provider fetch puts the completeness caveat on the page. /// A task that represents the asynchronous operation. [Fact] @@ -61,6 +67,120 @@ public sealed class AlarmsPageTruncationBannerTests Assert.Contains("Active Alarms", html, StringComparison.Ordinal); } + /// + /// The push path. A snapshot_status frame arriving on the page's + /// in-process alarm-feed subscription raises the caveat on its own, with + /// no poll tick behind it — the poll is a 3-second reconcile baseline, and + /// an operator should not stare at an un-caveated alarm list for up to + /// three seconds after the gateway has already decided the set is capped. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task AlarmsPage_WhenFeedPushesTruncated_RaisesTheBannerWithoutAPollTick() + { + ScriptedAlarmFeed feed = new(); + await using ServiceProvider provider = BuildPushServices(feed); + await using HtmlRenderer renderer = new( + provider, + provider.GetRequiredService()); + + HtmlRootComponent page = await renderer.Dispatcher.InvokeAsync( + () => renderer.RenderComponentAsync()); + + // The one poll answer this page will ever get said "complete", so everything + // the banner does from here is the feed's doing. + Assert.DoesNotContain(BannerMarker, await HtmlAsync(renderer, page), StringComparison.Ordinal); + + await feed.PushAsync(SnapshotStatusFrame(truncated: true)); + + await WaitForHtmlAsync( + renderer, + page, + html => html.Contains(BannerMarker, StringComparison.Ordinal), + "banner to appear after a truncated snapshot_status frame"); + } + + /// + /// The clearing edge, page-side. Absence authority comes back when the + /// gateway says so; a banner that only ever went up would caveat the alarm + /// list for the life of the circuit. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task AlarmsPage_WhenFeedPushesComplete_ClearsTheBannerWithoutAPollTick() + { + ScriptedAlarmFeed feed = new(); + await using ServiceProvider provider = BuildPushServices(feed); + await using HtmlRenderer renderer = new( + provider, + provider.GetRequiredService()); + + HtmlRootComponent page = await renderer.Dispatcher.InvokeAsync( + () => renderer.RenderComponentAsync()); + + await feed.PushAsync(SnapshotStatusFrame(truncated: true)); + await WaitForHtmlAsync( + renderer, + page, + html => html.Contains(BannerMarker, StringComparison.Ordinal), + "banner to appear before the clearing frame is pushed"); + + await feed.PushAsync(SnapshotStatusFrame(truncated: false)); + + await WaitForHtmlAsync( + renderer, + page, + html => !html.Contains(BannerMarker, StringComparison.Ordinal), + "banner to clear after a complete snapshot_status frame"); + } + + private static AlarmFeedMessage SnapshotStatusFrame(bool truncated) + { + return new AlarmFeedMessage + { + SnapshotStatus = new AlarmSnapshotStatus { Truncated = truncated }, + }; + } + + private static ServiceProvider BuildPushServices(ScriptedAlarmFeed feed) + { + ServiceCollection services = new(); + services.AddLogging(); + services.AddSingleton(); + services.AddSingleton(feed); + services.AddSingleton>( + Options.Create(new GatewayOptions { Alarms = new AlarmsOptions { Enabled = true } })); + return services.BuildServiceProvider(); + } + + private static Task HtmlAsync(HtmlRenderer renderer, HtmlRootComponent page) + { + // Serialization has to happen on the renderer's dispatcher, and it reads the + // component's current render tree — so it reflects renders the page's feed + // loop queued after the initial quiescent render. + return renderer.Dispatcher.InvokeAsync(page.ToHtmlString); + } + + private static async Task WaitForHtmlAsync( + HtmlRenderer renderer, + HtmlRootComponent page, + Func predicate, + string expectation) + { + Stopwatch elapsed = Stopwatch.StartNew(); + while (elapsed.Elapsed < RenderWaitTimeout) + { + if (predicate(await HtmlAsync(renderer, page))) + { + return; + } + + await Task.Delay(TimeSpan.FromMilliseconds(20)); + } + + Assert.Fail($"Timed out after {RenderWaitTimeout.TotalSeconds:N0}s waiting for the {expectation}."); + } + private static async Task RenderAsync(bool snapshotTruncated) { ServiceCollection services = new(); @@ -102,4 +222,93 @@ public sealed class AlarmsPageTruncationBannerTests WorkerProcessId: null, SnapshotTruncated: snapshotTruncated)); } + + // Answers exactly one poll — the inline first pass — and parks every later tick + // until the page's disposal cancels it. The parking is what makes the push tests + // measure the push: a second tick would re-assert the poll's own verdict, and + // could either mask a banner the feed raised or raise one the feed did not. + private sealed class SinglePollLiveDataService : IDashboardLiveDataService + { + private int _polls; + + /// + public Task ReadAsync( + IReadOnlyCollection tagAddresses, + CancellationToken cancellationToken) => + Task.FromResult(DashboardLiveReadResult.Empty); + + /// + public async Task QueryAlarmsAsync(CancellationToken cancellationToken) + { + if (Interlocked.Increment(ref _polls) > 1) + { + await Task.Delay(Timeout.Infinite, cancellationToken).ConfigureAwait(false); + } + + return new DashboardAlarmQueryResult( + Alarms: [], + Error: null, + WorkerProcessId: null, + SnapshotTruncated: false); + } + } + + // A hand-driven stand-in for the alarm feed: the test writes the frames the real + // monitor would push. Unbounded and never completed, so a frame written before the + // page's loop attaches is still delivered, and the loop never has to resubscribe. + private sealed class ScriptedAlarmFeed : IGatewayAlarmService + { + private readonly Channel _frames = + Channel.CreateUnbounded(new UnboundedChannelOptions + { + SingleReader = false, + SingleWriter = false, + }); + + /// + public GatewayAlarmMonitorState State => GatewayAlarmMonitorState.Monitoring; + + /// + public string? LastError => null; + + /// + public int? WorkerProcessId => null; + + /// + public IReadOnlyList CurrentAlarms => []; + + /// + public bool SnapshotTruncated { get; set; } + + /// Pushes one frame onto the feed the page is subscribed to. + /// The feed frame to deliver. + /// A task that represents the asynchronous operation. + public ValueTask PushAsync(AlarmFeedMessage message) => _frames.Writer.WriteAsync(message); + + /// + public async IAsyncEnumerable StreamAsync( + string? alarmFilterPrefix, + [EnumeratorCancellation] CancellationToken cancellationToken) + { + await foreach (AlarmFeedMessage message in _frames.Reader + .ReadAllAsync(cancellationToken) + .ConfigureAwait(false)) + { + yield return message; + } + } + + /// + public Task AcknowledgeAsync( + AcknowledgeAlarmRequest request, + CancellationToken cancellationToken) + { + return Task.FromResult(new AcknowledgeAlarmReply + { + CorrelationId = request.ClientCorrelationId, + ProtocolStatus = new ProtocolStatus { Code = ProtocolStatusCode.Ok }, + DiagnosticMessage = string.Empty, + }); + } + } } From 7da52b65b7002fd19bcade77df7cc64389981fca Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:46:08 -0400 Subject: [PATCH 07/21] =?UTF-8?q?fix(worker-tests):=20model=20the=20STA=20?= =?UTF-8?q?pump=20at=20heartbeat=20capture=20=E2=80=94=20WorkerPipeSession?= =?UTF-8?q?Tests=20long-in-flight=20repro?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RunAsync_LongInFlightCommandThatKeepsPumping_DoesNotFaultAndDeliversReply failed deterministically on the Windows box with a StaHung fault whose command_method was empty and whose staleness was 233 ms — i.e. a fault raised with no command in flight, before the scenario under test began. FakeRuntimeSession stamps LastStaActivityUtc once, at construction, and the test only started refreshing it after the blocked dispatch signalled. Everything between those two points — handshake, STA init, the first heartbeat — captured a snapshot already stale past the compressed 50 ms grace, with no correlation id for the watchdog to suppress on, so the watchdog correctly reported the fake as hung. The harness, not the product, was wrong: StaRuntime.ThreadMain calls MarkActivity() on every WaitForWorkOrMessages iteration, so a live worker is never captured stale, idle or busy. Model that where it belongs — FakeRuntimeSession.RefreshStaActivityOnCapture (opt in, default off) stamps activity at each CaptureHeartbeat and leaves the rest of the snapshot alone — and arm it before RunAsync so the first beat is covered. The test-owned refresh loop goes away with it; a thread-pool loop racing a compressed grace could not have held the invariant anyway. Scenario intent is unchanged and slightly stronger: the command still blocks in dispatch across 30 heartbeats (~600 ms, many multiples of the 100 ms stuck ceiling), no frame may be a fault, and the reply must still arrive. The reply leg is now fault-checked too (previously it skipped frames blindly), and the pump keeps running across the release, as it does in production while the reply is marshalled off the STA. Fault assertions now report the category and diagnostic message instead of a bare body-case mismatch. Test-only change; no product code, frame protocol, or STA rule touched. --- .../Ipc/WorkerPipeSessionTests.cs | 99 ++++++++++++------- .../TestSupport/FakeRuntimeSession.cs | 43 ++++++++ 2 files changed, 109 insertions(+), 33 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs index 826e1b0..30e7044 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs @@ -1453,8 +1453,15 @@ public sealed class WorkerPipeSessionTests /// HeartbeatStuckCeiling (75 s in production) keeps its activity /// timestamp fresh. This test compresses the clock — a 100 ms ceiling /// with a command in flight across a window many multiples longer — and - /// models the pump refresh by continuously advancing the snapshot's - /// LastStaActivityUtc while the command blocks. Contrast + /// models the pump refresh with + /// , which + /// stamps activity at every heartbeat capture exactly as the pump's + /// per-iteration MarkActivity() does. The refresh has to be in + /// effect from construction, not from the moment the command blocks: + /// with a 50 ms grace, the idle window covering handshake and startup + /// carries no correlation id for the watchdog to suppress on, so a fake + /// whose activity timestamp is frozen at construction is reported + /// StaHung before the scenario under test even starts. Contrast /// , /// where a frozen timestamp beyond the ceiling correctly faults; here /// the refreshed timestamp must keep the fault suppressed and let the @@ -1469,6 +1476,13 @@ public sealed class WorkerPipeSessionTests FakeRuntimeSession runtime = new() { BlockDispatch = true, + + // The pump refreshes STA activity on every wait iteration, so every + // heartbeat capture on a healthy worker sees fresh activity — while + // a command holds the STA and while it is idle alike. Armed before + // RunAsync so the very first beat, sent as soon as the session is + // Ready, is already covered. + RefreshStaActivityOnCapture = true, }; WorkerPipeSession session = CreatePipeSession( pipePair.WorkerStream, @@ -1490,49 +1504,48 @@ public sealed class WorkerPipeSessionTests runtime.DispatchStarted.Wait(TimeSpan.FromSeconds(5)), "The long command must reach the runtime and begin dispatch."); - // Model the pump refreshing STA activity on each wait iteration: keep - // the snapshot's LastStaActivityUtc current while the command is in - // flight. - using CancellationTokenSource pumpRefresh = new(); - Task refreshLoop = Task.Run( - async () => - { - while (!pumpRefresh.IsCancellationRequested) - { - runtime.SetSnapshot(new WorkerRuntimeHeartbeatSnapshot( - DateTimeOffset.UtcNow, - pendingCommandCount: 1, - outboundEventQueueDepth: 0, - lastEventSequence: 0, - currentCommandCorrelationId: "long-bulk-read")); - await Task.Delay(TimeSpan.FromMilliseconds(20)).ConfigureAwait(false); - } - }); + // Publish the in-flight shape the heartbeat then reports for the whole + // blocked window; only LastStaActivityUtc moves after this, refreshed by + // the modelled pump at each capture. + runtime.SetSnapshot(new WorkerRuntimeHeartbeatSnapshot( + DateTimeOffset.UtcNow, + pendingCommandCount: 1, + outboundEventQueueDepth: 0, + lastEventSequence: 0, + currentCommandCorrelationId: "long-bulk-read")); // Inspect a bounded number of frames over a window many multiples of the // 100 ms ceiling (at least 30 heartbeats at 20 ms ~ 600 ms). None may be // a WorkerFault while activity is continuously refreshed. const int framesToInspect = 30; - for (int index = 0; index < framesToInspect; index++) + int frameIndex = 0; + for (; frameIndex < framesToInspect; frameIndex++) { WorkerEnvelope envelope = await pipePair.GatewayReader .ReadAsync(cancellation.Token); - Assert.NotEqual( - WorkerEnvelope.BodyOneofCase.WorkerFault, - envelope.BodyCase); + AssertNotFault(envelope, frameIndex); } - // Stop refreshing and release the command; its reply must be delivered - // because the session never faulted (state stayed Ready). - pumpRefresh.Cancel(); - await refreshLoop; + // Release the command with the pump still running — as it is in + // production while the reply is marshalled off the STA. The reply must + // be delivered (the session never faulted, so its state stayed Ready), + // and no frame on the way to it may be a fault either. runtime.ReleaseDispatch(); - WorkerEnvelope reply = await ReadUntilAsync( - pipePair.GatewayReader, - WorkerEnvelope.BodyOneofCase.WorkerCommandReply, - envelope => envelope.CorrelationId == "long-bulk-read", - cancellation.Token); + WorkerEnvelope reply; + while (true) + { + WorkerEnvelope envelope = await pipePair.GatewayReader + .ReadAsync(cancellation.Token); + AssertNotFault(envelope, frameIndex++); + if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerCommandReply + && envelope.CorrelationId == "long-bulk-read") + { + reply = envelope; + break; + } + } + Assert.Equal( ProtocolStatusCode.Ok, reply.WorkerCommandReply.Reply.ProtocolStatus.Code); @@ -2217,6 +2230,26 @@ public sealed class WorkerPipeSessionTests /// Predicate to match against envelope. /// Token to cancel the asynchronous operation. /// The matching envelope. + /// + /// Fails when the frame is a WorkerFault, naming the category and diagnostic message. + /// A bare body-case comparison reports only "expected not WorkerFault", which says nothing + /// about which watchdog or protocol path produced it — the one fact needed to tell a + /// regression from a harness that mis-models the runtime. + /// + /// Frame read from the gateway end. + /// Ordinal of the frame within the inspected run. + private static void AssertNotFault(WorkerEnvelope envelope, int frameIndex) + { + if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerFault) + { + return; + } + + Assert.Fail( + $"Frame {frameIndex} is a WorkerFault ({envelope.WorkerFault.Category}): " + + envelope.WorkerFault.DiagnosticMessage); + } + private static async Task ReadUntilAsync( WorkerFrameReader reader, WorkerEnvelope.BodyOneofCase expectedBody, diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs index b5a4252..c7b9ebf 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs @@ -127,11 +127,54 @@ internal sealed class FakeRuntimeSession : IWorkerRuntimeSession }); } + private bool refreshStaActivityOnCapture; + + /// + /// When set, stamps the snapshot's + /// LastStaActivityUtc with the capture time and leaves every other field as the last + /// left it. Models a live STA whose pump calls + /// MarkActivity() on each wait iteration (StaRuntime.ThreadMain), so a healthy + /// worker is never captured stale — which a watchdog test needs to hold for the whole + /// session, including the handshake window before any command exists for the watchdog to + /// suppress on. A test-owned refresh loop cannot hold it: it is a thread-pool continuation + /// racing a compressed grace, and the gap between this fake being constructed and that loop's + /// first tick is already enough to look hung. + /// + public bool RefreshStaActivityOnCapture + { + get + { + lock (gate) + { + return refreshStaActivityOnCapture; + } + } + + set + { + lock (gate) + { + refreshStaActivityOnCapture = value; + } + } + } + /// public WorkerRuntimeHeartbeatSnapshot CaptureHeartbeat() { lock (gate) { + if (refreshStaActivityOnCapture) + { + snapshot = new WorkerRuntimeHeartbeatSnapshot( + DateTimeOffset.UtcNow, + snapshot.PendingCommandCount, + snapshot.OutboundEventQueueDepth, + snapshot.LastEventSequence, + snapshot.CurrentCommandCorrelationId, + snapshot.StaCallInProgress); + } + return snapshot; } } From d3ac52758c00033f9cdf44c08fa9018c531be41a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:48:45 -0400 Subject: [PATCH 08/21] docs(reviews): re-verify doc-only Server-0xx resolutions; re-fix regressions Audited every Server-0xx finding whose Resolution described a documentation-only or comment-only change, and spot-checked the doc sub-claims of otherwise test-backed resolutions. 20 entries annotated in place (append-only; no historical resolution text rewritten). Two corrections had not survived and are re-applied: Server-040: the MapGroupsToRoles lookup-precedence comment moved intact into DashboardGroupRoleMapping (792e3f9) and was then deleted wholesale by fca978d, a sweep meant only to strip (Server-NNN) tracking markers. That also removed a later, substantive paragraph recording that the shared ZB.MOM.WW.Auth.Ldap provider pre-strips groups to short RDN names, so a full-DN GroupToRole key is unsupported. Both paragraphs restored, minus the tracking IDs. Server-009: the WAL / busy_timeout note vanished when the Storage section of docs/Authentication.md was rewritten to delegate connection-factory detail to ZB.MOM.WW.Auth.ApiKeys. The behavior is still live in the library (confirmed against 0.2.1), so the fix is prose-only. Server-011/014/022/023 are annotated as moot rather than regressed: the IAlarmRpcDispatcher trio was deleted in dc9c0c9 and no stale 'not yet wired' / 'PR A.6/A.7' prose survives in Server source. Server-038's documented v1 ACL gap was later closed by IDashboardSessionAcl, so its remarks are current. Comment/doc-only; no logic changes. --- code-reviews/Server/findings.md | 40 +++++++++---------- docs/Authentication.md | 11 +++++ .../Dashboard/DashboardGroupRoleMapping.cs | 17 ++++++++ 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/code-reviews/Server/findings.md b/code-reviews/Server/findings.md index 72b66f8..a9202c2 100644 --- a/code-reviews/Server/findings.md +++ b/code-reviews/Server/findings.md @@ -259,7 +259,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Add a startup hosted service that finds and kills stale worker processes (by executable path / a well-known argument or environment marker) before the server accepts sessions, or update the design docs if reattachment/cleanup is deliberately deferred. -**Resolution:** Resolved 2026-05-18. Confirmed against source: no code path enumerated or killed leftover workers. Added `IRunningProcessInspector` / `SystemRunningProcessInspector` (a testable seam over `Process.GetProcessesByName`/`Kill`), `OrphanWorkerTerminator` (kills processes matched by the configured worker executable path, or by image name when the x64 gateway cannot introspect the x86 worker's `MainModule`, skipping the current process and tolerating per-process kill failures), and `OrphanWorkerCleanupHostedService` (best-effort `IHostedService`). The hosted service is registered in `AddWorkerProcessLauncher` ahead of `AddGatewaySessions` so cleanup runs before the server accepts sessions. `gateway.md` updated to describe the implemented behavior. Regression tests: `OrphanWorkerTerminatorTests` (`KillsWorkerProcessesMatchingConfiguredExecutablePath`, `KillsImageNameMatchWhenExecutablePathUnreadable`, `DoesNotKillUnrelatedProcessSharingImageName`, `DoesNotKillCurrentProcess`, `ContinuesWhenOneKillThrows`). +**Resolution:** Resolved 2026-05-18. Confirmed against source: no code path enumerated or killed leftover workers. Added `IRunningProcessInspector` / `SystemRunningProcessInspector` (a testable seam over `Process.GetProcessesByName`/`Kill`), `OrphanWorkerTerminator` (kills processes matched by the configured worker executable path, or by image name when the x64 gateway cannot introspect the x86 worker's `MainModule`, skipping the current process and tolerating per-process kill failures), and `OrphanWorkerCleanupHostedService` (best-effort `IHostedService`). The hosted service is registered in `AddWorkerProcessLauncher` ahead of `AddGatewaySessions` so cleanup runs before the server accepts sessions. `gateway.md` updated to describe the implemented behavior. Regression tests: `OrphanWorkerTerminatorTests` (`KillsWorkerProcessesMatchingConfiguredExecutablePath`, `KillsImageNameMatchWhenExecutablePathUnreadable`, `DoesNotKillUnrelatedProcessSharingImageName`, `DoesNotKillCurrentProcess`, `ContinuesWhenOneKillThrows`). Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; the `gateway.md` orphan-cleanup prose (`OrphanWorkerCleanupHostedService` running `OrphanWorkerTerminator` once on startup, before the server accepts sessions) is still there. ### Server-003 @@ -364,7 +364,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Set `Pooling`, a non-zero `DefaultTimeout`/`busy_timeout`, and enable WAL (`PRAGMA journal_mode=WAL`) once at startup so concurrent readers/writers degrade gracefully. -**Resolution:** Resolved 2026-05-18. Confirmed against source: the connection string set only `DataSource` and `Mode`. `AuthSqliteConnectionFactory.CreateConnection` now also sets `Pooling = true` and a non-zero `DefaultTimeout`. A new `OpenConnectionAsync(CancellationToken)` opens the connection and applies `PRAGMA journal_mode=WAL` and `PRAGMA busy_timeout` (5 s); WAL is a persistent database-level setting so re-applying it per connection is a cheap no-op, while `busy_timeout` is per-connection state. All nine auth-store call sites (`SqliteApiKeyAdminStore`, `SqliteApiKeyAuditStore`, `SqliteApiKeyStore`, `SqliteAuthStoreMigrator`) were switched from `CreateConnection()` + `OpenAsync()` to `OpenConnectionAsync()`. `docs/Authentication.md` updated to describe the WAL/busy-timeout behavior. Regression test: `SqliteAuthStoreTests.OpenConnectionAsync_EnablesWalJournalModeAndBusyTimeout`. +**Resolution:** Resolved 2026-05-18. Confirmed against source: the connection string set only `DataSource` and `Mode`. `AuthSqliteConnectionFactory.CreateConnection` now also sets `Pooling = true` and a non-zero `DefaultTimeout`. A new `OpenConnectionAsync(CancellationToken)` opens the connection and applies `PRAGMA journal_mode=WAL` and `PRAGMA busy_timeout` (5 s); WAL is a persistent database-level setting so re-applying it per connection is a cheap no-op, while `busy_timeout` is per-connection state. All nine auth-store call sites (`SqliteApiKeyAdminStore`, `SqliteApiKeyAuditStore`, `SqliteApiKeyStore`, `SqliteAuthStoreMigrator`) were switched from `CreateConnection()` + `OpenAsync()` to `OpenConnectionAsync()`. `docs/Authentication.md` updated to describe the WAL/busy-timeout behavior. Regression test: `SqliteAuthStoreTests.OpenConnectionAsync_EnablesWalJournalModeAndBusyTimeout`. Regressed or never applied; re-fixed 2026-08-18 in `docs/Authentication.md` (feat/followups-tickets) — the doc sub-claim did not survive: the Storage section was later rewritten to delegate schema/connection-factory detail to `ZB.MOM.WW.Auth.ApiKeys` and the WAL/busy-timeout paragraph went with it, leaving no prose for the behavior anywhere. The behavior itself is intact (the library's `AuthSqliteConnectionFactory.OpenConnectionAsync` still issues both pragmas — confirmed against the 0.2.1 package), so the fix is prose-only: a paragraph restating the pooled-connection + `journal_mode=WAL` + 5 s `busy_timeout` contract and why it is load-bearing for the per-request last-used stamp and the per-denial audit append. ### Server-010 @@ -379,7 +379,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Either hide/disable the Rotate action for revoked keys in `ApiKeysPage.razor`, require an explicit confirmation, or have `RotateAsync` preserve `revoked_utc` and add a separate explicit "reactivate" operation. -**Resolution:** Resolved 2026-05-18. Confirmed against source: `ApiKeysPage.razor` rendered the Rotate button unconditionally while Revoke was already gated on `key.RevokedUtc is null`. Took the lowest-risk recommended option — the dashboard now renders the Rotate (and Revoke) actions only for keys whose status is `Active`; a revoked key shows a "No actions" placeholder, so an operator cannot un-revoke a deliberately disabled key as a side effect of a rotation. `RotateAsync`'s store-level behavior is unchanged (rotation by `key_id` still clears `revoked_utc`, which the CLI relies on); `docs/Authentication.md` updated to document both the store behavior and the dashboard restriction. No automated test added: the change is pure conditional Razor rendering and the test project has no bUnit component-rendering harness; the underlying `DashboardApiKeyManagementService` is already unit-tested. +**Resolution:** Resolved 2026-05-18. Confirmed against source: `ApiKeysPage.razor` rendered the Rotate button unconditionally while Revoke was already gated on `key.RevokedUtc is null`. Took the lowest-risk recommended option — the dashboard now renders the Rotate (and Revoke) actions only for keys whose status is `Active`; a revoked key shows a "No actions" placeholder, so an operator cannot un-revoke a deliberately disabled key as a side effect of a rotation. `RotateAsync`'s store-level behavior is unchanged (rotation by `key_id` still clears `revoked_utc`, which the CLI relies on); `docs/Authentication.md` updated to document both the store behavior and the dashboard restriction. No automated test added: the change is pure conditional Razor rendering and the test project has no bUnit component-rendering harness; the underlying `DashboardApiKeyManagementService` is already unit-tested. Re-verified present 2026-08-18 (feat/followups-tickets) — `ApiKeysPage.razor` still gates both Rotate and Revoke on `key.RevokedUtc is null`, with the "Rotate clears revoked_utc" comment intact. ### Server-011 @@ -394,7 +394,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Add the needed `using` directives, drop the inline fully-qualified names, and convert to a primary constructor for consistency. -**Resolution:** Resolved 2026-05-18. Confirmed against source. Converted `WorkerAlarmRpcDispatcher` to a primary constructor with the standard `?? throw new ArgumentNullException(...)` field-initializer guard; dropped the inline `System.Guid` / `System.ArgumentNullException` qualifications (using implicit `using System;`); removed redundant `using System.Collections.Generic;` / `System.Threading` / `System.Threading.Tasks;` directives (covered by `ImplicitUsings`); replaced the two `if (... is null) throw new System.ArgumentNullException(...)` checks with `ArgumentNullException.ThrowIfNull`. The stale class-level ``/`` ("Replaces NotWiredAlarmRpcDispatcher once ... wired in", "partially wired", "returns an Unimplemented diagnostic") were corrected to describe the actual GUID-vs-`Provider!Group.Tag` handling — overlapping with Server-014. No behavior change, so no new test; existing `WorkerAlarmRpcDispatcherTests` continue to pass and the project builds warning-free under `TreatWarningsAsErrors`. +**Resolution:** Resolved 2026-05-18. Confirmed against source. Converted `WorkerAlarmRpcDispatcher` to a primary constructor with the standard `?? throw new ArgumentNullException(...)` field-initializer guard; dropped the inline `System.Guid` / `System.ArgumentNullException` qualifications (using implicit `using System;`); removed redundant `using System.Collections.Generic;` / `System.Threading` / `System.Threading.Tasks;` directives (covered by `ImplicitUsings`); replaced the two `if (... is null) throw new System.ArgumentNullException(...)` checks with `ArgumentNullException.ThrowIfNull`. The stale class-level ``/`` ("Replaces NotWiredAlarmRpcDispatcher once ... wired in", "partially wired", "returns an Unimplemented diagnostic") were corrected to describe the actual GUID-vs-`Provider!Group.Tag` handling — overlapping with Server-014. No behavior change, so no new test; existing `WorkerAlarmRpcDispatcherTests` continue to pass and the project builds warning-free under `TreatWarningsAsErrors`. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but no longer verifiable in place — `WorkerAlarmRpcDispatcher.cs` was **deleted** (not renamed) by the `dc9c0c9` project-rename/alarm-rework commit, which replaced the whole `IAlarmRpcDispatcher` trio with `GatewayAlarmMonitor` / `IGatewayAlarmService`. A repo-wide grep for the prose this finding removed ("not yet wired", "PR A.6/A.7", "worker-pending", "dev-rig") returns nothing in Server source, so the correction is moot rather than lost. Nothing to re-fix. ### Server-012 @@ -409,7 +409,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Update CLAUDE.md's scope list and the `apikey` example to the canonical `*:*` scope strings, per CLAUDE.md's own rule that docs change with the code. -**Resolution:** Resolved 2026-05-18. Confirmed against `GatewayScopes` (`session:open`, `session:close`, `invoke:read`, `invoke:write`, `invoke:secure`, `events:read`, `metadata:read`, `admin`). CLAUDE.md's Build/Test/Run `apikey create` example and the Authentication-section scope list were both updated to the canonical `*:*` strings. (Note: since finding Server-004 was resolved, the old example would now be actively rejected at create time rather than silently creating an unusable key, making the doc correction load-bearing.) Pure documentation change; no test. +**Resolution:** Resolved 2026-05-18. Confirmed against `GatewayScopes` (`session:open`, `session:close`, `invoke:read`, `invoke:write`, `invoke:secure`, `events:read`, `metadata:read`, `admin`). CLAUDE.md's Build/Test/Run `apikey create` example and the Authentication-section scope list were both updated to the canonical `*:*` strings. (Note: since finding Server-004 was resolved, the old example would now be actively rejected at create time rather than silently creating an unusable key, making the doc correction load-bearing.) Pure documentation change; no test. Regression re-fixed 2026-08-18 on feat/followup-closeout (a5f843c, f2a422b); scope lists re-verified present. (Both CLAUDE.md sites confirmed on feat/followups-tickets: the Build/Test/Run `apikey create-key` example carries the full canonical `session:open,session:close,invoke:read,invoke:write,invoke:secure,events:read,metadata:read,admin` set, and the Authentication-section list names the same eight strings.) ### Server-013 @@ -439,7 +439,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Update the `AcknowledgeAlarm`/`QueryActiveAlarms` remarks to reflect that `WorkerAlarmRpcDispatcher` is the wired default, and describe its actual GUID-vs-`Provider!Group.Tag` handling. -**Resolution:** Resolved 2026-05-18. Confirmed against source: `SessionServiceCollectionExtensions` registers `WorkerAlarmRpcDispatcher` as `IAlarmRpcDispatcher`, so the "not yet wired" / "empty stream until PR A.2" / "PR A.6/A.7 follow-up" prose in the `AcknowledgeAlarm` and `QueryActiveAlarms` `` and inline comments was stale. Rewrote both `` blocks and both inline comments to state that DI binds the production `WorkerAlarmRpcDispatcher`, that it routes over the worker pipe IPC, and that `AcknowledgeAlarm` handles a canonical-GUID reference (→ `AcknowledgeAlarmCommand`) and a `Provider!Group.Tag` reference (→ `AcknowledgeAlarmByNameCommand`), with `NotWiredAlarmRpcDispatcher` being only the null fallback. The matching stale `WorkerAlarmRpcDispatcher` class-level XML doc was corrected as part of Server-011. Pure documentation/comment change; no test. +**Resolution:** Resolved 2026-05-18. Confirmed against source: `SessionServiceCollectionExtensions` registers `WorkerAlarmRpcDispatcher` as `IAlarmRpcDispatcher`, so the "not yet wired" / "empty stream until PR A.2" / "PR A.6/A.7 follow-up" prose in the `AcknowledgeAlarm` and `QueryActiveAlarms` `` and inline comments was stale. Rewrote both `` blocks and both inline comments to state that DI binds the production `WorkerAlarmRpcDispatcher`, that it routes over the worker pipe IPC, and that `AcknowledgeAlarm` handles a canonical-GUID reference (→ `AcknowledgeAlarmCommand`) and a `Provider!Group.Tag` reference (→ `AcknowledgeAlarmByNameCommand`), with `NotWiredAlarmRpcDispatcher` being only the null fallback. The matching stale `WorkerAlarmRpcDispatcher` class-level XML doc was corrected as part of Server-011. Pure documentation/comment change; no test. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but superseded — `MxAccessGatewayService.AcknowledgeAlarm` / `QueryActiveAlarms` still exist but were rewritten onto `IGatewayAlarmService` (the `IAlarmRpcDispatcher` seam is gone), and both now carry a bare `` rather than the rewritten ``. The stale prose this finding removed is absent repo-wide, so there is nothing to re-fix; the rewritten remarks describing a now-deleted dispatcher would be wrong if restored. ### Server-015 @@ -454,7 +454,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Make `CloseAsync` mutate `_state` through the existing `TransitionTo(...)` helper (or acquire `_syncRoot` around the reads/writes) so all `_state` access uses the same lock. Either extend `TransitionTo` to accept the `Closing` and `Closed` transitions (it already handles `Faulted`/`Closed` precedence) or refactor `CloseAsync` to call a private `TrySetClosing()` / `MarkClosed()` that locks `_syncRoot`. Add a regression test that forces a `TransitionTo(Ready)` after `CloseAsync` has set `Closing` and asserts the session does not flip back to `Ready`. -**Resolution:** 2026-05-20 — Unified the close path on `_syncRoot`. `GatewaySession.CloseAsync` (`src/MxGateway.Server/Sessions/GatewaySession.cs`) now mutates `_state` only through two private `_syncRoot`-locked helpers — `TryBeginClose` (writes `Closing`, returns the prior `_closeStarted`) and `MarkClosed` (writes `Closed`) — so every `_state` read/write in the session uses the same lock; `_closeLock` keeps its role of serializing concurrent close attempts. `TransitionTo` was tightened to refuse a transition out of `Closing` to anything other than `Closed`/`Faulted` so a late lifecycle callback cannot walk a closing session back to `Ready`. `docs/Sessions.md` updated to describe the unified lock discipline and the extended terminal precedence. Regression tests in `src/MxGateway.Tests/Gateway/Sessions/GatewaySessionTests.cs`: `TransitionTo_AfterCloseStarted_DoesNotOverwriteClosing` (the named scenario — `BlockingShutdownWorkerClient` parks the close inside `worker.ShutdownAsync` so the test can call `TransitionTo(Ready)` between the `Closing` and `Closed` writes and assert the state stays `Closing`) and `MarkFaulted_AfterCloseCompletes_DoesNotResurrectSession`. +**Resolution:** 2026-05-20 — Unified the close path on `_syncRoot`. `GatewaySession.CloseAsync` (`src/MxGateway.Server/Sessions/GatewaySession.cs`) now mutates `_state` only through two private `_syncRoot`-locked helpers — `TryBeginClose` (writes `Closing`, returns the prior `_closeStarted`) and `MarkClosed` (writes `Closed`) — so every `_state` read/write in the session uses the same lock; `_closeLock` keeps its role of serializing concurrent close attempts. `TransitionTo` was tightened to refuse a transition out of `Closing` to anything other than `Closed`/`Faulted` so a late lifecycle callback cannot walk a closing session back to `Ready`. `docs/Sessions.md` updated to describe the unified lock discipline and the extended terminal precedence. Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; `docs/Sessions.md` still states that both close-related writes go through `_syncRoot` via `TryBeginClose`/`MarkClosed` and that `_closeLock` only serializes concurrent close attempts. Regression tests in `src/MxGateway.Tests/Gateway/Sessions/GatewaySessionTests.cs`: `TransitionTo_AfterCloseStarted_DoesNotOverwriteClosing` (the named scenario — `BlockingShutdownWorkerClient` parks the close inside `worker.ShutdownAsync` so the test can call `TransitionTo(Ready)` between the `Closing` and `Closed` writes and assert the state stays `Closing`) and `MarkFaulted_AfterCloseCompletes_DoesNotResurrectSession`. ### Server-016 @@ -484,7 +484,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Add explicit arms to `ResolveRequiredScope`: map `AcknowledgeAlarmRequest` to `GatewayScopes.InvokeWrite` (parity with other write actions; ack changes alarm state) and `QueryActiveAlarmsRequest` to `GatewayScopes.MetadataRead` or `GatewayScopes.InvokeRead`. Update `docs/Authorization.md` to list both. Extend `GatewayGrpcScopeResolverTests` with the new mappings and an assertion that every request type defined by `mxaccess_gateway.proto` is named in the resolver (the test can enumerate the assembly's request types so a future RPC cannot quietly add itself only via the admin fallback). -**Resolution:** 2026-05-20 — Added explicit `AcknowledgeAlarmRequest => GatewayScopes.InvokeWrite` and `QueryActiveAlarmsRequest => GatewayScopes.EventsRead` arms to `GatewayGrpcScopeResolver.ResolveRequiredScope` (`src/MxGateway.Server/Security/Authorization/GatewayGrpcScopeResolver.cs:21-22`). `InvokeWrite` matches the existing `MxCommandKind.Write*` mapping because ack mutates alarm state; `EventsRead` matches `StreamEventsRequest` and `MxCommandKind.DrainEvents` because querying active alarms reads the same alarm/event surface. Extended `GatewayGrpcScopeResolverTests` with two new `InlineData` rows covering both request types (`src/MxGateway.Tests/Security/Authorization/GatewayGrpcScopeResolverTests.cs:16-17`) and added four interceptor-level cases in `GatewayGrpcAuthorizationInterceptorTests` (`UnaryServerHandler_AcknowledgeAlarmMissingScope_ReturnsPermissionDenied`, `UnaryServerHandler_AcknowledgeAlarmWithScope_RunsHandler`, `ServerStreamingServerHandler_QueryActiveAlarmsMissingScope_ReturnsPermissionDenied`, `ServerStreamingServerHandler_QueryActiveAlarmsWithScope_RunsHandler`) proving each new RPC denies callers lacking the chosen scope and runs the handler when the scope is held. Updated `docs/Authorization.md` (resolver snippet and Scope Catalog table) to list both RPCs against their scopes. `dotnet test ... --filter FullyQualifiedName~GatewayGrpcAuthorizationInterceptorTests` → 14 passed, 0 failed; resolver tests 28 passed, 0 failed. +**Resolution:** 2026-05-20 — Added explicit `AcknowledgeAlarmRequest => GatewayScopes.InvokeWrite` and `QueryActiveAlarmsRequest => GatewayScopes.EventsRead` arms to `GatewayGrpcScopeResolver.ResolveRequiredScope` (`src/MxGateway.Server/Security/Authorization/GatewayGrpcScopeResolver.cs:21-22`). `InvokeWrite` matches the existing `MxCommandKind.Write*` mapping because ack mutates alarm state; `EventsRead` matches `StreamEventsRequest` and `MxCommandKind.DrainEvents` because querying active alarms reads the same alarm/event surface. Extended `GatewayGrpcScopeResolverTests` with two new `InlineData` rows covering both request types (`src/MxGateway.Tests/Security/Authorization/GatewayGrpcScopeResolverTests.cs:16-17`) and added four interceptor-level cases in `GatewayGrpcAuthorizationInterceptorTests` (`UnaryServerHandler_AcknowledgeAlarmMissingScope_ReturnsPermissionDenied`, `UnaryServerHandler_AcknowledgeAlarmWithScope_RunsHandler`, `ServerStreamingServerHandler_QueryActiveAlarmsMissingScope_ReturnsPermissionDenied`, `ServerStreamingServerHandler_QueryActiveAlarmsWithScope_RunsHandler`) proving each new RPC denies callers lacking the chosen scope and runs the handler when the scope is held. Updated `docs/Authorization.md` (resolver snippet and Scope Catalog table) to list both RPCs against their scopes. Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; the snippet still shows `AcknowledgeAlarmRequest => GatewayScopes.InvokeWrite` / `QueryActiveAlarmsRequest => GatewayScopes.EventsRead` and both RPCs still appear in the Scope Catalog rows. `dotnet test ... --filter FullyQualifiedName~GatewayGrpcAuthorizationInterceptorTests` → 14 passed, 0 failed; resolver tests 28 passed, 0 failed. ### Server-018 @@ -559,7 +559,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Rewrite the `IAlarmRpcDispatcher` `` block to match the language now used on `WorkerAlarmRpcDispatcher` and on the gRPC service: DI binds `WorkerAlarmRpcDispatcher` by default; `NotWiredAlarmRpcDispatcher` is only the null fallback for tests/DI omission. Drop the "PR A.6 / A.7" prefix from the `` — the interface is now the public alarm-RPC seam. -**Resolution:** 2026-05-20 — Rewrote `IAlarmRpcDispatcher`'s `` and `` (`src/MxGateway.Server/Sessions/IAlarmRpcDispatcher.cs`) to match the language now used on `WorkerAlarmRpcDispatcher` and on `MxAccessGatewayService.AcknowledgeAlarm` / `QueryActiveAlarms`: dropped the stale "PR A.6 / A.7" prefix from the summary, and replaced the "this PR ships a not-yet-wired default that returns a clear worker-pending diagnostic" clause with the correct statement that DI binds the production `WorkerAlarmRpcDispatcher` by default and `NotWiredAlarmRpcDispatcher` is only the null fallback for DI omission / standalone tests. Pure documentation change; no test. +**Resolution:** 2026-05-20 — Rewrote `IAlarmRpcDispatcher`'s `` and `` (`src/MxGateway.Server/Sessions/IAlarmRpcDispatcher.cs`) to match the language now used on `WorkerAlarmRpcDispatcher` and on `MxAccessGatewayService.AcknowledgeAlarm` / `QueryActiveAlarms`: dropped the stale "PR A.6 / A.7" prefix from the summary, and replaced the "this PR ships a not-yet-wired default that returns a clear worker-pending diagnostic" clause with the correct statement that DI binds the production `WorkerAlarmRpcDispatcher` by default and `NotWiredAlarmRpcDispatcher` is only the null fallback for DI omission / standalone tests. Pure documentation change; no test. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but no longer verifiable in place — `IAlarmRpcDispatcher.cs` was deleted with the rest of the dispatcher trio in `dc9c0c9`. No stale "PR A.6/A.7" or "not-yet-wired" prose survives anywhere in Server source. Nothing to re-fix. ### Server-023 @@ -574,7 +574,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Replace the `` and `` on `NotWiredAlarmRpcDispatcher` with text that matches the language now used on the interface and `WorkerAlarmRpcDispatcher` — "null fallback `IAlarmRpcDispatcher` used when no dispatcher is registered (DI omission / standalone tests); production wires `WorkerAlarmRpcDispatcher`." Either drop the `AcknowledgeAsync` diagnostic string's dev-rig framing entirely or shorten it to "alarm dispatcher is not registered." `#pragma warning disable CS1998` on `QueryActiveAlarmsAsync` is correct here (empty stream is intentional for the null fallback) and should stay. -**Resolution:** 2026-05-20 — Rewrote `NotWiredAlarmRpcDispatcher` summary/remarks as the null-fallback dispatcher and shortened the `AcknowledgeAsync` diagnostic to "Alarm dispatcher is not registered."; updated the two tests that asserted the old "worker"-prefixed diagnostic. +**Resolution:** 2026-05-20 — Rewrote `NotWiredAlarmRpcDispatcher` summary/remarks as the null-fallback dispatcher and shortened the `AcknowledgeAsync` diagnostic to "Alarm dispatcher is not registered."; updated the two tests that asserted the old "worker"-prefixed diagnostic. Re-verified 2026-08-18 (feat/followups-tickets): not regressed but no longer verifiable in place — `NotWiredAlarmRpcDispatcher.cs` and its tests were deleted with the dispatcher trio in `dc9c0c9`. Nothing to re-fix. ### Server-024 @@ -637,7 +637,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Update the `ResolveCommandScope` snippet to include the four bulk-write arms. Update the Constraint Enforcement prose to enumerate the bulk read/write commands that are actually filtered, and reference the per-entry index-ordered merge that `BulkConstraintPlan.MergeDeniedInto` performs. Adding `ReadBulk` to the `InvokeRead` row of the Scope Catalog would also be useful — the table currently lists `Register`/`AddItem`/`Advise` against `InvokeRead` but not `ReadBulk`. -**Resolution:** 2026-05-20 — Updated the `ResolveCommandScope` snippet in `docs/Authorization.md` to enumerate the four bulk-write arms (`WriteBulk`/`Write2Bulk` against `InvokeWrite`, `WriteSecuredBulk`/`WriteSecured2Bulk` against `InvokeSecure`); expanded the Constraint Enforcement prose to list `ReadBulk` and all four bulk-write commands and to call out `BulkConstraintPlan.MergeDeniedInto`'s index-ordered merge; added `ReadBulk` to the `InvokeRead` row of the Scope Catalog. +**Resolution:** 2026-05-20 — Updated the `ResolveCommandScope` snippet in `docs/Authorization.md` to enumerate the four bulk-write arms (`WriteBulk`/`Write2Bulk` against `InvokeWrite`, `WriteSecuredBulk`/`WriteSecured2Bulk` against `InvokeSecure`); expanded the Constraint Enforcement prose to list `ReadBulk` and all four bulk-write commands and to call out `BulkConstraintPlan.MergeDeniedInto`'s index-ordered merge; added `ReadBulk` to the `InvokeRead` row of the Scope Catalog. Re-verified present 2026-08-18 (feat/followups-tickets) — all three corrections still stand in `docs/Authorization.md`: the snippet carries the four bulk-write arms, the Constraint Enforcement prose names `ReadBulk` plus the four bulk-write commands and `BulkConstraintPlan.MergeDeniedInto`, and `ReadBulk` is in the `InvokeRead` catalog row. ### Server-028 @@ -670,7 +670,7 @@ closed. New findings filed against this pass: Server-051..053. **Recommendation:** Either (a) extend the advertised list with `bulk-read-command` and `bulk-write-commands` (`WriteBulk` / `Write2Bulk` / `WriteSecuredBulk` / `WriteSecured2Bulk` collectively), or (b) document in `gateway.md` and `docs/Contracts.md` that `Capabilities` is informational only and not the contract version. Option (a) is the simplest forward-compatible fix and keeps the capability token shape clients are already familiar with. -**Resolution:** 2026-05-20 — Extended the `OpenSession` capabilities list with `bulk-read-commands` and `bulk-write-commands` alongside the existing `bulk-subscribe-commands` token, so clients that gate on capability strings have an explicit signal for the bulk-read and bulk-write families. +**Resolution:** 2026-05-20 — Extended the `OpenSession` capabilities list with `bulk-read-commands` and `bulk-write-commands` alongside the existing `bulk-subscribe-commands` token, so clients that gate on capability strings have an explicit signal for the bulk-read and bulk-write families. Re-verified present 2026-08-18 (feat/followups-tickets) — `MxAccessGatewayService.OpenSession` still adds all three tokens. ### Server-030 @@ -839,7 +839,7 @@ Add a regression test that advises N items without an active `StreamEvents` cons **Recommendation:** Before the EventsHub is exercised by Admin-only sessions or session-scoped Viewer roles, gate `SubscribeSession` on a session-access check — either via a per-session role check in the hub method itself, or by storing a per-user allowed-session-id set in the connection's `Context.Items` at connect time and rejecting subscribes outside that set. The current dashboard surfaces only a per-page Session Details view that the page can prove it's authorized for, but as soon as a Viewer role exists the gap matters. -**Resolution:** 2026-05-24 — Documented the v1 acceptance per the prompt's "practical fix for v1" direction. Added a detailed `` block to `EventsHub.SubscribeSession` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs`) stating that (a) in v1 the hub-level `HubClientsPolicy` only requires one of the dashboard roles (Admin or Viewer) and both may subscribe to any session id, (b) this is acceptable today because the dashboard's per-session views show non-secret session metadata any authenticated user can already see and value logging is gated by the same redaction policy, and (c) the per-session ACL that gates the gRPC `StreamEvents` RPC is intentionally not yet mirrored here. Added an explicit `TODO(per-session-acl)` describing the future enforcement seam — once a role/scope is introduced that scopes a Viewer to a specific session or tenant, add a session-access check at this method (inline on `Context.User` claims/`Context.Items`, or via a dedicated authorization policy applied to the hub method). No code-behavior change in this pass; the per-session ACL data model design is out of scope for the resolution window. No new regression test (the change is documentation-only). +**Resolution:** 2026-05-24 — Documented the v1 acceptance per the prompt's "practical fix for v1" direction. Added a detailed `` block to `EventsHub.SubscribeSession` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHub.cs`) stating that (a) in v1 the hub-level `HubClientsPolicy` only requires one of the dashboard roles (Admin or Viewer) and both may subscribe to any session id, (b) this is acceptable today because the dashboard's per-session views show non-secret session metadata any authenticated user can already see and value logging is gated by the same redaction policy, and (c) the per-session ACL that gates the gRPC `StreamEvents` RPC is intentionally not yet mirrored here. Added an explicit `TODO(per-session-acl)` describing the future enforcement seam — once a role/scope is introduced that scopes a Viewer to a specific session or tenant, add a session-access check at this method (inline on `Context.User` claims/`Context.Items`, or via a dedicated authorization policy applied to the hub method). No code-behavior change in this pass; the per-session ACL data model design is out of scope for the resolution window. No new regression test (the change is documentation-only). Re-verified 2026-08-18 (feat/followups-tickets): the documented v1 gap was subsequently **closed**, not regressed — `EventsHub` now takes `IDashboardSessionAcl` and `SubscribeSession` throws `HubException("Not authorized for this session.")` when `CanViewSession` denies (SEC-25 / TST-15). The `TODO(per-session-acl)` is correctly gone and the `` block now describes the shipped tag-scoped gate (admin bypass first, tag intersection, `Dashboard:UntaggedSessionVisibility`, phantom-id denial for non-Admins). Nothing to re-fix; the finding's own recommendation is what shipped. ### Server-039 @@ -869,7 +869,7 @@ Add a regression test that advises N items without an active `StreamEvents` cons **Recommendation:** Add a one-line comment above the loop explaining the precedence: full DN/CN literal first, leading-RDN fallback second. Mention the case-insensitive map comparer (`OrdinalIgnoreCase`) so the next reader doesn't ask why `"GwAdmin"` matches `"gwadmin"`. -**Resolution:** 2026-05-24 — Added a precedence comment block above the lookup in `MapGroupsToRoles` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs:156-163`) explaining that the full literal group string is tried first and the leading-RDN value (e.g. `GwAdmin` extracted from `ou=GwAdmin,ou=groups,...`) is the fallback, and back-referencing `DashboardOptions.GroupToRole` as the source of the `OrdinalIgnoreCase` comparer so a maintainer sees why `"GwAdmin"` matches `"gwadmin"`. No code change — existing `DashboardAuthenticatorTests.MapGroupsToRoles_ResolvesByShortNameAndDistinguishedName` already pins both the full-match and RDN-fallback paths and the case-insensitive lookup; pure documentation-only resolution, no new test. +**Resolution:** 2026-05-24 — Added a precedence comment block above the lookup in `MapGroupsToRoles` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthenticator.cs:156-163`) explaining that the full literal group string is tried first and the leading-RDN value (e.g. `GwAdmin` extracted from `ou=GwAdmin,ou=groups,...`) is the fallback, and back-referencing `DashboardOptions.GroupToRole` as the source of the `OrdinalIgnoreCase` comparer so a maintainer sees why `"GwAdmin"` matches `"gwadmin"`. No code change — existing `DashboardAuthenticatorTests.MapGroupsToRoles_ResolvesByShortNameAndDistinguishedName` already pins both the full-match and RDN-fallback paths and the case-insensitive lookup; pure documentation-only resolution, no new test. Regressed or never applied; re-fixed 2026-08-18 in `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs` (feat/followups-tickets). Chain of events: the lookup moved out of `DashboardAuthenticator` into the shared `DashboardGroupRoleMapping` helper in `792e3f9` (the `IGroupRoleMapper` seam) **with the comment intact**, then `fca978d` ("docs(src): add missing XML docs and strip tracking-ID comments") deleted the entire block rather than just the `(Server-040)` marker inside it. That also took out a second, substantive paragraph added later — the note that the shared `ZB.MOM.WW.Auth.Ldap` provider already strips groups to short RDN names, making the RDN fallback a no-op on the live login path and making a full-DN `GroupToRole` **key** unsupported. Both paragraphs are restored, minus the tracking IDs (that part of the sweep's intent is respected). ### Server-041 @@ -914,7 +914,7 @@ Add a regression test that advises N items without an active `StreamEvents` cons **Recommendation:** Add a `` block to `HubTokenService` noting "Registered as a singleton in `AddGatewayDashboard`; the underlying `ITimeLimitedDataProtector` is thread-safe and shared across hub-token issuance and validation." Optionally add a comment near the DI registration explaining the lifetime contract. -**Resolution:** 2026-05-24 — Added a `` block to `HubTokenService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/HubTokenService.cs`) documenting that the service is registered as a singleton in `DashboardServiceCollectionExtensions.AddGatewayDashboard` and is shared by two consumer scopes — `DashboardHubConnectionFactory` (scoped, per-circuit; calls `Issue` from the cookie-authenticated dashboard) and `HubTokenAuthenticationHandler` (transient, per-request; calls `Validate` from the SignalR negotiate / connection path). Notes that the underlying `ITimeLimitedDataProtector` is thread-safe so concurrent mint/validate from any number of callers is safe, and explicitly asks future maintainers to preserve the singleton lifetime to keep the protector instance stable. Pure documentation change; no test. +**Resolution:** 2026-05-24 — Added a `` block to `HubTokenService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/HubTokenService.cs`) documenting that the service is registered as a singleton in `DashboardServiceCollectionExtensions.AddGatewayDashboard` and is shared by two consumer scopes — `DashboardHubConnectionFactory` (scoped, per-circuit; calls `Issue` from the cookie-authenticated dashboard) and `HubTokenAuthenticationHandler` (transient, per-request; calls `Validate` from the SignalR negotiate / connection path). Notes that the underlying `ITimeLimitedDataProtector` is thread-safe so concurrent mint/validate from any number of callers is safe, and explicitly asks future maintainers to preserve the singleton lifetime to keep the protector instance stable. Pure documentation change; no test. Re-verified present 2026-08-18 (feat/followups-tickets) — the `` block still names the `AddGatewayDashboard` singleton registration, the two consumer scopes, the thread-safe `ITimeLimitedDataProtector`, and the preserve-the-lifetime request. One consumer was renamed since (the issuing side is now the `/hubs/token` endpoint rather than `DashboardHubConnectionFactory`) and the doc tracked that change correctly. ## Re-review 2026-05-24 (commit 42b0037) @@ -983,7 +983,7 @@ The user-visible difference: rotating/revoking/deleting a key vs closing/killing **Recommendation:** Align `ApiKeysPage.ConfirmPendingAsync` with the sessions pages: hold `PendingAction`, set `IsBusy = true`, run the action, then clear `PendingAction` in the `finally`. The current ApiKeysPage shape was inherited from before the dialog existed (when the confirmation was a `confirm()` JS call); the dialog component change can flatten the difference now. As a smaller alternative, document the divergence on the component's XML doc — but the shared component should ideally be used consistently. -**Resolution:** 2026-05-24 — Took the recommended alignment. `ApiKeysPage.ConfirmPendingAsync` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor`) now holds `PendingAction` for the duration of the awaited action (so the shared `ConfirmDialog` renders its `IsBusy` in-flight state on the dialog itself, matching the sessions pages) and clears it in `finally` regardless of outcome. The action is captured up front so a clear in `finally` works even when the action throws. `RunManagementActionAsync` continues to drive `IsBusy = true` inside its own `try/finally`, so the dialog now correctly disables Confirm/Cancel while the awaited service call runs. Pure UX-consistency change; no new automated test (no bUnit harness in the test project — same precedent as Server-010). +**Resolution:** 2026-05-24 — Took the recommended alignment. `ApiKeysPage.ConfirmPendingAsync` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor`) now holds `PendingAction` for the duration of the awaited action (so the shared `ConfirmDialog` renders its `IsBusy` in-flight state on the dialog itself, matching the sessions pages) and clears it in `finally` regardless of outcome. The action is captured up front so a clear in `finally` works even when the action throws. `RunManagementActionAsync` continues to drive `IsBusy = true` inside its own `try/finally`, so the dialog now correctly disables Confirm/Cancel while the awaited service call runs. Pure UX-consistency change; no new automated test (no bUnit harness in the test project — same precedent as Server-010). Re-verified present 2026-08-18 (feat/followups-tickets) — `ApiKeysPage.ConfirmPendingAsync` still captures the action up front, awaits inside `try`, and clears `PendingAction` in `finally`; the explanatory comment survives. ### Server-048 @@ -1017,7 +1017,7 @@ The user-visible difference: rotating/revoking/deleting a key vs closing/killing **Recommendation:** Add `` blocks to `IDashboardSessionAdminService.CanManage` (states the Admin-role gate), `CloseSessionAsync` and `KillWorkerAsync` (state that missing sessions return `DashboardSessionAdminResult.Fail(...)` rather than throwing, and that the audit log captures actor + remote IP). Add `` and `` for the request/response shape. The same sweep can pick up the longstanding gap on `IDashboardApiKeyManagementService` if the team wants — but the new file is the load-bearing one. -**Resolution:** 2026-05-24 — Added `` + `` blocks to every member of `IDashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardSessionAdminService.cs`): an interface-level `` describing the Admin-role gate, audit log shape, and `DashboardSessionAdminResult.Fail` semantics; per-member docs on `CanManage`, `CloseSessionAsync`, and `KillWorkerAsync` calling out the missing-session-returns-Fail contract and the `dashboard-admin-kill` reason constant that reaches the worker-kill audit log and `mxgateway.workers.killed` counter tag. `DashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs`) picked up a class-level `` + `` describing the per-page audit-log seam, plus `` on each public method. Pure documentation change; no test (the behavioral contracts the docs describe are already exercised by the existing `DashboardSessionAdminServiceTests` cases). +**Resolution:** 2026-05-24 — Added `` + `` blocks to every member of `IDashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardSessionAdminService.cs`): an interface-level `` describing the Admin-role gate, audit log shape, and `DashboardSessionAdminResult.Fail` semantics; per-member docs on `CanManage`, `CloseSessionAsync`, and `KillWorkerAsync` calling out the missing-session-returns-Fail contract and the `dashboard-admin-kill` reason constant that reaches the worker-kill audit log and `mxgateway.workers.killed` counter tag. `DashboardSessionAdminService` (`src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs`) picked up a class-level `` + `` describing the per-page audit-log seam, plus `` on each public method. Pure documentation change; no test (the behavioral contracts the docs describe are already exercised by the existing `DashboardSessionAdminServiceTests` cases). Re-verified present 2026-08-18 (feat/followups-tickets) — the interface-level `` and all three per-member blocks are intact, including the `dashboard-admin-kill` reason constant and the missing-session-returns-`Fail` contract. ### Server-050 @@ -1078,7 +1078,7 @@ catch (Exception ex) { _logger.LogWarning(ex, "...continuing with configuration- **Recommendation:** For (1), align the `IAlarmWatchListResolver` doc with whatever Server-051 settles on. For (2), either restrict the exclude to GR-discovered rows (apply `RemoveAll` before appending the `IncludeAttributes` entries) or update the option XML doc and `GatewayConfiguration.md` to say excludes are applied to the merged GR-plus-include list and therefore also suppress matching explicit includes. -**Resolution:** Resolved 2026-06-15. (1) No longer over-promises: the Server-051 fix makes the implementation propagate `OperationCanceledException`, so the `IAlarmWatchListResolver.ResolveAsync` `` doc is now accurate and was left unchanged. (2) Kept the "excludes win" code behaviour (excludes applied to the merged GR-plus-include list) and corrected the prose to match: `AlarmDiscoveryOptions.ExcludeAttributes` XML doc and `docs/GatewayConfiguration.md:247` now state the exclude runs after the GR rows and explicit `IncludeAttributes` are combined, so an exclude matching an explicit include suppresses it too. The "excludes win" precedence is pinned by `AlarmWatchListResolverTests.ResolveAsync_ExcludeAlsoSuppressesMatchingExplicitInclude`. +**Resolution:** Resolved 2026-06-15. (1) No longer over-promises: the Server-051 fix makes the implementation propagate `OperationCanceledException`, so the `IAlarmWatchListResolver.ResolveAsync` `` doc is now accurate and was left unchanged. (2) Kept the "excludes win" code behaviour (excludes applied to the merged GR-plus-include list) and corrected the prose to match: `AlarmDiscoveryOptions.ExcludeAttributes` XML doc and `docs/GatewayConfiguration.md:247` now state the exclude runs after the GR rows and explicit `IncludeAttributes` are combined, so an exclude matching an explicit include suppresses it too. The "excludes win" precedence is pinned by `AlarmWatchListResolverTests.ResolveAsync_ExcludeAlsoSuppressesMatchingExplicitInclude`. Re-verified present 2026-08-18 (feat/followups-tickets) — both doc corrections stand: `IAlarmWatchListResolver.ResolveAsync`'s `` still documents cancellation propagation (and the implementation still honors it), and the "excludes win over explicit includes" wording is present on `AlarmDiscoveryOptions.ExcludeAttributes` (now in `Configuration/AlarmFallbackOptions.cs`) and in the `docs/GatewayConfiguration.md` option table. ### Server-053 @@ -1113,7 +1113,7 @@ Additionally, `GatewayAlarmMonitor.ApplyProviderModeChangeAsync` increments the **Recommendation:** Update both `DesignDecisions.md` sections and the revisit list to describe the shipped behavior (gated by `AllowMultipleEventSubscribers`, `DetachGraceSeconds`, replay options), and amend the CLAUDE.md convention bullet. -**Resolution:** 2026-06-16: updated `docs/DesignDecisions.md` (Session Reconnect section rewritten to describe the shipped detach-grace + replay-on-reconnect behavior with config references; Event Subscribers section rewritten to describe the config-gated multi-subscriber fan-out, mode-dependent `FailFast` semantics, and internal vs external subscriber distinction; Later Revisit Items list removes the two shipped items and records them as shipped with config cross-references) and the `CLAUDE.md` conventions bullet to describe the shipped config-gated multi-subscriber + reconnect-replay behavior while preserving the one-worker-per-session invariant. +**Resolution:** 2026-06-16: updated `docs/DesignDecisions.md` (Session Reconnect section rewritten to describe the shipped detach-grace + replay-on-reconnect behavior with config references; Event Subscribers section rewritten to describe the config-gated multi-subscriber fan-out, mode-dependent `FailFast` semantics, and internal vs external subscriber distinction; Later Revisit Items list removes the two shipped items and records them as shipped with config cross-references) and the `CLAUDE.md` conventions bullet to describe the shipped config-gated multi-subscriber + reconnect-replay behavior while preserving the one-worker-per-session invariant. Re-verified present 2026-08-18 (feat/followups-tickets) — `docs/DesignDecisions.md` still records the "no reconnectable sessions" constraint as superseded and documents `DetachGraceSeconds` / `AllowMultipleEventSubscribers` / `ReplayBufferCapacity`, the Later Revisit Items list still cross-references them as shipped, and the `CLAUDE.md` bullet still carries the config-gated wording. ### Server-055 @@ -1158,7 +1158,7 @@ Additionally, `GatewayAlarmMonitor.ApplyProviderModeChangeAsync` increments the **Recommendation:** Either (a) extend `NormalizeOutboundCommand` and the `MapCommand` tracking path to normalize each `AddItemBulk.TagAddresses` entry (and `AddBufferedItem.ItemDefinition`) the same `IsArray`-gated way, keeping the constraint check, the worker bind, and the stored `SessionItemRegistration.TagAddress` consistent; or (b) if bulk-add normalization is intentionally out of scope for this feature, state that explicitly in `gateway.md` and the client READMEs (alongside the existing `ReadBulk` carve-out) so clients know bulk-added array handles must carry the `[]` suffix themselves to be writable. -**Resolution:** 2026-06-18 — Took option (a). Root cause confirmed: `NormalizeOutboundCommand` had no `AddItemBulk`/`AddBufferedItem` case, so the worker bound bare (non-write-capable) array handles for those paths while single-add was already fixed. Added `AddItemBulk` (normalizes each `TagAddresses` entry in place) and `AddBufferedItem` (normalizes `ItemDefinition`) cases to `NormalizeOutboundCommand`; added the matching `AddBufferedItem` normalization to the `TrackCommandReply`/`MapCommand` tracking path (its registration keys off the command's `ItemDefinition`). `AddItemBulk` tracking needs no change — the worker echoes the already-suffixed address back in each `SubscribeResult.TagAddress`, which `TrackBulkItems` stores. Authz is unchanged and consistent: `FilterTagBulkAsync` checks the bare address through `ConstraintEnforcer.ResolveTarget`'s `[]` fallback, mirroring single-add. Updated `gateway.md` and all five client READMEs (dotnet/go/python/rust/java) so the add-family normalization no longer reads as AddItem-only; the `ReadBulk` carve-out stays. Regression tests: `GatewayArrayWriteWiringTests.AddItemBulk_BareArrayAddress_NormalizedOnWireAndInRegistration`, `.AddBufferedItem_BareArrayAddress_NormalizedOnWireAndInRegistration`. +**Resolution:** 2026-06-18 — Took option (a). Root cause confirmed: `NormalizeOutboundCommand` had no `AddItemBulk`/`AddBufferedItem` case, so the worker bound bare (non-write-capable) array handles for those paths while single-add was already fixed. Added `AddItemBulk` (normalizes each `TagAddresses` entry in place) and `AddBufferedItem` (normalizes `ItemDefinition`) cases to `NormalizeOutboundCommand`; added the matching `AddBufferedItem` normalization to the `TrackCommandReply`/`MapCommand` tracking path (its registration keys off the command's `ItemDefinition`). `AddItemBulk` tracking needs no change — the worker echoes the already-suffixed address back in each `SubscribeResult.TagAddress`, which `TrackBulkItems` stores. Authz is unchanged and consistent: `FilterTagBulkAsync` checks the bare address through `ConstraintEnforcer.ResolveTarget`'s `[]` fallback, mirroring single-add. Updated `gateway.md` and all five client READMEs (dotnet/go/python/rust/java) so the add-family normalization no longer reads as AddItem-only; the `ReadBulk` carve-out stays. Regression tests: `GatewayArrayWriteWiringTests.AddItemBulk_BareArrayAddress_NormalizedOnWireAndInRegistration`, `.AddBufferedItem_BareArrayAddress_NormalizedOnWireAndInRegistration`. Re-verified present 2026-08-18 (feat/followups-tickets) — doc sub-claim only; `gateway.md` still describes normalization across the whole add family (`AddItem`/`AddItem2`/`AddItemBulk`/`AddBufferedItem`) and all five client READMEs still name the bulk and buffered members (Python in its snake_case form: `add_item_bulk` / `add_buffered_item`). ### Server-058 diff --git a/docs/Authentication.md b/docs/Authentication.md index 7544c71..bdae026 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -175,6 +175,17 @@ optional serialized constraints, and the `created_utc`, `last_used_utc`, belong to `ZB.MOM.WW.Auth.ApiKeys`, this document does not restate their column readers or SQL; consult the library for that detail. +One connection-level behavior is worth stating here because it is what keeps the +store usable under concurrency rather than an implementation detail of the schema: +the library's `AuthSqliteConnectionFactory.OpenConnectionAsync` opens pooled +connections with a non-zero command timeout and applies `PRAGMA journal_mode=WAL` +and `PRAGMA busy_timeout` (5 s). Last-used stamping runs on every authenticated +request and the audit store appends on every denial, so without WAL and a busy +timeout those concurrent writers would surface `SQLITE_BUSY` as a hard failure on +the request path instead of degrading gracefully. WAL is a persistent +database-level setting, so re-applying it per connection is a cheap no-op; +`busy_timeout` is per-connection state and must be set each time. + ### Audit trail The library emits its own API-key audit entries (from the admin verbs — create, diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs index 53b9d50..4cd177a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupRoleMapping.cs @@ -31,6 +31,23 @@ internal static class DashboardGroupRoleMapping { string normalizedGroup = group.Trim(); + // Lookup precedence: the full literal group string is tried first; only if + // that misses do we fall back to the leading RDN value (e.g. "GwAdmin" + // extracted from "ou=GwAdmin,ou=groups,..."). The map's comparer is + // OrdinalIgnoreCase (see DashboardOptions.GroupToRole), so "GwAdmin" and + // "gwadmin" both match. + // + // With the shared ZB.MOM.WW.Auth.Ldap provider, groups arrive here already + // stripped to short RDN names (the library calls FirstRdnValue before + // returning them). So through the live login path the full-string branch + // only ever sees short names and the RDN fallback is effectively a no-op — + // they collapse to the same key. The fallback is retained because this + // mapping is also reachable directly via the IGroupRoleMapper seam + // (DashboardGroupRoleMapper), where a caller could still pass a full DN. + // CONSEQUENCE: configuring a full-DN GroupToRole *key* (e.g. + // "ou=GwAdmin,ou=groups,...") is UNSUPPORTED with the shared library — the + // incoming group is a short name, so it will never equal a full-DN key. + // Keep GroupToRole keys as short group names. if (groupToRole.TryGetValue(normalizedGroup, out string? mapped) || groupToRole.TryGetValue(ExtractFirstRdnValue(normalizedGroup), out mapped)) { From f57a6ae5ffb97cb5a83fa7334615f7a6b6112691 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:49:20 -0400 Subject: [PATCH 09/21] fix(dashboard): alarms feed doc row + monitor-contract comment + mid-truncation attach test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The In-process page feeds table still described the alarms page's subscription as provider status only, contradicting the three passages updated in 7b6dfba. The loop's catch-all comment (and the cadence bullet that repeated it) claimed the monitor completes a subscriber's stream on restart. It does not: ClearCache pushes snapshot_status(false) through the still-open channel, and a subscriber is only completed-with-error on a failed TryWrite, or removed by its own disposal. The SnapshotStatus arm's priming claim had no test behind it — every push test attached to an untruncated feed and pushed the edge itself. ScriptedAlarmFeed now replays an optional open sequence, and a new test attaches to a feed already primed provider_status then snapshot_status(truncated) and asserts the banner comes up with no edge pushed after render. --- docs/GatewayDashboardDesign.md | 8 +-- .../Components/Pages/AlarmsPage.razor | 11 ++-- .../AlarmsPageTruncationBannerTests.cs | 52 +++++++++++++++++++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/docs/GatewayDashboardDesign.md b/docs/GatewayDashboardDesign.md index 5bd7241..0f6d16e 100644 --- a/docs/GatewayDashboardDesign.md +++ b/docs/GatewayDashboardDesign.md @@ -186,7 +186,7 @@ hubs stay for the audience that genuinely needs a wire. |---|---|---| | every page deriving from `DashboardPageBase` | `IDashboardSnapshotFeed.WatchAsync` | `DashboardSnapshotFeed` (singleton) multicasting one `IDashboardSnapshotService.WatchSnapshotsAsync` enumeration | | `SessionDetailsPage` | `IDashboardSessionEventSubscriber.Subscribe(sessionId)` | `DashboardEventBroadcaster` — the same singleton the session mirror publishes to, registered behind both interfaces | -| `AlarmsPage` | `IGatewayAlarmService.StreamAsync` | the central alarm monitor, **provider status only**; the alarm rows still come from the 3 s `QueryAlarmsAsync` poll | +| `AlarmsPage` | `IGatewayAlarmService.StreamAsync` | the central alarm monitor, **gateway-status frames only** — `provider_status` for the badge and `snapshot_status` for the truncated-snapshot banner; the alarm rows still come from the 3 s `QueryAlarmsAsync` poll | The snapshot feed multicasts rather than handing each page its own enumeration: `WatchSnapshotsAsync` is not multicast on its own — each enumeration owns a timer @@ -288,9 +288,9 @@ Both seams consume the same producing services, so they share these cadences: to its internal dashboard-mirror subscriber (independent of any gRPC `StreamEvents`); - the alarms page's status feed resubscribes one second after its `IGatewayAlarmService.StreamAsync` enumeration ends — the monitor completes a - subscriber's stream when it falls behind and again when it restarts, both - recoverable by resubscribing — and the badge and banner hold their last values in - between. That feed carries both gateway-status frames: `provider_status` drives the + subscriber's stream only when that subscriber has fallen behind, which resubscribing + recovers (a monitor restart keeps the channel and pushes cleared status frames through + it) — and the badge and banner hold their last values in between. That feed carries both gateway-status frames: `provider_status` drives the badge, and `snapshot_status` drives the truncated-snapshot banner, so the caveat appears on the monitor's verdict change rather than up to three seconds later. Every subscriber is primed with a `snapshot_status` frame at open, so a page attaching diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor index 8c3ad85..d12ecbd 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor @@ -258,10 +258,13 @@ } catch { - // The monitor completes a subscriber's stream when it falls behind, and - // again when the monitor restarts. Both are recoverable by resubscribing; - // the badge and banner hold their last values in the meantime, and the - // resubscribe is primed with the current ones. + // The monitor drops a subscriber whose queue it cannot write to, completing + // that stream with an error; short of cancellation or disposal that is the + // only way this enumeration ends. A monitor restart is NOT one of them — it + // keeps the channel and pushes the cleared status frames through it — so this + // catch is the fell-behind case, recoverable by resubscribing. The badge and + // banner hold their last values meanwhile, and the resubscribe is primed with + // the current ones. } try diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs index 8a7e7fc..d14339b 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/AlarmsPageTruncationBannerTests.cs @@ -134,6 +134,44 @@ public sealed class AlarmsPageTruncationBannerTests "banner to clear after a complete snapshot_status frame"); } + /// + /// Attach while the verdict is already truncated. The page carries no + /// priming logic of its own — it relies on StreamAsync opening every + /// subscription with a snapshot_status baseline — so the caveat has + /// to come up off the open sequence alone, with no edge pushed afterwards. + /// A page that only handled the edge would show an un-caveated alarm list + /// to every operator who opened it after the truncation began. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task AlarmsPage_AttachingToAnAlreadyTruncatedFeed_RaisesTheBannerFromThePriming() + { + // The monitor's open sequence: provider status, then the unconditional + // completeness baseline. Nothing is pushed after this. + ScriptedAlarmFeed feed = new() + { + Priming = + [ + new AlarmFeedMessage { ProviderStatus = new AlarmProviderStatus() }, + SnapshotStatusFrame(truncated: true), + ], + }; + + await using ServiceProvider provider = BuildPushServices(feed); + await using HtmlRenderer renderer = new( + provider, + provider.GetRequiredService()); + + HtmlRootComponent page = await renderer.Dispatcher.InvokeAsync( + () => renderer.RenderComponentAsync()); + + await WaitForHtmlAsync( + renderer, + page, + html => html.Contains(BannerMarker, StringComparison.Ordinal), + "banner to appear from the feed's open-time snapshot_status baseline"); + } + private static AlarmFeedMessage SnapshotStatusFrame(bool truncated) { return new AlarmFeedMessage @@ -280,6 +318,14 @@ public sealed class AlarmsPageTruncationBannerTests /// public bool SnapshotTruncated { get; set; } + /// + /// Frames replayed at the head of every subscription, standing in for the + /// monitor's open sequence (provider status, then the unconditional + /// completeness baseline). Empty means the subscriber sees only what the + /// test pushes. + /// + public IReadOnlyList Priming { get; init; } = []; + /// Pushes one frame onto the feed the page is subscribed to. /// The feed frame to deliver. /// A task that represents the asynchronous operation. @@ -290,6 +336,12 @@ public sealed class AlarmsPageTruncationBannerTests string? alarmFilterPrefix, [EnumeratorCancellation] CancellationToken cancellationToken) { + foreach (AlarmFeedMessage primed in Priming) + { + cancellationToken.ThrowIfCancellationRequested(); + yield return primed; + } + await foreach (AlarmFeedMessage message in _frames.Reader .ReadAllAsync(cancellationToken) .ConfigureAwait(false)) From d1ae43d0d319658713507e3336544626e780b5a3 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 06:56:58 -0400 Subject: [PATCH 10/21] =?UTF-8?q?docs(probe):=20ack-leg=20third=20attempt?= =?UTF-8?q?=20=E2=80=94=20unavailable=20by=20configuration,=20GUID=20leg?= =?UTF-8?q?=20still=20assumed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/AlarmProbeFindings.md | 143 ++++++++++++++++++++++++++++++++++++- 1 file changed, 141 insertions(+), 2 deletions(-) diff --git a/docs/AlarmProbeFindings.md b/docs/AlarmProbeFindings.md index 91afcb9..f062ba6 100644 --- a/docs/AlarmProbeFindings.md +++ b/docs/AlarmProbeFindings.md @@ -24,13 +24,13 @@ the blocker rather than rediscovering it. |---|---| | GUID stable across polls and `ALM → RTN` | Answered — yes (2026-05-01 capture in `AlarmClientDiscovery.md`, reconfirmed 2026-08-18) | | GUID stable across clear-then-re-raise | Answered 2026-08-18 — **no**, a re-raise mints a new GUID | -| GUID stable across `UNACK → ACK` | **Open** — the rig cannot be driven into an acknowledged state at all | +| GUID stable across `UNACK → ACK` | **Open** — the rig cannot be driven into an acknowledged state; 2026-08-18 (third attempt) identifies the reason as the test attribute's `MxSecurityOperate` classification, which no non-interactive ack surface on the rig can satisfy | | `COUNT` = total active vs records-in-reply under a capped fetch | Answered 2026-08-18 — **records in the reply** | The 2026-08-17 run below is kept because it is the record of the wrong-verb blocker. The 2026-08-18 run cleared that blocker with `AuthenticateUser` + `WriteSecured` and answered two of the three questions; the acknowledge leg is now blocked on something narrower and -different, described in "Second attempt". +different, described in "Second attempt" and diagnosed in "Third attempt". ## First attempt (2026-08-17): plain `Write` @@ -264,8 +264,147 @@ MXAccess-side entry point and the wnwrap-side entry point is inert. spurious Clear plus a spurious Raise, which is the same shape the now-observed re-raise behaviour produces and which `ComputeTransitions` already handles as two instances. +## Third attempt (2026-08-18): why the acknowledge is refused + +The second attempt left two candidate explanations for `AlarmAckByName` returning `rc=0` +and changing nothing: the rig enforces an acknowledgement security requirement the wnwrap +consumer cannot meet, or wnwrap's ack is simply broken here. This attempt was read-only — +no writes, no alarms raised, no configuration touched — and settles the first question: +the requirement is **enforced**, and the alarm attribute's security classification is what +enforces it. + +### Method + +Read-only inspection of the `ZB` Galaxy Repository over `sqlcmd -S localhost -d ZB -E`, +plus the already-built `mxa` CLI (`C:\Users\dohertj2\Desktop\wwtools\mxaccesscli\src\MxAccess.Cli\bin\x86\Release\net48\mxa.exe`) +for runtime reads. Nothing was written and nothing was installed. The `lmxopcua\gr` schema +notes referenced elsewhere in this repo are **not present on this box** (`Test-Path` is +`False`), so the schema was located by querying `sys.tables` / `sys.columns` directly. + +### The test attribute is classified `MxSecurityOperate` + +UDA security classification lives in `dynamic_attribute.security_classification`, keyed by +`gobject_id` — for the `$TestMachine` template that is `1055`: + +``` +attribute_name | security_classification | mx_attribute_category +ProtectedValue | 2 | 10 +ProtectedValue1 | 3 | 10 +TestAlarm001 | 1 | 10 +TestAlarm002 | 1 | 10 +TestAlarm003 | 1 | 10 +TestChangingInt | 1 | 10 +``` + +The enum is pinned by two independent sources rather than assumed. `ProtectedValue` and +`ProtectedValue1` are the mxaccess analysis project's documented *secured-write* and +*verified-write* fixtures (`C:\Users\dohertj2\Desktop\mxaccess\docs\galaxy-test-fixtures.md`), +and `docs/NMX-COM-Contracts.md` in the same project records "Galaxy security classification +(`2` for `SecuredWrite`, `3` for VerifiedWrite)" — so `2` and `3` land on exactly the two +attributes that are supposed to carry them. `galaxy-test-fixtures.md` also records the +provisioning verb used for every UDA in that inventory: `--security MxSecurityOperate`. +`TestAlarm001` therefore reads `1` = **`MxSecurityOperate`**. + +That is the missing piece from 2026-08-17. `Operate` is not free access: it requires an +authenticated galaxy identity holding Operate permission on the object's security group. +An unauthenticated `Write` is refused with `SecurityError` `1008`, and the same write after +`AuthenticateUser` succeeds — which is precisely the pair of results both prior attempts +recorded. Galaxy security is live on this rig, not disabled. + +`wwAlarmConsumerClass.AlarmAckByName` carries an operator *name* string and a comment. It +carries no authenticated user id, no credential, and no token — there is no parameter on +either the 6-arg or the 8-arg overload that could convey one. A consumer calling it cannot +satisfy an `Operate` classification, and `rc=0` followed by no state change is what an ack +dropped downstream of an accepted call looks like. + +The one step this stops short of is a direct experiment: the rig's security configuration +is out of scope for a read-only probe, so "an `Operate` alarm cannot be acknowledged +without an authenticated identity" is inferred from the classification plus the observed +`1008`/`rc=0` pattern rather than observed by relaxing the classification and watching the +ack start working. The human step below is exactly that experiment. + +### There is no writeable `.Ack` attribute — the second attempt targeted the only one there is + +A plausible reading of the second attempt was that it wrote the wrong sub-attribute: +ArchestrA alarm extensions are commonly described as exposing a writeable `.Ack` alongside +the read-only `.Acked`. On this galaxy they do not. Every `Ack`-named attribute the alarm +primitives define: + +``` +attribute_name | security_classification | mx_attribute_category +Acked | -1 | 2 +AckMsg | 0 | 6 +Bad.Acked | -1 | 2 +Bad.AckMsg | 0 | 6 +TimeAlarmAcked | -1 | 2 +AlarmAckCnt | -1 | 2 +AlarmAckErrorsCnt | -1 | 2 +AlarmMostUrgentAcked | -1 | 2 +AlarmUnAckedCnt | -1 | 2 +``` + +No `Ack`. `Acked` carries `security_classification = -1` — no classification at all, which +is what a non-writeable attribute carries, as distinct from the `1` on the writeable +`TestAlarm001`. That is the configuration-side counterpart of the `detail=1007` +operational refusal the second attempt got when it wrote `.Acked`: not a permission +refusal, but an attribute that has no write path to refuse. The second attempt had already +found the only MXAccess-side candidate, and it is read-only by definition. + +### No non-interactive acknowledge surface is installed + +The `wwtools` collection on the box (`aalogcli`, `aot`, `graccesscli`, `grdb`, `histdb`, +`mbproxy`, `mxaccesscli`, `secrets`) is the most likely home for a scriptable ack. There +isn't one. `mxa --help` lists `diag`, `info`, `read`, `read-batch`, `subscribe`, +`subscribe-batch`, `write`, `write-batch` — a tag data-plane only, with no alarm surface. +`graccesscli` is a Galaxy Repository configuration tool (`object uda add`, `instance +deploy`), which acts at configure/deploy time and not on live alarm state. Reading the +galaxy's own authentication mode at runtime is also unavailable: `Galaxy.AuthenticationMode` +does not resolve over MXAccess (`Category=4 Detail=6`), and the value is not in +`dynamic_attribute` — only the attribute *definition* names `AuthenticationMode` and +`_AuthenticationModeEnum` exist in `attribute_definition`. + +So every remaining acknowledge surface on this rig is interactive: the IDE's alarm client, +InTouch, or an ArchestrA graphic bound to the alarm. Driving those is out of scope. + +### Status of the acknowledge leg + +**Unavailable by configuration, and the GUID question stays assumed.** The two are separate +statements and both matter: + +- The wnwrap ack path is unavailable on this rig as configured, for an identified reason + rather than an unknown one. This is a real finding for the gateway: `AcknowledgeByName` + will behave the same way — accepted, inert — on any customer galaxy whose alarmed + attributes carry a non-free-access security classification. It is worth noting in the + alarm client's documentation that a silent `rc=0` is not proof of acknowledgement. +- Whether wnwrap re-mints the record GUID on `UNACK_ALM → ACK_ALM` is still unobserved, and + after three attempts it stays assumed. As the second attempt already noted, this remains + a documentation gap rather than a correctness one: a re-minted GUID produces a spurious + Clear plus a spurious Raise, the same shape the observed re-raise behaviour produces, and + `ComputeTransitions` already handles that correctly as two instances. + +#### What a human would need to do + +Either of these answers it; the second is cheaper and also confirms or refutes the +classification hypothesis above, which the read-only probe could only infer. + +1. Acknowledge `TestMachine_001.TestAlarm001` from an interactive System Platform client + (IDE alarm client, InTouch, or an ArchestrA graphic) while a wnwrap probe polls + `GetXmlCurrentAlarms2` against `\\DESKTOP-6JL3KKO\Galaxy!TestArea`, and record whether + `STATE` reaches `ACK_ALM` and whether `GUID` survives the transition. Raise the alarm + first with `AuthenticateUser` + `WriteSecured` as the second attempt did. +2. Reclassify `TestAlarm001` on the `$TestMachine` template to free access and redeploy — + `graccesscli object uda ... --security MxSecurityFreeAccess` against `$TestMachine`, + then `instance deploy TestMachine_001` — and re-run the second attempt's ack probe + unchanged. If `AlarmAckByName` then moves `STATE` to `ACK_ALM`, the classification is + confirmed as the blocker and the GUID question is answered in the same run. Restore the + classification to `MxSecurityOperate` afterwards, since the secured-write fixtures in + `WorkerLiveMxAccessSmokeTests` depend on the alarm UDAs being secured. + ### Rig state left behind +The third attempt changed nothing. `TestMachine_001.TestAlarm001` and its `.Acked` subtag +both read `false` at the end of the session, matching the state the second attempt left. + The three `TestMachine_00{1,2,3}.TestAlarm001` UDAs are back to `false` and their `.InAlarm` subtags read `false`, but each leaves a `UNACK_RTN` record in the wnwrap snapshot, since nothing can acknowledge them away. `SnapshotActiveAlarms` counts only From 462850afb749346d13fca35d6e1895ead24a7804 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:01:16 -0400 Subject: [PATCH 11/21] fix(worker-tests): doc-comment placement + watchdog-window headroom in long-in-flight test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on 7da52b6: 1. The AssertNotFault doc block landed between the predicate-overload ReadUntilAsync's doc block and the helper, so the compiler attached the merged block to the helper and ReadUntilAsync lost its docs entirely. Helper and its docs moved above ReadUntilAsync, whose docs are back where they belong. 2. The compressed watchdog windows (50 ms grace, 100 ms ceiling) reintroduced the load-sensitivity the fix removed, one layer down. ReportWatchdogFaultIfNeededAsync measures staleness AFTER the heartbeat frame is written and flushed over the real named pipe, so a beat whose pipe I/O outlasts the ceiling faults a healthy session no matter how fresh the captured activity was. At 100 ms that is a plausible stall on a loaded box, and this is the one test asserting the watchdog NEVER fires. Widened to a 200 ms grace and a 1 s ceiling — still two orders of magnitude under the 75 s production default. The inspection loop is now bounded by a 2 s window (twice the ceiling, so a fake whose activity stopped advancing still accumulates past it and faults) with a 30-frame floor, rather than a fixed 30 frames that no longer outran the wider ceiling. The floor keeps a window that saw almost no beats from passing as a clean one. Two seconds stays well inside FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight for the whole window. 3. AssertNotFault renamed AssertNotWorkerFault, matching the WorkerFault body case it tests. Scenario intent unchanged: long in-flight command, pump refreshing, zero fault frames, reply delivered. Test-only. --- .../Ipc/WorkerPipeSessionTests.cs | 61 +++++++++++-------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs index 30e7044..1bef756 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.IO.Pipes; using System.Runtime.InteropServices; @@ -1451,17 +1452,17 @@ public sealed class WorkerPipeSessionTests /// refresh LastActivityUtc on every wait iteration, so a healthy /// ReadBulk holding the STA far longer than /// HeartbeatStuckCeiling (75 s in production) keeps its activity - /// timestamp fresh. This test compresses the clock — a 100 ms ceiling - /// with a command in flight across a window many multiples longer — and - /// models the pump refresh with + /// timestamp fresh. This test compresses the clock — a 1 s ceiling with + /// a command in flight across a window twice as long — and models the + /// pump refresh with /// , which /// stamps activity at every heartbeat capture exactly as the pump's /// per-iteration MarkActivity() does. The refresh has to be in - /// effect from construction, not from the moment the command blocks: - /// with a 50 ms grace, the idle window covering handshake and startup - /// carries no correlation id for the watchdog to suppress on, so a fake - /// whose activity timestamp is frozen at construction is reported - /// StaHung before the scenario under test even starts. Contrast + /// effect from construction, not from the moment the command blocks: the + /// idle window covering handshake and startup carries no correlation id + /// for the watchdog to suppress on, so a fake whose activity timestamp is + /// frozen at construction is reported StaHung before the scenario + /// under test even starts. Contrast /// , /// where a frozen timestamp beyond the ceiling correctly faults; here /// the refreshed timestamp must keep the fault suppressed and let the @@ -1489,9 +1490,15 @@ public sealed class WorkerPipeSessionTests runtime, new WorkerPipeSessionOptions { + // Compressed relative to production (75 s ceiling), but no further than the real + // pipe underneath can carry. ReportWatchdogFaultIfNeededAsync measures staleness + // AFTER the heartbeat frame has been written and flushed, so any beat whose pipe + // I/O outlasts the ceiling faults a healthy session. At a 100 ms ceiling that is a + // plausible stall on a loaded box; at 1 s it is not — and this is the one test + // asserting the watchdog NEVER fires, so it has to hold under load. HeartbeatInterval = TimeSpan.FromMilliseconds(20), - HeartbeatGrace = TimeSpan.FromMilliseconds(50), - HeartbeatStuckCeiling = TimeSpan.FromMilliseconds(100), + HeartbeatGrace = TimeSpan.FromMilliseconds(200), + HeartbeatStuckCeiling = TimeSpan.FromSeconds(1), }); Task runTask = session.RunAsync(cancellation.Token); await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token); @@ -1514,16 +1521,22 @@ public sealed class WorkerPipeSessionTests lastEventSequence: 0, currentCommandCorrelationId: "long-bulk-read")); - // Inspect a bounded number of frames over a window many multiples of the - // 100 ms ceiling (at least 30 heartbeats at 20 ms ~ 600 ms). None may be - // a WorkerFault while activity is continuously refreshed. - const int framesToInspect = 30; + // Inspect frames across a window twice the stuck ceiling — long enough that a fake whose + // activity timestamp stopped advancing would accumulate staleness past the ceiling and + // fault — and require the beats to have actually flowed while it ran, so an inspection + // that saw a couple of frames and timed out cannot pass for a clean window. None may be a + // WorkerFault while activity is continuously refreshed. The window stays well inside + // FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight + // throughout. + TimeSpan inspectionWindow = TimeSpan.FromSeconds(2); + const int minimumFramesInspected = 30; + Stopwatch inspection = Stopwatch.StartNew(); int frameIndex = 0; - for (; frameIndex < framesToInspect; frameIndex++) + while (inspection.Elapsed < inspectionWindow || frameIndex < minimumFramesInspected) { WorkerEnvelope envelope = await pipePair.GatewayReader .ReadAsync(cancellation.Token); - AssertNotFault(envelope, frameIndex); + AssertNotWorkerFault(envelope, frameIndex++); } // Release the command with the pump still running — as it is in @@ -1537,7 +1550,7 @@ public sealed class WorkerPipeSessionTests { WorkerEnvelope envelope = await pipePair.GatewayReader .ReadAsync(cancellation.Token); - AssertNotFault(envelope, frameIndex++); + AssertNotWorkerFault(envelope, frameIndex++); if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerCommandReply && envelope.CorrelationId == "long-bulk-read") { @@ -2224,12 +2237,6 @@ public sealed class WorkerPipeSessionTests cancellationToken); } - /// Reads frames until one matches the expected body type and predicate. - /// Frame reader. - /// Expected body case. - /// Predicate to match against envelope. - /// Token to cancel the asynchronous operation. - /// The matching envelope. /// /// Fails when the frame is a WorkerFault, naming the category and diagnostic message. /// A bare body-case comparison reports only "expected not WorkerFault", which says nothing @@ -2238,7 +2245,7 @@ public sealed class WorkerPipeSessionTests /// /// Frame read from the gateway end. /// Ordinal of the frame within the inspected run. - private static void AssertNotFault(WorkerEnvelope envelope, int frameIndex) + private static void AssertNotWorkerFault(WorkerEnvelope envelope, int frameIndex) { if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerFault) { @@ -2250,6 +2257,12 @@ public sealed class WorkerPipeSessionTests + envelope.WorkerFault.DiagnosticMessage); } + /// Reads frames until one matches the expected body type and predicate. + /// Frame reader. + /// Expected body case. + /// Predicate to match against envelope. + /// Token to cancel the asynchronous operation. + /// The matching envelope. private static async Task ReadUntilAsync( WorkerFrameReader reader, WorkerEnvelope.BodyOneofCase expectedBody, From bc22792a36b9dde4e5414039a7a7be0031a4acaa Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:03:20 -0400 Subject: [PATCH 12/21] docs(probe): close superseded unblock bullet, hedge the summary, evidence gobject_id, ack-caveat in RPC docs --- docs/AlarmProbeFindings.md | 17 +++++++++++++---- docs/Grpc.md | 2 ++ gateway.md | 8 ++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/AlarmProbeFindings.md b/docs/AlarmProbeFindings.md index f062ba6..2e2be68 100644 --- a/docs/AlarmProbeFindings.md +++ b/docs/AlarmProbeFindings.md @@ -258,7 +258,9 @@ MXAccess-side entry point and the wnwrap-side entry point is inert. requirement that the wnwrap consumer, which passes an operator *name* string and no authenticated identity, cannot meet. If so, ack over wnwrap is not merely untested here but unavailable by configuration, and the gateway's `AcknowledgeByName` path needs the - same treatment on any customer galaxy configured that way. + same treatment on any customer galaxy configured that way. (Answered by the Third attempt + below — enforced by the alarm attribute's `MxSecurityOperate` security classification, an + engine-level write-security setting, not a separate `alarmmgr`-side ack policy.) - If neither lands, the acknowledge leg stays assumed. It is worth restating that this is a documentation gap, not a correctness one: a re-minted GUID on acknowledge would produce a spurious Clear plus a spurious Raise, which is the same shape the now-observed re-raise @@ -284,7 +286,12 @@ notes referenced elsewhere in this repo are **not present on this box** (`Test-P ### The test attribute is classified `MxSecurityOperate` UDA security classification lives in `dynamic_attribute.security_classification`, keyed by -`gobject_id` — for the `$TestMachine` template that is `1055`: +`gobject_id`. For the `$TestMachine` template that is `1055`: + +``` +SELECT gobject_id, tag_name, hierarchical_name FROM gobject WHERE gobject_id=1055; +1055|$TestMachine|$TestMachine +``` ``` attribute_name | security_classification | mx_attribute_category @@ -374,8 +381,10 @@ statements and both matter: - The wnwrap ack path is unavailable on this rig as configured, for an identified reason rather than an unknown one. This is a real finding for the gateway: `AcknowledgeByName` will behave the same way — accepted, inert — on any customer galaxy whose alarmed - attributes carry a non-free-access security classification. It is worth noting in the - alarm client's documentation that a silent `rc=0` is not proof of acknowledgement. + attributes carry a non-free-access security classification (inferred from the mechanism — + no `AlarmAckByName` overload can carry a credential — not confirmed by relaxing the + classification and re-testing; see "What a human would need to do"). It is worth noting in + the alarm client's documentation that a silent `rc=0` is not proof of acknowledgement. - Whether wnwrap re-mints the record GUID on `UNACK_ALM → ACK_ALM` is still unobserved, and after three attempts it stays assumed. As the second attempt already noted, this remains a documentation gap rather than a correctness one: a re-minted GUID produces a spurious diff --git a/docs/Grpc.md b/docs/Grpc.md index 6d7794e..fa39591 100644 --- a/docs/Grpc.md +++ b/docs/Grpc.md @@ -92,6 +92,8 @@ An accepted gRPC command payload can still be too large for the worker pipe: the `AcknowledgeAlarm` is a unary, **session-less** RPC that acknowledges a single alarm. The handler validates `alarm_full_reference` inline (it does not run through `MxAccessGrpcRequestValidator`) and delegates to `IGatewayAlarmService.AcknowledgeAsync`. The always-on `GatewayAlarmMonitor` routes the ack over its own gateway-managed worker session — clients no longer open a session to acknowledge an alarm. A reference that parses as a canonical GUID forwards to `AcknowledgeAlarmCommand`; a `Provider!Group.Tag` reference forwards to `AcknowledgeAlarmByNameCommand`. +An `OK` response means the alarm provider accepted the acknowledgement, not that it applied it: the gateway forwards the ack and reports what the provider returned, in keeping with the parity rule. On galaxies whose alarmed attributes carry a non-free-access security classification the by-name ack is accepted and inert — the underlying `AlarmAckByName` conveys an operator name rather than an authenticated identity — so a client that needs to confirm an acknowledgement should watch for the resulting transition on `StreamAlarms` instead of treating `OK` as proof. See [Alarm Probe Findings](./AlarmProbeFindings.md). + ### `StreamAlarms` `StreamAlarms` is a server-streaming, **session-less** RPC that attaches to the gateway's central alarm feed. The handler delegates to `IGatewayAlarmService.StreamAsync`. The stream opens with a `provider_status` and a `snapshot_status` `AlarmFeedMessage` (the current provider mode and snapshot-completeness verdict), then one `AlarmFeedMessage` carrying an `active_alarm` per currently-active alarm (the ConditionRefresh snapshot), then a single `snapshot_complete`, then a `transition` for every subsequent raise / acknowledge / clear — interleaved with a further `provider_status` on each failover/failback and a further `snapshot_status` on each change of the truncation verdict. It is served by the always-on `GatewayAlarmMonitor`, which owns a single gateway-managed worker session and fans out to every attached client — clients no longer open a session of their own. `alarm_filter_prefix`, when set, scopes the stream to a sub-tree. diff --git a/gateway.md b/gateway.md index c46b102..1520193 100644 --- a/gateway.md +++ b/gateway.md @@ -227,6 +227,14 @@ ack. If the attribute has no writable ack-comment subtag configured, the RPC returns `FailedPrecondition`. In alarm-manager mode, `AlarmAckByName` is used as before. +**Acceptance is not application:** the ack is forwarded to the provider, and a +successful return means the provider accepted the call, not that the +acknowledgement was applied. On galaxies whose alarmed attributes carry a +non-free-access security classification, `AlarmAckByName` returns `rc=0` and the +alarm stays `UNACK_ALM` — it carries an operator name, not an authenticated +identity. Confirm an ack by the resulting transition, never by the return code. +See `docs/AlarmProbeFindings.md`. + **Degraded state visibility:** every subtag-mode transition carries `degraded = true` and `source_provider = ALARM_PROVIDER_MODE_SUBTAG` on the `OnAlarmTransitionEvent` and `ActiveAlarmSnapshot` proto fields. The From aaeb86b1327df1b1363d7c5077ef6b946fa40e2d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:08:47 -0400 Subject: [PATCH 13/21] fix(worker-tests): decouple FakeRuntimeSession dispatch safety-net from the inspection window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BlockDispatch branch waited 5 real seconds and then proceeded regardless — it does not branch on the wait's result. The long-in-flight test's inspection loop is bounded by elapsed time and a frame floor, so on a loaded box (the documented 4-5x slowdown class) it can plausibly outrun that 5 s. When it does, the reply is emitted mid-window, AssertNotWorkerFault waves it past, and the reply leg then waits for a reply already gone by — failing at the 20 s cancellation with no message, on exactly the loaded-box run the widened windows exist to survive. The wait is a pure safety net: nothing asserts on it firing, and every test that blocks dispatch releases it explicitly (ReleaseDispatch, or a WorkerShutdown envelope, both of which Set the event) — none reaches the timeout on a healthy run. Named it BlockedDispatchSafetyNet and raised it to 30 s, above any window a test opens and above the 20 s cancellation those tests arm, so a wedged test always fails on its own token with its own message. Dispose still releases the wait, so teardown never waits on it either. Inline rationale in the test now states the decoupling and what a close pairing would cost, rather than asserting the window stays inside a 5 s ceiling. Nothing else changed. --- .../Ipc/WorkerPipeSessionTests.cs | 9 ++++++--- .../TestSupport/FakeRuntimeSession.cs | 13 ++++++++++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs index 1bef756..dca5936 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs @@ -1525,9 +1525,12 @@ public sealed class WorkerPipeSessionTests // activity timestamp stopped advancing would accumulate staleness past the ceiling and // fault — and require the beats to have actually flowed while it ran, so an inspection // that saw a couple of frames and timed out cannot pass for a clean window. None may be a - // WorkerFault while activity is continuously refreshed. The window stays well inside - // FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight - // throughout. + // WorkerFault while activity is continuously refreshed. Nothing here is racing + // FakeRuntimeSession's blocked-dispatch backstop: that wait is a safety net sized far above + // any window a test opens (and above this test's own cancellation), so the command stays in + // flight for however long a loaded box stretches the loop. Were the two close together, a + // slow run would take the reply mid-window and then fail waiting for a reply already gone + // by — a cancellation at teardown, naming nothing. TimeSpan inspectionWindow = TimeSpan.FromSeconds(2); const int minimumFramesInspected = 30; Stopwatch inspection = Stopwatch.StartNew(); diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs index c7b9ebf..0f89658 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs @@ -20,6 +20,17 @@ namespace ZB.MOM.WW.MxGateway.Worker.Tests.TestSupport; /// internal sealed class FakeRuntimeSession : IWorkerRuntimeSession { + /// + /// Backstop on the wait so a test that never releases leaves no + /// thread parked forever. It is a safety net, never a scenario's timing budget: nothing + /// asserts on it firing, and a test whose blocked window outruns it silently gets its reply + /// mid-window, which then fails as an opaque cancellation somewhere later. Kept far above + /// any test's window — and above the 20 s cancellation those tests arm — so the test's own + /// token always fails first, with its own message. releases the wait + /// regardless, so teardown never depends on this elapsing. + /// + private static readonly TimeSpan BlockedDispatchSafetyNet = TimeSpan.FromSeconds(30); + private readonly ManualResetEventSlim releaseDispatch = new(false); private readonly object gate = new(); private readonly Queue events = new(); @@ -91,7 +102,7 @@ internal sealed class FakeRuntimeSession : IWorkerRuntimeSession if (BlockDispatch) { - releaseDispatch.Wait(TimeSpan.FromSeconds(5)); + releaseDispatch.Wait(BlockedDispatchSafetyNet); } SetSnapshot(new WorkerRuntimeHeartbeatSnapshot( From 5417222f443cd948ed9176634d3520a6f3c0669d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:13:46 -0400 Subject: [PATCH 14/21] chore(plan): tasks 1-8 completed with review outcomes; task 9 in progress --- ...6-08-18-followups-and-tickets.md.tasks.json | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json b/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json index 6a82449..d57e32a 100644 --- a/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json +++ b/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json @@ -1,15 +1,15 @@ { "planPath": "docs/plans/2026-08-18-followups-and-tickets.md", "tasks": [ - {"id": 1, "subject": "Task 1: check-codegen Check 4 — Windows .exe banner normalization", "status": "pending"}, - {"id": 2, "subject": "Task 2: Gradle 9 checkGeneratedClean via ProviderFactory.exec", "status": "pending"}, - {"id": 3, "subject": "Task 3: SettingsPage RecentFaultLimit/RecentSessionLimit rows", "status": "pending"}, - {"id": 4, "subject": "Task 4: Authentication.md runnable-as-written examples pass", "status": "pending"}, - {"id": 5, "subject": "Task 5: AlarmsPage push-driven truncation banner (snapshot_status)", "status": "pending"}, - {"id": 6, "subject": "Task 6: Server-0xx doc-only resolution audit", "status": "pending", "blockedBy": [4]}, - {"id": 7, "subject": "Task 7: WorkerPipeSessionTests deterministic failure — investigate + fix (windev)", "status": "pending"}, - {"id": 8, "subject": "Task 8: Ack-leg probe bounded unblock attempt (windev rig)", "status": "pending", "blockedBy": [7]}, - {"id": 9, "subject": "Task 9: Windev toolchain pin + full verification", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8]}, + {"id": 1, "subject": "Task 1: check-codegen Check 4 — Windows .exe banner normalization", "status": "completed", "commit": "c94c4d4", "review": "Approved (Haiku code review, no issues); macOS check-codegen 4/4; Windows-side proof deferred to Task 9"}, + {"id": 2, "subject": "Task 2: Gradle 9 checkGeneratedClean via ProviderFactory.exec", "status": "completed", "commit": "df45cb4", "review": "Approved; verified on Gradle 9.5.1 (local toolchain already Gradle 9 — task was dead here), clean-pass and forced-stale-fail both proven"}, + {"id": 3, "subject": "Task 3: SettingsPage RecentFaultLimit/RecentSessionLimit rows", "status": "completed", "commit": "a390fe1", "review": "Approved; fail-first render test with non-default values, 3/3 filtered"}, + {"id": 4, "subject": "Task 4: Authentication.md runnable-as-written examples pass", "status": "completed", "commit": "2b1efb5", "review": "Approved; three examples gained session:open, ops.audit correctly left session-less (metadata:read gates session-less Galaxy RPCs)"}, + {"id": 5, "subject": "Task 5: AlarmsPage push-driven truncation banner (snapshot_status)", "status": "completed", "commits": ["7b6dfba", "f57a6ae"], "review": "Spec compliant (rename judged in-scope); code review Approved after fixes: stale doc table row, monitor-contract comment (restart does NOT complete subscribers), mid-truncation attach priming test; 5/5 + 370/370 filtered"}, + {"id": 6, "subject": "Task 6: Server-0xx doc-only resolution audit", "status": "completed", "commit": "d3ac527", "review": "Spec compliant; code review Approved (controller verified DashboardGroupRoleMapping change comment-only). 20 entries audited; 2 regressions re-fixed (Server-040 swept comment block incl. RDN pre-strip paragraph; Server-009 WAL/busy-timeout prose); 4 moot (target deleted); Server-038 closed-not-regressed. Systemic finding: fca978d tracking-marker sweep deleted substantive prose in at least one place"}, + {"id": 7, "subject": "Task 7: WorkerPipeSessionTests deterministic failure — investigate + fix (windev)", "status": "completed", "commits": ["7da52b6", "462850a", "aaeb86b"], "review": "High-risk chain: spec compliant; code review Approved after two fix rounds (doc-comment attachment; watchdog-window headroom 200ms/1s + 2s window/30-frame floor; 30s BlockedDispatchSafetyNet > 20s CTS makes the mid-window auto-release race structurally unreachable). Root cause: test-harness defect — FakeRuntimeSession stamped LastStaActivityUtc only at construction, watchdog correctly faulted StaHung pre-dispatch. Test-only fix; windev worker suite 524 passed/11 skipped (was 523+1 fail)"}, + {"id": 8, "subject": "Task 8: Ack-leg probe bounded unblock attempt (windev rig)", "status": "completed", "commits": ["d1ae43d", "bc22792"], "review": "Spec compliant; code review Approved after fixes (superseded bullet closed, summary hedged, gobject_id evidenced, ack-caveat added to gateway.md + Grpc.md as authorized scope extension). Outcome: ack unavailable-by-configuration (MxSecurityOperate classification, no credential-bearing AlarmAckByName overload — inferred, honestly caveated); no .Ack attribute exists; no non-interactive platform-side surface; GUID-across-ack row stays Open with reason updated"}, + {"id": 9, "subject": "Task 9: Windev toolchain pin + full verification", "status": "in_progress", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8]}, {"id": 10, "subject": "Task 10: Bookkeeping — close the follow-ups record", "status": "pending", "blockedBy": [9]} ], "lastUpdated": "2026-08-18" From 1605f542ccc5d6ad0bbeaef670cc132bb232a806 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:23:33 -0400 Subject: [PATCH 15/21] docs(probe): align RPC-doc hedging with the findings doc; attribute rig-state inventory --- docs/AlarmProbeFindings.md | 3 ++- docs/Grpc.md | 2 +- gateway.md | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/AlarmProbeFindings.md b/docs/AlarmProbeFindings.md index 2e2be68..f47f7e8 100644 --- a/docs/AlarmProbeFindings.md +++ b/docs/AlarmProbeFindings.md @@ -414,7 +414,8 @@ classification hypothesis above, which the read-only probe could only infer. The third attempt changed nothing. `TestMachine_001.TestAlarm001` and its `.Acked` subtag both read `false` at the end of the session, matching the state the second attempt left. -The three `TestMachine_00{1,2,3}.TestAlarm001` UDAs are back to `false` and their +The second attempt's inventory, unchanged since: the three +`TestMachine_00{1,2,3}.TestAlarm001` UDAs are back to `false` and their `.InAlarm` subtags read `false`, but each leaves a `UNACK_RTN` record in the wnwrap snapshot, since nothing can acknowledge them away. `SnapshotActiveAlarms` counts only `UNACK_ALM` and `ACK_ALM` as active, so these are inert for the gateway; they will clear on diff --git a/docs/Grpc.md b/docs/Grpc.md index fa39591..e548e91 100644 --- a/docs/Grpc.md +++ b/docs/Grpc.md @@ -92,7 +92,7 @@ An accepted gRPC command payload can still be too large for the worker pipe: the `AcknowledgeAlarm` is a unary, **session-less** RPC that acknowledges a single alarm. The handler validates `alarm_full_reference` inline (it does not run through `MxAccessGrpcRequestValidator`) and delegates to `IGatewayAlarmService.AcknowledgeAsync`. The always-on `GatewayAlarmMonitor` routes the ack over its own gateway-managed worker session — clients no longer open a session to acknowledge an alarm. A reference that parses as a canonical GUID forwards to `AcknowledgeAlarmCommand`; a `Provider!Group.Tag` reference forwards to `AcknowledgeAlarmByNameCommand`. -An `OK` response means the alarm provider accepted the acknowledgement, not that it applied it: the gateway forwards the ack and reports what the provider returned, in keeping with the parity rule. On galaxies whose alarmed attributes carry a non-free-access security classification the by-name ack is accepted and inert — the underlying `AlarmAckByName` conveys an operator name rather than an authenticated identity — so a client that needs to confirm an acknowledgement should watch for the resulting transition on `StreamAlarms` instead of treating `OK` as proof. See [Alarm Probe Findings](./AlarmProbeFindings.md). +An `OK` response means the alarm provider accepted the acknowledgement, not that it applied it: the gateway forwards the ack and reports what the provider returned, in keeping with the parity rule. On galaxies whose alarmed attributes carry a non-free-access security classification the by-name ack is accepted and inert — the underlying `AlarmAckByName` conveys an operator name rather than an authenticated identity, observed on the probe rig with the mechanism inferred rather than confirmed — so a client that needs to confirm an acknowledgement should watch for the resulting transition on `StreamAlarms` instead of treating `OK` as proof. See [Alarm Probe Findings](./AlarmProbeFindings.md). ### `StreamAlarms` diff --git a/gateway.md b/gateway.md index 1520193..77c113f 100644 --- a/gateway.md +++ b/gateway.md @@ -232,8 +232,9 @@ successful return means the provider accepted the call, not that the acknowledgement was applied. On galaxies whose alarmed attributes carry a non-free-access security classification, `AlarmAckByName` returns `rc=0` and the alarm stays `UNACK_ALM` — it carries an operator name, not an authenticated -identity. Confirm an ack by the resulting transition, never by the return code. -See `docs/AlarmProbeFindings.md`. +identity. (Observed on the probe rig; the mechanism is inferred.) Confirm an ack +by the resulting transition, never by the return code. See +`docs/AlarmProbeFindings.md`. **Degraded state visibility:** every subtag-mode transition carries `degraded = true` and `source_provider = ALARM_PROVIDER_MODE_SUBTAG` on the From 6b5c737b0425ad149f614d46c22025f1ef84a1ee Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:23:37 -0400 Subject: [PATCH 16/21] docs(toolchain): windev protoc-gen-go-grpc updated to pinned 1.6.2 --- docs/ToolchainLinks.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/ToolchainLinks.md b/docs/ToolchainLinks.md index d5859f8..e4a0ac7 100644 --- a/docs/ToolchainLinks.md +++ b/docs/ToolchainLinks.md @@ -69,8 +69,8 @@ a .NET Framework or COM interop build needs classic Visual Studio MSBuild. | Tool | Version | Path | | --- | --- | --- | | Go | 1.26.2 windows/amd64 | `C:\Program Files\Go\bin\go.exe` | -| protoc-gen-go | latest installed by `go install` | `C:\Users\dohertj2\go\bin\protoc-gen-go.exe` | -| protoc-gen-go-grpc | latest installed by `go install` | `C:\Users\dohertj2\go\bin\protoc-gen-go-grpc.exe` | +| protoc-gen-go | v1.36.11 (pinned) | `C:\Users\dohertj2\go\bin\protoc-gen-go.exe` | +| protoc-gen-go-grpc | 1.6.2 (pinned) | `C:\Users\dohertj2\go\bin\protoc-gen-go-grpc.exe` | Environment: @@ -80,11 +80,14 @@ GOPATH=C:\Users\dohertj2\go Go plugin bin=C:\Users\dohertj2\go\bin ``` -Installed plugin commands: +Installed plugin commands. Install the pinned versions, not `@latest`: the committed Go +bindings stamp the plugin version in their headers, so `clients/go/generate-proto.ps1` +throws when a plugin reports anything else. That script is the source of truth for the +pin — change it there first, then update this table. ```powershell -go install google.golang.org/protobuf/cmd/protoc-gen-go@latest -go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest +go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.11 +go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.2 ``` ## Rust From db7b1db947350e9d802ab644f764c07e7bc9deb4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:23:59 -0400 Subject: [PATCH 17/21] =?UTF-8?q?docs(dashboard):=20GroupToRole/GroupToTag?= =?UTF-8?q?=20keys=20are=20short=20names=20=E2=80=94=20full-DN=20keys=20ne?= =?UTF-8?q?ver=20match=20the=20pre-stripped=20groups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the Server-040 comment restoration (d3ac527). The restored comment in DashboardGroupRoleMapping records that the shared ZB.MOM.WW.Auth.Ldap provider hands the mapper groups already stripped to short RDN names, so a full-DN GroupToRole *key* can never match. Three doc sites still told operators the opposite, and the tag mapper carried the same consequence with no comment at all. - docs/GatewayConfiguration.md: GroupToRole and GroupToTag rows both said keys may be 'short CN or full DN — leading-RDN match'. Corrected to short group names only, with the reason (pre-stripped groups) and a note that the mapper's full-string-then-RDN fallback only matters to non-library callers of the IGroupRoleMapper seam. - docs/GatewayDashboardDesign.md: the login-flow bullet said the user's memberOf '(or short CN)' is matched against GroupToRole; it now describes what actually happens — groups arrive short and are matched against short-name keys. - DashboardGroupTagMapping: short comment at the lookup pointing at the DashboardGroupRoleMapping comment, since it reuses ExtractFirstRdnValue and inherits the same full-DN-key-unsupported consequence. Both doc rows are pre-existing on main — they predate this branch and were not introduced by the Server-040 work; only their incorrectness was surfaced by it. Comment/doc-only; no logic changes. --- docs/GatewayConfiguration.md | 4 ++-- docs/GatewayDashboardDesign.md | 6 ++++-- .../Dashboard/DashboardGroupTagMapping.cs | 7 +++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/GatewayConfiguration.md b/docs/GatewayConfiguration.md index 4ce63b8..5970259 100644 --- a/docs/GatewayConfiguration.md +++ b/docs/GatewayConfiguration.md @@ -193,8 +193,8 @@ events (a "gap") and must re-snapshot; whatever is still retained is replayed. | `MxGateway:Dashboard:RecentFaultLimit` | `100` | Maximum number of fault summaries projected into each dashboard snapshot. | | `MxGateway:Dashboard:RecentSessionLimit` | `200` | Maximum number of session summaries projected into each dashboard snapshot. | | `MxGateway:Dashboard:ShowTagValues` | `false` | Controls whether tag values reach the dashboard at all. `false` (the default) suppresses them on all three seams that carry one: (1) the **events hub mirror** — `DashboardEventBroadcaster` blanks `MxEvent.value` plus the alarm body's `current_value`/`limit_value` from a deep-cloned copy before it reaches any hub subscriber (see `docs/GatewayDashboardDesign.md`'s `EventsHub` row); (2) the **alarms hub** — `AlarmsHubPublisher` clears `current_value`/`limit_value` from a deep-cloned copy of each `AlarmFeedMessage`, on both value-bearing payload arms (`transition` and `active_alarm`), before broadcasting to `/hubs/alarms`; (3) the **`/browse` live-value panel** — `DashboardLiveDataService` substitutes the literal `[redacted]` for the value text of each **successfully read** tag; a failed read keeps its `-` placeholder (there was no value to suppress, and the row's error is left untouched, so the two cannot contradict each other). Both hub redactions clone: the source `MxEvent` is shared with the gRPC event stream and the replay ring, and the source `AlarmFeedMessage` fans out to gRPC `StreamAlarms` subscribers, so neither is mutated in place and **no gRPC client is affected by this flag** — it is a dashboard-display control only. Everything that is not the value survives on every seam: tag reference, alarm reference/severity/state/operator, data type, quality, status, and timestamps still render, so the dashboard stays diagnostic without disclosing process values. This is one of two independent layers: `IDashboardSessionAcl` decides *which* sessions a caller may subscribe to at all (see `GroupToTag` / `UntaggedSessionVisibility` below), while this flag decides what a permitted subscriber sees. Setting it `true` exposes tag values to everyone the ACL admits — every Administrator, plus each Viewer holding a matching tag — and, because the alarms hub is session-less, exposes alarm values to every dashboard client that can reach `/hubs/alarms`. | -| `MxGateway:Dashboard:GroupToRole` | _(empty)_ | LDAP group → dashboard role mapping. Keys are LDAP group names (short CN or full DN — leading-RDN match). Values must be `Administrator` (read/write, API-key CRUD) or `Viewer` (read-only) — matched ordinally by the startup validator, so the spelling is exact and `Admin` is rejected. A user whose LDAP groups don't intersect this map cannot sign in; with no mapping at all, only the loopback bypass admits anyone. | -| `MxGateway:Dashboard:GroupToTag` | _(empty)_ | LDAP group → dashboard visibility tags. Keys follow the same convention as `GroupToRole` (short CN or full DN — leading-RDN match, case-insensitive); values are tag lists. A dashboard user's granted tag set is the union over the groups they belong to; an unmapped group contributes nothing. **Visibility only:** tags scope which sessions' event streams a Viewer may observe on the dashboard — they never grant or deny data access, which stays with the API key's scopes and constraints. Independent of `GroupToRole`: a group may appear in either map, both, or neither. Empty (the default) means Viewers hold no tags, so under the default `UntaggedSessionVisibility` they observe no session's events. | +| `MxGateway:Dashboard:GroupToRole` | _(empty)_ | LDAP group → dashboard role mapping. Keys must be **short** LDAP group names — the CN / first-RDN value, e.g. `GwAdmin`, not `ou=GwAdmin,ou=groups,dc=zb,dc=local` — matched case-insensitively. The shared `ZB.MOM.WW.Auth.Ldap` provider delivers a user's groups already stripped to short names, so a full-DN key can never match and the group silently maps to nothing. (The mapper does try the full literal string before falling back to the leading-RDN value, but on the live login path both branches see the same short name; the fallback only matters to non-library callers of the `IGroupRoleMapper` seam.) Values must be `Administrator` (read/write, API-key CRUD) or `Viewer` (read-only) — matched ordinally by the startup validator, so the spelling is exact and `Admin` is rejected. A user whose LDAP groups don't intersect this map cannot sign in; with no mapping at all, only the loopback bypass admits anyone. | +| `MxGateway:Dashboard:GroupToTag` | _(empty)_ | LDAP group → dashboard visibility tags. Keys follow the same convention as `GroupToRole` — short LDAP group names (CN / first-RDN value), matched case-insensitively; a full-DN key never matches the pre-stripped groups the LDAP provider returns. Values are tag lists. A dashboard user's granted tag set is the union over the groups they belong to; an unmapped group contributes nothing. **Visibility only:** tags scope which sessions' event streams a Viewer may observe on the dashboard — they never grant or deny data access, which stays with the API key's scopes and constraints. Independent of `GroupToRole`: a group may appear in either map, both, or neither. Empty (the default) means Viewers hold no tags, so under the default `UntaggedSessionVisibility` they observe no session's events. | | `MxGateway:Dashboard:UntaggedSessionVisibility` | `AdminOnly` | Who may observe a session that carries no tags (its owning API key declared none). `AdminOnly` (default, fail-closed) restricts untagged sessions to dashboard Administrators. `AllViewers` shows them to every Viewer — opt-in for a single-tenant deployment that wants the pre-tag behaviour. Administrators always see every session regardless of tags. | | `MxGateway:Dashboard:DisableLogin` | `false` | Dev/test only. When `true`, replaces the cookie authentication handler with `DashboardAutoLoginAuthenticationHandler`, which auto-authenticates every dashboard request — including requests from remote browsers, not just loopback — as `AutoLoginUser` holding both `Administrator` and `Viewer` roles. No login form, LDAP bind, or cookie is involved. A loud one-time startup warning is logged. Differs from `AllowAnonymousLocalhost`: `DisableLogin` mints a real authenticated principal (so role-gated write affordances appear), whereas `AllowAnonymousLocalhost` satisfies the authorization requirement on loopback only without minting a principal (write affordances stay hidden). Never enable in production. **Production hard-stop (SEC-04):** when the host runs in the `Production` environment and `DisableLogin` is `true`, startup validation fails and the process aborts — the flag is only accepted outside Production, where the one-time startup warning still fires. | | `MxGateway:Dashboard:AutoLoginUser` | `(null)` | Username stamped on the synthetic principal when `DisableLogin` is `true`. Default `(null)` — a null or blank value falls back to `multi-role`. Has no effect when `DisableLogin` is `false`. | diff --git a/docs/GatewayDashboardDesign.md b/docs/GatewayDashboardDesign.md index 0f6d16e..abbc3f8 100644 --- a/docs/GatewayDashboardDesign.md +++ b/docs/GatewayDashboardDesign.md @@ -690,8 +690,10 @@ Implemented behavior: - a static `/login` HTML form posts username/password to the gateway; - `DashboardAuthenticator` binds against `MxGateway:Ldap` (service-account bind, user search, candidate bind) using `Novell.Directory.Ldap.NETStandard`; -- the user's `memberOf` (or short CN) is matched against - `MxGateway:Dashboard:GroupToRole`; the resolved role(s) are emitted as +- the user's groups arrive from the LDAP provider already stripped to short + names (the CN / first-RDN value of each `memberOf` entry) and are matched + against the short-name keys of `MxGateway:Dashboard:GroupToRole` — a full-DN + key there never matches; the resolved role(s) are emitted as `ClaimTypes.Role` claims, alongside the per-group `mxgateway:ldap_group` claims; - a successful login signs in the `MxGateway.Dashboard` cookie scheme diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupTagMapping.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupTagMapping.cs index dc54fd7..84be574 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupTagMapping.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardGroupTagMapping.cs @@ -35,6 +35,13 @@ internal static class DashboardGroupTagMapping { string normalizedGroup = group.Trim(); + // Same lookup semantics as DashboardGroupRoleMapping.MapGroupsToRoles — + // full literal group string first, leading-RDN value as the fallback, over + // an OrdinalIgnoreCase map. See the comment there for the consequence that + // applies verbatim here: the shared ZB.MOM.WW.Auth.Ldap provider delivers + // groups already stripped to short RDN names, so a full-DN GroupToTag *key* + // is UNSUPPORTED — it can never equal the short name that arrives. Keep + // GroupToTag keys as short group names. if (!groupToTag.TryGetValue(normalizedGroup, out string[]? granted) && !groupToTag.TryGetValue( DashboardGroupRoleMapping.ExtractFirstRdnValue(normalizedGroup), From 753d0705357d3780515df03703813c9e3aae1d3d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:24:39 -0400 Subject: [PATCH 18/21] style(worker-tests): field ordering in FakeRuntimeSession MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshStaActivityOnCapture was declared between DispatchAsync and the property that wraps it, rather than with the other instance fields. Moved up to the field block per the member ordering in docs/style-guides/CSharpStyleGuide.md. Declaration move only — no behavior, no other edits. --- .../TestSupport/FakeRuntimeSession.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs index 0f89658..456205d 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/TestSupport/FakeRuntimeSession.cs @@ -42,6 +42,7 @@ internal sealed class FakeRuntimeSession : IWorkerRuntimeSession // disposed SemaphoreSlim would turn that shutdown into an ObjectDisposedException. private readonly SemaphoreSlim eventSignal = new(0, 1); private TimeSpan? lastWaitForEventsTimeout; + private bool refreshStaActivityOnCapture; private WorkerRuntimeHeartbeatSnapshot snapshot = new( DateTimeOffset.UtcNow, pendingCommandCount: 0, @@ -138,8 +139,6 @@ internal sealed class FakeRuntimeSession : IWorkerRuntimeSession }); } - private bool refreshStaActivityOnCapture; - /// /// When set, stamps the snapshot's /// LastStaActivityUtc with the capture time and leaves every other field as the last From 8ae0c2f3d3c632bb35841cc71cf255a29d0f9ebd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:25:42 -0400 Subject: [PATCH 19/21] fix(codegen): PS5.1-safe quoting for native-exe args in generate-proto probes --- clients/python/generate-proto.ps1 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clients/python/generate-proto.ps1 b/clients/python/generate-proto.ps1 index a98eb29..863a21c 100644 --- a/clients/python/generate-proto.ps1 +++ b/clients/python/generate-proto.ps1 @@ -36,7 +36,11 @@ function Resolve-Python { function Assert-GrpcioToolsVersion { param([string]$Python) - $version = (& $Python -c 'import grpc_tools; from importlib.metadata import version; print(version("grpcio-tools"))').Trim() + # Windows PowerShell 5.1 strips embedded double quotes when passing an argument to a native + # exe (pwsh 7 does not), so a Python literal quoted with " " here would arrive at Python as + # print(version(grpcio-tools)) -> NameError. Use single quotes for the Python string literal + # inside this double-quoted PowerShell string so the quoting survives on both hosts. + $version = (& $Python -c "import grpc_tools; from importlib.metadata import version; print(version('grpcio-tools'))").Trim() if ($version -ne $PinnedGrpcioToolsVersion) { throw "grpcio-tools $version is installed, but regeneration is pinned to $PinnedGrpcioToolsVersion. " + "Install the pin (python -m pip install 'grpcio-tools==$PinnedGrpcioToolsVersion') before regenerating, " + From fb68bdb699e5842c6749d412aa095c337c2ba6b9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:25:57 -0400 Subject: [PATCH 20/21] test(dashboard): settings render test proves label-value pairing --- .../SettingsPageTagVisibilityRenderTests.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs index 0c8f7e9..0a99f8e 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Dashboard/SettingsPageTagVisibilityRenderTests.cs @@ -74,8 +74,9 @@ public sealed class SettingsPageTagVisibilityRenderTests } /// - /// RecentFaultLimit and RecentSessionLimit reach the page. Non-default values - /// prove the projection is plumbed through, not just that the defaults happen to render. + /// RecentFaultLimit and RecentSessionLimit reach the page, each value landing in + /// the row its own label names — not merely present somewhere in the document. Non-default + /// values prove the projection is plumbed through, not just that the defaults happen to render. /// /// A task that represents the asynchronous operation. [Fact] @@ -87,10 +88,11 @@ public sealed class SettingsPageTagVisibilityRenderTests RecentSessionLimit = 456, }); - Assert.Contains("Recent fault limit", html, StringComparison.Ordinal); - Assert.Contains("123", html, StringComparison.Ordinal); - Assert.Contains("Recent session limit", html, StringComparison.Ordinal); - Assert.Contains("456", html, StringComparison.Ordinal); + // Combined label+value fragments, matching the razor's exact markup (no whitespace between + // tags): a bare Contains on "123"/"456" would pass even if the values landed in the wrong + // row, or any other row on the page. + Assert.Contains("""Recent fault limit123""", html, StringComparison.Ordinal); + Assert.Contains("""Recent session limit456""", html, StringComparison.Ordinal); } private static async Task RenderAsync(DashboardOptions dashboard) From f4b974eec10bb4452eeb851213884be181eb38b2 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:29:15 -0400 Subject: [PATCH 21/21] chore(plan): followups-and-tickets as-built record; prior follow-ups closed --- docs/plans/2026-08-17-followup-closeout.md | 34 ++++++++-- .../plans/2026-08-18-followups-and-tickets.md | 65 +++++++++++++++++++ ...-08-18-followups-and-tickets.md.tasks.json | 4 +- 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/docs/plans/2026-08-17-followup-closeout.md b/docs/plans/2026-08-17-followup-closeout.md index 4847a40..4cc0cbe 100644 --- a/docs/plans/2026-08-17-followup-closeout.md +++ b/docs/plans/2026-08-17-followup-closeout.md @@ -480,22 +480,40 @@ design; they need only a cheap tip re-build there. rig shows the new GUID replaces the record. The prose that implied coexistence was corrected rather than the code. -Follow-ups recorded, not started: +Follow-ups recorded, not started — **all closed 2026-08-18 on `feat/followups-tickets`** +(plan `docs/plans/2026-08-18-followups-and-tickets.md`; per-bullet closing commits below): - `WorkerPipeSessionTests.RunAsync_LongInFlightCommandThatKeepsPumping_DoesNotFaultAndDeliversReply` fails deterministically on windev **and on `main`** — pre-existing, needs its own - investigation. + investigation. *Closed (`7da52b6`, `462850a`, `aaeb86b`): test-harness defect — the fake + runtime session stamped STA activity only at construction, so the watchdog correctly + faulted `StaHung` pre-dispatch. Test-only fix; windev worker suite fully green (524/11 + skipped) for the first time.* - `check-codegen.ps1` Check 4 is unrunnable on Windows: the `protoc-gen-go` version banner carries a `.exe` suffix that the exact-string compare in - `clients/go/generate-proto.ps1:10,55` does not tolerate. -- Windev has `protoc-gen-go-grpc` 1.6.1 against the repo's pinned 1.6.2. + `clients/go/generate-proto.ps1:10,55` does not tolerate. *Closed (`c94c4d4`, plus + `8ae0c2f` for a second Windows-only blocker found during verification: PS 5.1 strips + embedded double quotes from the Python probe's `-c` argument). Check 4 verified 4/4 on + windev under both PowerShell 5.1 and pwsh 7.* +- Windev has `protoc-gen-go-grpc` 1.6.1 against the repo's pinned 1.6.2. *Closed: 1.6.2 + installed on windev; `docs/ToolchainLinks.md` corrected from `@latest` to the pinned + install commands (`6b5c737`).* - `clients/java`'s `checkGeneratedClean` is dead under Gradle 9 (`Project.exec` was - removed); it needs `ExecOperations` injection to work again. + removed); it needs `ExecOperations` injection to work again. *Closed (`df45cb4`) via + `ProviderFactory.exec`; verified on Gradle 9.5.1 (macOS) and 9.4.1 (windev), including + a configuration-cache ordering proof.* - `SettingsPage` renders every `EffectiveDashboardConfiguration` member except `RecentFaultLimit` / `RecentSessionLimit` (pre-existing, predates this branch). + *Closed (`a390fe1`, test tightened in `fb68bdb`).* - The ack-leg probe stays blocked; unblock paths are in `docs/AlarmProbeFindings.md`. + *Closed as answered-why (`d1ae43d`, `bc22792`, `1605f54`): the ack is unavailable by + configuration — the test attributes carry `MxSecurityOperate` and no `AlarmAckByName` + overload can carry a credential (inferred, caveated). The GUID-across-ack question + itself stays assumed; the remaining paths need a human at an interactive client.* - The dashboard `AlarmsPage` truncation banner is still poll-driven — it could consume - the new `snapshot_status` feed frame instead. + the new `snapshot_status` feed frame instead. *Closed (`7b6dfba`, `f57a6ae`): the + page's status feed loop now consumes `snapshot_status`; the poll stays as reconcile + baseline.* - **A closed code-review finding regressed, or was never applied.** Server-012 (`code-reviews/Server/findings.md:405-412`) is recorded *Resolved 2026-05-18* and claims it corrected two scope lists to the canonical `*:*` strings — the `CLAUDE.md` @@ -504,7 +522,9 @@ Follow-ups recorded, not started: with a third instance the finding never covered (`docs/Authentication.md`'s `ops.alice` example). The bookkeeping is the follow-up: other `Server-0xx` entries marked Resolved with documentation-only fixes should be spot-checked for the same pattern, since a - finding that reads Resolved is not otherwise re-examined. + finding that reads Resolved is not otherwise re-examined. *Closed (`d3ac527`): 20 + doc-only resolutions audited; two more regressions found and re-fixed (Server-040, + Server-009); four moot (target files deleted); annotations recorded in findings.md.* Explicitly decided, not an omission: **`../scadaproj/CLAUDE.md` needs no update.** The umbrella index records the *set* of `.proto` files this repo owns, and that set is diff --git a/docs/plans/2026-08-18-followups-and-tickets.md b/docs/plans/2026-08-18-followups-and-tickets.md index 76a8e8d..dda175c 100644 --- a/docs/plans/2026-08-18-followups-and-tickets.md +++ b/docs/plans/2026-08-18-followups-and-tickets.md @@ -459,3 +459,68 @@ accepted-but-inert (`rc=0`, state stays `UNACK_ALM`), and `.Acked` is write-reje Wave 4 = Task 10, final integration review, then hold for the user's merge decision. - **Push cadence:** controller pushes the branch before any windev task needs its code there (Task 7 investigates on-branch; Tasks 8–9 need the wave-1/2 tips). + +--- + +## As-built record (2026-08-18) + +All 10 tasks completed on `feat/followups-tickets`; every classification-driven review +chain resolved **Approved** (Tasks 5, 7, and 8 after fix rounds; Task 7's high-risk chain +took two rounds, ending with the mid-window auto-release race made structurally +unreachable — 30 s `BlockedDispatchSafetyNet` > every test's 20 s cancellation). The +final integration review returned *Ready with reservations*; every reservation was fixed +on-branch: I-2 (`6b5c737` — `ToolchainLinks.md` recorded `@latest` for plugins the +regeneration script hard-pins), I-3 (`db7b1db` — three doc sites claimed full-DN +`GroupToRole`/`GroupToTag` keys work; the Server-040 restoration proves they never match +the pre-stripped groups; rows were pre-existing on `main`), M-2/M-5 (`1605f54`), M-3 +(`fb68bdb`), M-4 (`753d070`), and M-1 was discharged by evidence (a +`--configuration-cache --rerun-tasks` discriminator proved `providers.exec` evaluates at +execution time after `generateProto`; no change needed). + +Verification (Task 9, windev CI clone at `5417222`, post-fix commits re-verified +individually by their owners at their pushed tips): full `slnx` 0W/0E; worker x86 **524 +passed / 11 skipped / 0 failed** — the suite's first fully green windev run; gateway +1155/1155 with no reruns; `check-codegen.ps1` **4/4 on Windows** under pwsh 7, and after +`8ae0c2f` also end-to-end under Windows PowerShell 5.1; Gradle 9.4.1 +`checkGeneratedClean` green on windev (and 9.5.1 on macOS); live MXAccess smoke 8/8; +toolchain pins now real on windev (`protoc-gen-go v1.36.11`, `protoc-gen-go-grpc 1.6.2`). + +Notable findings made along the way: + +- **Check 4 had a second Windows-only blocker** behind the `.exe` banner: Windows + PowerShell 5.1 strips embedded double quotes when marshaling native-exe args, breaking + the Python `grpcio-tools` version probe (`clients/python/generate-proto.ps1:39`). + Fixed (`8ae0c2f`) by swapping quote nesting; no other native-exe invocation in the + codegen scripts carries the pattern. +- **The ack leg is blocked by galaxy security configuration**, not a missing verb: the + probe fixtures carry `MxSecurityOperate`, and no `AlarmAckByName` overload can convey + an authenticated identity (inferred from mechanism, honestly caveated). `gateway.md` + and `docs/Grpc.md` now carry the "acceptance is not application" caveat on the + acknowledge RPC. +- **The `fca978d` tracking-marker sweep deleted substantive prose** along with markers in + at least one place (Server-040's precedence comment plus an operator-facing RDN + paragraph that was never part of the finding). A targeted re-read of that commit's + larger comment deletions is ticket-worthy (203 files swept; only the audited file was + examined). +- **`generateProto`'s up-to-date check does not notice an out-of-band deletion** of a + single generated file (protobuf-gradle-plugin behavior, orthogonal to this branch) — a + manually deleted generated file stays missing until a `.proto` change or + `--rerun-tasks` invalidates the task. +- Windev process notes: the CI clone sat on a detached HEAD, so bare `git pull` silently + no-ops (use `git pull origin `); concurrent MSBuilds on the clone can kill each + other's child nodes (MSB4166) — `-m:1` avoids it; ssh can transiently refuse with "Too + many authentication failures" under agent concurrency (back off and retry). + +Follow-ups recorded, not started (deliberately small): + +- The `fca978d` sweep re-read (above) — the one genuinely ticket-worthy item. +- `DashboardGroupTagMapping`'s class-level `` still describes the lookup as + "full DN first, leading-RDN fallback" — accurate mechanics, but it could point at the + short-name-keys consequence the inline comment now records. +- wwtools `mxa read` human-readable formatter throws `RuntimeBinderException` on a failed + read (`ReadCommand.cs:137`); `--llm-json` works. Different repo, noted here so it isn't + lost. + +Explicitly decided, not an omission: **`../scadaproj/CLAUDE.md` needs no update** — no +`.proto`, contract, command, or architecture fact the umbrella index records changed on +this branch. diff --git a/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json b/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json index d57e32a..a8501b0 100644 --- a/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json +++ b/docs/plans/2026-08-18-followups-and-tickets.md.tasks.json @@ -9,8 +9,8 @@ {"id": 6, "subject": "Task 6: Server-0xx doc-only resolution audit", "status": "completed", "commit": "d3ac527", "review": "Spec compliant; code review Approved (controller verified DashboardGroupRoleMapping change comment-only). 20 entries audited; 2 regressions re-fixed (Server-040 swept comment block incl. RDN pre-strip paragraph; Server-009 WAL/busy-timeout prose); 4 moot (target deleted); Server-038 closed-not-regressed. Systemic finding: fca978d tracking-marker sweep deleted substantive prose in at least one place"}, {"id": 7, "subject": "Task 7: WorkerPipeSessionTests deterministic failure — investigate + fix (windev)", "status": "completed", "commits": ["7da52b6", "462850a", "aaeb86b"], "review": "High-risk chain: spec compliant; code review Approved after two fix rounds (doc-comment attachment; watchdog-window headroom 200ms/1s + 2s window/30-frame floor; 30s BlockedDispatchSafetyNet > 20s CTS makes the mid-window auto-release race structurally unreachable). Root cause: test-harness defect — FakeRuntimeSession stamped LastStaActivityUtc only at construction, watchdog correctly faulted StaHung pre-dispatch. Test-only fix; windev worker suite 524 passed/11 skipped (was 523+1 fail)"}, {"id": 8, "subject": "Task 8: Ack-leg probe bounded unblock attempt (windev rig)", "status": "completed", "commits": ["d1ae43d", "bc22792"], "review": "Spec compliant; code review Approved after fixes (superseded bullet closed, summary hedged, gobject_id evidenced, ack-caveat added to gateway.md + Grpc.md as authorized scope extension). Outcome: ack unavailable-by-configuration (MxSecurityOperate classification, no credential-bearing AlarmAckByName overload — inferred, honestly caveated); no .Ack attribute exists; no non-interactive platform-side surface; GUID-across-ack row stays Open with reason updated"}, - {"id": 9, "subject": "Task 9: Windev toolchain pin + full verification", "status": "in_progress", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8]}, - {"id": 10, "subject": "Task 10: Bookkeeping — close the follow-ups record", "status": "pending", "blockedBy": [9]} + {"id": 9, "subject": "Task 9: Windev toolchain pin + full verification", "status": "completed", "verifiedAt": "5417222", "commit": "6b5c737", "result": "protoc-gen-go-grpc 1.6.2 installed; slnx 0W/0E; worker x86 524/11/0; gateway 1155/1155; check-codegen 4/4 (pwsh 7; and under PS 5.1 after 8ae0c2f fixed the quote-stripping probe bug it surfaced); Gradle 9.4.1 checkGeneratedClean green; live smoke 8/8; ToolchainLinks @latest rows corrected to pins (integration-review I-2)"}, + {"id": 10, "subject": "Task 10: Bookkeeping — close the follow-ups record", "status": "completed", "review": "None (trivial); closes the 2026-08-17 plan's follow-ups block with per-bullet commits, appends the as-built record (integration-review resolution: I-2 6b5c737, I-3 db7b1db, M-2/M-5 1605f54, M-3 fb68bdb, M-4 753d070, M-1 discharged by evidence; plus 8ae0c2f PS5.1 fix), final statuses here"} ], "lastUpdated": "2026-08-18" }