Files
mxaccessgw/archreview/2026-08-19/remediation/00-tracking.md
T
Joseph Doherty fa364ce139
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m23s
ci / java (push) Successful in 2m27s
ci / portable (push) Successful in 9m25s
chore(archreview): cycle-3 remediation integration gate recorded (gateway 1183/1183, windev worker 524/524)
2026-08-20 09:41:49 -04:00

16 KiB
Raw Blame History

MxAccessGateway — Remediation Plan & Tracking (2026-08-19 review)

Source review: ../00-overall.md (HEAD 8b3e3b3, committed as f4919ba). Generated 2026-08-20. Tracks the 34 cycle-3 IDs (GWC-31+, WRK-32+, IPC-33+, SEC-37+, CLI-46+, TST-31+) plus the tracker-housekeeping actions the overall asked for. Cycle-1/2 trackers remain the record for their own IDs.

Status legend: Not started · In progress · Done · Blocked (external) · Deferred (P2) · N/A.

Execution model

This plan is executed as seven parallel batches, each by one subagent working in an isolated git worktree on its own fix/c3-<batch> branch, merged back into main by the coordinator in the order listed (lowest conflict surface first). Every batch must: build the affected component(s) on macOS (NonWindows.slnx — the x86 worker cannot build here), run only the filtered tests it touches, update docs in the same commit, and cite finding IDs in commit messages (fix(IPC-34): …). Worker-side changes (batches E, F) are verified afterwards on windev via windev-worker-ci.ps1 -Mode test.

Model assignment rationale: opus for batches that change concurrency/authorization logic or require reasoning across several interacting files with regression tests; sonnet for contained, well-specified edits with a clear recipe (version bumps, doc reconciliation, a two-line worker fix); haiku for none — every batch either touches code invariants or needs judgement about stale prose, where the cost of a wrong mechanical sweep exceeds the saving.

Batch Branch Findings Model Why this model Status
A fix/c3-client-versions CLI-46 sonnet Mechanical five-client bump + README note + one script guard; recipe is fully specified Done
B fix/c3-glauth-doc TST-31 sonnet Reconcile a table against one TOML source of truth, re-derive one default, change an assert to a log Done
C fix/c3-tracker-hygiene GWC-31 + housekeeping (close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31) sonnet Cross-file comment/ID sweep needs care about what each ID means, but no logic changes Done (3619152)
D fix/c3-dashboard-acl SEC-40, SEC-41, GWC-38 opus Authorization semantics, fail-closed decisions, claims re-resolution; each needs a regression test Done (b177636 GWC-38, 2ef7b93 SEC-41+SEC-40, 8257a03 comment fixup; branch not merged/pushed)
E fix/c3-ipc-boundaries IPC-33, IPC-39, IPC-34, IPC-35 opus Changes the WorkerClient command lifecycle, the options validator, and the worker drain budget; race-sensitive Done (577a0db IPC-33, a71663a IPC-39, b27035b IPC-34, ab41362 IPC-35; branch fix/c3-ipc-boundaries, not merged/pushed) — worker half verified on windev (f2ab9cc, Worker.Tests 524/524)
F fix/c3-worker-residuals WRK-32, WRK-33 (+WRK-34 opportunistic) sonnet Small, precisely located worker edits + doc prose; cannot be built on macOS so the agent's job is a careful, minimal diff Done (windev x86 build + Worker.Tests 524/524 green on f2ab9cc)
G fix/c3-session-lifecycle GWC-32, GWC-34, GWC-37 opus Distributor/dispose lifecycle races and an alarm-monitor re-resolve path; needs lock-discipline reasoning Done (058d8d5, 282a0e7, 1932217)

Out of scope for this execution (recorded, not dropped):

ID Status Why
TST-32 Blocked (external) Gitea runner registration-token reset is a web-UI action on the Gitea admin page; no repo change. Operator to-do.
SEC-37 Blocked (external) Family-wide: TLS on the shared GLAuth lives in scadaproj/infra/glauth/ (separate repo + docker host). Plan separately; after it lands, move the deployed service to Production.
CLI-47/48/49/50, IPC-36, IPC-37, TST-33/34/35/36, GWC-33/35/36, SEC-38/39, IPC-38, WRK-34 Deferred (P2) Roadmap tier 10; WRK-34 may be picked up by batch F if trivial.

