dohertj2/mxaccessgw
1
0
Fork 0
Code Issues 14 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(IPC-23,IPC-24,IPC-25,IPC-32): proto-comment regen wave + codegen-freshness guards

Browse Source 
Proto comments (comment-only, no wire change):
- mxaccess_worker.proto GatewayHello.max_frame_bytes: every worker->gateway frame
  must serialize within the negotiated max; reply builders truncate (IPC-23).
- mxaccess_gateway.proto DrainEventsReply: count-cap + byte-cap, drain-until-empty
  caller contract (IPC-23).
- mxaccess_gateway.proto ReplayGap.oldest_available_sequence: empty-ring value is
  highest-observed+1, oldest-1 resume formula stays valid (GWC-25 deferred amendment).

Regen wave: Contracts/Generated (C# XML doc), rust vendored protos (byte-copy),
Go bindings (worker binding was genuinely stale - lacked MaxFrameBytes entirely),
Python worker _pb2 (real descriptor delta), Java aggregates (javadoc, zero
protobuf-version churn under the pinned toolchain), client descriptor set.

IPC-24: pinned Java toolchain regenerates with no gencode-version churn, so the
unconditional churn-revert step in ci.yml is a fossil - deleted it; git diff is
now a true message-level drift gate for the single-file Java aggregates.

IPC-25: pin protoc-gen-go v1.36.11 / protoc-gen-go-grpc 1.6.2 in the Go generate
script (+ fix a latent pwsh-7 parse bug); add Check 4 to check-codegen.ps1
(regenerate Go+Python bindings, fail on diff, tool-missing fails not skips); add
the pinned-generator installs to the portable CI job.

IPC-32: relabel check-codegen banners 1/4..4/4 (folded into the Check 4 edit).

Docs: ClientProtoGeneration.md, Contracts.md, GatewayTesting.md, build.gradle
checkGeneratedClean caveat. Tracking: IPC-23/24/25/32 -> Done, GWC-25 proto note
resolved, change-log 2026-08-07.
This commit is contained in:
Joseph Doherty
2026-08-07 07:32:22 -04:00
parent 97f79e79ef
commit eacdd2d453
22 changed files with 469 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/2026-07-12/remediation/00-tracking.md
+5 -4
View File
@@ -44,12 +44,12 @@ Sequenced by cluster; a cluster is one change set.
| CLI-35 | Medium | S | GWC-25 (coord) | Done | Python CLI `stream-events` crashes on a ReplayGap |
| CLI-36 | Medium | S | GWC-25 (coord) | Done | Go CLI `stream-events` silently destroys the ReplayGap signal |
| WRK-21 | Medium | M | owns IPC-23 fix; WRK-28 same batch | Done | DrainEvents bound is count-based only; oversized reply kills the session and loses the drained events |
| IPC-23 | Medium | S | WRK-21 | In progress — mechanics landed with WRK-21; proto-comment/doc wave pending | DrainEvents contract requirements (reply fits negotiated max, no event loss, drain-until-empty) + proto-comment/doc wave |
| IPC-23 | Medium | S | WRK-21 | Done | DrainEvents contract requirements (reply fits negotiated max, no event loss, drain-until-empty) + proto-comment/doc wave |
| IPC-30 | Low | M | WRK-21 (same batch) | Done | Oversized event frame stays session-fatal by design, but the death becomes structured (fault frame + logged identity) |
| SEC-31 | Medium | M | — | Done | Failure limiter partitions on attacker-controlled key id and blocks before verification (lockout DoS) |
| SEC-32 | Low | S | SEC-31 | Done | Failure-limiter LRU flushable by junk-token spray; token prefix never validated |
| IPC-24 | Medium | S | — | Not started | CI's unconditional Java churn-revert masks real drift |
| IPC-25 | Medium | M | — | Not started | Regenerate stale Go/Python worker bindings + add binding-freshness guard (Check 4) to check-codegen.ps1 |
| IPC-24 | Medium | S | — | Done | CI's unconditional Java churn-revert masks real drift |
| IPC-25 | Medium | M | — | Done | Regenerate stale Go/Python worker bindings + add binding-freshness guard (Check 4) to check-codegen.ps1 |
## Finding registers by domain
@@ -93,7 +93,7 @@ Full design + implementation for each row lives in the linked domain doc under i
| IPC-29 | Low | — | S | WRK-26 (discharged by) | Not started | WorkerFrameProtocol.md missing write-scheduling/sequencing section |
| IPC-30 | Low | P0 | M | WRK-21 (same batch) | Done | Oversized event frame: keep session-fatal, make the death structured |
| IPC-31 | Info | — | — | — | N/A | Gateway creation-time sequence stamping accepted; diagnostic-only, decision recorded |
| IPC-32 | Info | — | S | IPC-25 (folded in) | Not started | check-codegen banner relabel 1/4…4/4 |
| IPC-32 | Info | — | S | IPC-25 (folded in) | Done | check-codegen banner relabel 1/4…4/4 |
### Security & dashboard — [40-security-dashboard.md](40-security-dashboard.md)
@@ -174,3 +174,4 @@ Sequence these together rather than piecemeal — several are one change set spa
| 2026-08-07 | **GWC-28, GWC-29, GWC-30, TST-28 → `Done`** (branch `fix/gwc-28-29-30-polish`). GWC-28: `WorkerClient.WriteLoopAsync` now stamps `envelope.Sequence = unchecked(++_nextSequence)` immediately before `_writer.WriteAsync`, and `CreateEnvelope` leaves it unset; `_nextSequence` dropped from `long` + `Interlocked` to a plain `ulong` touched only by the write loop (the channel's single consumer, `SingleReader = true`), so wire order and sequence order are the same thing by construction. Mirrors the worker's WRK-04 stamping, which the gateway half had never received; `gateway.md`'s envelope-sequence rule now states that both sides stamp at write inside their single write path and that inbound enforcement (still open, old **GWC-10**) would rely on it. New `WorkerClientTests.ConcurrentInvokesEmitStrictlyIncreasingSequencesOnTheWire` (32 parallel invokes, sequences asserted strictly increasing in wire order) failed 3/3 pre-fix. GWC-29: added `MxAccessGrpcMapper.MapCommand(MxCommand)`; `Invoke` no longer deep-clones the whole `MxCommandRequest` just to overwrite and discard its command. The one clone inside `MapCommand` stays and is documented as required — `commandToInvoke` may be the gRPC-owned `request.Command` and is read again after dispatch by `TrackCommandReply`, so it is what keeps `CreateCommandEnvelope`'s no-aliasing invariant true. New `MxAccessGrpcMapperTests.MapCommandFromCommandClonesPayload` (isolation + both overloads equal under a `FakeTimeProvider`). GWC-30: `WorkerFrameReader` reuses a per-instance `_lengthPrefix` scratch buffer instead of allocating 4 bytes per frame, with a class remark that `ReadAsync` is not reentrant (single read loop per `WorkerClient`; handshake reads complete before the loop starts); guarded by new `WorkerFrameProtocolTests.ReadAsync_WithMultipleFramesOnOneReader_ParsesEveryFrame` (5 frames, varying payload lengths, one reader). TST-28: new `[Theory] WorkerClientTests.StartAsync_SendsGatewayHelloWithConfiguredMaxFrameBytes` over the default and a 2 MiB override via `FakeWorkerHarness.CreateConnectedPairAsync(maxMessageBytes:)` — test-only, and the mutation check (hard-code `MaxFrameBytes = 0`) failed both cases before being reverted. Verification: `NonWindows.slnx` 0 warnings/0 errors; `WorkerClientTests` 25 passed, `WorkerFrameProtocolTests` 11 passed, `MxAccessGrpcMapperTests` 6 passed, `MxAccessGatewayService*` 29 passed, full gateway suite 844 passed / 0 failed (`TMPDIR=/tmp` on macOS). |
| 2026-08-07 | **WRK-21 + WRK-28 + WRK-23 + IPC-30 → `Done`** (branch `fix/wrk-21-drain-cluster`, commits `33ba612` + test-fixture follow-ups `7c2eaf0`/`a256560`). WRK-21: `MxAccessEventQueue` gains a byte-budgeted `Drain(maxEvents, maxTotalBytes)` returning the new `WorkerEventDrainResult`, sizing inside the queue lock so an event that will not fit is never dequeued; `CreateDrainEventsReply` budgets against the negotiated frame max less a 64 KiB wrapper reserve and reports truncation through the existing `DiagnosticMessage` (no proto change), satisfying IPC-23 R1–R3; both reply-write seams (`HandleControlCommandAsync`, `ProcessCommandAsync`) now catch `MessageTooLarge` and answer the correlation with an `InvalidRequest` reply instead of unwinding/faulting the session. WRK-28: the 10,000 ceiling moved to `GatewayContractInfo.MaxDrainEventsPerCommand`, referenced by the gateway validator and the worker clamp (C# const, no `.proto` change). WRK-23: `WorkerFrameWriter` peek-stamps then commits `Sequence` only immediately before the stream write, so rejections leave no wire gap. IPC-30: an oversized event frame stays session-fatal but writes a `PROTOCOL_VIOLATION` `WorkerFault` with `command_method = EventDrain` naming family/handles/sequence/sizes (never the value) before exiting. Docs same commit: `MxAccessWorkerInstanceDesign.md`, `WorkerFrameProtocol.md`, `gateway.md`. **IPC-23 → `In progress`** — mechanics landed here; the proto-comment/doc wave (and its regen fan-out) is still pending and must not be folded into this branch. **Evidence** — macOS: `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` 0 warnings/0 errors, `dotnet test …MxGateway.Tests --filter FullyQualifiedName~MxAccessGrpcRequestValidator` 4/4 passed. windev (`scripts/ci/windev-worker-ci.ps1 -Sha a2565604 -Mode test`, 2026-08-07 06:47): x86 Worker build 0 warnings/0 errors, `Worker.Tests` **367 passed / 0 failed / 11 skipped** (skips are the live-MXAccess/dev-rig opt-ins), script exit 0. **Harness note:** `PipePair` runs both pipe ends in one process with blocking `FlushFileBuffers` per frame, so it wedges on multi-MB frames or after ~85 large round trips; the pipe tests therefore negotiate a 128 KiB frame maximum and walk 1,000 events to empty, while the full 10,000-event drain-to-empty no-loss proof runs at the queue layer (`MxAccessEventQueueTests`). |
| 2026-08-07 | Code-review follow-ups on the same branch (commit `6bc3f9b`). (1) **Important** — `ResolveDrainReplyByteBudget` was a step, not a floor: just above the 64 KiB reserve the budget collapsed to a few bytes (exactly 1024 at the validator floor `MaxMessageBytes = 1024 + 64 KiB`), so a byte-heavy `DrainEvents` truncated on every call and the drain-until-empty loop never terminated. Now `Math.Max(frameMax - reserve, frameMax / 2)` — monotonic, never below half the frame max. New test `WorkerPipeSessionTests.DrainEvents_AtValidatorFloorFrameMax_MakesProgressAndTerminates` drives a byte-heavy queue at the exact validator floor and asserts drain-to-empty with no head reported oversized. (2) **Hardening** — the reply-too-large fallback write is now itself size-guarded (`WriteReplyTooLargeFallbackAsync`, shared by the control and STA reply seams) so a pathologically tiny negotiated max below the gateway floor (the WRK-24 gap) cannot make even the backstop session-fatal; log-and-swallow, comment points at WRK-24. (3) **Comment** — corrected the `RepeatedFieldOverheadBytes` docs: `WorkerEvent.CalculateSize()` already includes the event's tag+length, so the 8 bytes is pure slack, not wrapper compensation. **Evidence** — macOS build 0/0, validator filter 4/4. windev (`windev-worker-ci.ps1 -Sha 6bc3f9b -Mode test`, 07:07): x86 Worker build 0/0, `Worker.Tests` **368 passed / 0 failed / 11 skipped**, script exit 0. (An earlier run of the same SHA flaked on the pre-existing `RunAsync_WhenStaActivityIsStale_WritesWatchdogFault` — a 5 s CTS timeout under first-run load, untouched by this change; it passed on the clean re-run and in both prior full runs.) |
| 2026-08-07 | **IPC-23 + IPC-24 + IPC-25 + IPC-32 → `Done`; GWC-25 deferred proto-comment resolved** (branch `fix/ipc-24-25-codegen`). The proto-comment/doc + regen wave for the P0 codegen-freshness cluster. **Proto comments (comment-only):** `mxaccess_worker.proto` `GatewayHello.max_frame_bytes` gained the every-frame-must-fit / reply-builders-truncate sentence (IPC-23); `mxaccess_gateway.proto` `DrainEventsReply` gained the count-cap+byte-cap / drain-until-empty comment (IPC-23); `mxaccess_gateway.proto` `ReplayGap.oldest_available_sequence` gained the empty-ring value definition (`highest observed + 1`, `oldest − 1` formula stays valid) resolving GWC-25's deferred amendment. **Regen wave:** `Contracts/Generated/{MxaccessGateway,MxaccessWorker}.cs` (XML-doc only, no descriptor delta), `clients/rust/protos/{mxaccess_gateway,mxaccess_worker}.proto` byte-copied, Go `internal/generated/{mxaccess_gateway,mxaccess_worker}.pb.go` (worker binding was genuinely stale — it lacked the whole `MaxFrameBytes` field/accessor/rawDesc, now refreshed; gateway.pb.go got the new doc comments), Python `generated/mxaccess_worker_pb2.py` (real descriptor delta — `max_frame_bytes` field + cascaded offsets), Java aggregates `MxaccessGateway.java`/`MxaccessWorker.java` (javadoc from the new proto comments — **zero** protobuf-version churn under the pinned toolchain), and the client descriptor set `mxaccessgw-client-v1.protoset`. **IPC-24:** regenerating Java on the pinned toolchain (grpc 1.76.0 / protobuf 4.33.1, Homebrew JDK 17) produced only the legitimate javadoc delta with no `validateProtobufGencodeVersion`/`major=/minor=/patch=` churn — so the preferred path was taken: the unconditional churn-revert step and its comment were **deleted** from `.gitea/workflows/ci.yml` (the `git diff --exit-code` gate is now a true message-level drift gate for the single-file Java aggregates). **IPC-25:** pinned the Go generators in `clients/go/generate-proto.ps1` (`protoc-gen-go` v1.36.11 assert, `protoc-gen-go-grpc` 1.6.2 assert, protoc warn-only; also fixed a latent `(if …)`→`$(if …)` parse bug that broke the script under pwsh 7); added **Check 4** to `scripts/check-codegen.ps1` (regenerate Go+Python bindings, fail on any diff, tool-missing FAILS not skips) and relabeled the banners `1/4`…`4/4` + header comment (**IPC-32** folded in); added the pinned-generator installs to the `portable` CI job before the codegen step. **Docs same commit:** `ClientProtoGeneration.md` (pinned-versions table + Go/Python Check-4 guard + Java-revert-deleted note), `Contracts.md` (four-check enumeration), `GatewayTesting.md` (java job + four-check prose), `clients/java/zb-mom-ww-mxgateway-client/build.gradle` checkGeneratedClean caveat. **IPC-23 DrainEvents-truncation doc deviation:** `docs/Grpc.md` has no DrainEvents row (it documents only the seven public RPCs; DrainEvents is a worker diagnostic), and `docs/WorkerFrameProtocol.md` (owned by the parallel doc batch) already carries the byte-cap/drain-until-empty prose via WRK-21 — so no Grpc.md edit was made. **Evidence (macOS):** `pwsh scripts/check-codegen.ps1` all four checks green (banners `1/4`…`4/4`); `dotnet build …NonWindows.slnx` 0 warnings/0 errors; `dotnet test …Tests --filter ~ClientProtoInputTests` green; `grep -c max_frame_bytes` → Go worker 2, Python worker 1 (both non-zero, were 0); `clients/go` gofmt clean + `go build` + `go test` ok; `clients/python` `pytest` 163 passed/1 skipped; Java `gradle generateProto` clean under JDK 17. **Negative-path proof:** added a scratch field to `mxaccess_worker.proto`, regenerated only `Contracts/Generated/`, reran `check-codegen.ps1` and confirmed **Check 4 fails naming both `clients/go/internal/generated` and `clients/python/…/generated`**, then reverted. **Not published** (CLI-39 gates Go/Python republish). |