lmxopcua/archreview/07-client-tooling-engineering.md

# Architecture Review 07 — Client Tooling, Analyzers, and the Cross-Cutting Engineering System

| | |
|---|---|
| **Date** | 2026-08-19 |
| **Commit** | `12b0035a` (master, clean tree) |
| **Reviewer** | Architecture review agent (deep-review sweep, slice 07 — **second update**; round 3) |
| **Supersedes** | The 2026-07-12 re-review at `f6eaa267` (itself superseding the 2026-07-08 original at `9cad9ed0`). ~1274 commits landed in the window, including the R2-12 repo-hygiene batch (`67de2c94` + the `r2/12` merge), four new driver test fleets (MQTT/Sparkplug, MTConnect, SQL, Calculation), the `Host.Tests` unit tier, the full-codebase review system (`code-reviews/`, 65 modules), and the LocalDb 0.2.1 / SQLitePCLRaw 2.1.12 / HistorianGateway 0.3.0 pin updates. |
| **Scope** | `src/Client/*` (Client.CLI, Client.Shared, Client.UI) + `tests/Client/*`; `src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers` + `tests/Tooling/*`; cross-cutting: `Directory.Build.props`, `Directory.Packages.props`, `NuGet.config`, `ZB.MOM.WW.OtOpcUa.slnx`, `.github/workflows/`, `ci/`, `scripts/`, overall test architecture (57 test projects), docs freshness, repo-root hygiene |

---

## Prior-finding status (f6eaa267 → 12b0035a)

Verified against actual code/tree state, not remediation claims. `src/Client` production code is
**again byte-identical** to the prior baseline (`git log f6eaa267..HEAD -- src/Client` is empty — a
third review round with zero client-code commits), so every client-code finding carries over
verbatim. The R2-12 hygiene batch, by contrast, checked out completely: every one of its seven
claimed closures held.

