Six domain reports + overall synthesis. All 47 cycle-2 Done claims verified at HEAD with zero false Dones and zero regressions; 34 new findings (0 Critical, 0 High, 4 Medium, 25 Low, 5 Info): GWC-31..38, WRK-32..34, IPC-33..39, SEC-37..41, CLI-46..50, TST-31..36.
24 KiB
Language Clients — Architecture Review (Cycle 3)
Date: 2026-08-19 · Commit: 8b3e3b3 (main) · Baseline: 4f5371f (2026-07-12 cycle) · Method: static review on macOS (no builds, no tests run)
Scope: clients/dotnet (lib + CLI + tests), clients/go (mxgw-go), clients/java, clients/python, clients/rust (lib + CLI + build.rs + vendored protos), cross-client parity, docs/CrossLanguageSmokeMatrix.md, docs/ClientLibrariesDesign.md, docs/ClientPackaging.md, docs/ClientBehaviorFixtures.md, client READMEs, scripts/pack-clients.ps1 / scripts/tag-go-module.ps1. Prior findings tracked as CLI-01..34 (archreview/remediation/00-tracking.md) and CLI-35..45 (archreview/2026-07-12/remediation/50-clients.md + 00-tracking.md), plus NEXT-02 (archreview/2026-07-12/remediation/90-candidate-findings-next-cycle.md).
Since 4f5371f the client family absorbed: the full CLI-35..45 remediation train, NEXT-02, the 0.2.0/0.2.1 family publish (2026-08-07, 5b153da), TST-24 real-wire tests for .NET and Python (a8f86b5), alarm truncation-flag regen (d9ea8a8), the feed-level snapshot_status rollout (c748361 + 4d4f443), StreamAlarms open-sequence doc truth (c3c603f), and Gradle-9 survival for checkGeneratedClean (df45cb4).
Prior-finding verification
2026-07-12 cycle (CLI-35..45) and NEXT-02
| ID | Claimed status | Verified? | Evidence (file:line at HEAD) | Notes |
|---|---|---|---|---|
| CLI-35 | Done | Yes | clients/python/src/zb_mom_ww_mxgateway_cli/commands.py:24 (imports ReplayGap), :1660-1666 (_event_row branches on isinstance(item, ReplayGap) and emits {"replayGap": {"requestedAfterSequence": …, "oldestAvailableSequence": …}}); regression clients/python/tests/test_cli.py:935 (test_stream_events_renders_replay_gap, asserts exit 0, the gap row at index 0, the normal event row after, :971-974) |
Exactly the designed fix, camelCase row matching the cross-CLI shape. |
| CLI-36 | Done | Yes | clients/go/cmd/mxgw-go/main.go:1012-1029 (if result.IsReplayGap() → JSON replayGapCursors struct :70, text REPLAY_GAP requested_after=%d oldest_available=%d); test clients/go/cmd/mxgw-go/main_test.go:734 (TestRunStreamEventsPrintsReplayGap over a real fake gateway :698-713, both output modes) |
Gap counted toward -limit as designed. |
| NEXT-02 | Resolved 2026-08-10 | Yes | .NET clients/dotnet/ZB.MOM.WW.MxGateway.Client.Cli/MxGatewayClientCli.cs:1425-1429 (stream loop branch), :1845-1846 (EventToJsonElement inside --json aggregate), :1858-1873 (FormatReplayGapRow — hand-built so cursors are JSON numbers, not proto-JSON uint64 strings); Java clients/java/zb-mom-ww-mxgateway-cli/.../MxGatewayCli.java:1665-1674 (replayGap JSON row + REPLAY_GAP text line); tests MxGatewayCliTests.java:909-937, clients/dotnet/ZB.MOM.WW.MxGateway.Client.Tests/MxGatewayClientCliTests.cs (grep FormatReplayGapRow/replayGap hits); docs/CrossLanguageSmokeMatrix.md:43-67 now records one converged contract with a per-CLI table |
All five CLIs emit the same two camelCase keys with numeric cursors; the matrix doc explains why the row is hand-typed (proto3 JSON stringifies uint64). Genuinely converged. |
| CLI-37 | Done | Yes — all five conformant on category |
.NET MxStatusProxyExtensions.cs:15-20 (Category is MxStatusCategory.Ok only); Go clients/go/mxgateway/status.go:15-17 (nil ∥ category == OK, doc :7-14 states the diagnostics-only rule); Java MxStatuses.java:33-35 (+ success() Javadoc :55-57 marks it diagnostic); Python errors.py:144-152; Rust error.rs:407-413 (status.category != Ok as i32, doc :383-388 covers the unspecified-category-fails edge); shared fixtures clients/proto/fixtures/behavior/command-replies/write.status-category-error-success-set.reply.json + write.status-category-ok-success-zero.reply.json, registered in the manifest, documented in docs/ClientBehaviorFixtures.md (Reply Validation Conformance table), and consumed by all five suites (MxCommandReplyExtensionsTests.cs, protofixtures_test.go, MxGatewayFixtureTests.java, test_errors.py, client_behavior.rs) |
The 4-vs-1 divergence is gone; the edge decisions (absent entry = success, present-unspecified = failure) are uniform and documented. |
| CLI-38 | Done | Yes — all five on hresult < 0 |
.NET MxCommandReplyExtensions.cs:38 (HasHresult && Hresult < 0); Go errors.go:299 (GetHresult() < 0); Java MxGatewayErrors.java:52 (getHresult() < 0); Python errors.py:137; Rust error.rs:407; fixtures write.hresult-s-false.reply.json (S_FALSE=1 → success) + write.hresult-e-fail.reply.json (E_FAIL → failure) wired into all five suites |
Lands prior-cycle CLI-08 (cycle-1 tracker row updated to Done via CLI-38 — verified). docs/ClientLibrariesDesign.md:112-125 is now true, and states both rules explicitly. |
| CLI-39 | Done | Yes (bump + both guards); pattern has already recurred → CLI-46 | Versions: clients/rust/Cargo.toml:3/:28 = 0.2.0, clients/python/pyproject.toml:9 + version.py:3 = 0.2.0 (+ new drift test test_cli.py:17 test_version_matches_pyproject_toml, closing the CLI-26 residual), clients/go/mxgateway/version.go = 0.2.0, .../ZB.MOM.WW.MxGateway.Client.csproj:22 = 0.2.0, Contracts .csproj:10 = 0.2.0 (440e7cf), Java build.gradle = 0.2.1. Guards: scripts/tag-go-module.ps1:40-53 (tag fails on ClientVersion mismatch — the CLI-21 guard finally implemented), scripts/pack-clients.ps1:91-156 (Test-GiteaPackageExists + hard abort "never force-overwrites"). Publish record: docs/ClientPackaging.md:102-113 (2026-08-07: nuget/pypi/cargo 0.2.0, maven 0.2.1 with the rationale for the Java exception, Go tag clients/go/v0.2.0 — tag exists in this repo) |
Executed to the letter, including the doc's "bump before every publish" rule and the collision-guard rationale. However the same hazard is live again at HEAD: post-publish library changes (incl. a shipped Python connect() fix) sit at the already-published 0.2.0/0.2.1 — see CLI-46. |
| CLI-40 | Done | Yes | Java MxGatewaySecrets.java:62-78 (redactExact); .NET MxGatewaySecretRedaction.cs (Redact/RedactReply:55-68/RedactStatus:81-89/RedactStatuses — scrubs message and the structured reply accessors, the 0d874f9 follow-up); Rust error.rs:208-231 (MxAccessError.secrets + with_secrets, hand-written Debug prints only the count :202-204); fixtures authenticate-user.echoed-credential.reply.json + .echoed-credential-mxaccess-failure.reply.json in the manifest, consumed by all five suites (MxGatewaySessionReplyContractTests.cs, command_reply_fixtures_test.go, MxGatewayCredentialReplyTests.java, test_malformed_reply.py/test_errors.py, client_behavior.rs) |
The echoed-credential fixture is exactly the test the prior review said was missing ("the fakes don't echo"). |
| CLI-41 | Done | Yes | Go errors.go:164-176 (MalformedReplyError); Python errors.py:56; Java MxGatewayMalformedReplyException.java + throw sites MxGatewaySession.java:889/:917/:948 (redaction rethrow preserves the type :1143-1144); .NET MxGatewayMalformedReplyException.cs; Rust session.rs:1024-1042 (payload → present return_value → Error::MalformedReply, now uniform across its extractors); fixtures authenticate-user.missing-payload.reply.json + authenticate-user.return-value-only.reply.json; contract documented in docs/ClientLibrariesDesign.md ("Malformed-reply extraction" paragraph) |
Silent-0 user ids and the .NET NRE are gone; the payload → return_value → typed-error ladder is uniform. |
| CLI-42 | Done | Yes | clients/rust/README.md:24-36 (repo-path-first / vendored-fallback resolution, same-commit refresh rule, both-direction drift guard); docs/ClientPackaging.md:199-213 (same, plus why cargo package runs with verification); 711793f extended check-codegen Check 3 to sweep both directions (canonical proto missing a vendored copy now fails) |
Vendored copies verified byte-identical to Contracts at HEAD (all three protos, diff clean). |
| CLI-43 | Done | Yes | docs/style-guides/JavaStyleGuide.md:8 — "Target Java 17 (the Ignition 8.3 baseline; the client build enforces …)" |
grep "Java 21" in style-guides: clean. |
| CLI-44 | Done | Yes | clients/go/mxgateway/session.go:1077 (Recv-error path uses sendTerminalEventResult), :1096-1111 (bypasses the overflow substitution via the reserved slot) |
Root cause of a genuine stream error is no longer relabeled ErrSlowConsumer. |
| CLI-45 | Done | Yes | Canonical env MXGATEWAY_VERIFY_PASSWORD in all five: Go main.go:452 (default) + :483-491 (fail-fast before dialing, message names flag+var only); Java MxGatewayCli.java:194 + :1166-1179 (ParameterException on null/blank); Rust main.rs:151 (clap default) + test :2644-2660; Python commands.py:38 (DEFAULT_VERIFY_PASSWORD_ENV); .NET MxGatewayClientCli.cs:353-359 (canonical default + MXGATEWAY_VERIFY_USER_PASSWORD/--verify-user-password* kept as deprecated aliases, resolution order documented :385-386 and in help :2455-2456). Tests: Go main_test.go:605 (RejectsEmptyPassword) + :632 (ReadsPasswordFromCanonicalEnv). Contract documented at docs/CrossLanguageSmokeMatrix.md:90-119, incl. the parity rationale (libraries still transmit whatever they are given) and the .NET deprecation window |
One exported variable now drives all five CLIs; empty credential is a usage error everywhere. |
Done claims that don't fully hold: none. Every 2026-07-12 remediation claim checked out at HEAD with the designed shape (fixtures-first conformance, docs in the same commit, guards actually implemented — including the CLI-21 tag guard skipped last cycle). This is the cleanest verification pass of the three cycles.
Cycle-1 residue (CLI-05..33, previously Open)
Re-verified briefly at HEAD; all unchanged unless noted:
- CLI-08 → Done via CLI-38 (see above); cycle-1 tracker row correctly updated.
- Still open, unchanged: CLI-05 (
MxGatewaySession.cs:19internal ctor, no .NETAttachSession), CLI-06 (:1501-1505DisposeAsync→ unboundedCloseAsync), CLI-07 (MxGatewayClient.cs:311), CLI-09 (no typed auth errors in Goerrors.go), CLI-10 (client.go:64/:68WithBlock/DialContext), CLI-11 (library hasRequireCertificateValidation(client.go:239) butmxgw-gostill exposes no flag), CLI-13 (MxGatewayClient.java:248hardcoded 16), CLI-14 (no TOFU runtime warning inoptions.py), CLI-17/20 (TLS posture divergence: .NET accept-allMxGatewayClient.cs:369, Go skip-verify, JavaInsecureTrustManagerFactoryMxGatewayClient.java:391, Python TOFU, Rust strict), CLI-19 (duplicateInternalsVisibleTo), CLI-22 (session.go:1176-1181empty correlation id on rand failure), CLI-23 (nil-vs-empty asymmetry — now at least documented in theWriteBulkdoc commentsession.go:431-432), CLI-25 (close()doesn't await terminationMxGatewayClient.java:347-351), CLI-27 (session.pyclose still lock-free), CLI-28 (bottom-of-file importsession.py), CLI-31 (mxgw-cli/src/main.rsnow 2,991 lines — grew again; the other CLIs sit at 1,668–2,478), CLI-32 (no .NET/Java client-side bulk cap), CLI-33 (no backpressure section indocs/ClientBehaviorFixtures.md).
New findings
CLI-46 — Version constants again equal the already-published 0.2.0/0.2.1 while post-publish changes shipped — including the fix for a published-broken Python connect()
Severity: Medium · Category: packaging / release hygiene
Files: clients/python/pyproject.toml:9 + clients/python/src/zb_mom_ww_mxgateway/version.py:3 (0.2.0 = the pypi artifact published 2026-08-07, docs/ClientPackaging.md:102-113); clients/python/src/zb_mom_ww_mxgateway/client.py:61-72 (the fixed connect()); post-publish library commits a8f86b5 (2026-08-10), d9ea8a8, c748361 (2026-08-17/18); guard scripts/pack-clients.ps1:145-156
The published pypi 0.2.0 Python client cannot open a real connection at all: its connect() built the grpc.aio channel inside asyncio.to_thread, and a grpc.aio channel binds to the event loop current on the constructing thread, so every non-stub connection raised "There is no current event loop in thread" (TST-24's first real-wire run caught it; fixed in-tree by a8f86b5 — resolve_channel_security runs off-loop, create_channel on-loop). That fix has sat unpublished for nine days while the version constant still reads 0.2.0. Since the publish, all five libraries also gained wire-surface additions at the published numbers (alarm truncation fields d9ea8a8, feed-level snapshot_status bindings c748361; Java at its published 0.2.1). This is precisely the CLI-39 pattern one cycle later — now mitigated (the pack-script collision guard refuses a same-version publish, and the bump-together deferral is deliberate) but the live registry keeps serving a broken Python client, and nothing in the tree records that 0.2.0-as-published ≠ 0.2.0-as-source.
Recommendation: cut the next release train soon (0.2.1 four clients / 0.2.2 Java, or 0.3.0 across the board), prioritizing the Python publish; until then, note the known-broken connect() against pypi 0.2.0 in clients/python/README.md/docs/ClientPackaging.md (or yank the pypi artifact). Consider bumping version constants in the same commit as the first post-publish library change rather than at publish time — the collision guard then enforces nothing because nothing needs catching.
CLI-47 — snapshot-status/provider-status rendering diverges: .NET text mode hides the flag on the frames that matter most; Rust's JSON is the lone always-explicit shape
Severity: Low · Category: cross-client parity / CLI
Files: .NET MxGatewayClientCli.cs:15 (JsonFormatter.Default — omits default-valued proto3 fields) + :1563-1566 (text arms format the payload through it); Go main.go:1112-1114 (snapshot-status truncated=%t); Java MxGatewayCli.java:2309-2312 (truncated=%b) + protoJson :2362-2368 (default printer); Rust main.rs:2262-2264 (summary) + :2314-2318 (hand-built JSON, always includes "truncated"); Python CLI renders generic MessageToDict
The c748361 rollout achieved surface parity (all five render the new arm) but not rendering parity. The snapshot_status clearing frame — truncated = false, emitted on monitor restart, explicitly called out by the commit and docs/Grpc.md as a frame consumers must see — renders in the .NET CLI's text mode as snapshot-status {} (proto3 JSON omits the false bool), while Go/Java/Rust print truncated=false explicitly. The same applies to a healthy provider-status (degraded=false invisible in .NET text). In JSON mode the split flips: Rust's hand-built row always carries "truncated", while .NET/Go/Java/Python protobuf formatters emit {"snapshotStatus": {}} for the clearing frame. Semantically recoverable (absent = false), but the same class of divergence NEXT-02 just cleaned up for ReplayGap, where the cross-CLI row was deliberately hand-specified.
Recommendation: hand-format the .NET text arms like the other three (snapshot-status truncated={bool}, provider-status mode=… degraded=… reason=…); decide one JSON shape (either Rust adopts formatter-omission or the matrix documents Rust's explicitness the way CrossLanguageSmokeMatrix.md:54-67 documents the ReplayGap row's deliberate choices).
CLI-48 — The new alarm-feed renderer arms are untested in three of the four CLIs that changed; the Go CLI has no stream-alarms test at all
Severity: Low · Category: testing coverage
Files: clients/go/cmd/mxgw-go/main_test.go (test inventory :16-734 — no stream-alarms/formatAlarmFeedMessage test of any kind); .NET MxGatewayClientCliTests.cs:671-706 (stream-alarms test feeds only ActiveAlarm + SnapshotComplete); Java MxGatewayCliTests.java:653-687 (asserts active-alarm/snapshot-complete/transition arms only); Rust main.rs:2986 (the one snapshot-status render test, added by 4d4f443)
The review-nits commit covered only the Rust render; the .NET/Go/Java provider-status and snapshot-status arms shipped with zero assertions. The .NET {}-rendering divergence (CLI-47) is exactly what a text-mode assertion on a truncated=false frame would have caught — cycle 2's lesson ("each new epic achieves surface parity but re-introduces behavioral divergence") repeating at the test layer.
Recommendation: one fake-fed stream-alarms test per CLI whose feed includes a provider_status and a snapshot_status frame (including the false/clearing shape), asserting the rendered row in both output modes; for Go this also creates the missing stream-alarms scaffold.
CLI-49 — Go library exposes no aliases for AlarmProviderStatus/AlarmSnapshotStatus, whose generated types live in an internal package; the AlarmFeedMessage alias doc names three of five payload arms
Severity: Low · Category: code organization & conventions / documentation
Files: clients/go/mxgateway/types.go:129-137 (alias block: AlarmFeedMessage, ActiveAlarmSnapshot, OnAlarmTransitionEvent — but not the two status payloads; ReplayGap got its alias at :38 when it shipped); generated types under clients/go/internal/generated (Go internal visibility)
Because the generated package is internal, a Go consumer cannot name *generated.AlarmSnapshotStatus: they can call message.GetSnapshotStatus().GetTruncated() on an inferred value but cannot declare a variable, struct field, or function parameter of the type — unlike every other StreamAlarms payload, which types.go deliberately aliases for exactly this reason. (AlarmProviderStatus has had the same gap since provider-mode shipped.) The AlarmFeedMessage alias comment (types.go:130-132) still describes the oneof as "an active-alarm snapshot row, a snapshot-complete sentinel, or a transition" — stale by two arms.
Recommendation: add AlarmProviderStatus = pb.AlarmProviderStatus and AlarmSnapshotStatus = pb.AlarmSnapshotStatus aliases and refresh the AlarmFeedMessage comment to name all five arms.
CLI-50 — Python CLI stream-alarms default (--max-messages 1, collect-then-print) now returns only the provider-status preamble — the README's own example fetches no alarm data
Severity: Low · Category: cross-client parity / CLI usability
Files: clients/python/src/zb_mom_ww_mxgateway_cli/commands.py:514 (--max-messages default 1), :1539-1551 (_collect_alarm_messages — bounded collect, printed once at the end), :1122-1134; clients/python/README.md:451 (mxgw-py stream-alarms --max-messages 1 --json); feed order per docs/Grpc.md/c3c603f: provider_status → snapshot_status → cached active_alarm → snapshot_complete
The other four CLIs stream rows as they arrive and default to unbounded (--limit 0 / --max-events 0). Python is the outlier twice over: it buffers then prints (so it cannot tail a live feed at all), and its default of 1 — reasonable when the first frame was an alarm — now returns exactly one provider_status frame, since the feed gained a two-frame status preamble. The README example therefore demonstrates a command that yields no alarm rows. Pre-existing shape, but the preamble growth this cycle changed what the default actually returns.
Recommendation: raise the default (or document that N status frames precede the data) and update the README example; longer term consider printing rows incrementally like the other four CLIs.
Severity roll-up (new findings)
| Severity | Count | IDs |
|---|---|---|
| High | 0 | — |
| Medium | 1 | CLI-46 |
| Low | 4 | CLI-47, CLI-48, CLI-49, CLI-50 |
Cross-client parity note (updated)
The 2026-07-12 divergence table has largely collapsed. Converged this cycle, with shared fixtures pinning the bytes: HRESULT < 0 (CLI-38), status verdict = category != OK (CLI-37), exact-secret scrub incl. echoed diagnostics (CLI-40), malformed-reply ladder (CLI-41), CLI ReplayGap row (CLI-35/36 + NEXT-02, all five, same keys, numeric cursors), credential env-var contract (CLI-45). What remains divergent:
| Behavior | .NET | Go | Java | Python | Rust |
|---|---|---|---|---|---|
| Version vs published artifact (CLI-46) | 0.2.0 = shipped | 0.2.0 = shipped | 0.2.1 = shipped | 0.2.0 = shipped (broken connect() on pypi) |
0.2.0 = shipped |
| CLI text render of snapshot/provider status (CLI-47) | proto-JSON, flag invisible when false | explicit truncated=… |
explicit | n/a (JSON only) | explicit ✓ |
CLI JSON shape of snapshot_status (CLI-47) |
omits false | omits false | omits false | omits false | always explicit |
| New renderer arms tested (CLI-48) | no | no (no stream-alarms test) | no | n/a (generic dump) | yes ✓ |
stream-alarms consumption model (CLI-50) |
stream, unbounded default | stream, unbounded | stream, unbounded | collect, default 1 | stream, unbounded |
| Session re-attach (CLI-05) | No | Yes | Yes | Yes | Yes |
| Client-side bulk cap (CLI-32) | No | Yes | No | Yes | Yes |
| Event backpressure (CLI-13/33) | unbuffered | 16+1, loud, true root cause ✓ | 16, cancel+error | unbuffered | unbuffered |
| TLS default (CLI-17) | accept-all | accept-all | accept-all | TOFU pin | strict pin |
| Typed auth errors (CLI-09) | Yes | No | Yes | Yes | Yes |
| Real-wire test harness | Kestrel h2c ✓ (TST-24) | bufconn ✓ | InProcessServer ✓ | grpc.aio loopback ✓ (TST-24) | tonic loopback ✓ |
CLI subcommand coverage remains uneven by design and is now honestly specified (docs/CrossLanguageSmokeMatrix.md:163-186: .NET all nine, Rust + unregister/credential pair, Go/Python/Java credential pair only) — verified accurate against all five dispatch tables at HEAD, including the note that every gap is CLI-surface-only (all five libraries carry all typed helpers, confirmed for the Rust bulk family at session.rs:605-654).
What is good
- The remediation train executed at design fidelity. All eleven 2026-07-12 findings landed exactly as their remediation designs specified — including the parts prior cycles skipped (the CLI-21 tag guard, the CLI-02/42 doc half, the CLI-26 pyproject-drift test). The conformance findings (CLI-37/38/40/41) all went fixtures-first: ten shared command-reply fixtures under
clients/proto/fixtures/behavior/command-replies/, each consumed by all five suites, so the canonical rules are pinned in bytes, not prose. - The publish pipeline is now guarded end to end: version-vs-tag check in
tag-go-module.ps1, registry-collision hard-stop inpack-clients.ps1, publish coordinates and the Java 0.2.1 exception recorded indocs/ClientPackaging.mdwith the reasoning ("these are the numbers a future release bumps off"). - TST-24 paid for itself immediately: the first real-wire run of the Python client found
connect()could never work outside stubs — a defect every mock-based test had actively asserted as correct. All five clients now have genuine-transport tests (framing, metadata, status codes, ReplayGap, PERMISSION_DENIED mapping), and CI runs all five suites plus a Java generated-tree drift check (.gitea/workflows/ci.yml:84-142). - Generated-code hygiene is the best it has been: vendored Rust protos byte-identical to Contracts (verified, all three files); check-codegen Check 3 sweeps both directions; the Java toolchain pin eliminated the spurious ~64k-line protobuf churn (per
build.gradle:61-70, IPC-24) andcheckGeneratedCleansurvived Gradle 9 (df45cb4); Python regen stays on the pinned grpcio-tools 1.80.0 / protobuf 6.31.1 baseline (pyproject.toml:41-44, generated header confirms 6.31.1); even a proto-comment-only change (c3c603f) regenerated every downstream tree in the same commit. - Parity discipline held through the alarm epic: no client wraps or reinterprets
AlarmFeedMessage/ActiveAlarmSnapshot— the truncation and status flags pass through the generated types untouched in all five, and every README explains thefrom_truncated_snapshot/snapshot_statussemantics with the same don't-reconcile-deletions guidance, explicitly disambiguating them from the per-recorddegradedflag. - The smoke-matrix doc is a model of honest divergence-recording: where the CLIs still differ (subcommand coverage, JSON formatting quirks per language), the doc specifies the delta instead of leaving it to be discovered — the exact practice this review series asked for two cycles ago.