ScadaBridge/archreview/08-conventions-tests-underdeveloped.md

Architecture Review 08 — Cross-Cutting Conventions, Test Posture, Underdeveloped Areas

Reviewer domain: cross-cutting conventions, code organization, test posture, repo-wide underdeveloped-areas sweep, docs-code drift. Individual subsystems are covered in depth by the other seven reviews; this one covers the seams and the whole.

Scope

• src/ — all 26 projects: Commons structure, project reference graph, options pattern, POCO/repository conventions, UTC and correlation-ID discipline, message-contract evolution.
• tests/ — all 30 test projects: breadth, quality sampling, integration/performance posture, slnx membership.
• Repo-wide grep sweep for TODO / FIXME / HACK / NotImplementedException / "deferred" / "follow-up" / "not shipped", plus deferred-work inventory from docs/plans/ and CLAUDE.md.
• Docs-code sync spot checks: README component table, docs/requirements/Component-*.md (KpiHistory, StoreAndForward, Transport, ClusterInfrastructure), docs/components/ reference docs.

Method: .slnx and .csproj graph extraction, directory-structure listing, targeted greps over src/, tests/, docs/, and direct reads of the files behind every claim below. Read-only except this report.

Maturity Verdict

This is an unusually disciplined codebase for its size (~190k src LOC, ~156k test LOC, ~5,000 test cases across 30 projects). The documented conventions are not aspirational — they are actually followed: Commons is genuinely persistence-ignorant, the Types/Interfaces/Entities/Messages hierarchy is real, UTC discipline is total (zero DateTime.Now in src), correlation IDs are on every cross-cluster request/response plus the ManagementEnvelope, and the project reference graph has no site→central contamination. The TODO surface is astonishingly small (two real TODOs in all of src) and almost every deferral is deliberately documented at the deferral site with a pointer to the design doc. The gaps that exist are narrow and specific: one genuine spec-vs-code contradiction (Transport claims Area membership travels in bundles; the exporter always emits null), the CLI.Tests slnx exclusion that keeps 279 tests out of every solution-level test run, options-validation coverage on only ~6 of ~20 option classes, and a thin PerformanceTests project that is a correctness-at-small-scale suite rather than a performance suite. Deferred-work inventory is sizeable (~20 tracked items) but almost entirely intentional and logged; only two or three items carry real operational risk.

---

1. Conventions & Organization

1.1 Commons hierarchy — followed, with undocumented extensions [Low]

src/ZB.MOM.WW.ScadaBridge.Commons/ follows the documented Types/, Interfaces/, Entities/, Messages/ layout with domain-area subfolders exactly as Component-Commons.md describes (284 .cs files; e.g. Entities/{Audit,Deployment,ExternalSystems,InboundApi,Instances,Kpi,Notifications,Schemas,Scripts,SecuredWrites,Security,Sites,Templates}, Messages/{Artifacts,Audit,Communication,DataConnection,DebugView,Deployment,Health,InboundApi,Instance,Integration,Lifecycle,Management,Notification,RemoteQuery,ScriptExecution,Streaming}).

Three top-level folders exist beyond the documented four: Observability/, Serialization/ (protocol endpoint-config serializers), and Validators/. These are reasonable homes, but the convention doc lists only the four canonical folders. Either document them in Component-Commons.md or fold them in (Serialization arguably belongs under Types/).

1.2 POCO persistence-ignorance — verified clean [Pass]

Grep for EF Core / DataAnnotations attributes across Commons returns only one hit, and it is an XML-doc <see cref> to DbUpdateConcurrencyException in src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs:64 — a documentation reference, not a dependency. The Commons csproj has zero EF package references (its only PackageReference is the shared ZB.MOM.WW.Audit lib). Entities are genuinely persistence-ignorant.

1.3 Repository interfaces vs implementations — 1:1, with a deliberate site-side exception [Low]

14 repository interfaces in Commons/Interfaces/Repositories/, 14 implementation files in ConfigurationDatabase/Repositories/ — exact match. Two additional implementations live in src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Repositories/ (SiteExternalSystemRepository, SiteNotificationRepository): read-only SQLite-backed site-side implementations that throw NotSupportedException on writes. This is a sensible pattern (same interface, site-local store), but it is an undocumented exception to the "implementations in Configuration Database" rule.

