Files
mxaccessgw/docs/plans/2026-08-25-cycle-3-p2-followups.md
T
2026-08-25 10:40:55 -04:00

406 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Cycle 3 P2 And Follow-Up Implementation Plan
> **For Codex:** REQUIRED SUB-SKILL: Use `executeplan` to implement this plan task-by-task.
**Goal:** Close the 16 still-open cycle-3 P2 findings, correct WRK-34's tracker status, implement the three integration-gate follow-ups, and publish the completed client release train.
**Architecture:** Work proceeds in seven reviewable batches. Small tracker and documentation corrections land first; focused gateway, security, and secondary-contract fixes follow; client parity is completed before a single protobuf regeneration wave adds live alarm watch-list refresh and the remaining public comments; Windows CI is extended before the final windev gate; packages are published only from the integrated `main` release commit.
**Tech Stack:** .NET 10/C#, .NET Framework 4.8 x86, protobuf/Grpc.Tools, PowerShell 5.1/7, Go, Python/pytest, Rust/Cargo, Java/Gradle, Gitea Actions and package registries.
---
## Preconditions
- Work only in `.worktrees/c3-p2-followups` on `codex/c3-p2-followups`.
- Preserve the clean baseline recorded on 2026-08-20/25: `NonWindows.slnx` builds with zero warnings and the gateway suite passes 1183/1183.
- Follow red-green-refactor for every behavior change. Documentation-only changes use an explicit failing grep/check as their red gate.
- After every batch, update `archreview/2026-08-19/remediation/00-tracking.md` with status, commits, and exact verification output before committing.
- Worker/protobuf changes require windev x86 verification before Done.
### Task 1: Batch H — Tracker And Documentation Currency
**Findings:** WRK-34 tracker correction, GWC-36, IPC-38, TST-33, TST-34, TST-36
**Files:**
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/EventSubscriberReplayAttachment.cs`
- Modify: `docs/Grpc.md`
- Modify: `stillpending.md`
- Modify: `.gitea/workflows/ci.yml`
- Modify: `REVIEW-PROCESS.md`
- Modify: `CLAUDE.md`
- Modify: `docs/Sessions.md`
- Modify: `docs/DesignDecisions.md`
**Step 1: Capture the failing currency checks**
Run:
```bash
git grep -n '`event` scope' -- CLAUDE.md docs/
rg -n 'not safe to rely|Phase 5, not started|6 \[LiveMxAccessFact\]|wire behavior is unverified|still scheduled.*Phase 4' stillpending.md
rg -n 'regen-readme.py|check-code-reviews-readme' .gitea/workflows/ci.yml
rg -n 'DrainEvents' docs/Grpc.md
```
Expected: stale scope and pending claims are found; CI and `DrainEvents` checks find no intended entry.
**Step 2: Correct GWC-36 and TST-36**
Change `OldestAvailableSequence` to document retained-oldest or `highest observed + 1` when an empty ring has a gap, with `0` only when `Gap` is false. Replace prose references to the nonexistent ``event`` scope with ``events:read`` without renaming identifiers such as `GatewayScopes.EventsRead`.
**Step 3: Correct IPC-38 and TST-33**
Add a `DrainEvents` command row to `docs/Grpc.md` covering the 10,000-event count cap, byte cap, truncation diagnostic, drain-until-empty contract, and removal from the normal session event stream. Reconcile the five stale `stillpending.md` claims and stamp the file as a historical inventory whose live status comes from archreview trackers.
**Step 4: Wire TST-34**
Add this portable-job step next to the existing repository consistency checks:
```yaml
- name: Code-reviews README consistency
run: python code-reviews/regen-readme.py --check
```
Qualify the interpreter note in `REVIEW-PROCESS.md`: Windows may require `python`; macOS/Linux normally use `python3`, while CI uses its configured `python`.
**Step 5: Re-run the checks**
Run:
```bash
python3 code-reviews/regen-readme.py --check
git grep -n '`event` scope' -- CLAUDE.md docs/ || true
rg -n 'not safe to rely|Phase 5, not started|6 \[LiveMxAccessFact\]|wire behavior is unverified|still scheduled.*Phase 4' stillpending.md || true
rg -n 'DrainEvents' docs/Grpc.md
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
```
Expected: README check passes; stale greps return nothing; DrainEvents is documented; build has zero warnings/errors.
**Step 6: Update tracker and commit**
Mark the five P2 findings Done and WRK-34 Done (`3679e59`), record verification, then commit:
```bash
git add .gitea/workflows/ci.yml REVIEW-PROCESS.md CLAUDE.md stillpending.md docs/Grpc.md docs/Sessions.md docs/DesignDecisions.md src/ZB.MOM.WW.MxGateway.Server/Sessions/EventSubscriberReplayAttachment.cs archreview/2026-08-19/remediation/00-tracking.md
git commit -m "chore(GWC-36,IPC-38,TST-33,TST-34,TST-36): close cycle-3 currency gaps"
```
### Task 2: Batch I — Gateway Correctness And Security
**Findings:** GWC-33, GWC-35, SEC-38, SEC-39
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerClient.cs`
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerClientTests.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Alarms/GatewayAlarmMonitor.cs`
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Alarms/GatewayAlarmMonitorAttachOrderTests.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs`
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs`
- Modify: `docs/Authentication.md`
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
**Step 1: RED for GWC-33**
Add a worker-client test that stages an event, faults the client without disposing it, and asserts the aggregate queue-depth observable stops counting that client while the faulted object remains alive. Run:
```bash
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter FullyQualifiedName~WorkerClientTests
```
Expected: the new assertion fails because the registration is disposed only by `DisposeAsync`.
**Step 2: GREEN for GWC-33**
Make queue-depth registration disposal idempotent and call it on both terminal transitions and disposal. Do not clear the faulted client's diagnostic state or shorten `FaultedGraceSeconds` retention. Re-run the WorkerClient filter and commit the focused change.
**Step 3: RED/GREEN for GWC-35**
Add a monitor test whose first reconcile contains `ActiveAcked` and whose next reconcile carries a newer `Active` instance. Assert one `Raise` repair carrying the incoming instance identity. Verify red, then generalize the both-present state delta:
```csharp
AlarmTransitionKind kind = incoming.Value.CurrentState == AlarmConditionState.ActiveAcked
? AlarmTransitionKind.Acknowledge
: AlarmTransitionKind.Raise;
```
Keep the existing duplicate suppression keyed by original raise timestamp. Re-run `GatewayAlarmMonitorAttachOrderTests`.
**Step 4: RED/GREEN for SEC-38**
Add a limiter test that fills all 32 peer key-ID partitions, then attacks one valid victim key ID through the fallback from multiple peers. Assert the victim aggregate trips. Change resolution/recording so every validly-shaped presented key ID consults and feeds its aggregate even if its peer partition fell back; continue using the resolved partition key for the peer layer and the existing capped aggregate eviction for cardinality. Update class remarks and `docs/Authentication.md`.
**Step 5: RED/GREEN for SEC-39**
Add a test that records fallback failures, calls `Reset` with a null effective key ID, and asserts the fallback remains tracked. Tighten ownership:
```csharp
if (effectiveKeyId is not null
&& string.Equals(effectiveKeyId, partition.KeyId, StringComparison.Ordinal))
{
RemovePartition(partitionKey);
}
```
**Step 6: Verify, track, and commit**
Run:
```bash
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~WorkerClientTests|FullyQualifiedName~GatewayAlarmMonitorAttachOrderTests|FullyQualifiedName~ApiKeyFailureLimiterTests"
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
```
Expected: all filtered tests pass; zero warnings/errors. Update tracker with per-finding commits and counts.
### Task 3: Batch J — Secondary Contract Guards
**Findings:** IPC-36, IPC-37
**Files:**
- Create: `src/ZB.MOM.WW.MxGateway.Contracts/WorkerLaunchEnvironment.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerProcessLauncher.cs`
- Modify: worker consumers under `src/ZB.MOM.WW.MxGateway.Worker/Bootstrap`, `Ipc`, and `MxAccess`
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerProcessLauncherTests.cs`
- Create: `src/ZB.MOM.WW.MxGateway.Tests/Contracts/GalaxyContractParityTests.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Contracts/ZB.MOM.WW.MxGateway.Contracts.csproj`
- Modify: `docs/WorkerProcessLauncher.md`
- Modify: `docs/MxAccessWorkerInstanceDesign.md`
- Modify: `docs/Contracts.md`
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
**Step 1: RED for IPC-36**
Change the launcher test to enumerate the six constants expected on `WorkerLaunchEnvironment` and assert each is present in `ProcessStartInfo.Environment`. Run the test and confirm compilation fails because the shared class does not exist.
**Step 2: GREEN for IPC-36**
Create a net48-safe static class containing six `public const string` names. Replace both gateway and worker literals with these constants; remove the duplicate public constants from `WorkerProcessLauncher` unless compatibility requires obsolete aliases. Add a launch-environment table with variable, source configuration key, and worker default. Refresh Write/Write2 wording.
**Step 3: RED for IPC-37**
Add `GalaxyContractParityTests` that converts both loaded descriptors with `ToProto()`, clears `SourceCodeInfo`, normalizes `Options.CsharpNamespace` and file `Name`, then compares deterministic bytes. Before the assertion, mutate a clone in a local test helper to prove the comparison reports the differing symbol/message clearly; the real descriptor assertion must pass at HEAD.
**Step 4: Document and verify**
Record the parity guard in the Contracts project comment and `docs/Contracts.md`. Run:
```bash
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~WorkerProcessLauncherTests|FullyQualifiedName~GalaxyContractParityTests"
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
```
Then update tracker and commit IPC-36/37.
### Task 4: Batch K — Client Alarm-Feed Parity
**Findings:** CLI-47, CLI-48, CLI-49, CLI-50
**Files:**
- Modify: `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs`
- Test: `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs`
- Test/modify: `clients/go/cmd/mxgw-go/main.go`, `main_test.go`
- Modify: `clients/go/mxgateway/types.go`
- Test/modify: Java CLI and `MxGatewayCliTests.java`
- Modify/test: `clients/python/src/zb_mom_ww_mxgateway_cli/commands.py`, `clients/python/tests/`
- Modify: all affected client READMEs and `docs/CrossLanguageSmokeMatrix.md`
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
**Step 1: RED/GREEN for CLI-47 and CLI-48**
In .NET, Go, and Java tests, feed `provider_status { degraded: false }` and `snapshot_status { truncated: false }`; assert exact text rows and protobuf-JSON behavior. Confirm .NET text fails as `{}` and Go lacks a stream-alarm harness. Hand-format .NET text to match the explicit rows; add the missing Go and Java coverage without changing their correct output.
**Step 2: RED/GREEN for CLI-49**
Add a Go compile-time consumer test that declares variables of `mxgateway.AlarmProviderStatus` and `mxgateway.AlarmSnapshotStatus`. Confirm red, then add aliases and update the five-arm `AlarmFeedMessage` comment.
**Step 3: RED/GREEN for CLI-50**
Add Python CLI tests proving default `stream-alarms` emits provider status, snapshot status, cached alarms, and later live messages incrementally until timeout/cancellation; add a finite `--max-messages` test. Change the default to `0` (unbounded) and print rows as consumed instead of collecting first. Keep the maximum bound for positive finite values and ensure iterator cleanup occurs in `finally`.
**Step 4: Verify all clients**
Run the authoritative commands from `docs/ClientPackaging.md`:
```bash
dotnet test clients/dotnet/ZB.MOM.WW.MxGateway.Client.slnx
(cd clients/go && go test ./...)
(cd clients/python && python -m pytest)
(cd clients/rust && cargo test --workspace)
(cd clients/java && JAVA_HOME=/opt/homebrew/opt/openjdk@17 gradle test)
```
Revert only proven generator churn; do not revert source changes. Update tracker and commit CLI-47–50.
### Task 5: Batch L — Live Watch-List Refresh And Protobuf Wave
**Follow-ups:** complete GWC-37 application path, WRK-32 public comment
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto`
- Regenerate: Contracts and every client generated tree required by `docs/Contracts.md`
- Modify/test: worker alarm command executor, handler, failover/subtag consumers, and Worker.Tests
- Modify/test: `src/ZB.MOM.WW.MxGateway.Server/Alarms/GatewayAlarmMonitor.cs` and alarm monitor tests
- Modify: alarm, worker protocol, configuration, and client contract documentation
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
**Step 1: RED — contract round trip**
Add a protobuf round-trip test for:
```proto
message UpdateAlarmWatchListCommand {
repeated AlarmSubtagTarget watch_list = 1;
}
```
Add a new non-conflicting `MxCommand` oneof field and `MxCommandKind`, plus an explicit `fallback_enabled` field on `SubscribeAlarmsCommand`. Confirm the test cannot compile before editing the proto.
**Step 2: RED — worker routing and failure preservation**
Add Worker.Tests covering command routing, rejection without an active/fallback-capable subscription, replacement setup failure preserving the previous standby, primary-mode replacement keeping primary active, and standby-mode replacement keeping subtag active.
**Step 3: GREEN — worker hot-swap**
Implement `IAlarmCommandHandler.UpdateWatchList`. Retain the subscription expression. Refactor `FailoverAlarmConsumer` so standby replacement performs build/subscribe/attach before swap and disposes the old standby after swap. No COM operation may leave the STA. In auto mode, build the composite with an empty standby when `fallback_enabled` is true; leave an intentionally alarm-manager-only subscription unchanged.
**Step 4: RED — gateway periodic update**
Extend `GatewayAlarmMonitorProviderModeTests`/`AlarmFailoverEndToEndTests` with these scenarios:
- startup discovery fails/empty, later reconcile resolves targets, exactly one update command is sent;
- unchanged targets send no update;
- changed targets update the sent fingerprint only after an OK reply;
- worker rejection retains the prior fingerprint and degraded coverage reason;
- failover edge performs an immediate refresh before publishing status.
**Step 5: GREEN — gateway refresh path**
Reuse `ReconcileIntervalSeconds`. Resolve only when mode can use subtags, compare canonical full-target fingerprints, invoke the new command on material change/recovery, and reconcile after success. Do not add a timer or option.
**Step 6: Complete the shared proto comments**
In the same proto edit:
- replace the IPC-36 internal environment-variable name with `MxGateway:Worker:WriteCompletionWaitMilliseconds`;
- describe WRK-32 correlation as recency-based per handle and name the sequential timed-out-predecessor stale-callback case, not only concurrent writes.
**Step 7: Regenerate once and run portable checks**
Follow `docs/Contracts.md` exactly, then run:
```bash
pwsh scripts/check-codegen.ps1
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~ProtobufContractRoundTripTests|FullyQualifiedName~AlarmFailoverEndToEndTests|FullyQualifiedName~GatewayAlarmMonitorProviderModeTests|FullyQualifiedName~ClientProtoInputTests"
```
Run all five client suites again because generated trees changed.
**Step 8: Verify worker on windev**
Commit the candidate, then run the repository's windev test mode at that SHA. Required evidence: x86 Worker build success and Worker.Tests with zero failures. Only then mark the watch-list/WRK-32 follow-ups Done in the tracker and commit any evidence-only tracker update.
### Task 6: Batch M — Windows Gateway Suite In Nightly CI
**Finding:** TST-35
**Files:**
- Modify: `scripts/ci/windev-worker-ci.ps1`
- Modify: `.gitea/workflows/ci.yml` comments if needed
- Modify: `docs/GatewayTesting.md`
- Modify: `scripts/ci/README.md`
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
**Step 1: Add an acceptance check before behavior**
Document the expected `live` order and add a script-level/static assertion or dry-run seam showing gateway tests are absent before the change.
**Step 2: Extend live mode**
After Worker.Tests and full-slnx build, run:
```powershell
Invoke-Native 'gateway tests on Windows' {
dotnet test 'src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj' -c Release --no-build
}
```
Keep it before the opt-in live-MXAccess smoke, under the existing host lock. Log the actual result; do not enforce a historical count.
**Step 3: Verify on windev**
Run `live` mode at the committed SHA. Require Worker.Tests, gateway tests, full solution build, and live smoke to complete successfully. Update docs/tracker with date, SHA, counts, and duration; commit TST-35.
### Task 7: Batch N — Full Integration And Release
**Follow-up:** publish the five clients and Contracts
**Files:**
- Modify after publication: `docs/ClientPackaging.md`
- Modify: `archreview/2026-08-19/remediation/00-tracking.md`
**Step 1: Full local integration gate**
Run:
```bash
git diff --check
pwsh scripts/check-codegen.ps1
pwsh scripts/check-client-versions.ps1
python3 code-reviews/regen-readme.py --check
dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx
dotnet test src/ZB.MOM.WW.MxGateway.NonWindows.slnx --no-build --no-restore
```
Run all five client suites. Require zero unexpected working-tree changes.
**Step 2: Final windev gate**
Run `windev-worker-ci.ps1 -Mode live` at the exact candidate SHA. Record Worker.Tests, gateway tests, live smoke, and full solution build evidence.
**Step 3: Integrate before release**
Use `superpowers:finishing-a-development-branch`. Merge/push the verified branch to `main`, confirm CI green, and resolve the exact release commit SHA. Do not publish artifacts from an unmerged feature commit.
**Step 4: Preflight exact coordinates**
Confirm these are absent (HTTP 404) immediately before publishing:
- NuGet `ZB.MOM.WW.MxGateway.Client` 0.2.1
- NuGet `ZB.MOM.WW.MxGateway.Contracts` 0.2.1
- PyPI `zb-mom-ww-mxaccess-gateway-client` 0.2.1
- Cargo `zb-mom-ww-mxgateway-client` 0.2.1
- Maven `com.zb.mom.ww.mxgateway:zb-mom-ww-mxgateway-client` 0.2.2
- Git tag `clients/go/v0.2.1`
Abort rather than overwrite if any coordinate exists unexpectedly.
**Step 5: Publish from clean `main`**
Run the documented `scripts/pack-clients.ps1 -Publish` commands for .NET, Python, Rust, and Java with required credentials, then:
```powershell
pwsh scripts/tag-go-module.ps1 -Version v0.2.1 -Push
```
The Go tag must point to the exact integrated release commit.
**Step 6: Verify release and close tracker**
Query every exact registry coordinate for HTTP 200 and resolve the Go module/tag remotely. Update `docs/ClientPackaging.md` and the tracker with release SHA, coordinates, and verification date. Run the doc checks, commit the release record, and push it.
## Final Tasklist Exit Gate
- [ ] All 16 open P2 findings are Done with commit/test evidence.
- [ ] WRK-34 is corrected to Done (`3679e59`).
- [ ] Live watch-list refresh passes portable and windev tests.
- [ ] WRK-32 public proto comment is regenerated through every client surface.
- [ ] TST-35 nightly Windows gateway suite is proven on windev.
- [ ] Full NonWindows gateway suite and all five client suites pass.
- [ ] Worker.Tests and Windows gateway suite pass at the integrated release SHA.
- [ ] Five client releases plus Contracts are confirmed in registries; Go tag resolves.
- [ ] `archreview/2026-08-19/remediation/00-tracking.md` contains no stale Deferred row for completed work.