Batch specifications

A — CLI-46 client release-train bump (sonnet)

  • Bump .NET/Go/Python/Rust clients 0.2.0 → 0.2.1, Java 0.2.1 → 0.2.2 in every version-bearing file (pyproject.toml + version.py, Cargo.toml workspace + any version = consumers, Directory.Build.props/csproj <Version>, Java build.gradle/gradle.properties, Go module is tag-driven — update any Version const if one exists). Follow docs/ClientPackaging.md for the authoritative file list.
  • clients/python/README.md + docs/ClientPackaging.md: note that published pypi 0.2.0 has a broken connect() (event-loop binding, fixed by a8f86b5) and is superseded by 0.2.1.
  • Add a PR-time check to scripts/check-codegen.ps1 (or a new scripts/check-client-versions.ps1 wired where the other checks run): fail if any clients/<lang> source differs from its last publish tag while the version constant still equals that tag's version. Document in docs/ClientPackaging.md.
  • Verify: dotnet build clients/dotnet/...slnx; cargo check --workspace; python -m pytest (clients/python); JAVA_HOME=/opt/homebrew/opt/openjdk@17 gradle test then revert spurious generated-proto churn; go build ./... && go test ./....

B — TST-31 glauth.md + live-test credential (sonnet)

  • Reconcile glauth.md against ~/Desktop/scadaproj/infra/glauth/config.toml: ReadOnly gid, the pre-provisioned users table (drop readonly/readonly123, fix admin password), the worked example at :124. Keep the OPC-UA taxonomy note.
  • Re-derive MXGATEWAY_LIVE_MXACCESS_WRITE_SECURED_PASSWORD default in docs/GatewayTesting.md and the ?? "admin123" literal in WorkerLiveMxAccessSmokeTests.cs from the config; make the test log (not assert) whether AuthenticateUser succeeded.
  • Verify: dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx (IntegrationTests compiles on mac? if excluded, note it).

C — GWC-31 + tracker housekeeping (sonnet)

  • Sweep code comments citing unregistered GWC-30/GWC-31 (perf pass) in WorkerClient.cs, GatewayMetrics.cs, WorkerClientTests.cs, worker WorkerFrameReader/Writer.cs to PERF-nn keyed to task numbers in docs/plans/2026-08-15-perf-review-remediation.md; add a PERF-ID table to that plan. Do not touch the genuine GWC-30 (frame-reader prefix) citations if any exist.
  • archreview/REVIEW-PROCESS.md (or wherever the ID rule lives): state that GWC/WRK/IPC/SEC/CLI/TST prefixes are reserved for archreview registers.
  • Cycle-2 tracker (archreview/2026-07-12/remediation/00-tracking.md + domain docs): GWC-16 → Won't fix (rationale from cycle-3 10-gateway-core.md), IPC-08 → Done (7c1ea12, residual IPC-34), IPC-31 → Done (superseded by GWC-28), refresh TST-06/TST-17 rows, register WRK-31 (interim ID burned by e913dab) so it is never reused.
  • Verify: dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx; dotnet test --filter FullyQualifiedName~WorkerClientTests.

D — SEC-40, SEC-41, GWC-38 (opus)

  • SEC-41: DashboardSessionAcl re-resolves tags from LdapGroupClaimType claims via DashboardGroupTagMapping.MapGroupsToTags against current options (as HubTokenService does) instead of trusting login-stamped tag claims; keep stamped claims only as fallback when no group claims exist (document). Test: principal with stale tag claim + group whose grant was removed → denied.
  • SEC-40: decide tighten: AlarmsHub gets a dedicated policy requiring a real role claim (anonymous-loopback/Disabled bypass excluded) — and document in docs/GatewayDashboardDesign.md ACL section + docs/GatewayConfiguration.md GroupToTag row that alarm metadata remains galaxy-wide for authenticated Viewers (central monitor, no per-session provenance). Test for the policy.
  • GWC-38: GatewayBrowseScopeProvider throws RpcException(PermissionDenied) (and logs) when Current is null and API-key auth is enabled; unchanged when auth mode is Disabled. Test both branches.
  • Verify: build Server; dotnet test --filter "FullyQualifiedName~DashboardSessionAcl|FullyQualifiedName~AlarmsHub|FullyQualifiedName~BrowseScope" (adjust to real class names).

