Joseph Doherty
59856b8c63
Adds the 2026-07-08 architecture review (00-overall + six domain reports) and a remediation/ tree: one design+implementation doc per domain covering every finding, plus 00-tracking.md as the master progress tracker. - 153 findings with stable IDs (GWC/WRK/IPC/SEC/CLI/TST), each with design rationale, implementation steps, tests, docs, and verification. - Tracker rolls findings up by severity and P0/P1/P2 roadmap tier, records cross-cutting clusters and per-finding status (all Not started). - Planning docs only; no source changes.
148 lines
26 KiB
Markdown
148 lines
26 KiB
Markdown
# Language Clients — Architecture Review
|
||
|
||
## Scope & method
|
||
|
||
This review covers the five official client libraries and their CLIs: `clients/dotnet`, `clients/go`, `clients/java`, `clients/python`, `clients/rust`, plus `clients/proto` and the client-facing docs (`docs/ClientLibrariesDesign.md`, `docs/ClientPackaging.md`, `docs/ClientBehaviorFixtures.md`, `docs/CrossLanguageSmokeMatrix.md`). All clients consume the shared protos in `src/ZB.MOM.WW.MxGateway.Contracts/Protos`. The review is static (macOS tree, no builds run): every handwritten source file in each client library was read, CLIs were spot-checked, generated directories were verified to exist and were excluded from style review. All findings cite `path:line` in the repository root. The Java client was reviewed specifically for JDK 17 compatibility following the `feat/jdk17-client-retarget` work.
|
||
|
||
## Executive summary
|
||
|
||
- All five clients implement the same core shape — connect, open/close session, typed Register/AddItem/Advise/Write helpers, raw `Invoke` escape hatch, event streaming with a resume cursor, alarm RPCs, and Galaxy browse with a lazy walker — and all five load the shared behavior fixtures in tests. Overall maturity is high and unusually uniform for a five-language surface.
|
||
- The Java JDK 21→17 retarget is correct in build config (`toolchain 17` + `options.release = 17`) and no JDK 21+ APIs remain in source; however three docs still say Java 21 (`clients/java/README.md:354`, `clients/java/JavaClientDesign.md:34`, `docs/ClientPackaging.md:193`), violating the repo's docs-in-same-commit rule.
|
||
- The published Rust crate cannot build outside this repository: `build.rs` resolves protos at `../../src/ZB.MOM.WW.MxGateway.Contracts/Protos` and packaging runs `cargo package/publish --no-verify`, so the defect is never caught. This is the most serious packaging finding.
|
||
- The Go `Session.Events()` path silently cancels the stream and closes the results channel when its 16-slot buffer fills — the consumer cannot distinguish a backpressure disconnect from normal stream end. This is the most serious runtime finding.
|
||
- The Rust client validates only `protocol_status` on `invoke`; it never inspects `hresult` or the `MXSTATUS_PROXY` array, so a reply with an OK protocol envelope but failing per-item statuses reads as success — every other client raises. Conversely, .NET/Go/Java treat *any* nonzero HRESULT as failure (positive success codes like `S_FALSE` misclassified); only Python uses the correct `hresult < 0` COM semantics.
|
||
- No client exposes typed helpers for `WriteSecured`/`WriteSecured2` (single-item), `AuthenticateUser`, `ArchestrAUserToId`, `AdviseSupervisory`, `AddBufferedItem`, `SetBufferedUpdateInterval`, `Suspend`, or `Activate`, even though the wire contract supports all of them (`src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto:150-160`). The secured-write parity path is reachable only via raw `Invoke`.
|
||
- Reconnect-with-replay is supported everywhere via `after_worker_sequence`, and four of five clients can re-attach a session wrapper to an existing session id; .NET cannot (its `MxGatewaySession` constructor is internal with no factory).
|
||
- Version constants drift in three clients: Go reports `0.1.0-dev`, Rust `version.rs` says `0.1.0-dev` while `Cargo.toml` says `0.1.2`, Python `version.py` says `0.1.0` while `pyproject.toml` says `0.1.2`. Only Java (0.2.0) is consistent.
|
||
- TLS posture is intentionally lenient but inconsistent across languages: .NET/Go/Java skip verification by default, Python does a blocking trust-on-first-use certificate pin, Rust is strict pin-only. The docs acknowledge this, but the Go CLI cannot even opt into strict validation.
|
||
- `docs/ClientPackaging.md` has drifted badly from the Python client's actual naming (package, module paths, CLI module) and the .NET solution filename.
|
||
|
||
## Cross-client parity matrix
|
||
|
||
| Capability | .NET | Go | Java | Python | Rust |
|
||
|---|---|---|---|---|---|
|
||
| Open/close session (typed + raw) | Yes | Yes | Yes | Yes | Yes |
|
||
| Re-attach wrapper to existing session id | **No** (internal ctor) | Yes (`NewSessionForID`) | Yes (`forSessionId`) | Yes (ctor) | Yes (`client.session()`) |
|
||
| Register / AddItem / AddItem2 / Advise / UnAdvise / RemoveItem | Yes | Yes | Yes | Yes | Yes |
|
||
| Unregister typed helper | **No** | Yes | Yes | Yes | **No** |
|
||
| Write / Write2 typed | Yes | Yes | Yes | Yes | Yes |
|
||
| WriteSecured / WriteSecured2 single-item typed | No | No | No | No | No |
|
||
| AuthenticateUser / ArchestrAUserToId typed | No | No | No | No | No |
|
||
| AdviseSupervisory / buffered / Suspend / Activate typed | No | No | No | No | No |
|
||
| Bulk add/advise/remove/unadvise/subscribe/unsubscribe | Yes | Yes | Yes | Yes | Yes |
|
||
| WriteBulk / Write2Bulk / WriteSecured(2)Bulk | Yes | Yes | Yes | Yes | Yes |
|
||
| ReadBulk | Yes | Yes | Yes | Yes | Yes |
|
||
| Sparse array write helper (`WriteArrayElements`) | Yes | Yes | Yes | Yes | Yes |
|
||
| Client-side 1000-item bulk cap | **No** | Yes | **No** | Yes | Yes |
|
||
| Event stream (idiomatic primitive) | `IAsyncEnumerable` | channel | iterator + observer | async iterator | `Stream` |
|
||
| Resume cursor (`after_worker_sequence`) | Yes | Yes | Yes | Yes | Yes |
|
||
| Automatic reconnect loop / `ReplayGap` handling | No | No | No | No | No |
|
||
| Alarm RPCs (Ack / StreamAlarms / QueryActiveAlarms) | Yes | Yes | Yes | Yes | Yes |
|
||
| Galaxy browse + lazy walker + WatchDeployEvents | Yes | Yes | Yes | Yes | Yes |
|
||
| Typed auth errors (Unauthenticated vs PermissionDenied) | Yes | **No** | Yes | Yes | Yes |
|
||
| MXAccess HRESULT/status-array validation | Yes (`!=0`) | Yes (`!=0`) | Yes (`!=0`) | Yes (`<0`, correct) | **No** |
|
||
| Automatic transient retry | Yes (Polly) | No | No | No | No |
|
||
| TLS default posture | skip-verify | skip-verify | skip-verify | TOFU pin | strict pin-only |
|
||
| Version constant matches package version | n/a (no version in csproj) | **No** (`0.1.0-dev`) | Yes (0.2.0) | **No** (0.1.0 vs 0.1.2) | **No** (`0.1.0-dev` vs 0.1.2) |
|
||
| Fixture-driven unit tests | Yes | Yes | Yes | Yes | Yes |
|
||
|
||
## Findings — .NET (`clients/dotnet`)
|
||
|
||
**D1 — Medium.** A session wrapper cannot be reconstructed from an existing session id, so reconnect-with-replay after client restart loses all typed helpers. `MxGatewaySession`'s constructor is `internal` (`clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs:19`) and no factory equivalent to Go's `NewSessionForID` (`clients/go/mxgateway/session.go:70`) or Java's `forSessionId` exists. Impact: the gateway's `DetachGraceSeconds`/replay features are usable only through the raw stub. Recommendation: add a public `MxGatewayClient.AttachSession(string sessionId)` factory.
|
||
|
||
**D2 — Medium.** `MxGatewaySession.DisposeAsync` calls `CloseAsync()` with no cancellation token or timeout (`clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs:881-885`); when the gateway is unreachable, `await using` disposal blocks for the full retry-pipeline budget and then throws from disposal, masking the original exception. Recommendation: swallow or time-bound close failures in the disposal path.
|
||
|
||
**D3 — Medium.** The retry budget self-defeats on timeouts: `ExecuteSafeUnaryAsync` caps the whole retry pipeline with `CancelAfter(Options.DefaultCallTimeout)` while each attempt also gets a `DefaultCallTimeout` deadline (`clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewayClient.cs:305-316` with `:269-303`), so a first attempt that ends in `DeadlineExceeded` exhausts the outer budget and the configured retries (`MxGatewayClientRetryPolicy.cs:62-67` lists `DeadlineExceeded` as retryable) never run. Recommendation: give the outer budget headroom (e.g., attempts × delay) or drop `DeadlineExceeded` from the retryable set.
|
||
|
||
**D4 — Medium.** `EnsureMxAccessSuccess` treats any nonzero HRESULT as failure (`clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxCommandReplyExtensions.cs:32`), misclassifying positive COM success codes (e.g. `S_FALSE = 1`); Python's `hresult < 0` (`clients/python/src/zb_mom_ww_mxgateway/errors.py:133`) is the correct COM semantics. Same defect in Go (`clients/go/mxgateway/errors.go:121`) and Java (`.../client/MxGatewayErrors.java:50`). Impact: a parity-preserving gateway reply carrying a success HRESULT other than 0 throws in three clients and passes in one. Recommendation: align all clients on `hresult < 0`.
|
||
|
||
**D5 — Low.** No `<Version>` property in `clients/dotnet/ZB.MOM.WW.MxGateway.Client/ZB.MOM.WW.MxGateway.Client.csproj:14-28`; the published NuGet version (0.1.2) is supplied out-of-band at pack time, so the source tree does not record what ships. Recommendation: add `<Version>` to the csproj.
|
||
|
||
**D6 — Low.** Duplicate `InternalsVisibleTo` declared in both `Properties/AssemblyInfo.cs:3` and the csproj `AssemblyAttribute` block (`ZB.MOM.WW.MxGateway.Client.csproj:36-40`). Harmless but redundant; keep one.
|
||
|
||
**D7 — Low.** With `UseTls` and no CA file, the default (`RequireCertificateValidation = false`) installs an accept-all certificate callback (`clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewayClient.cs:361-364`). Documented and intentional for the internal-tool posture (`MxGatewayClientOptions.cs:31-36`), but it means `--tls` without a CA gives no server authentication. See cross-cutting C4.
|
||
|
||
Otherwise the .NET client is in good shape: retries are correctly restricted to idempotent commands (`MxGatewayClientRetryPolicy.cs:45-50`), the stream enumerators map `RpcException` per read and honor `EnumeratorCancellation` (`GrpcMxGatewayClientTransport.cs:78-109`), close is idempotent under a lock (`MxGatewaySession.cs:42-67`), style follows `CSharpStyleGuide.md` (sealed types, `Async` suffix, file-scoped namespaces), and the test project covers options, TLS handler, session, alarms, Galaxy, values, statuses, and CLI redaction.
|
||
|
||
## Findings — Go (`clients/go`)
|
||
|
||
**G1 — High.** `Session.Events()`/`EventsAfter()` silently terminate the stream on consumer backpressure: when the 16-slot results channel is full, `sendEventResult` cancels the stream and returns without queuing any terminal error, and the goroutine closes the channel (`clients/go/mxgateway/session.go:751-768`, spawn at `:700-742`). The consumer sees a closed channel — indistinguishable from graceful server end — and events are lost with no signal. The documented contract ("until … a terminal error is sent", `session.go:675-677`) does not mention this. Impact: silent data loss for any consumer that stalls for 16 events. Recommendation: enqueue a sentinel `EventResult{Err: ErrSlowConsumer}` before closing (guarantee one reserved slot), or drop the buffered-cancel variant in favor of the blocking `SubscribeEvents` path, which correctly rides gRPC flow control.
|
||
|
||
**G2 — Medium.** No typed auth error mapping: all RPC failures are wrapped in the generic `GatewayError` (`clients/go/mxgateway/errors.go:9-34`; e.g. `client.go:110-133`), so distinguishing `Unauthenticated` from `PermissionDenied` requires `status.Code(errors.Unwrap(err))`. `docs/ClientLibrariesDesign.md:153` requires the two be treated distinctly, and the other four clients expose typed auth errors. Recommendation: add `AuthenticationError`/`AuthorizationError` wrappers (or sentinel errors) in `errors.go`.
|
||
|
||
**G3 — Medium.** `Dial` uses deprecated `grpc.DialContext` with `grpc.WithBlock()` (`clients/go/mxgateway/client.go:60-68`); grpc-go ≥1.63 deprecates both in favor of `grpc.NewClient` with lazy connection. Impact: future grpc-go upgrades and `go vet`/staticcheck noise; blocking dial also hides per-RPC connection errors semantics that the rest of the ecosystem now expects. Recommendation: migrate to `grpc.NewClient` and surface readiness via a first `Ping`.
|
||
|
||
**G4 — Medium.** The CLI cannot opt into strict TLS validation: `dialForCommand` never sets `Options.RequireCertificateValidation` and no flag exists for it (`clients/go/cmd/mxgw-go/main.go:1151-1158`), so every non-CA-pinned TLS run of `mxgw-go` is skip-verify even though the library supports strictness (`clients/go/mxgateway/client.go:231-241`). Recommendation: add `-require-certificate-validation`.
|
||
|
||
**G5 — Low.** `ClientVersion = "0.1.0-dev"` (`clients/go/mxgateway/version.go:5-7`) is stale relative to the tagged module releases published via `scripts/tag-go-module.ps1`. Recommendation: bump on release as part of the tagging script.
|
||
|
||
**G6 — Low.** `newCorrelationID` swallows `crypto/rand` errors and returns an empty correlation id (`clients/go/mxgateway/session.go:786-792`); a fallback (timestamp counter) would preserve traceability.
|
||
|
||
**G7 — Low.** Nil-vs-empty asymmetry: `WriteBulk`/`ReadBulk` short-circuit an empty (non-nil) slice locally (`session.go:399-407`, `:524-532`) while `AddItemBulk`/`SubscribeBulk` send an empty command to the wire (`session.go:255-274`). Harmless but inconsistent within one file.
|
||
|
||
Otherwise the Go client is idiomatic: contexts propagate everywhere, `%w`-style wrapping via `Unwrap`, correct capped-timeout merging in `callContext` (`client.go:190-205`), CLI redacts the API key before JSON output (`cmd/mxgw-go/main.go:1162-1177`), and test coverage (session, TLS, galaxy, alarms, fixtures) is broad.
|
||
|
||
## Findings — Java (`clients/java`)
|
||
|
||
**J1 — Verified (no defect).** The JDK 17 retarget is complete and correct: `java.toolchain.languageVersion = 17` plus `options.release = 17` (`clients/java/build.gradle:20-31`) guarantees both language level and API surface are 17-bounded. A scan of all handwritten sources found no JDK 21+ APIs (no `SequencedCollection`/`getFirst`/`reversed()`, no virtual threads, no `ScopedValue`, no string templates, no `Math.clamp`); the language features present (records `MxStatuses.java:45`, pattern `instanceof`, `switch` expressions `MxGatewayErrors.java:17-25`) are all ≤17. Gradle/grpc/protobuf dependency versions (`build.gradle:5-11`) are 17-compatible.
|
||
|
||
**J2 — Medium.** Docs still claim Java 21 after the retarget: `clients/java/README.md:354` ("Java 21 Gradle toolchain"), `clients/java/JavaClientDesign.md:34-35`, and `docs/ClientPackaging.md:193`. CLAUDE.md's same-commit docs rule is violated on the retarget's own branch. Recommendation: sweep all three in this branch before merge.
|
||
|
||
**J3 — Medium.** The event-stream buffer is hardcoded to 16 with no configuration: `MxGatewayClient.streamEvents` constructs `new MxEventStream(16)` (`clients/java/zb-mom-ww-mxgateway-client/src/main/java/com/zb/mom/ww/mxgateway/client/MxGatewayClient.java:248`), and overflow cancels the RPC (`.../client/MxEventStream.java:124-132`). Unlike Go, the failure is at least surfaced as `MxGatewayException("…queue overflowed")`, but a consumer that stalls for 16 events is disconnected even though gRPC has native flow control that could simply be leaned on. Recommendation: make capacity an option and/or use `disableAutoRequestWithInitial`-style manual flow control instead of cancel-on-overflow.
|
||
|
||
**J4 — Low.** `MxEventStream` is a single-consumer iterator with unsynchronized `next` state (`MxEventStream.java:31`, `:65-92`); this constraint is not documented. Add a Javadoc note.
|
||
|
||
**J5 — Low.** `close()` initiates `shutdown()` without awaiting termination (`MxGatewayClient.java:346-351`); acceptable given `closeAndAwaitTermination()` exists (`:360-367`), but try-with-resources users can leak a channel briefly at JVM exit. Consider making `close()` await a short bound.
|
||
|
||
Otherwise the Java client is the most complete: blocking, future, and async stub variants with correct deadline layering (unary vs stream, `MxGatewayClient.java:418-430`), a full typed exception hierarchy with credential redaction (`MxGatewayErrors.java:13-29`, `MxGatewaySecrets.java`), the only in-process end-to-end CLI test harness (`zb-mom-ww-mxgateway-cli/src/test/java/.../InProcessGatewayHarness.java`), and consistent 0.2.0 versioning between Gradle (`build.gradle:16`) and `MxGatewayClientVersion.java:12`.
|
||
|
||
## Findings — Python (`clients/python`)
|
||
|
||
**P1 — Medium.** The default TLS path performs a blocking, unverified certificate fetch and pins it (trust-on-first-use) (`clients/python/src/zb_mom_ww_mxgateway/options.py:108-160`), and silently defaults the SNI override to `localhost` (`options.py:150-154`). This is documented and bounded (probe timeout, `asyncio.to_thread` in `client.py:61-63`), but it is the only client that opens a second out-of-band TCP+TLS connection per channel, and TOFU is vulnerable to first-contact interception. Recommendation: document the MITM window in the README threat model and prefer `ca_file` in examples.
|
||
|
||
**P2 — Low.** Version mismatch: `pyproject.toml` says `0.1.2` (`clients/python/pyproject.toml:9`) while `version.py` says `__version__ = "0.1.0"` (`clients/python/src/zb_mom_ww_mxgateway/version.py:3`), so `mxgw-py version` reports the wrong version. Recommendation: derive one from the other (e.g. `importlib.metadata.version`).
|
||
|
||
**P3 — Low.** `Session.close()` is not concurrency-safe (no lock around `_closed`, `session.py:38-55`) and repeated closes return a locally synthesized `CloseSessionReply` rather than the cached server reply — divergent from .NET/Go which cache the real reply. Minor; align with a cached-reply pattern.
|
||
|
||
**P4 — Low.** Circular-import workaround: `from .client import GatewayClient` at the bottom of `session.py:590` (`# noqa: E402`). Works, but a `TYPE_CHECKING` import plus a string annotation would remove the runtime cycle.
|
||
|
||
Otherwise the Python client is strong: correct `hresult < 0` semantics (`errors.py:133`), fully typed (`from __future__ import annotations`, precise unions), GIL-friendly pure-asyncio design with stream iterators that cancel the call on generator exit (`client.py:230-263`), packaging correctly capped at `setuptools <77` for the ≤2.3 metadata constraint (`pyproject.toml:2-5`), and the largest per-client test suite (13 test modules including regression files for prior review findings).
|
||
|
||
## Findings — Rust (`clients/rust`)
|
||
|
||
**R1 — High.** The published crate cannot build outside this repository. `build.rs` resolves the protos two directories above the crate (`clients/rust/build.rs:8-16`: "clients/rust must live two levels below the repository root") and `src/generated.rs` is only `tonic::include_proto!` of build output (`clients/rust/src/generated.rs:16-40`); `Cargo.toml` does not vendor the `.proto` files into the package. The packaging script masks this by using `cargo package --no-verify` and `cargo publish --no-verify` (`scripts/pack-clients.ps1:190-211`). Any consumer of the Gitea-published `zb-mom-ww-mxgateway-client 0.1.2` fails in `build.rs` at first `cargo build`. Recommendation: copy the three protos into the crate (e.g. `clients/rust/protos/`, listed in `include`), fall back to them when the repo path is absent, and remove `--no-verify` so `cargo package` verifies buildability.
|
||
|
||
**R2 — High.** `invoke` validates only `protocol_status` and never inspects `hresult` or the `MXSTATUS_PROXY` array: `ensure_command_success` checks `code == Ok` only (`clients/rust/src/error.rs:214-226`, used by `client.rs:177-179`). Every other client performs a second MXAccess-level check (e.g. `MxCommandReplyExtensions.cs:27-41`, `errors.go:117-130`, `MxGatewayErrors.java:46-58`, `errors.py:122-148`), because a reply can carry an OK protocol envelope with failing per-item statuses. Impact: a Rust `session.write(...)` can report success while MXAccess rejected the write; there is also no distinct MxAccess error variant to catch. Recommendation: add an `ensure_mxaccess_success` pass (hresult `< 0` + statuses) and an `Error::MxAccess` variant.
|
||
|
||
**R3 — Low.** `CLIENT_VERSION = "0.1.0-dev"` with a doc comment claiming it "Mirrors Cargo.toml" while `Cargo.toml` says `0.1.2` (`clients/rust/src/version.rs:6-7` vs `clients/rust/Cargo.toml:3`). Recommendation: `pub const CLIENT_VERSION: &str = env!("CARGO_PKG_VERSION");`.
|
||
|
||
**R4 — Low.** No `unregister` typed helper (session surface at `clients/rust/src/session.rs:119-231` covers register/add/advise/remove but not Unregister); Go/Java/Python have it. Add for symmetry.
|
||
|
||
**R5 — Low.** The CLI is a single 2,699-line `main.rs` (`clients/rust/crates/mxgw-cli/src/main.rs`) — the largest single file in the client tree; the Windows stack-size workaround it forced (`clients/rust/.cargo/config.toml:1-19`) is itself evidence the command enum has outgrown one module. Split subcommands into modules.
|
||
|
||
Otherwise the Rust client is clean: clippy-conscious generated-module allowances (`generated.rs:18`, `#![warn(missing_docs)]` in `lib.rs:12`), a well-structured `thiserror` enum with boxed `tonic::Status`, credential scrubbing in error messages with a unit test (`error.rs:256-289`), cheap `Clone` client over a shared channel, correct unary-vs-stream timeout split (`client.rs:280-293`), and bulk caps enforced client-side (`session.rs:29`).
|
||
|
||
## Findings — cross-cutting
|
||
|
||
**X1 — High.** The MXAccess command parity gap is uniform: no client exposes typed helpers for single-item `WriteSecured`/`WriteSecured2`, `AuthenticateUser`, `ArchestrAUserToId`, `AdviseSupervisory`, `AddBufferedItem`, `SetBufferedUpdateInterval`, `Suspend`, or `Activate`, although the contract defines all of them (`src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto:150-160`) and `gateway.md` documents `AdviseSupervisory` as a precondition for user-attributed plain writes. Only the .NET, Go, and Python CLIs offer `advise-supervisory` via hand-built raw commands (`clients/go/cmd/mxgw-go/main.go:364-391`, `clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:456-470`, `clients/python/src/zb_mom_ww_mxgateway_cli/commands.py:287-291`); Java and Rust CLIs lack even that. Impact: the secured-write path (AuthenticateUser → WriteSecured) and the buffered-event family (`OnBufferedDataChange`) cannot be exercised through any typed client API. Recommendation: add the missing typed session helpers to all five clients, starting with `adviseSupervisory`, `writeSecured`, and `authenticateUser`.
|
||
|
||
**X2 — Medium.** No client handles the `ReplayGap` sentinel or offers a reconnect loop. All five expose the `after_worker_sequence` resume cursor (e.g. `MxGatewaySession.cs:865-876`, `session.go:682-688`, `MxGatewaySession.java:718-722`, `session.py:571-582`, `session.rs:648`), which matches the "no client-side reconnect" v1 non-goal in `docs/ClientLibrariesDesign.md:64-70`, but since the gateway shipped `DetachGraceSeconds` + replay, none of the clients document or type the `ReplayGap` event a resuming consumer must expect (`grep ReplayGap clients/` → no handwritten hits). Recommendation: at minimum document gap detection per client README; longer term add a resume helper.
|
||
|
||
**X3 — Medium.** `docs/ClientPackaging.md` has drifted from reality: Python package named `mxaccess-gateway-client` and generated dir `src/mxgateway/generated` (`docs/ClientPackaging.md:159-160`) vs actual `zb-mom-ww-mxaccess-gateway-client` (`clients/python/pyproject.toml:8`) and `src/zb_mom_ww_mxgateway/generated`; CLI module `python -m mxgateway_cli` (`ClientPackaging.md:187`) vs actual `zb_mom_ww_mxgateway_cli`; .NET solution `ZB.MOM.WW.MxGateway.Client.sln` (`ClientPackaging.md:51-52`) vs actual `.slnx`; Java 21 (`:193`, see J2). `docs/ClientLibrariesDesign.md:410` repeats the stale Python generated path. Recommendation: one doc sweep commit.
|
||
|
||
**X4 — Medium.** TLS default posture is inconsistent across the five clients: accept-any-cert in .NET (`MxGatewayClient.cs:361-364`), Go (`client.go:236-240`, `InsecureSkipVerify`), and Java (`MxGatewayClient.java:387-395`, `InsecureTrustManagerFactory`); TOFU pinning in Python (`options.py:133-154`); strict pin-only in Rust. `docs/CrossLanguageSmokeMatrix.md:58-66` documents the divergence, but the practical result is that the same `--tls`-without-CA invocation authenticates the server in one language, half-authenticates in another, and not at all in three. Recommendation: converge on the Python TOFU model or at least emit a one-line warning when verification is disabled.
|
||
|
||
**X5 — Low.** Client-side bulk caps differ: Go/Python/Rust enforce 1,000 items locally (`session.go:19`, `session.py:11`, `session.rs:29`) while .NET and Java send unbounded lists and rely on the gateway. Harmless but produces different error types for the same oversized call. Align (either all enforce or none).
|
||
|
||
**X6 — Low.** Event-stream backpressure semantics differ by language: Go buffered-16/silent-cancel (G1), Java buffered-16/error-cancel (J3), .NET/Python/Rust unbuffered (native gRPC flow control, pushing backpressure to the gateway where the documented fail-fast policy applies). The per-language behavior under a slow consumer is a parity-relevant observable and belongs in `docs/ClientBehaviorFixtures.md`.
|
||
|
||
**X7 — Low.** Generated-code hygiene is good everywhere: `clients/go/internal/generated`, `clients/java/src/main/generated` (6 tracked files), `clients/python/src/zb_mom_ww_mxgateway/generated` exist and match the manifest; Rust generates into `OUT_DIR` with a `.gitkeep` placeholder; `clients/dotnet/generated` is intentionally absent because the .NET client references the Contracts project directly (`ZB.MOM.WW.MxGateway.Client.csproj:4`), which `docs/ClientPackaging.md:38-40` correctly describes. Python `build/` and `.pytest_cache/` exist on disk but are not git-tracked.
|
||
|
||
## Top 5 recommendations
|
||
|
||
1. **Fix Rust crate publishability (R1):** vendor the three `.proto` files into the crate, add a repo-path fallback in `build.rs`, and drop `--no-verify` from `scripts/pack-clients.ps1` so `cargo package` proves the crate builds standalone.
|
||
2. **Fix Go silent stream termination (G1):** reserve a slot for a terminal `EventResult{Err: ...}` before closing the `Events()` channel so slow-consumer disconnects are observable, or deprecate the buffered-cancel path in favor of `SubscribeEvents`.
|
||
3. **Add MXAccess-level reply validation to Rust and align HRESULT semantics everywhere (R2, D4):** Rust must check `hresult` and the status array; .NET/Go/Java should switch from `hresult != 0` to `hresult < 0` to match COM semantics and Python.
|
||
4. **Close the typed-command parity gap (X1):** add `adviseSupervisory`, single-item `writeSecured`/`writeSecured2`, and `authenticateUser` helpers to all five session APIs (the wire already supports them), then the buffered-item family.
|
||
5. **One doc-and-version sweep on the retarget branch (J2, X3, G5, P2, R3):** update the three "Java 21" references, the Python naming in `ClientPackaging.md`/`ClientLibrariesDesign.md`, the `.sln` → `.slnx` reference, and reconcile the Go/Rust/Python version constants with their published package versions.
|