| ID | One-liner (prior) | Verified status |
|---|---|---|
| S-3 | Nightly E2E workflow is a permanent green no-op | **CLOSED** — `v2-e2e.yml` deleted by `67de2c94` (R2-12); `.github/workflows/` contains only `v2-ci.yml`. Zero tests carry `Category=E2E` (grep re-verified), so nothing pretends to be an E2E tier anymore. The documented follow-up (a fresh workflow WITH a zero-match guard once real E2ETests exist) remains a follow-up, not a gap. |
| S-4 | Driver-suite fixed sleeps are CI-exposed flake risk | **STILL OPEN, evolving** — occurrences grew ~112 → ~219 with the new driver fleet (Galaxy 37, Modbus 29, Host.IntegrationTests 27, AbCip 24, FOCAS 23, S7 16, **Mqtt 16**, TwinCAT/Sql/MTConnect 9 each, Sql.Browser 8, …). Mitigating: the house is applying the bounded-watch pattern reactively where flakes surface — `45d67092` replaced the historian recorder tests' fixed sleeps with bounded watches; `31227285` serialized meter-asserting Runtime suites (cross-collection counter pollution); many of the new suites' delays are deliberate absence-observation windows with positive controls (see U-11), not startup races. Watch-and-fix remains the right posture. |
| S-5 | Client.Shared subscription lock-discipline gaps | **STILL OPEN** — `OpcUaClientService.cs:262` (unguarded null-check/create), `:695-696` (failover nulls outside the lock), `UnsubscribeAlarmsAsync` not through the semaphore. File untouched for a third round. |
| S-6 | Global `Log.Logger` swap per CLI command | **STILL OPEN** — `src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs:123` (`Log.CloseAndFlush`), `:133` (static `Log.Logger` assignment). Unchanged. |
| S-7 | No `global.json`, but v2-ci.yml claims one pins the SDK | **STILL OPEN** — no `global.json` at root; `v2-ci.yml:10-12` still says "pinned via global.json at the repo root" (with the fallback hedge). Stakes rose slightly: the Roslyn pin comment now names an exact SDK band ("SDK 10.0.105 ships compiler 5.0.0.0 … until the SDK rolls to 10.0.110+", `Directory.Packages.props:46-49`) while CI floats on `dotnet-version: 10.0.x` (`v2-ci.yml:35,48`). |
| S-8 | First-caller interval wins for the shared data subscription | **STILL OPEN** — `OpcUaClientService.cs:262`. |
| S-9 | Strict skip-gate floor is `MIN_EXECUTED=1` (catches total outage only) | **STILL OPEN** — `scripts/ci/assert-not-all-skipped.sh:20` default 1; the unit leg (`v2-ci.yml:66-68`) sets no override. A partial-discovery outage (e.g. the mixed xunit v2/v3 population dropping one adapter) still passes green. |
| P-1 | CI does uncached restore+build passes per push | **STILL OPEN, narrowed again** — 4 legs → **2** (`build` + `unit-tests`, `v2-ci.yml:29-75`) after P-5's matrix deletion; but each job still restores from scratch, the `build` job's output is still discarded (unit leg re-builds during `dotnet test --no-restore`), and there is still no `actions/cache` for `~/.nuget/packages`. |
| P-2 | Recursive browse is N+1 over `HasChildrenAsync` | **STILL OPEN** — `OpcUaClientService.cs:230-231`; unchanged, acceptable for a diagnostic tool. |
| P-3 | (positive) probe-once integration-fixture pattern | **STILL VALID, more load-bearing** — now 14 `*.IntegrationTests` projects (was 10) probe-once-and-skip inside the whole-solution CI leg; the three new fixtures follow (and improve on) the pattern — see U-11. |
| P-4 | Unbounded channel in CLI subscribe output | **STILL OPEN** (cosmetic) — unchanged. |
| P-5 | CI integration matrix fully redundant with the unit leg | **CLOSED** — the `integration` job was deleted by `67de2c94`; `v2-ci.yml:77-86` keeps an honest tombstone comment documenting the S-2-option-#2 follow-up (an opc-plc `services:` leg gated at `MIN_EXECUTED=1`) that would make a *new* integration job earn its keep. |
| C-2 | TWE never reached Client/Tooling src (or most tests) | **STILL OPEN** — exactly 4 src projects lack `TreatWarningsAsErrors`: `Client.CLI`, `Client.Shared`, `Client.UI`, `Analyzers` (verified by exhaustive csproj scan; 56/60 src projects opted in). Test-side: 11/57 opted in (the four new driver test suites did not). |
| C-3 | (positive) CPM discipline + valid audit carve-out | **STILL VALID, improved** — the GHSA-2m69-gcr7-jv3q `NuGetAuditSuppress` was **removed** (no `NuGetAuditSuppress` anywhere in the tree) exactly per its recorded removal condition, replaced by the surgical `SQLitePCLRaw.bundle_e_sqlite3` 2.1.12 pin (`Directory.Packages.props:130-136`) + the bare `PackageReference` in `Core.AlarmHistorian.csproj:21` — the documented right way to fix a transitive CVE without `CentralPackageTransitivePinningEnabled`. The big window additions (MQTTnet 5.2.0.1603, Npgsql 10.0.3, Oracle.ManagedDataAccess.Core 23.26.300, LocalDb 0.2.1, HistorianGateway 0.3.0) all carry rationale comments and pins; `packageSourceMapping` extended correctly (`NuGet.config:9-33`). |
| C-4 | Three xunit 2.9.2 holdouts | **STILL OPEN, count grew 3 → 4** — the pin moved 2.9.2→2.9.3 with an honest upstream-blocker comment (`Directory.Packages.props:164-169`: no xunit.v3 Akka TestKit exists), but the **new `Host.Tests` joined the holdout set** (4 projects reference `xunit` v2; all 4 carry `Akka.TestKit.Xunit2`). The pin comment now drifts twice: it names 3 holdouts (there are 4) and "All 44 other test projects" (there are 57 total). Upstream-blocked on Akka; fix the comment now, migrate when the TestKit ships. |
| C-5 | No `.editorconfig`; StyleGuide.md says "ScadaBridge" | **MOSTLY CLOSED** — root `.editorconfig` landed (R2-12; deliberately suggestion-severity-only so it cannot break TWE builds, per its own header comment) and `StyleGuide.md:3` now reads "OtOpcUa". Residual: still no `dotnet format`/style CI step, so the conventions are documented but unenforced (folded into U-7). |
| C-6 | csproj boilerplate duplicates Directory.Build.props | **STILL OPEN** — and the new projects inherit the habit (e.g. `Driver.Mqtt.Tests.csproj` restates `TargetFramework`, as does `Client.Shared.csproj`). |
| C-7 | OTOPCUA0001 is warning-only exactly where TWE is missing | **STILL OPEN** — analyzer wiring itself re-verified live tree-wide (`Directory.Build.props:30-36`, `OutputItemType="Analyzer"`, excluding only the analyzer + its tests); the four new driver test/integration projects that call capability methods at the wire level all carry **commented, scoped** `NoWarn` (e.g. `Driver.Mqtt.IntegrationTests.csproj:3-8`, `Driver.Sql.Tests.csproj:3-7`, `Driver.MTConnect.Tests.csproj:3-7`) — the triage discipline held through the fleet growth. The TWE-dependence interaction with C-2 is unchanged. |
| U-1 | (positive) Client.UI is real and maturing | **STILL VALID** — unchanged (untouched in the window); 126 headless tests still CI-run. |
| U-2 | Host has no unit tier | **CLOSED** — `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/` exists: 102 tests covering exactly the DI-composition/wiring class this finding asked for (`ActiveTierRegistrationTests`, `MeshRegistrationRoleScopingTests`, `ConfigDbApiKeyStoreTests`/`ArtifactApiKeyStoreTests`, auth interceptors, `SecretsPreHostContentRootSourceTests`, `LogPipelineDiagnosticsTests`). In the slnx, so CI-gated. |
| U-3 | docs/Client.CLI.md documents 8 of 14 commands | **CLOSED** — all 13 commands now have sections (`ack` §243, `confirm` §264, `shelve` §285, `enable` §315, `disable` §333, `redundancy` §351); the command inventory is 13 files under `Commands/` and 13 doc sections — 13/13. The "104 unit tests" claim re-counted exact (104). |
| U-4 | Root planning-file sprawl contradicts its own rules | **CLOSED (with a recurrence — see U-10)** — `looseends.md` deleted; `pending.md`/`current.md` untracked+gitignored; `stillpending.md` + `HISTORIAN-GATEWAY-INTEGRATION-ISSUES.md` archived under `docs/plans/`. Tracked root .md set is now CLAUDE/README/REVIEW-PROCESS/StyleGuide + `deferment.md` — the last being a new root status register with its own drift (U-10). |
| U-5 | 7 orphaned proprietary AVEVA DLLs tracked in `lib/` | **CLOSED** — `git ls-files lib/` empty, the directory is gone from disk entirely, and `.gitignore:56` now ignores `lib/`. Zero-reference proven before removal (R2-12). |
| U-6 | sql_login.txt local-only; Wonderware husk dirs on disk | **STILL OPEN (Low, local-only)** — `sql_login.txt` still on disk at root (untracked, `.gitignore:47`); `export-clean-copy.bat` still lacks an explicit exclusion. The `src/` Wonderware husks are gone; two untracked `bin`/`obj`-only test husks remain (`tests/Drivers/…Historian.Wonderware{,.Client}.Tests/`). |
| U-7 | Missing CI stages inventory | **STILL OPEN, narrowed** — `.editorconfig` now exists but no `dotnet format` step reads it; no explicit failing NuGet-audit step; `scripts/check-code-reviews-readme.ps1` and `code-reviews/regen-readme.py --check` (both real, both used manually) are not CI-gated — increasingly wasteful now that the review index covers 65 modules (U-12). The prior `ci/ab-server.lock.json` item resolved itself structurally: the lockfile exists and is consumed by the AbCip fixture docs/tests, and `docs/v2/test-data-sources.md` no longer claims a phantom Actions step. |
| U-8 | CLI security posture is dev-tool-grade | **STILL OPEN** — `ConnectionSettings.AutoAcceptCertificates` still defaults `true` (`Models/ConnectionSettings.cs:41`); `-P` argv-visible; undocumented. |
| U-9 | STATUS.md merged to master still claims nothing is merged | **CLOSED** — `archreview/plans/STATUS.md:13` retitles the topology table "HISTORICAL — as of 2026-07-08; since merged + pushed" and the resume-facts strikethrough (`STATUS.md:116-119`) records the supersession explicitly. |