Vestigial seam [Low/Medium]: SiteNotificationRepository (registered scoped in src/ZB.MOM.WW.ScadaBridge.SiteRuntime/ServiceCollectionExtensions.cs:65) reads the site-local notification_lists table — but per the central-only notification design, SiteStorageService.PurgeCentralOnlyNotificationConfigAsync (src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Persistence/SiteStorageService.cs:703) deliberately empties that table on every artifact apply. So the registered repository can only ever return empty results, and StoreNotificationListAsync (line 721) still exists as a write path into a table the design says must stay empty. The purge is well-commented and correct as a security cleanup; the live read/write surface around an intentionally-empty table is dead weight and a trap for a future maintainer. Recommend removing StoreNotificationListAsync/list-read paths or marking them [Obsolete] with the design rationale.

1.4 Project reference graph — sane; no site→central contamination [Pass, one Low note]

Extracted from all 26 csproj files:

• Commons is a leaf (no project refs). Everything depends on it; it depends on nothing.
• Site-side projects never reference central-only projects. SiteRuntime → Commons, Communication, DataConnectionLayer, ScriptAnalysis, HealthMonitoring, SiteEventLogging, StoreAndForward — critically, no reference to ConfigurationDatabase (EF/central SQL), ManagementService, CentralUI, or TemplateEngine. Same for DataConnectionLayer, StoreAndForward, SiteEventLogging.
• Central-only stack is correctly layered: AuditLog/SiteCallAudit/Transport → ConfigurationDatabase; CentralUI → the central components + ManagementService; Host references everything (composition root, as designed).
• CLI → Commons only — correctly thin (HTTP client to the Management API).
• DelmiaNotifier → nothing — correctly standalone/BCL-only.

One smell [Low]: Communication → HealthMonitoring (src/ZB.MOM.WW.ScadaBridge.Communication/Actors/CentralCommunicationActor.cs:13). A transport layer depending on a monitoring component is a mild layering inversion; the health-aggregator abstraction it consumes could live in Commons. Cosmetic [Low]: ManagementService.csproj uses a Windows-style ..\ separator for its InboundAPI reference while every other reference in the repo uses ../ — harmless to MSBuild, inconsistent style.

1.5 Options pattern — ownership right, validation coverage partial [Medium]

Every component owns its options class in its own project (31 *Options.cs files across src; none in Commons) — the ownership convention is fully honored. But eager validation is wired for only ~6 components: IValidateOptions<>/ValidateDataAnnotations/ValidateOnStart appear only in AuditLog, KpiHistory, NotificationOutbox (+SmsOptions), HealthMonitoring, Security, ClusterInfrastructure, plus the Host StartupValidator. Components binding options without a validator include Communication, DataConnectionLayer, DeploymentManager, ExternalSystemGateway, InboundAPI, ManagementService, NotificationService, SiteCallAudit, SiteEventLogging, SiteRuntime, StoreAndForward, and Transport (e.g. src/ZB.MOM.WW.ScadaBridge.Transport/ServiceCollectionExtensions.cs:25 — AddOptions<TransportOptions>().BindConfiguration(...) with no .ValidateOnStart()). A malformed appsettings section in those components surfaces at first use rather than at startup, which is exactly what the Host's readiness-gating philosophy is supposed to prevent. Recommend a sweep adding validators (the HealthMonitoring/ClusterInfrastructure IValidateOptions implementations are ready-made patterns to copy).

1.6 UTC timestamps — total compliance [Pass]

Zero occurrences of DateTime.Now/DateTimeOffset.Now (non-Utc) in all of src (grep across .cs and .razor). Every timestamp uses UtcNow or stores datetime2/ISO-8601 UTC.

1.7 Correlation IDs — convention followed [Pass]

96 CorrelationId declarations across Commons/Messages/ cover the full cross-cluster request/response surface (RouteToInstance, Get/SetAttribute(s), Subscribe*, WriteTag*, ParkedMessage*, NotificationOutboxQueries, SiteCallQueries, EventLogQuery, DeploymentStateQuery, WaitForAttribute, etc.). Management commands themselves carry no per-command field, but they always travel inside ManagementEnvelope(User, Command, CorrelationId) with ManagementSuccess/ManagementError/ManagementUnauthorized echoing it (Commons/Messages/Management/ManagementEnvelope.cs:7-11) — the convention holds at the envelope level.