Outcome (done, branch fix/c3-dashboard-acl, commits b177636, 2ef7b93, 8257a03; not merged, not pushed). Decisions worth carrying forward:

  • SEC-41 fallback: the ACL re-resolves from mxgateway:ldap_group claims when the principal has any, and falls back to the stamped zb:dashboardtag claims only when it has none. That is exactly the hub-token principal — HubTokenService.Validate rehydrates roles and tags but not groups, so re-resolution already happened at mint. The fallback cannot re-open the cookie gap, because DashboardAuthenticator stamps both claim kinds from one group list.
  • SEC-40 requirement type: new AuthenticatedDashboardRoleRequirement + AuthenticatedDashboardRoleHandler and a MxGateway.Dashboard.Alarms policy. DashboardAuthorizationHandler is AuthorizationHandler<DashboardAuthorizationRequirement>, so it is never invoked for the new type — the same structural device the Secrets UI's secrets:manage already relies on. Both hub schemes are retained on the policy.
  • SEC-40 scope answer: the /alarms Blazor page carries the SAME policy (it reads the central monitor in process — gating only the hub left the disclosure one navigation away), and the side-rail Alarms link is wrapped in an AuthorizeView on that policy. AlarmsHubPublisher is unchanged and stays unfiltered by design: audience is decided at hub admission and the feed carries no per-session provenance.
  • GWC-38: PermissionDenied + warning only when Authentication:Mode=ApiKey; Mode=Disabled still returns unscoped. GalaxyRepositoryHostWiringTests phase 1 was changed to push an unconstrained identity rather than none — that is production's real unscoped case.
  • New/changed tests: AlarmFeedAuthorizationTests (new, 7), DashboardSessionAclTests (+5), GatewayBrowseScopeProviderTests (+2), SecretsNavRenderTests (+2, and it must register the new policy — an unregistered policy name throws inside AuthorizeView rather than denying). Filtered run over the touched classes: 92 passed / 0 failed; NonWindows.slnx builds zero-warning.

E — IPC-33, IPC-39, IPC-34, IPC-35 (opus)

  • IPC-33 option (a): GatewayOptionsValidator adds the upper coupling Worker:MaxMessageBytes ≤ Protocol:MaxGrpcMessageBytes + 2×reserve; worker ResolveDrainReplyByteBudget becomes MaxMessageBytes − 2×DrainReplyFrameHeadroomBytes (floor MaxMessageBytes/2); update docs/GatewayConfiguration.md, docs/WorkerFrameProtocol.md. Test: GatewayOptionsTests case for the new bound.
  • IPC-39: pre-check compares against MaxMessageBytes − MaxSequenceEncodingBytes (11); fix the comment direction.
  • IPC-34: caller-cancellation arms call TrySendCancelForAbandonedCommand (renamed) gated on removed, skipped when _stopCts is the source; new WorkerClientErrorCode.CallerCancelled replacing the GatewayShutdown mislabel, mapped to Cancelled in the gRPC layer; update docs/Grpc.md mapping line, gateway.md:913-919, docs/GatewayProcessDesign.md:596-602. Test: twin of the timeout test driven by the caller token.
  • IPC-35: ReadHandshakeEnvelopeAsync special-cases WorkerFault → WorkerClientException(WorkerFaulted, CreateWorkerFaultMessage(...)). Test: fake writes WorkerFault{MxaccessCreationFailed} instead of WorkerReady, category text surfaces.
  • Verify: build Server; dotnet test --filter "FullyQualifiedName~WorkerClientTests|FullyQualifiedName~GatewayOptions". Worker half → windev afterwards.