**Net for the window:** 8 CLOSED (S-3, P-5, U-2, U-3, U-4, U-5, U-9, and C-5 mostly), 1 improved-in-kind
(C-3's suppress→pin), 13 still open — of which the six client-code findings (S-5/S-6/S-8/P-2/P-4/U-8)
have now survived **two full review rounds untouched** because nobody has edited `src/Client` since
before round 1. Every R2-12 remediation claim in `STATUS.md` checked out against the tree. One prior
finding got *worse* by count (C-4: 3→4 holdouts, via the otherwise-welcome `Host.Tests`).

---

## Architecture Overview

### Client stack

Unchanged for the third consecutive review (zero `src/Client` commits since before `9cad9ed0`):
**Client.Shared** (975-line `OpcUaClientService` behind five adapter seams), **Client.CLI**
(CliFx, 13 commands, session-per-invocation), **Client.UI** (Avalonia, 126 headless tests). The
stack is stable and CI-run; its known lock-discipline and posture findings are frozen with it.

### Tooling

**OTOPCUA0001** remains wired tree-wide via `Directory.Build.props:30-36` and the discipline
propagated to the new driver fleet: every new wire-level test project carries the commented `NoWarn`
with the 07/C-1 rationale, none silently. The analyzer remains the standing regression guard for the
capability-invoker dispatch rule.

### Test architecture map (grew substantially)

**57 test projects** (was 47; verified `git ls-files 'tests/**/*.csproj' | wc -l`), all in the slnx
(119 tracked csprojs, 117 slnx entries — the only exclusions are the two `tools/mem-probe/` probes,
U-13):

1. **`*.Tests`** — unit suites; xunit.v3 + Shouldly except the four Akka-TestKit-blocked v2
   holdouts (AdminUI, ControlPlane, Runtime, **Host**). New since `f6eaa267`: `Host.Tests` (102),
   `Driver.Mqtt.Tests` (**541**), `Driver.MTConnect.Tests` (**408**), `Driver.Sql.Tests` (263) +
   `Driver.Sql.Browser.Tests`, `Driver.Calculation.Tests` (44 — proportionate to a 13-file
   pseudo-driver).
2. **`*.IntegrationTests`** — 14 projects (was 10; new: Mqtt, MTConnect, Sql, AbLegacy), probe-once
   fixtures, `Assert.Skip` when unreachable. The MQTT fixture deliberately inverts the
   default-endpoint convention — **no default endpoint, unset env ⇒ skip** — with a documented
   rationale (per-machine generated credentials would turn "probe and hope" into auth failures;
   `MqttFixture.cs:9-19`). `dotnet test` offline on macOS stays green (verified pattern across all
   three new fixtures).
3. **`Category=LiveIntegration`** — env-gated live suites (MQTT live matrix, HistorianGateway);
   further env-gated live tests without the category (Galaxy gateway smokes, S7 blackhole/outage,
   SQL blackhole) all `Assert.Skip` on absent env vars — CI-safe by construction.
4. **`Category=E2E`** — the tier no longer exists anywhere: no tests, no workflow (S-3 closed).

### Build / CI pipeline shape

- `Directory.Build.props` — net10.0 defaults + tree-wide analyzer injection; the CVE carve-out is
  **gone** (resolved, C-3).
- `Directory.Packages.props` — CPM throughout, zero floating versions (verified), every
  window addition rationale-commented.
- CI (`v2-ci.yml`) — two jobs: `build`, then the whole-solution unit leg with the strict
  fail-on-skip gate (`scripts/ci/assert-not-all-skipped.sh`) and trx upload. The redundant
  integration matrix and the no-op nightly are deleted (P-5, S-3).
- `code-reviews/` — new: a 65-module review index (`findings.md` per module, generated README with
  `--check`, process in `REVIEW-PROCESS.md`) — a real engineering-system asset (U-12).

---

## Findings

Severity scale: **Critical** (broken now, corrupts trust) / **High** (material gap, fix soon)
/ **Medium** (real debt) / **Low** (polish).

Closed prior findings live only in the status table above. Still-open findings keep their IDs; new
findings continue each scheme's numbering (S-10, P-6, U-10…U-13).

### 1. STABILITY

#### S-4 (Medium) — fixed sleeps grew with the fleet; the deflake pattern is applied reactively, which is working
~219 `Task.Delay`/`Thread.Sleep` occurrences across driver + Host suites (was ~112), the growth
coming from the four new fleets. The character changed, though: the new suites' delays are mostly
*bounded observation windows for absence assertions with positive controls* (e.g.
`MqttConnectionTests.cs:592-616` — a no-reconnect-storm window whose companion asserts `retries ≥ 1`
under the same clock; `:660-682` bounds the backoff rate from both sides), which is the
house-documented correct idiom, not a startup race. And where genuine flakes surfaced, they were
fixed with the readiness/bounded-watch pattern (`45d67092` historian recorder; `31227285`
meter-suite serialization; `409a58b6` non-vacuous reprobe test). **Recommendation** unchanged:
keep the watch-and-fix posture; do not pre-emptively rewrite the absence windows — they are load-bearing.

#### S-5 (Medium) — Client.Shared subscription lock-discipline gaps, third round unchanged
`OpcUaClientService.cs:262` (check/create of `_dataSubscription` outside the lock), `:695-696`
(failover nulls both subscriptions without the lock, contradicting `DisconnectAsync`'s stated
assumption at `:147-148`), `UnsubscribeAlarmsAsync` not routed through `_alarmSubscribeSemaphore`.
Near-unhittable for the session-per-command CLI; real for long-lived Client.UI usage. The finding
is now old enough that it should either be fixed (small: an async gate + take the lock in
`RunFailoverAsync`) or explicitly waived with a comment at the site.

#### S-6 (Medium) — global `Log.Logger` swap per CLI command
Unchanged: `CommandBase.cs:123,133`. Per-execution instance `ILogger` remains the fix.

#### S-7 (Low) — no `global.json`; the workflow's claim persists; the Roslyn pin's SDK-band coupling tightened
Still no `global.json`; `v2-ci.yml:10-12` still asserts one exists (hedged). Meanwhile the
`Microsoft.CodeAnalysis.CSharp` 5.0.0 pin rationale now depends on an exact SDK patch band
(`Directory.Packages.props:46-49`: "until the SDK rolls to 10.0.110+") while CI floats
`dotnet-version: 10.0.x` — so the day the hosted runner picks up a newer 10.0.x SDK, the
ScriptAnalysis compiler-parity assumption changes with no repo-side signal. **Recommendation:**
add `global.json` (`rollForward: latestFeature` at most) — it now protects a documented invariant,
not just a comment's honesty.

#### S-8 (Low) — first-caller interval wins for the shared data subscription
Unchanged (`OpcUaClientService.cs:262`).

#### S-9 (Low) — the strict skip-gate floor is still `MIN_EXECUTED=1`
`assert-not-all-skipped.sh:20` defaults to 1 and the unit leg sets no override
(`v2-ci.yml:66-68`). The whole-solution run now executes well north of 5,000 tests, so the
distance between "gate passes" and "coverage intact" widened. Raising the floor to a conservative
constant (e.g. 3000) is still a one-line change that catches the partial-discovery-outage class —
now including the four-holdout mixed xunit v2/v3 population (C-4) losing an adapter.

#### S-10 (Low, NEW) — three review rounds in, no CI run has ever been evidenced from the repo
The pipeline has been statically verified three times (here included: YAML well-formed, script
logic correct, filters sound), but the repo contains no artifact of an actual green run on the
Gitea act_runner (`origin` is `gitea.dohertylan.com`; no badge, no recorded run link, no
runner-fixing commits that would imply one). Given this slice's whole thesis — "built and tested
but plugged into nothing" — the CI pipeline itself is the one remaining component whose *execution*
is unproven from inside the tree. **Recommendation:** record one green run (a README badge, or a
line in `deferment.md`/STATUS with the run URL) so the next review can verify the runner leg the
way everything else here is verified.

### 2. PERFORMANCE

#### P-1 (Low, narrowed again) — 2 uncached restore+build passes per push
Down from 7 (round 1) → 4 (round 2) → 2 legs, entirely by deleting redundant jobs. The remaining
waste is now small but pure: no `actions/cache` for NuGet keyed on `Directory.Packages.props`,
and the `build` job's output is discarded (the unit leg rebuilds under `dotnet test`). Severity
lowered to Low — fix opportunistically.

#### P-2 (Low) — recursive browse is N+1 over `HasChildrenAsync`
Unchanged (`OpcUaClientService.cs:230-231`). Acceptable for a diagnostic tool.

#### P-3 (Low, positive) — integration-fixture cost engineering held through the fleet growth
Now 14 probe-once projects; the three new fixtures adopted the pattern and the MQTT one improved
it (env-gate before probe, so no wasted dials at hosts that were never provisioned).

#### P-4 (Low) — CLI subscription output channel unbounded
Unchanged; cosmetic.

#### P-6 (Medium, NEW) — the whole-solution unit leg's wall-clock is growing linearly with the driver fleet, and the new MQTT suite carries real-time socket windows
The single self-maintaining leg was the right fix for coverage, but its cost curve deserves a
look before the next two drivers land: `Driver.Mqtt.Tests` (541 tests) runs a real in-process
TCP `MiniBroker`/`BlackholeBroker` and its connection suite alone contains ~45s+ of deliberate
real-time windows (`MqttConnectionTests.cs:613` 3s, `:675` 8s, `:708` 5s, `:824` 9s, `:859` 7s,
`:931` 5s, `:1024`/`:1100` 4s each) — each individually justified (absence assertions with
positive controls, backoff-rate bounds), but cumulatively a fixed tax on every push that xunit
parallelism only partly amortizes. `Driver.MTConnect.Tests` (408) and `Driver.Sql.Tests` (263)
are cheaper but add to the same leg. **Recommendation:** publish per-project durations from the
trx (the data is already uploaded) and set a soft budget; when the leg crosses ~15 min, split the
slow socket suites behind a `Category=SlowUnit` second job rather than un-writing the windows.

### 3. CONVENTIONS

#### C-2 (Medium) — `TreatWarningsAsErrors` still absent from exactly the Client/Tooling src slice
The gap set is unchanged and now precisely enumerable: `Client.CLI`, `Client.Shared`, `Client.UI`,
`Analyzers` are the only 4 of 60 src csprojs without TWE. The prior argument stands verbatim — the
client stack is v2-era code with no legacy-warning debt, and closing this is also what gives
OTOPCUA0001 teeth there (C-7). Test-side opt-in is 11/57; the four new driver test fleets did not
opt in (they follow the majority convention, so no new debt class — just more of it).

#### C-3 (Low, positive) — CPM discipline is exemplary and passed a real test this window
The window was this finding's stress test and it held: the audit suppress was removed per its own
recorded exit condition and replaced with the textbook surgical transitive pin
(`Directory.Packages.props:130-136` + `Core.AlarmHistorian.csproj:21`); three genuinely risky new
dependencies were pinned with honest rationale (MQTTnet v5 chosen over SparkplugNet's stale 4.x
pin with the Sparkplug layer hand-rolled instead, `Directory.Packages.props:79-86`; managed-only
Npgsql/Oracle with the "no native client" constraint stated, `:93-110`); `packageSourceMapping`
was extended for every new `ZB.MOM.WW.*` package (LocalDb resolves from the Gitea feed, not the
stale-prone folder feed — only `MxGateway.*` stays on `./nuget-packages`, currently in sync at
0.2.0).

#### C-4 (Low) — xunit v2 holdouts grew 3 → 4, and the pin comment drifted
`AdminUI.Tests`, `ControlPlane.Tests`, `Runtime.Tests` + **`Host.Tests`** (new) reference `xunit`
2.9.3; all four carry `Akka.TestKit.Xunit2`, so the block is genuinely upstream (no xunit.v3 Akka
TestKit as of the pin comment's survey). Two comment drifts to fix now:
`Directory.Packages.props:164-169` names 3 holdouts and "All 44 other test projects" — the tree
has 4 and 57. Migrate all four when Akka ships a v3 TestKit; until then the mixed-population risk
in the single CI leg is why S-9's floor should rise.

#### C-5 (Low, residual only) — `.editorconfig` exists; nothing enforces it
The R2-12 `.editorconfig` is deliberately suggestion-only (its header says so — nothing in it can
fail a TWE build), and there is still no `dotnet format --verify-no-changes` CI step. That
combination means the file documents conventions without enforcing any. Fine as a first step;
the enforcement half lives in U-7's inventory.

#### C-6 (Low) — csproj boilerplate still duplicates Directory.Build.props
Unchanged, and propagating: the new driver test csprojs restate `TargetFramework` etc. Strip on
next sweep; consider a `tests/Directory.Build.props` layer to absorb the common test-project
shape (xunit refs, `NoWarn`, IsPackable) while at it.

#### C-7 (Low) — OTOPCUA0001 enforcement strength remains TWE-dependent
Unchanged interaction (see C-2). Re-verified this round: the wiring is live
(`Directory.Build.props:30-36`), the new projects' suppressions are scoped and commented, and no
uncommented `NoWarn` for the rule exists anywhere. If TWE promotion keeps stalling, escalate the
rule to error-by-default in the analyzer — its intentional-call story has now survived a
four-driver fleet expansion without a single undocumented suppression.

### 4. UNDERDEVELOPED AREAS

#### U-1 — Client.UI: real product, adequately tested, unchanged
Verdict stands (126 headless tests, CI-run). The AXAML-binding blind spot note stands with it.
Worth stating plainly after three rounds: the client stack is receiving **zero** investment while
the server side moves fast — acceptable for a diagnostic tool, but S-5/U-8 should be fixed or
formally waived rather than carried open indefinitely.

#### U-6 (Low) — local-only hygiene
Unchanged: `sql_login.txt` on disk (gitignored, `.gitignore:47`), no `export-clean-copy.bat`
exclusion; two untracked Wonderware test-husk dirs (`bin`/`obj` only) safe to delete locally.

#### U-7 (Low, narrowed) — missing CI stages inventory
Remaining un-gated: `dotnet format` (the `.editorconfig` now exists to back it), an explicit
failing NuGet-audit step, and — newly the biggest item — the review-system freshness checks that
already exist as scripts: `code-reviews/regen-readme.py --check` and
`scripts/check-code-reviews-readme.ps1`. With 65 modules indexed (U-12), a drifted generated
README is now the most likely doc-rot vector, and the check is one CI line away.

#### U-8 (Low) — CLI security posture is dev-tool-grade
Unchanged (`ConnectionSettings.cs:41` `AutoAcceptCertificates = true`; `-P` argv-visible;
undocumented). Document the posture; add `--strict-certs`.

#### U-10 (Low, NEW) — `deferment.md`: the root status register drifts against itself
The register that replaced the U-4 sprawl is genuinely better (single file, source-verified
method, explicit baseline discipline) but exhibits the exact self-drift class the house has
already named: its header pins "**Current tree:** `master` @ `4bdce910`, 2026-08-04"
(`deferment.md:16`) while its own §9 carries entries through 2026-08-10 (last commit `4d549571`,
"eighth round"), and HEAD is now 2026-08-19 — so the "live status" section is both internally
mislabeled and ~9 days behind a tree that closed items it lists. Same failure shape as the closed
U-9/S-7: a bookkeeping doc whose header contradicts its body erodes trust in the parts still
accurate. **Recommendation:** update the header on every §9 append (or drop the "Current tree"
line entirely — `git log -1 -- deferment.md` already dates it).

#### U-11 (positive, NEW) — the four new driver test fleets meet, and mostly exceed, the house bar
Assessed against the house criteria (probe-once fixtures, skip-clean offline, falsification
controls, rationale'd suppressions):
- **Scale proportionate to surface:** Mqtt 541 / MTConnect 408 / Sql 263+Browser / Calculation 44
  (a 13-file pseudo-driver) unit tests; all in the slnx, all CI-gated.
- **Fixtures:** all three new `*.IntegrationTests` are `Assert.Skip`-gated; the MQTT fixture's
  no-default-endpoint design (`MqttFixture.cs:9-19`) is a deliberate, documented improvement over
  the probe-a-well-known-host convention for credentialed fixtures. Offline `dotnet test` stays
  green (the CI-viability property).
- **Infrastructure ownership:** the project-owned C# Sparkplug edge-node simulator
  (`Driver.Mqtt.IntegrationTests/SparkplugSimulator/`, TWE'd) rather than a third-party sim;
  the SQL suites gained an **orphan-test-database reaper** (`8946644b`) that was then itself
  hardened against injection/hangs (`580b77ce`: GUID-suffix validation, `QUOTENAME`, short
  reaper connect timeout) — test infrastructure treated as production code.
- **Falsification discipline:** absence windows carry positive controls (MqttConnectionTests
  backoff bounds both sides); suppression comments state *why* per project.
The bar this review series set in round 1 ("driver suites are the flake frontier") is being met
by the new arrivals better than by the legacy suites.

#### U-12 (positive, NEW) — the per-module review system is a real engineering asset; gate its freshness
`code-reviews/` (70 entries incl. 65 modules), generated README with `--check`, per-module
`findings.md` as source of truth, process codified in root `REVIEW-PROCESS.md`, and a recorded
fleet-wide zero-open state (2026-08-10). This is the kind of cross-cutting quality machinery this
slice exists to look for. Its one gap is U-7's: nothing in CI runs `regen-readme.py --check`, so
the generated index can silently drift from the findings files it summarizes.

#### U-13 (Low, NEW) — `tools/mem-probe/` is tracked but outside the slnx, so CI never compiles it
`LeanContext.csproj` + `MemProbe.csproj` are the only two tracked csprojs absent from
`ZB.MOM.WW.OtOpcUa.slnx` (117 of 119). They are a legitimate measurement harness with design-doc
provenance (`3834400f`, the VT memory-scalability work), but as-is they will bit-rot invisibly —
the exact class the slnx-driven CI leg was built to prevent. **Recommendation:** either add them
to the slnx (they're tiny) or move them under the design doc's plan directory as recorded
artifacts if they are one-shot.

---

## Maturity Ratings (1 = ad hoc, 5 = exemplary)

| Dimension | 2026-07-12 | 2026-08-19 | Justification |
|---|---|---|---|
| Stability | 3.5 | **4** | The last always-green surface is gone (S-3: the no-op nightly deleted rather than left lying), the whole-solution CI leg + strict skip-gate survived a 47→57-project fleet expansion untouched, new live/fixture suites are uniformly skip-clean offline, and flakes that surfaced were fixed with the house bounded-watch pattern rather than retries. Held below 4.5 by: no evidenced CI run in-repo after three rounds (S-10), the `MIN_EXECUTED=1` floor (S-9), and the Client.Shared lock gaps frozen for a third round (S-5). |
| Performance | 3.5 | **3.5** | Net wash again: CI legs 4→2 and the redundant matrix is gone, but restore/build stays uncached and the unit leg's wall-clock now grows linearly with the driver fleet, including ~45s of real-time socket windows in one new suite (P-6). Client-side perf posture unchanged (untouched code). |
| Conventions | 4 | **4** | Held, and stress-tested: the analyzer wiring + scoped-suppression discipline propagated cleanly to four new driver fleets; CPM discipline passed a real CVE-pin transition and three risky dependency adoptions with documented rationale; `.editorconfig` + StyleGuide fix landed. Held below 4.5 by the same TWE gap (C-2/C-7, now exactly 4 projects), the xunit holdout set *growing* to 4 with a drifted pin comment (C-4), and conventions documented-but-unenforced (C-5 residual). |
| Underdeveloped areas | 3 | **4** | This bucket moved most: `lib/` DLLs gone, root sprawl resolved, CLI docs complete (13/13, counts exact), `Host.Tests` closed the last coverage-matrix hole, STATUS.md corrected, and two genuinely new assets landed (the 65-module review system, the deferment register). Held below 4.5 by the register's self-drift (U-10), the ungated freshness checks (U-7/U-12), and the client stack's frozen open findings (U-1 note, U-8). |

---

## Top recommendations (ordered)

1. Evidence one green CI run from the Gitea runner and record it in-repo (S-10); raise the unit-leg `MIN_EXECUTED` floor while you're in the file (S-9) — together these close the last "is the gate itself real?" question.
2. Add `regen-readme.py --check` + `check-code-reviews-readme.ps1` as CI steps (U-7/U-12) — one line each, protecting the 65-module review index.
3. Add TWE to the four Client/Tooling src projects (C-2) — unchanged ask, third round; it is also what hardens OTOPCUA0001 there (C-7).
4. Fix or formally waive the three `OpcUaClientService` lock gaps (S-5) — the code is stable, so the fix is cheap and final; document the CLI cert/credential posture (U-8).
5. Add `global.json` (S-7) — it now guards the documented Roslyn/SDK band invariant, not just a comment.
6. Keep `deferment.md`'s header honest on every append (U-10); fix the xunit pin comment's counts (C-4); put `tools/mem-probe` in the slnx or archive it (U-13).
7. Watch the unit leg's duration as the fleet grows; split slow socket suites behind a category before un-writing any absence windows (P-6, S-4).