1.8 Message contract evolution — convention by discipline, not enforcement [Low]

Messages are plain C# records with defaulted trailing parameters (additive-friendly, e.g. CreateDataConnectionCommand(..., string? BackupConfiguration = null, int FailoverRetryCount = 3) in Commons/Messages/Management/DataConnectionCommands.cs). The Akka HOCON in Host/Actors/AkkaHostedService.cs:233 configures no custom serializer, so ClusterClient traffic rides Akka.NET's default JSON serialization — tolerant of additive fields. The gRPC side does have contract-lock tests (tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/Protos/CachedTelemetryProtoTests.cs, PullAuditEventsProtoTests.cs), but there is no equivalent guard (snapshot/round-trip tests against old shapes) for the ClusterClient record contracts. Given the "system-wide artifact version skew across sites is supported" design decision, a small contract-compatibility test suite for the highest-traffic ClusterClient messages would convert the additive-only rule from convention to enforcement.

1.9 Naming/namespace consistency [Pass]

Consistent ZB.MOM.WW.ScadaBridge.<Component> namespaces mirroring folder structure in every file sampled (SiteRuntime, Transport, Commons, KpiHistory, ManagementService). Test projects mirror src names 1:1 plus IntegrationTests/PerformanceTests/PlaywrightTests/Transport.IntegrationTests.

---

2. Test Posture & Quality

2.1 Breadth — every component has a test project; volume is high

30 test projects; ~5,050 [Fact]/[Theory] methods; ~156k test LOC against ~190k src LOC (src figure inflated by ~75k in ConfigurationDatabase, mostly EF migrations). Per-project counts (files / LOC / test methods):

Project	Tests	Notes
CentralUI.Tests	829	bUnit page/component tests — largest suite
Commons.Tests	465	includes KpiSeriesBucketer, codecs, validators
SiteRuntime.Tests	457	Akka TestKit behavioral (see 2.2)
TemplateEngine.Tests	433
CLI.Tests	279	excluded from slnx (see 2.4)
AuditLog.Tests	237
ManagementService.Tests	234
ConfigurationDatabase.Tests	219	incl. migration tests
InboundAPI.Tests	218
Communication.Tests	211	incl. proto contract locks
DataConnectionLayer.Tests	190
Host.Tests	142	incl. CompositionRootTests
Security.Tests	133
NotificationOutbox.Tests / Transport.Tests	122 / 124
StoreAndForward.Tests	116
DeploymentManager.Tests	98
HealthMonitoring.Tests	84
IntegrationTests / ExternalSystemGateway.Tests	69 / 69
Transport.IntegrationTests	60
SiteEventLogging.Tests	54
ScriptAnalysis.Tests / NotificationService.Tests / CentralUI.PlaywrightTests	40 / 36 / 36
SiteCallAudit.Tests	31	thinnest per src LOC (1,642 loc / 31 tests)
DelmiaNotifier.Tests	23
ClusterInfrastructure.Tests	15	src is only 191 LOC (wiring lives in Host) — proportionate
KpiHistory.Tests	14	bucketer/query/chart tests live with owners (Commons.Tests, CentralUI.Tests) — coverage is distributed, not missing
PerformanceTests	10	see 2.3

Relatively thin spots: SiteCallAudit.Tests (31 tests for a reconciliation/KPI/relay component) and DeploymentManager.Tests (98 tests for the deployment pipeline, though much of its logic is exercised via ManagementService/Host/Integration suites).

2.2 Quality sampling — real behavioral tests, not shallow

tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/AlarmActorTests.cs is representative: full Akka TestKit harness, real compiled script library, TestProbe as the parent InstanceActor, asserts actual AlarmStateChanged messages with state/name payloads — genuine behavior, not constructor smoke tests. The SiteRuntime.Tests directory also carries targeted regression suites (InstanceActorChildAttributeRaceTests, DeploymentManagerRedeployTests, DeploymentManagerMediumFindingsTests — named for review findings, indicating review feedback gets locked in as tests). IntegrationTests covers real cross-cutting concerns: CentralFailoverTests, SiteFailoverTests, DualNodeRecoveryTests, RecoveryDrillTests, ReadinessTests, AuthFlowTests, SecurityHardeningTests, AuditTransactionTests, NotificationOutboxFlowTests, gRPC tests, plus a ScadaBridgeWebApplicationFactory. A 36-test Playwright E2E suite exists for the Central UI.