F — WRK-32, WRK-33 (sonnet)

  • WRK-33: ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask)); at WorkerPipeSession.cs:1235 (or catch-all-and-log in the observer).
  • WRK-32: one pumpStep() before capturing the CurrentVersion baseline in MxAccessCommandExecutor.cs; rewrite the executor doc comment (:996-1003) to state the recency-based contract; add the same to docs/MxAccessWorkerInstanceDesign.md write-completion section and the reply-field comment in mxaccess_gateway.proto (comment-only proto edit → regenerate all generated artifacts in the same commit, per the repo rule; if regen tooling is unavailable on macOS, skip the proto comment and say so).
  • WRK-34 if trivial: CreateFault(StaHung, "Watchdog", …) with the correlation id moved into the message.
  • Cannot build on macOS: keep diff minimal, net48-safe (no init-only/positional records). Verified on windev afterwards.

G — GWC-32, GWC-34, GWC-37 (opus)

  • GWC-32: DisposeAsync transitions _state to Closed under _syncRoot before completing channels (or if (_disposed || IsTerminalState()) return; in StageWorkerEvent); update the XML doc to name all three completion paths. Test: dispose a Ready client mid-dispatch → no ProtocolViolation fault/metric.
  • GWC-34: _eventDistributorDisposed flag set under _syncRoot when nulling; EnsureDistributorCreated throws SessionManagerException(SessionNotReady) when set. Test: attach after dispose → throws, no silent hang.
  • GWC-37: on ApplyProviderModeChangeAsync(toMode == Subtag) re-resolve the watch list and re-subscribe; additionally surface "zero targets resolved while mode can need them" on the provider status (Degraded + reason). Update docs/ alarm-subtag section. Test with the fake worker: GR discovery fails at start, succeeds at failover → watch list non-empty after failover.
  • Verify: build Server; dotnet test --filter "FullyQualifiedName~WorkerClientTests|FullyQualifiedName~GatewaySession|FullyQualifiedName~AlarmMonitor|FullyQualifiedName~AlarmWatchList".
  • Outcome (Done): 058d8d5 GWC-32, 282a0e7 GWC-34, 1932217 GWC-37 on fix/c3-session-lifecycle. Filtered run 176/176 green; NonWindows.slnx clean.
    • GWC-37 does not re-send the watch-list on the failover edge. The worker's AlarmCommandHandler.Subscribe rejects a second SubscribeAlarms while subscribed, and the only expressible replacement (UnsubscribeAlarms + SubscribeAlarms) rebuilds FailoverAlarmConsumer — restarting it on the primary that just failed and discarding the warm standby — so it would flap the feed rather than repair it. The re-resolve happens and any drift is reported on the same status frame as the failover; a monitor restart is the supported way to apply a new list. Adding a watch-list-update worker command is a possible future item, not done here.
    • GWC-37 changes IAlarmWatchListResolver.ResolveAsync to return AlarmWatchListResolution, and gives Degraded && Mode==ALARMMGR a real meaning (empty/undiscoverable standby) with a new dashboard label Alarm Manager (no subtag standby).
    • GWC-32's dispose path now also records WorkerStopped("dispose") via the existing once-latch, closing a workers_running gauge leak on the dispose-a-Ready-client path. GWC-12's double-dispose interlock got a natural home (the _disposed claim moved under _syncRoot) but is not claimed as fixed.

Merge order

C is merged last (its comment sweep touches the same files E and G edit). Order: A, B, F, D, G, E, C. After merge: full NonWindows build + one full gateway test run; then windev worker CI for E+F.

Integration gate (2026-08-20)

All seven branches merged into main (A ee68cc3, B f59c34a, C f2ab9cc, D 9f0c3fd, E cfbc995, F aa07497, G c6650c1). Merged tree: NonWindows.slnx 0 warnings; full gateway suite 1183/1183 passed; windev windev-worker-ci.ps1 -Sha f2ab9cc -Mode test: x86 Worker build succeeded, Worker.Tests 524 passed / 11 skipped / 0 failed. Follow-ups surfaced by the batches: a worker command to refresh a live alarm subscription's watch-list (would let GWC-37's re-resolve be applied, not just reported); WRK-32's proto reply-field comment deferred until the next proto regen wave; the five clients are bumped but not yet published.