f2a422b02f
Both lists predate the scope rename and would mislead anyone creating a key:
CLAUDE.md's Authentication section still named the pre-rename scopes, and
docs/Authentication.md's ops.alice example passed 'read,write', which
GatewayScopes.ValidateScopes rejects outright. Same defect family as the
Build/Test/Run sample fixed in a5f843c.
Recorded as a follow-up: code review finding Server-012 claims it fixed the two
CLAUDE.md lists on 2026-05-18, but neither correction was present — a Resolved
finding is not re-examined, so the sibling Server-0xx doc resolutions want a
spot-check for the same pattern.
513 lines
29 KiB
Markdown
513 lines
29 KiB
Markdown
# Follow-up Closeout Implementation Plan
|
||
|
||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development
|
||
> to implement this plan task-by-task in this session.
|
||
|
||
**Goal:** Close the follow-ups recorded in `docs/plans/2026-08-17-deferred-closeout.md`
|
||
(as-built notes, "Follow-ups recorded, not started"): the feed-level alarm-truncation
|
||
signal on `StreamAlarms`/`AlarmFeedMessage`, the `ShowTagValues` redaction gaps
|
||
(alarms hub + `/browse` live values — TST-16 residual), the codegen guard's
|
||
one-directional Rust check (the recorded "no guard" note is stale — Check 3 exists),
|
||
the dashboard settings page's missing `GroupToTag`/`UntaggedSessionVisibility` rows,
|
||
the ApiKeysPage `DashboardTags` display/input gaps, and a bounded retry of the wnwrap
|
||
alarm probes using the secured-write verb the first attempt didn't use.
|
||
|
||
**Architecture:** Same two-phase posture as the prior three plans. Gateway-side work
|
||
builds and tests on macOS via `NonWindows.slnx`; windev (`ssh windev`, PowerShell, CI
|
||
clone `C:\build\mxaccessgw-ci`) runs the probe retry and final full-matrix verification.
|
||
**This plan touches `.proto` contracts** (Task 1) — contracts regeneration and the
|
||
five-client rollout (Task 2) follow the exact pattern Task 9 of the previous plan used.
|
||
The gateway-side truncation edge needs **no worker change**: `GatewayAlarmMonitor`
|
||
already receives `snapshot_truncated` on every reconcile; the feed frame is raised
|
||
gateway-side (per-reconcile fidelity, matching the operator-facing caveat's semantics).
|
||
|
||
**Tech stack:** .NET 10 gateway / protobuf contracts / five language clients / Blazor
|
||
Server dashboard / PowerShell codegen scripts / windev live rig.
|
||
|
||
**Branch:** `feat/followup-closeout` off `main` (`ab3ff16`).
|
||
|
||
---
|
||
|
||
## Ground rules for every implementer subagent
|
||
|
||
- NEVER run `git stash`, `git reset`, `git clean`, or `git checkout <sha/branch>`.
|
||
Commit with explicit **pathspecs on the commit itself**:
|
||
`git commit -m "..." -- <paths>` — never `git add -A`, `git commit -a`, or a bare
|
||
`git commit` after add (a concurrent task's staged files would be swept in).
|
||
- Build/test mutual exclusion: before any `dotnet build`/`dotnet test`, acquire the lock via
|
||
`mkdir /private/tmp/claude-501/-Users-dohertj2-Desktop-MxAccessGateway/f36938ae-bbca-4245-b5c9-fac512d69e22/scratchpad/buildlock`
|
||
(retry with backoff while it fails); `rmdir` it on ALL exit paths, including failures.
|
||
- `TreatWarningsAsErrors=true`, `Nullable=enable` — new warnings break the build; fix, don't suppress.
|
||
- Follow `docs/style-guides/CSharpStyleGuide.md`: file-scoped namespaces, `sealed` by
|
||
default, `Async` suffix, MXAccess-aligned names.
|
||
- Update affected docs in the same commit as the source change.
|
||
- MXAccess parity is the contract; never synthesize events. The feed truncation frame is a
|
||
gateway-status frame (like `provider_status`), not a synthesized MXAccess event.
|
||
- Never log secrets, API keys, credentials, or tag values.
|
||
- The `Files:` block is the scope contract. If the task can't be done inside it, that's a
|
||
plan defect — surface it, don't silently expand scope.
|
||
- On macOS build `NonWindows.slnx`; the x86 Worker and full `slnx` only build on windev.
|
||
- `git commit` hitting `index.lock` contention → wait 5 s and retry.
|
||
|
||
---
|
||
|
||
## Task 1: Feed-level alarm-truncation signal (proto + gateway)
|
||
|
||
**Classification:** high-risk
|
||
**Estimated implement time:** ~5 min
|
||
**Parallelizable with:** Task 3, Task 4, Task 5, Task 6
|
||
|
||
**Files:**
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto`
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Alarms/GatewayAlarmMonitor.cs`
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Alarms/IGatewayAlarmService.cs` (doc comment only)
|
||
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Alarms/` (existing `GatewayAlarmMonitor` test class(es))
|
||
- Docs: `gateway.md`, `docs/Grpc.md`, `docs/DesignDecisions.md`
|
||
- Build output: `src/ZB.MOM.WW.MxGateway.Contracts/Generated/` (regenerated, committed)
|
||
|
||
Do NOT touch `docs/GatewayConfiguration.md` or `docs/GatewayDashboardDesign.md` (owned
|
||
by Task 3 this wave) or any `clients/**` path (Task 2).
|
||
|
||
**Spec.**
|
||
|
||
Proto (`mxaccess_gateway.proto`) — additive only:
|
||
|
||
1. New message next to `AlarmProviderStatus` (`:1025-1030`):
|
||
|
||
```proto
|
||
// Feed-level snapshot-completeness status. Emitted once on StreamAlarms open
|
||
// (after the initial provider_status frame, before the cached active_alarm frames)
|
||
// so late joiners learn the current verdict, and again on every change of the
|
||
// truncation verdict observed at reconcile. Mirrors the per-record
|
||
// ActiveAlarmSnapshot.from_truncated_snapshot caveat at feed level so live
|
||
// consumers (lmxopcua, ScadaBridge) can reason about completeness without
|
||
// polling QueryActiveAlarms. Additive in v1; absent frames mean "not truncated"
|
||
// only for streams opened against gateways that emit the frame at open.
|
||
message AlarmSnapshotStatus {
|
||
// True while the monitor's cached active-alarm set derives from a truncated
|
||
// (capped) worker fetch — the set may be missing alarms. Distinct from
|
||
// provider degradation (AlarmProviderStatus.degraded).
|
||
bool truncated = 1;
|
||
}
|
||
```
|
||
|
||
2. New oneof case in `AlarmFeedMessage` (`:1006-1023`, next free field 5):
|
||
`AlarmSnapshotStatus snapshot_status = 5;`
|
||
|
||
Gateway (`GatewayAlarmMonitor.cs`):
|
||
|
||
- `ApplyReconcile` (`:632-694`) currently records `_snapshotTruncated = snapshotTruncated`
|
||
at `:691` with a comment saying truncation "needs no special handling here". Change:
|
||
detect the edge (old value ≠ new value) and, when it changes, broadcast
|
||
`new AlarmFeedMessage { SnapshotStatus = new AlarmSnapshotStatus { Truncated = snapshotTruncated } }`
|
||
via `BroadcastToAll` (`:720-732` — the provider-status precedent at `:531`). Match the
|
||
existing locking discipline exactly: compute/record under `_sync` the way
|
||
`ApplyProviderModeChangeAsync` (`:513-566`) does, broadcast the way it does. Update the
|
||
now-stale comment at `:627-631`.
|
||
- Late-joiner priming in `StreamAsync` (`:748-800`): after the `provider_status` frame
|
||
(`:782`) and before the cached `active_alarm` frames (`:786`), yield one
|
||
`snapshot_status` frame carrying the current `_snapshotTruncated`, read under the same
|
||
lock that snapshots `_alarms` (`:763-780`) so the flag and the alarm set are a
|
||
consistent pair.
|
||
- `ClearCache` (`:742`) resets `_snapshotTruncated` to false — decide and document
|
||
whether the reset emits a frame (it should: a monitor restart that drops a truncated
|
||
verdict is a completeness change subscribers must see; emit via the same edge path).
|
||
- `MxAccessGatewayService.StreamAlarms` is a pass-through — no change.
|
||
- `IGatewayAlarmService.SnapshotTruncated` doc comment (`:41-59`): note the new
|
||
feed-level frame so the two surfaces cross-reference.
|
||
|
||
Tests (extend the existing GatewayAlarmMonitor test class(es), matching their fake/
|
||
reconcile-driving idiom):
|
||
1. Reconcile flipping truncated false→true emits exactly one `snapshot_status` frame
|
||
(truncated=true) to an attached subscriber; a second identical reconcile emits none.
|
||
2. true→false emits the clearing frame.
|
||
3. A subscriber attaching while truncated=true receives, in order: `provider_status`,
|
||
`snapshot_status(truncated=true)`, cached `active_alarm`s, `snapshot_complete`.
|
||
4. A subscriber attaching while truncated=false receives `snapshot_status(truncated=false)`
|
||
at open (the baseline frame is unconditional).
|
||
|
||
Docs, same commit:
|
||
- `gateway.md` §"truncated-snapshot visibility" (`:243-262`): add the feed-level frame,
|
||
emission rules (open + edges), and ordering.
|
||
- `docs/Grpc.md` `:105-120` (provider_status oneof case rules): add the fifth case with
|
||
its emission rules; `:95-99` StreamAlarms handler contract.
|
||
- `docs/DesignDecisions.md` `:206-230` ("truncation is reported per record…", dated
|
||
2026-08-17): extend the entry — per-record stays for `QueryActiveAlarms` (bare stream,
|
||
no envelope); the live feed now carries the set-level signal as a status frame.
|
||
|
||
**Step 1:** Write the four monitor tests first (they fail: no `SnapshotStatus` case).
|
||
Building the Tests project will fail to compile until the proto field exists — so add
|
||
the proto change, rebuild Contracts (regenerates `Generated/`), then confirm the tests
|
||
fail for behavioral reasons (no frame emitted), not compile errors.
|
||
**Step 2:** Implement the monitor changes; run
|
||
`dotnet test src/ZB.MOM.WW.MxGateway.Tests/... --filter "FullyQualifiedName~GatewayAlarmMonitor"` → all pass.
|
||
**Step 3:** Full macOS build (`NonWindows.slnx`) 0W/0E + run the alarm-area test filter.
|
||
**Step 4:** Update the three docs.
|
||
**Step 5:** Commit with pathspecs (proto, Generated, monitor, interface, tests, 3 docs).
|
||
|
||
---
|
||
|
||
## Task 2: Five-client rollout of `snapshot_status`
|
||
|
||
**Classification:** standard
|
||
**Estimated implement time:** ~5 min
|
||
**Parallelizable with:** Task 7 (windev-only; disjoint files)
|
||
**Blocked by:** Task 1, Task 3, Task 4
|
||
|
||
**Files:**
|
||
- Modify: `clients/rust/protos/mxaccess_gateway.proto` (byte-identical refresh from Contracts)
|
||
- Regenerate: `clients/proto/descriptors/mxaccessgw-client-v1.protoset`
|
||
(via `scripts/publish-client-proto-inputs.ps1`), Go + Python generated bindings
|
||
(per-client `generate-proto.ps1`)
|
||
- Modify: `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:1552-1565`
|
||
(feed renderer — add BOTH the missing `ProviderStatus` case and the new `SnapshotStatus` case)
|
||
- Modify: `clients/go/cmd/mxgw-go/main.go:1101-1114` (`formatAlarmFeedMessage` — same two cases)
|
||
- Modify: `clients/rust/crates/mxgw-cli/src/main.rs:2233-2264` and `:2266+`
|
||
(`alarm_feed_message_summary` / `alarm_feed_message_to_json` — add `SnapshotStatus` arm)
|
||
- Modify: READMEs — `clients/dotnet/README.md` (~`:152-158`), `clients/python/README.md`
|
||
(~`:118-123`), `clients/rust/README.md` (~`:126`), `clients/go/README.md` (~`:148-154`),
|
||
`clients/java/README.md` (~`:120-126`): one paragraph each on the feed-level frame
|
||
- Modify: `docs/GatewayDashboardDesign.md:276` (AlarmsHub payload-case row: add the new case)
|
||
|
||
**Spec.** Follow the exact rollout Task 9 of the prior plan used (recorded at
|
||
`docs/plans/2026-08-17-deferred-closeout.md:300-318`): regenerate everything from
|
||
Contracts, refresh the Rust vendored proto byte-identically, then verify with
|
||
`pwsh scripts/check-codegen.ps1` (all four checks green — note Task 4 may have added a
|
||
reverse-direction sweep to Check 3 by the time this runs; it must pass too).
|
||
Python/Java CLIs render generic protobuf-JSON — README paragraph only, no code.
|
||
Java's `build/resources/**` proto copies are untracked build output — do not commit.
|
||
|
||
**Steps:** regenerate → renderer cases → build/test each touched client per the
|
||
CLAUDE.md verification matrix (dotnet client slnx + tests; `gofmt`+`go build`+`go test`;
|
||
`cargo fmt`+`check`+`test`+`clippy -D warnings`; python `pytest`; java `gradle test`) →
|
||
`pwsh scripts/check-codegen.ps1` green → READMEs + dashboard-design row → commit with pathspecs.
|
||
|
||
---
|
||
|
||
## Task 3: Complete `ShowTagValues` coverage — alarms hub + `/browse` (TST-16 residual)
|
||
|
||
**Classification:** high-risk (security posture)
|
||
**Estimated implement time:** ~5 min
|
||
**Parallelizable with:** Task 1, Task 4, Task 5, Task 6
|
||
|
||
**Files:**
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/AlarmsHubPublisher.cs`
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardLiveDataService.cs`
|
||
- Modify (if value formatting sits there): `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardTagValue.cs`
|
||
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/AlarmsHubPublisherTests.cs` (new)
|
||
- Test: existing `DashboardLiveDataService` test class (extend), else new file alongside
|
||
- Docs: `docs/GatewayConfiguration.md:195` (the residual sentence),
|
||
`docs/GatewayDashboardDesign.md` (AlarmsHub row `:276` — redaction note; do NOT edit the
|
||
payload-case list, Task 2 owns that), `archreview/remediation/00-tracking.md:231`
|
||
(TST-16 residual row), `archreview/2026-07-12/60-testing-docs-gaps.md` (residual lines)
|
||
|
||
Do NOT touch `gateway.md` (owned by Task 1 this wave) or `SettingsPage.razor`/
|
||
`EffectiveDashboardConfiguration` (Task 5).
|
||
|
||
**Spec.**
|
||
|
||
Seam A — `AlarmsHubPublisher` (`:14-68`): today it forwards the raw `AlarmFeedMessage`
|
||
verbatim (`:40-43`), leaking `current_value`/`limit_value` from BOTH the `transition`
|
||
arm (proto `:882`, `:886`) and the `active_alarm` snapshot arm (proto `:931-932`) to any
|
||
`/hubs/alarms` browser client. Inject the dashboard options (mirror how
|
||
`DashboardEventBroadcaster.cs:53` captures `_showTagValues`); when `ShowTagValues` is
|
||
false, deep-`Clone()` the message and clear the four value fields before `SendAsync`.
|
||
**Never mutate the source message** — it is fanned out to gRPC `StreamAlarms` subscribers
|
||
and the AlarmsPage status loop (`DashboardEventBroadcaster.RedactValues` at `:247-259`
|
||
is the exact pattern, including the clone-only rule). When `ShowTagValues` is true,
|
||
forward as today. Frames with no value fields (`snapshot_complete`, `provider_status`,
|
||
and Task 1's `snapshot_status` if already merged — handle via default: clone only when
|
||
the arm carries values) pass through untouched.
|
||
|
||
Seam B — `/browse` live values: `BrowsePage.razor:133,:144,:147` renders
|
||
`value?.ValueText` unconditionally, fed by `DashboardLiveDataService.cs:99-101` →
|
||
`DashboardTagValue.ValueText` (`DashboardTagValue.cs:41-45`,
|
||
`DashboardMxValueFormatter.FormatValue`). Redact **at the service boundary** (per the
|
||
original TST-16 recommendation, `archreview/remediation/60-testing-docs-gaps.md:346-352`):
|
||
when `ShowTagValues` is false, `DashboardLiveDataService` produces `ValueText` as the
|
||
literal `"[redacted]"` instead of the formatted value; quality/timestamp columns stay.
|
||
No `BrowsePage.razor` change should be needed — if one turns out to be, that's a plan
|
||
defect to surface. Keep the redaction decision in one place; don't duplicate the check
|
||
in the page.
|
||
|
||
Tests:
|
||
- `AlarmsHubPublisherTests` (new): reuse the `CapturingHubContext` shape from
|
||
`DashboardEventBroadcasterTests.cs:23` and the `DashboardSnapshotPublisherTests`
|
||
BackgroundService-driving template (fake stream service, internal ctor if needed).
|
||
Cases: ShowTagValues=false redacts both arms' value fields but keeps metadata;
|
||
source message not mutated; ShowTagValues=true passes values through;
|
||
valueless frames forwarded intact.
|
||
- `DashboardLiveDataService`: ShowTagValues=false → `ValueText == "[redacted]"`;
|
||
true → formatted value.
|
||
|
||
Docs: rewrite `docs/GatewayConfiguration.md:195` — the flag now covers the events hub
|
||
mirror, the alarms hub, and `/browse`; state the "[redacted]" rendering. Mark the TST-16
|
||
residual rows closed in both archreview files (match their existing status wording).
|
||
|
||
**Steps:** tests first (fail) → implement both seams → targeted filters
|
||
(`~AlarmsHubPublisher`, `~DashboardLiveDataService`, plus `~DashboardEventBroadcaster`
|
||
regression) → macOS build 0W/0E → docs → commit with pathspecs.
|
||
|
||
---
|
||
|
||
## Task 4: Codegen guard — reverse-direction Check 3 + stale-note correction
|
||
|
||
**Classification:** small
|
||
**Estimated implement time:** ~3 min
|
||
**Parallelizable with:** Task 1, Task 3, Task 5, Task 6
|
||
|
||
**Files:**
|
||
- Modify: `scripts/check-codegen.ps1:75-95` (Check 3)
|
||
- Docs: `docs/GatewayTesting.md:687-698` (Check 3 description),
|
||
`docs/ClientPackaging.md:199-213` (one-line note), `clients/rust/README.md:25-38`
|
||
(refresh-rule note if wording changes)
|
||
|
||
**Spec.** The recorded follow-up ("no guard keeps `clients/rust/protos/` in sync") is
|
||
stale — Check 3 (`:75-95`) already SHA-256-compares vendored↔canonical and runs in CI
|
||
(`.gitea/workflows/ci.yml:76-78`). The real gap: `:79` iterates only the **vendored**
|
||
dir, so a newly added canonical proto with no vendored copy passes silently and would
|
||
break a standalone crate build later. Add the reverse sweep: iterate
|
||
`$canonicalProtoDir`; any canonical `*.proto` with no same-named vendored counterpart →
|
||
append to `$failures` with a copy-to instruction, same reporting style as `:81-89`.
|
||
Keep all-checks-always-run behavior. Do NOT edit the closeout plan's follow-ups block —
|
||
Task 9 owns that file.
|
||
|
||
**Steps:** edit script → verify locally: `pwsh scripts/check-codegen.ps1` runs Check 3
|
||
both directions green (macOS pwsh is fine for Check 3/hashing; if Checks 2/4 can't run
|
||
locally, run Check 3's logic standalone and say so) → prove the new failure mode by
|
||
temporarily copying a scratch proto into the canonical dir under the scratchpad — NOT
|
||
into the repo — or by dry-running the loop against a temp dir pair → docs → commit with
|
||
pathspecs.
|
||
|
||
---
|
||
|
||
## Task 5: Settings page — show `GroupToTag` and `UntaggedSessionVisibility`
|
||
|
||
**Classification:** small
|
||
**Estimated implement time:** ~3 min
|
||
**Parallelizable with:** Task 1, Task 3, Task 4, Task 6
|
||
|
||
**Files:**
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Configuration/EffectiveDashboardConfiguration.cs:3-10`
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Configuration/GatewayConfigurationProvider.cs:58-65`
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SettingsPage.razor`
|
||
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Configuration/GatewayConfigurationProviderTests.cs` (new)
|
||
- Test: page-render test alongside the existing dashboard render tests
|
||
(`AlarmsPageTruncationBannerTests.cs` is the template)
|
||
|
||
**Spec.** Add `GroupToTag` (`IReadOnlyDictionary<string, string[]>` or matching shape)
|
||
and `UntaggedSessionVisibility` to the `EffectiveDashboardConfiguration` record; populate
|
||
in `GatewayConfigurationProvider` (`:58-65`) from `DashboardOptions.GroupToTag` (`:78`)
|
||
and `.UntaggedSessionVisibility` (`:85`). Neither is a secret — no masking. Rendering:
|
||
`GroupToTag` row next to "Dashboard role mapping" (`SettingsPage.razor:50-67`), same
|
||
`<ul><li><code>group</code> → tag1, tag2</li></ul>` idiom with the `(none configured)`
|
||
empty case; `UntaggedSessionVisibility` as a scalar row in the Dashboard cluster
|
||
(`:77-80`). Tests: provider projection test (new ground — assert both members copied,
|
||
including the empty-dictionary case); render test asserting a configured mapping and the
|
||
visibility value appear in the emitted markup (HtmlRenderer idiom, no bUnit).
|
||
|
||
**Steps:** tests first → implement → `--filter` the two new test classes → macOS build
|
||
0W/0E → commit with pathspecs. (No doc change: `docs/GatewayConfiguration.md` already
|
||
documents both options; the settings page is self-describing.)
|
||
|
||
---
|
||
|
||
## Task 6: ApiKeysPage — `DashboardTags` in ConstraintText + create-form input
|
||
|
||
**Classification:** standard
|
||
**Estimated implement time:** ~4 min
|
||
**Parallelizable with:** Task 1, Task 3, Task 4, Task 5
|
||
|
||
**Files:**
|
||
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor`
|
||
- Test: new render/behavior test alongside the dashboard render tests
|
||
- Docs: `docs/Authorization.md` (dashboard API-key management section, if it describes
|
||
the create form's constraint fields)
|
||
|
||
**Spec.** Two defects, one page:
|
||
|
||
(a) `ConstraintText` (`:504-533`) never lists `DashboardTags`, so a tags-only key falls
|
||
through to `string.Join` of an empty list → `""` → `DashboardDisplay.Text` renders `-`,
|
||
while a truly unconstrained key says `unconstrained` — two spellings of one meaning,
|
||
except they're NOT the same meaning: the tags-only key IS constrained
|
||
(`ApiKeyConstraints.IsEmpty` counts tags, `ApiKeyConstraints.cs:62-71`). Fix by listing
|
||
it: `AddList(parts, "dashboard_tags", constraints.DashboardTags)` alongside the other
|
||
snake_case labels (~`:516`). `AddList` (`:535-541`) preserves order — keep that.
|
||
|
||
(b) The create form's Constraints section (`:97-133`) has no dashboard-tags input, so a
|
||
tags-only key can't be created from the dashboard at all. Add a textarea matching the
|
||
subtree/glob fields exactly: bind to a new `string DashboardTags` on `ApiKeyCreateModel`
|
||
(`:551-589`), clear it in `Reset()` (`:575-588`), split with the existing `ParseList`
|
||
(`:543-549`), and attach in `TryBuildCreateRequest` (`:415-455`) via object-initializer
|
||
on the `new ApiKeyConstraints(...)` at `:444-452` (init-only property):
|
||
`new ApiKeyConstraints(...) { DashboardTags = ParseList(CreateModel.DashboardTags) }`.
|
||
Help text: comma/newline-separated, matched case-insensitively against
|
||
`Dashboard:GroupToTag` grants; empty = untagged (visibility per
|
||
`UntaggedSessionVisibility`). Mirrors the CLI's `apikey create --dashboard-tags`.
|
||
|
||
Tests (HtmlRenderer page-render idiom): tags-only key renders `dashboard_tags: …` (not
|
||
`-`); unconstrained key still renders `unconstrained`; create-model round-trip —
|
||
`TryBuildCreateRequest` with a tags input yields constraints whose `DashboardTags`
|
||
matches (if the method is private, follow whatever access pattern the page's existing
|
||
tests use; an `internal`-for-testing hook is acceptable only if the repo already does
|
||
that elsewhere — otherwise drive through the rendered form or refactor minimally).
|
||
|
||
**Steps:** tests first → implement → `--filter` new test class → macOS build 0W/0E →
|
||
docs (only if `Authorization.md` enumerates the form fields) → commit with pathspecs.
|
||
|
||
---
|
||
|
||
## Task 7: Wnwrap alarm-probe retry — secured-write path (windev)
|
||
|
||
**Classification:** standard (investigation; may legitimately end blocked again)
|
||
**Estimated implement time:** ~10 min wall (windev round-trips)
|
||
**Parallelizable with:** Task 2
|
||
**Blocked by:** none (but runs in Wave 2 to keep windev free for it)
|
||
|
||
**Files:**
|
||
- Modify: `docs/AlarmProbeFindings.md` (append the attempt record + findings)
|
||
- Modify (only if findings answer the questions): comment-level updates in
|
||
`src/ZB.MOM.WW.MxGateway.Worker/MxAccess/WnWrapAlarmConsumer.cs:425-435, :490-505, :655-670`
|
||
- NO probe code committed; NO `Skip=` flips committed; throwaway harness lives only in
|
||
the windev CI clone and is deleted after.
|
||
|
||
**Spec.** The 2026-08-17 attempt failed because it used plain `Write` against classified
|
||
alarm UDAs (`SecurityError detail=1008`). The worker and live harness already implement
|
||
the right verb pair end-to-end (`WorkerLiveMxAccessSmokeTests.cs:445-583` —
|
||
`AuthenticateUser` then `WriteSecured`; on this rig `AuthenticateUser("Administrator","")`
|
||
is known to resolve to user id 1, `:649`). Retry on windev (`ssh windev`, CI clone):
|
||
|
||
1. Pull the branch; build what the probe needs (worker x86 + integration tests).
|
||
2. Drive `AuthenticateUser("Administrator","")` → `WriteSecured(true)` against
|
||
`TestMachine_001.TestAlarm001` (env overrides
|
||
`MXGATEWAY_LIVE_MXACCESS_WRITE_SECURED_USER`/`_PASSWORD` exist if a real Galaxy
|
||
account is available — never echo credential values). A throwaway variant of
|
||
`WnWrapConsumerProbeTests` (`Worker.Tests/Probes/`, `Skip=null` locally only,
|
||
`GROUP` fixed to the findings run's `TestArea`, `MaxAlarmsPerFetch` droppable to 1–2)
|
||
is the vehicle.
|
||
3. If the secured write lands: answer **Q1** — GUID stability across `UNACK→ACK`
|
||
(`AlarmAckByGUID`) and across clear-then-re-raise; and **Q2** — `ALARM_RECORDS/@COUNT`
|
||
total-active vs records-in-reply under `maxAlmCnt` 1–2 with all three TestMachine
|
||
alarms active. Record both in `AlarmProbeFindings.md`; update the three
|
||
`WnWrapAlarmConsumer` comment blocks to "observed" with the answer. **Do not change
|
||
`IsTruncatedFetch` behavior in this task** — if Q2 says `@COUNT` is total-active,
|
||
record that an exact-detection follow-up is now unblocked; the heuristic change is
|
||
its own reviewed task later.
|
||
4. If still refused: append the attempt (verb used, identity resolved, status line —
|
||
no secrets), and record which unblock paths remain (flip script re-enable /
|
||
real Galaxy account / reclassification). That outcome completes this task.
|
||
|
||
**Steps:** windev session → probe → findings written → clean up throwaway files on
|
||
windev → commit (docs + any comment updates) with pathspecs from the Mac tree.
|
||
|
||
---
|
||
|
||
## Task 8: Windev full verification
|
||
|
||
**Classification:** small (no review — verification gate)
|
||
**Estimated implement time:** ~15 min wall
|
||
**Blocked by:** Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7
|
||
|
||
Pull branch into `C:\build\mxaccessgw-ci`, then: full `slnx` build 0W/0E → worker x86
|
||
tests → gateway tests → live MXAccess smoke (`MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`,
|
||
`~WorkerLiveMxAccessSmokeTests`) → `pwsh scripts/check-codegen.ps1` all green.
|
||
Known quirk: first build after pull may fail CS2001/CS0016 on stale Contracts obj —
|
||
clear obj/bin and rebuild, not a regression. Record all counts.
|
||
|
||
---
|
||
|
||
## Task 9: Bookkeeping — follow-ups closure + plan record
|
||
|
||
**Classification:** trivial
|
||
**Estimated implement time:** ~2 min
|
||
**Blocked by:** Task 8
|
||
|
||
**Files:**
|
||
- Modify: `docs/plans/2026-08-17-deferred-closeout.md` (follow-ups block `:419-434`):
|
||
annotate each closed item with its closing commit; REWRITE the stale Rust-guard bullet
|
||
to record that Check 3 already existed and only the reverse sweep was missing; leave
|
||
anything genuinely still open (e.g. probe questions if Task 7 ended blocked;
|
||
`IsTruncatedFetch` exactness if Q2 unblocked it) accurately stated.
|
||
- Modify: `docs/plans/2026-08-17-followup-closeout.md` (this file): as-built notes.
|
||
- Modify: `docs/plans/2026-08-17-followup-closeout.md.tasks.json`: statuses.
|
||
|
||
---
|
||
|
||
## Execution notes for the orchestrator
|
||
|
||
- Branch `feat/followup-closeout` off `main` before Task 1.
|
||
- Opus implementers per user instruction; reviewer chain per Classification
|
||
(high-risk = spec-reviewer serial then code-reviewer; standard = parallel pair;
|
||
small = code-reviewer only; trivial = none).
|
||
- Waves: **Wave 1:** 1, 3, 4, 5, 6 (files disjoint, including docs — ownership lines in
|
||
each task are the contract) · **Wave 2:** 2 (after 1+3+4), 7 (windev) · **Wave 3:** 8 →
|
||
final integration review → 9.
|
||
- Each implementer gets its full task text + the ground rules block.
|
||
- Doc-file ownership this wave matters more than usual: `gateway.md`→Task 1,
|
||
`GatewayConfiguration.md`+`GatewayDashboardDesign.md`→Task 3 (Task 2 later adds the
|
||
payload-case row), closeout-plan follow-ups→Task 9 only.
|
||
- Task 7/8 run against windev over `ssh windev` (PowerShell); psbridge is fallback.
|
||
- Do not merge to `main` without user instruction.
|
||
|
||
---
|
||
|
||
## As-built notes (execution record, 2026-08-18)
|
||
|
||
All 9 tasks completed on `feat/followup-closeout`; every classification-driven review
|
||
chain resolved **Approved**. The final integration review came back *Ready with
|
||
reservations* — all of its non-blocking findings were fixed in `c3c603f`, leaving no
|
||
open review item.
|
||
|
||
Verification: macOS `NonWindows.slnx` 0W/0E; gateway filtered suites green (dashboard
|
||
276/276, alarm suites green); `scripts/check-codegen.ps1` 4/4 on macOS. **The full
|
||
five-language client matrix ran locally for the first time** — dotnet 133 passed /1
|
||
skipped, Go clean, Rust 108, Python 168 passed /1 skipped, Java 131. Windev at `90331b6`:
|
||
full `slnx` 0W/0E (one transient MSB4166 node crash, clean on retry), worker x86 523
|
||
passed / 11 skipped plus **one pre-existing deterministic failure that reproduces on
|
||
`main`** (`WorkerPipeSessionTests.RunAsync_LongInFlightCommandThatKeepsPumping_DoesNotFaultAndDeliversReply`),
|
||
gateway 1151/1151 on rerun (the known windev load flake), live MXAccess smoke 8/8. The
|
||
comment- and prose-only commits landed after `90331b6` are unverified on windev by
|
||
design; they need only a cheap tip re-build there.
|
||
|
||
- **A session-limit outage interrupted four reviewers mid-run.** All four resumed
|
||
cleanly on retry; no review was lost or silently truncated.
|
||
- **Task 2 surfaced two plan defects, both absorbed rather than deferred.** The spec
|
||
claimed the Java CLI renders alarm frames as generic JSON; it is in fact an exhaustive
|
||
`switch` that does not compile without an arm for the new case. The spec's work list
|
||
also omitted the Java generated-bindings tree.
|
||
- **A reviewer's Minor-2 was refuted, and the doc defect behind it fixed instead.** The
|
||
finding assumed alarm records coexist after a re-raise; fetch evidence from the probe
|
||
rig shows the new GUID replaces the record. The prose that implied coexistence was
|
||
corrected rather than the code.
|
||
|
||
Follow-ups recorded, not started:
|
||
|
||
- `WorkerPipeSessionTests.RunAsync_LongInFlightCommandThatKeepsPumping_DoesNotFaultAndDeliversReply`
|
||
fails deterministically on windev **and on `main`** — pre-existing, needs its own
|
||
investigation.
|
||
- `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/java`'s `checkGeneratedClean` is dead under Gradle 9 (`Project.exec` was
|
||
removed); it needs `ExecOperations` injection to work again.
|
||
- `SettingsPage` renders every `EffectiveDashboardConfiguration` member except
|
||
`RecentFaultLimit` / `RecentSessionLimit` (pre-existing, predates this branch).
|
||
- The ack-leg probe stays blocked; unblock paths are in `docs/AlarmProbeFindings.md`.
|
||
- The dashboard `AlarmsPage` truncation banner is still poll-driven — it could consume
|
||
the new `snapshot_status` feed frame instead.
|
||
- **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`
|
||
Build/Test/Run `apikey` sample and the `CLAUDE.md` Authentication-section scope list.
|
||
Neither correction was present when this branch looked; both were re-fixed here, along
|
||
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.
|
||
|
||
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
|
||
unchanged — Task 1 added a message and a field inside an existing proto, not a new
|
||
contract file.
|