2.3 PerformanceTests is real but minimal — a misnomer more than a stub [Medium]

tests/ZB.MOM.WW.ScadaBridge.PerformanceTests/ has 3 test files / 10 tests: HealthAggregationTests (10 sites × 100 updates, correctness assertions only, no timing), StaggeredStartupTests (one Stopwatch assertion < 1000ms), and AuditLog/HotPathLatencyTests (the best of the three: Stopwatch.GetTimestamp p95-under-threshold on the site audit hot path; its own doc comment acknowledges "no BenchmarkDotNet" as a deliberate choice). Nothing here exercises the load-bearing performance claims of the design — 25s failover, gRPC streaming throughput, S&F drain rate, per-subscriber stream backpressure. Either rename the project to reflect its actual scope or grow it toward the design's stated performance envelope (failover time and stream throughput being the two highest-value additions).

2.4 CLI.Tests still excluded from the slnx [High]

Verified against ZB.MOM.WW.ScadaBridge.slnx: 29 of 30 test projects are members; tests/ZB.MOM.WW.ScadaBridge.CLI.Tests is absent (the src CLI.csproj is in the slnx). This is a documented gotcha in CLAUDE.md and memory, but the durable fix — adding one <Project> line — has not been made, so dotnet test ZB.MOM.WW.ScadaBridge.slnx silently skips 279 tests and a green solution run can mask CLI tests that never compiled. There is no evident reason to keep it out; add it.

---

3. Underdeveloped Areas & Deferred-Work Inventory

The in-code TODO surface is remarkably small: exactly two real TODOs in all of src (both the same issue — Transport area export, EntitySerializer.cs:251/518), zero NotImplementedException in production code paths (the one that existed was deliberately deleted — see the comment in ClusterInfrastructure/ServiceCollectionExtensions.cs:34-47), and zero FIXME/HACK. 39 "deferred/follow-up" comment mentions are almost all self-documenting pointers to logged decisions.

Consolidated inventory (item, where noted, risk of leaving it):

#	Item	Where noted	Risk if left
1	Transport: Area membership doesn't travel in bundles — exporter hardcodes AreaName: null; importer machinery is fully built (BundleImporter.cs:3104-3168 resolves/creates areas)	src/.../Transport/Serialization/EntitySerializer.cs:251,518 (TODO); contradicts Component-Transport.md:21 + CLAUDE.md #24	High — spec and CLAUDE.md claim it ships; cross-env imports silently drop area organization; half-built feature invites confusion
2	CLI.Tests not in slnx	ZB.MOM.WW.ScadaBridge.slnx; CLAUDE.md gotcha	High — 279 tests invisible to solution-level CI/test runs
3	AuditLog reconciliation dials site NodeA only; NodeB failover endpoint selection deferred	src/.../AuditLog/Central/SiteEnumerator.cs:32,64, ISiteEnumerator.cs:16	Medium — during a site NodeA outage, telemetry-loss reconciliation (the safety net) is also unavailable; audit gaps persist until NodeA returns
4	Options validation missing on ~14 components (§1.5)	e.g. Transport/ServiceCollectionExtensions.cs:25	Medium — config typos surface at first use, not startup
5	SmsConfiguration.MaxRetryCount stored but not honored at dispatch (SMTP retry policy governs SMS)	Commons/Entities/Notifications/SmsConfiguration.cs:36	Medium — admin-visible knob that does nothing; misleading UI
6	Role-check case-sensitivity asymmetry: UI RequireClaim policies vs ManagementActor's case-insensitive check	ManagementService/ManagementActor.cs:1910	Medium — a role stored with non-canonical casing could pass one gate and fail the other; deliberately unaltered, separately deferred
7	SecuredWrite audit rows leave SourceNode NULL	CLAUDE.md (Security section, logged follow-up)	Low/Medium — per-node audit forensics blind spot for two-person writes specifically
8	Hash-chain tamper evidence (T1)	CLAUDE.md; docs/plans/2026-05-20-audit-log-code-roadmap.md:12; CLI verify-chain is a shipped no-op stub (CLI/Commands/AuditVerifyChainHelpers.cs:7)	Low/Medium — deferred to v1.x by locked decision; append-only DB roles are the current control; the no-op CLI verb slightly overstates capability
9	Parquet audit archival/export (T2) — endpoint returns 501	ManagementService/AuditEndpoints.cs:204-208; CLAUDE.md	Low — deferred v1.x; 501 + CLI messaging handle it honestly
10	Aggregated live alarm stream for Alarm Summary (currently snapshot fan-out + poll)	docs/plans/2026-06-18-m7-opcua-mxgateway-ux-design.md:252	Low/Medium — operator page scales with instance count; fan-out Ask per refresh is the known cost
11	Central-persisted, auditable OPC UA server-cert trust (v1 is site-local PKI only)	m7 design follow-ups; CLAUDE.md	Low — broadcast-to-both-nodes covers HA; no central governance/audit of trust decisions
12	Native-alarm-source-override CSV bulk import	m7 design follow-ups; CLAUDE.md (M7 UX)	Low — attribute-override CSV shipped; parity gap only
13	WaitForAttribute quality-gated ("Good"-only) mode	SiteRuntime/Scripts/ScriptRuntimeContext.cs:405	Low — planned enhancement per spec §4.2
14	WaitForAttribute not wired into CentralUI Test-Run sandbox	docs/plans/2026-06-17-waitfor-deferred-items.md:222	Low — Test-Run parity gap; production scripts unaffected
15	BrowseNext continuation: server's final-page signal not surfaced (client may issue one extra empty pull)	Commons/Messages/Management/BrowseCommands.cs:34	Low — one wasted round-trip
16	StubOpcUaClient throws on browse — test-infra gap for browse/search unit tests	m7 design "risks" (2026-06-18-m7-opcua-mxgateway-ux-design.md:245)	Low — limits offline coverage of browse UI
17	Unified notifications+site-calls outbox page	2026-06-15-stillpending-completion-design.md:118 (M9 deferrals)	Low — two pages remain; explicit decision
18	Folder drag-drop	same, [PERM] permanently deferred	None — closed decision; menu reorder shipped
19	Bundle signing (asymmetric, non-repudiation), direct cluster-to-cluster pull, differential bundles	docs/plans/2026-05-24-transport-design.md:402-408	Low — v1 manifest hash + passphrase AES-GCM held sufficient
20	Deployment EXPIRED-but-unpurged row purge	ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs:310 ("deferred TODO" — workaround in place)	Low — code already compensates when reading
21	SiteAuditBacklogReporter threshold consolidation into SqliteAuditWriterOptions	AuditLog/Site/SiteAuditBacklogReporter.cs:28	Low — config-shape cleanup
22	KPI history downsampling (hourly rollups)	Component-KpiHistory.md (YAGNI)	Low — 90-day retention bounds table size
23	Vestigial site-side notification list read/write surface (§1.3)	SiteRuntime/Persistence/SiteStorageService.cs:721, Repositories/SiteNotificationRepository.cs	Low/Medium — dead-but-live API around an intentionally empty table

Overall: ~20 open items, of which only #1–#4 warrant action before the next release; the rest are consciously logged, mostly with honest in-product behavior (501s, no-op stubs that say so).

---

4. Docs-Code Drift

Drift is exceptionally low for a docs-as-spec repo. Spot checks:

• Component-KpiHistory.md vs code — verified in-sync: KpiHistoryOptions defaults (SampleInterval 60s, RetentionDays 90 [1,3650], PurgeInterval 1d, DefaultMaxSeriesPoints 200 [2,5000]) match src/.../KpiHistory/KpiHistoryOptions.cs exactly, including range constraints; source placement (IKpiSampleSource impls with owners, bucketer in Commons, query service in CentralUI) matches the actual file layout and test placement.
• Component-StoreAndForward.md vs code — verified in-sync: the doc's nuanced notification-parking semantics (DefaultMaxRetries cap, maxRetries: 0 escape hatch), ICachedCallLifecycleObserver telemetry hook, and tracking-table-vs-buffer split all correspond to the shipped files (NotificationForwarder.cs, ParkedMessageHandlerActor.cs, StoreAndForwardService.cs, ReplicationService.cs).
• Component-ClusterInfrastructure.md vs code — in-sync, and exemplary: the code carries a comment (ClusterInfrastructure/ServiceCollectionExtensions.cs:34-47) explaining that a dead throwing extension was removed because the doc's "Code Placement" section settled ownership in Host — docs and code cross-reference each other.
• [High] Component-Transport.md:21 and CLAUDE.md #24 claim Area membership travels "carried by name" — but the exporter emits AreaName: null unconditionally (Transport/Serialization/EntitySerializer.cs:251-254) with a TODO, and BundleImporter.cs:511-518 confirms "Areas don't travel (see export TODO)". The import-side resolution (create-area-if-missing, BundleImporter.cs:3104-3168) is fully built, making the doc claim look shipped on casual inspection. Either finish the export (small: add Areas to EntityAggregate + the id→name lookup) or correct the doc/CLAUDE.md to say area membership is not yet carried.
• [Low] docs/components/ reference docs lag by three components: 24 per-component reference docs exist but ScriptAnalysis (#25), KpiHistory (#26), and DelmiaNotifier (#27) are missing, while README.md:111 claims "One doc per component (plus the shared TreeView)".
• [Low] DelmiaNotifier breaks the requirements-doc naming convention: no docs/requirements/Component-DelmiaNotifier.md; the README component table row 27 links to the project README instead. Defensible for an external tool, but CLAUDE.md's own Document Conventions say component documents live in docs/requirements/ as Component-<Name>.md — either add a thin spec doc or note the exception in the conventions section.
• README component table — all 27 rows present and links resolve to existing files; component descriptions match CLAUDE.md's component list. TreeView correctly footnoted as a sub-component. No stale cross-references found in the sampled docs.
• Historical docs/plans/ design docs contain superseded "future work" lists (e.g. transport-design §18 lists site-scoped transport as future, which M8 later shipped) — acceptable since they are dated decision records, and the living docs (Component-*.md, CLAUDE.md, README) reflect the shipped state.

---

Prioritized Recommendations

1. [High] Add tests/ZB.MOM.WW.ScadaBridge.CLI.Tests to ZB.MOM.WW.ScadaBridge.slnx. One line; removes a standing CI blind spot of 279 tests and retires a documented footgun.
2. [High] Resolve the Transport Area contradiction — either implement area export (importer already complete; add Areas to EntityAggregate and the id→name lookup in EntitySerializer.cs:251) or amend Component-Transport.md:21 + CLAUDE.md #24 to state area membership does not travel.
3. [Medium] Options-validation sweep: add IValidateOptions<>/ValidateOnStart to the ~14 unvalidated option classes (Transport, StoreAndForward, SiteRuntime, Communication, InboundAPI, etc.), copying the existing HealthMonitoring/ClusterInfrastructure validator pattern.
4. [Medium] Close the AuditLog reconciliation NodeB gap (SiteEnumerator.cs:64): dial NodeB when NodeA is unreachable — the shape (SiteEntry) already supports it, and reconciliation is the pipeline's loss-recovery safety net.
5. [Medium] Honor or remove SmsConfiguration.MaxRetryCount (Commons/Entities/Notifications/SmsConfiguration.cs:36) — an admin-editable field with no effect is worse than no field.
6. [Medium] Decide the PerformanceTests story: rename to reflect its correctness-at-scale scope, or add the two highest-value real measurements (failover time, gRPC stream throughput) behind the existing Category=Performance trait.
7. [Low/Medium] Excise the vestigial site-side notification-list surface (SiteNotificationRepository reads + StoreNotificationListAsync) now that the purge guarantees the table stays empty.
8. [Low] Unify role-string casing handling between UI RequireClaim policies and the ManagementActor check (ManagementActor.cs:1910) — normalize at write (already validated canonically) and compare consistently.
9. [Low] Backfill the three missing docs/components/ reference docs (ScriptAnalysis, KpiHistory, DelmiaNotifier) or scope the README claim.
10. [Low] Add contract-lock tests for the top ClusterClient message records (mirror the existing proto contract tests) to enforce, not just intend, additive-only evolution under version skew.