# Plan 07 — Client Tooling, Analyzers & Cross-Cutting Engineering System
> **Status (2026-07-08):** ✅ **C-1 DONE** — `fix/archreview-c1-wire-analyzer` `f0082af5` (OTOPCUA0001 wired
> tree-wide + ~280 hits triaged; negative-control-verified live; **surfaced the RESILIENCE-DISPATCH-GAP, task
> #10**). ✅ **S-1/S-2/S-4 DONE** — `fix/archreview-s1-s2-ci-coverage` `10b89830` (whole-solution unit leg +
> `scripts/ci/assert-not-all-skipped.sh` fail-on-skip gate + CLI sleep→readiness deflake; skip-gate proven
> locally, CI-run verification pending a push). Remaining in this domain: S-3 (nightly E2E no-op), S-7
> (global.json), U-3/U-4/U-5/U-6 hygiene, C-2..C-6. See [`STATUS.md`](STATUS.md).
| | |
|---|---|
| **Source report** | `archreview/07-client-tooling-engineering.md` |
| **Overall context** | `archreview/00-OVERALL.md` (theme #4 "verification system", action #6/#11) |
| **Base commit** | `9cad9ed0` (master); verified against current working tree 2026-07-08 |
| **Domain** | Client.CLI/Shared/UI, Analyzers, build/CPM, CI, test architecture, repo hygiene |
## Verification summary
26 findings in the report (S-1..S-8, P-1..P-4, C-1..C-6, U-1..U-8). Every actionable
finding was re-verified against the live tree; **all confirmed, none stale**. Four items are
explicitly "no-action / positive" in the report (P-3, P-4, C-3, U-1) and are recorded but not
planned. Key verifications performed:
- **C-1 CONFIRMED** — repo-wide grep for `ZB.MOM.WW.OtOpcUa.Analyzers` in `.csproj/.props/.targets`
returns only the analyzer's own csproj and its test project's `ProjectReference`. Zero
`OutputItemType="Analyzer"` anywhere in the tree (the only hit is inside the review markdown).
- **S-1/S-2 CONFIRMED** — `v2-ci.yml:47-52` gates exactly 5 unit projects; slnx contains 47
`*.Tests.csproj` (10 of them `*.IntegrationTests`). Integration job (`:61-76`) runs
fixture-less on `ubuntu-latest` with no `services:`. Category traits are declared via
`[Trait("Category", "…")]`, so a `Category!=E2E&Category!=LiveIntegration` filter is valid.
- **S-3 CONFIRMED** — grep for `Category…E2E` across `tests/**/*.cs` returns zero matches; the
nightly workflow's own header admits the no-op.
- **S-7 CONFIRMED** — no `global.json` at root (nor anywhere); `v2-ci.yml:10-12` claims one exists.
- **U-3 CONFIRMED** — `docs/Client.CLI.md` has zero sections for `confirm`, `shelve`, `disable`
(and only substring hits for `ack`/`enable`); all 5 alarm commands ship as command files.
- **U-4 CONFIRMED** — `git ls-files` shows `pending.md`, `current.md`, `looseends.md`,
`stillpending.md`, `HISTORIAN-GATEWAY-INTEGRATION-ISSUES.md` all tracked; `pending.md:3` itself
declares "HARD RULE: never stage `pending.md` / `current.md`".
- **U-5 CONFIRMED** — `git ls-files lib/` lists 7 tracked proprietary AVEVA DLLs; zero `HintPath`
hits reference `lib/` anywhere.
- **U-6 CONFIRMED** — `sql_login.txt` is `.gitignore:47`, `git check-ignore` matches, `git log`
empty (never committed). The retired `Driver.Historian.Wonderware*` dirs under `src/`+`tests/`
are untracked `bin/obj` husks (NOT tracked projects; not in slnx). Tracked Wonderware residue is
only `docs/drivers/Historian.Wonderware.md` (an intentional retired-stub) and 3
`code-reviews/Driver.Historian.Wonderware*/findings.md`.
- **C-2 CONFIRMED** — none of the 4 client/tooling src csprojs set `TreatWarningsAsErrors`.
- **C-5 CONFIRMED** — no `.editorconfig`; `StyleGuide.md:3` says "for all **ScadaBridge**
documentation".
- **S-5/S-6/U-8 CONFIRMED** — `OpcUaClientService.cs` lock gaps at cited lines; `CommandBase.cs:123`
`Log.CloseAndFlush()` + `:133` global `Log.Logger` swap; `CommandBase.cs:91`
`AutoAcceptCertificates = true`.
---
## Priority ordering
**Tier A — restore meaning to green CI (do first):** C-1, S-1, S-2, S-3, S-7.
**Tier B — repo hygiene:** U-5, U-4, U-6, C-5 (StyleGuide title).
**Tier C — docs:** U-3.
**Tier D — engineering debt (as capacity allows):** C-2, P-1, U-2, S-5, S-6, S-4, C-4, C-6, S-8, U-8, P-2.
No-action (recorded only): P-3, P-4, C-3, U-1, U-7.
---
## TIER A — Restore meaning to green CI
### C-1 (High) — Wire the OTOPCUA0001 analyzer into every project
**Restatement:** the sole custom analyzer enforces its CapabilityInvoker-wrapping rule against
nothing but its own 31 unit tests; no consuming project references it.
**Verification:** confirmed — only self + test `ProjectReference`; zero `OutputItemType="Analyzer"`.
**Root cause:** the "built-but-never-wired" house failure mode (theme #1). The analyzer project was
authored and unit-tested but the DI/build-graph wiring step was never done.
**Design:** inject the analyzer as an analyzer-type `ProjectReference` from `Directory.Build.props`,
conditioned to exclude the analyzer project itself (and its own build, to avoid a self-reference
cycle). Analyzer projects target `netstandard2.0`, so `ReferenceOutputAssembly="false"` +
`OutputItemType="Analyzer"` is the correct incantation — the analyzer DLL is loaded into the
compiler host, not linked. This makes OTOPCUA0001 run on every `src/` and `tests/` compilation.
Add to `Directory.Build.props` (after the existing `ItemGroup`):
```xml
```
(The `.Analyzers.Tests` exclusion is belt-and-suspenders: it already `ProjectReference`s the analyzer
directly; letting `Directory.Build.props` add a second analyzer-typed reference is harmless but noisy.)
**Triage sub-step (required, do NOT skip):** OTOPCUA0001 is a diagnostic. Its default severity must
be checked in `UnwrappedCapabilityCallAnalyzer` — if it is `Warning` and a src project has TWE on,
the wired-in analyzer can **break the build** on the first unwrapped call it finds. So the wiring
must be done in this order:
1. Wire the `ProjectReference` (above).
2. `dotnet build ZB.MOM.WW.OtOpcUa.slnx` and capture every OTOPCUA0001 hit.
3. For each hit, decide: real violation → route the call through `CapabilityInvoker`; intentional
(test doubles, the invoker's own internals) → add a scoped `#pragma warning disable OTOPCUA0001`
or a per-project `OTOPCUA0001` with a comment. The analyzer's own tests
demonstrate the intended pattern.
4. Only after the tree is clean should any project promote OTOPCUA0001 to error (out of scope here).
**Files to touch:** `Directory.Build.props` (add ItemGroup); plus any `.csproj` needing `NoWarn`
and any source files with genuine unwrapped calls surfaced by step 2.
**Verify:** `dotnet build ZB.MOM.WW.OtOpcUa.slnx` shows OTOPCUA0001 diagnostics on real code (proving
it is live); a deliberately-unwrapped call in a scratch file trips it; the analyzer's 31 tests still
pass. Confirm the build graph does not regress (`dotnet build` clean after triage).
**Effort:** M (wiring is S; the triage wave is the unknown — could be S or M depending on how many
unwrapped calls exist). **Risk:** Medium — a Warning-severity analyzer on TWE projects can fail the
build; the ordered triage above contains it. Blast radius is every compilation, so build once
locally before pushing.
---
### S-1 (High) — CI gates ~15% of the test matrix
**Restatement:** `v2-ci.yml` runs 5 of 47 unit-test projects; Client (388 tests), Analyzers (31),
all driver and most Core suites never run in CI.
**Verification:** confirmed (5 enumerated, 47 total).
**Root cause:** hand-maintained matrix predates most projects and was never widened; new projects
drift out of coverage silently.
**Design:** replace the hand-maintained `unit-tests` matrix with a single solution-wide leg that
excludes only the env-gated tiers. The skip-gated `*.IntegrationTests` fixtures already tolerate
unreachable endpoints (they `Assert.Skip`), so running them here is safe; but to keep the unit leg
fast and deterministic, split responsibilities:
- **`unit-tests` job:** one step —
`dotnet test ZB.MOM.WW.OtOpcUa.slnx --configuration Release --no-restore
--filter "Category!=E2E&Category!=LiveIntegration" --logger "trx;LogFileName=unit.trx"`
after an explicit `dotnet restore` + `dotnet build --no-restore` (share build via `--no-build`
where possible; see P-1). This runs all 47 projects in one invocation — no matrix to drift.
Rationale for the whole-solution filter over regenerating a matrix from the slnx: it is
self-maintaining (new project = automatically covered), matches CLAUDE.md's own
`dotnet test ZB.MOM.WW.OtOpcUa.slnx` guidance, and the trx output feeds S-2's skip-gate. The
integration split (Host.IntegrationTests / OpcUaServer.IntegrationTests as a distinct matrix leg)
stays, but is folded into the fail-on-skip work below (S-2).
**Files to touch:** `.github/workflows/v2-ci.yml` — replace the `unit-tests` job body.
**Verify:** push to a branch; the `unit-tests` job log shows Client.CLI/Shared/UI + Analyzers +
driver + Core suites executing (not skipped); wall-clock is acceptable (pair with P-1 caching).
**Effort:** S. **Risk:** Medium — first full-solution CI run will surface latent failures /
flakes that were never gated (esp. the fixed-sleep timing tests, S-4). That is the point, but budget
time to stabilize. Blast radius: CI only.
---
### S-2 (High) — "green" integration CI means "skipped"; skip is indistinguishable from pass
**Restatement:** the integration job runs fixture-less against `10.100.0.35` defaults → probe fails
→ `Assert.Skip` → job green. A real fixture outage cannot be told from a real pass.
**Verification:** confirmed — no `services:` in the integration job; fixtures hard-default to
`10.100.0.35` (Modbus/AbServer/Snap7/OpcPlc fixtures + `DriverTestConnectE2eTests`).
**Root cause:** the probe-once skip pattern is correct for dev boxes, but CI has no signal that the
tier ran nothing.
**Design — two complementary changes:**
1. **Make skips visible/failing.** Emit `--logger "trx"` from the integration leg and add a post-test
step that parses the trx and **fails the job when skipped-count exceeds a threshold** (e.g. any
integration test skipped when CI was supposed to have fixtures, or > N total). A small inline shell
or a tiny `scripts/ci/assert-not-all-skipped.ps1`/`.sh` reading the ``/`outcome="NotExecuted"`
counts. This is the minimal, always-applicable guard.
2. **(Optional, higher value) Actually start the reachable fixtures as workflow `services:`.** The
Modbus sim and opc-plc images run on any hosted runner; add them as `services:` and set
`MODBUS_SIM_ENDPOINT=localhost:5020` / `OPCUA_SIM_ENDPOINT=opc.tcp://localhost:50000` so those
probes connect and the tests actually run. S7/AbCip/Galaxy fixtures have no hosted-runner image and
stay legitimately skipped (guard #1 excludes them from the threshold, or scopes the threshold to
the modbus/opc-plc subset).
Recommend shipping **guard #1 first** (cheap, universal), then #2 as a follow-up.
**Files to touch:** `.github/workflows/v2-ci.yml` (integration job: add trx logger, post-step, and
optionally `services:` + endpoint env); optionally a new `scripts/ci/assert-not-all-skipped.*`.
**Verify:** deliberately point an endpoint at an unreachable host in a branch → integration job goes
**red** (not green). With services up, the modbus/opc-plc tests show as passed, not skipped.
**Effort:** S (guard #1) / M (with services). **Risk:** Low-Medium — a too-tight threshold could
red-flag legitimately-unavailable fixtures; scope the threshold to the images CI can actually run.
---
### S-3 (Medium) — nightly E2E workflow is a permanent green no-op
**Restatement:** `v2-e2e.yml` filters `Category=E2E`, which matches zero tests; it boots the full
docker-dev fleet for nothing and trains people to ignore a nightly green.
**Verification:** confirmed — zero `Category=E2E` in the tree; the workflow header admits it.
**Root cause:** the E2E project (`tests/Server/ZB.MOM.WW.OtOpcUa.E2ETests`) was scoped but never
landed.
**Design (pick one):**
- **(a) Disable the schedule until the project exists** — comment out the `schedule:` trigger, keep
`workflow_dispatch`, and add a guard step that fails if `Category=E2E` matches zero tests (so the
no-op can't silently return). Lowest effort; removes the false-green.
- **(b) Land a minimal E2E round-trip** — a single `[Trait("Category","E2E")]` test that, against the
booted docker-dev fleet, connects via `Client.Shared`/`Client.CLI` to `opc.tcp://localhost:4840`,
browses to a known seeded node, reads a value, and asserts Good. The `scripts/e2e/test-*.ps1`
harnesses show exactly what to assert. This gives the nightly job real meaning and exercises the
otherwise-uncovered Client stack end-to-end.
Recommend **(a) now** (removes the false signal immediately) with **(b) as the tracked follow-on** —
option (a)'s zero-match guard also protects (b) from regressing back to a no-op.
**Files to touch:** `.github/workflows/v2-e2e.yml`; for (b) a new `tests/Server/…E2ETests` project +
slnx entry.
**Verify:** (a) manual dispatch fails-fast on zero E2E tests; (b) the round-trip test passes against
the fleet and fails if the seeded node is absent.
**Effort:** S (a) / M-L (b). **Risk:** Low.
---
### S-7 (Low) — no `global.json` though the build depends on an exact SDK band
**Restatement:** `v2-ci.yml:10-12` claims a repo-root `global.json` pins the SDK; none exists.
Meanwhile `Directory.Packages.props:44-49` pins Roslyn 5.0.0 to match SDK 10.0.105's compiler.
**Verification:** confirmed — no `global.json`; the CI comment is false.
**Root cause:** the pin comment describes intended state that was never created; the Roslyn pin note
implicitly depends on SDK version stability the repo doesn't enforce.
**Design:** add a root `global.json`:
```json
{
"sdk": {
"version": "10.0.105",
"rollForward": "latestFeature"
}
}
```
`latestFeature` allows patch/feature SDK rolls within the 10.0.1xx band but keeps the major/minor
stable, so the Roslyn 5.0.0 pin's stated condition ("until the SDK rolls to 10.0.110+") becomes a
deliberate, visible decision rather than a silent drift. Cross-check the exact installed SDK version
before committing the `version` value. Update the Roslyn-pin comment in `Directory.Packages.props` to
reference `global.json` as the coupling point.
**Files to touch:** new `global.json`; comment tweak in `Directory.Packages.props`.
**Verify:** `dotnet --version` in-repo resolves within the band; CI `setup-dotnet` honors it (the
workflow comment becomes true). Build + tests unchanged.
**Effort:** S. **Risk:** Low — a too-strict `version`/`rollForward` could fail a runner lacking that
exact SDK; `latestFeature` mitigates.
---
## TIER B — Repo hygiene
### U-5 (Medium) — orphaned proprietary AVEVA DLLs tracked in `lib/`
**Restatement:** 7 committed vendor binaries in `lib/`, referenced by nothing — a
redistribution/licence risk in every clone.
**Verification:** confirmed — `git ls-files lib/` lists all 7; zero `HintPath` hits repo-wide.
**Root cause:** leftovers from the retired in-process MXAccess / Wonderware sidecar era; the
bitness/COM story now lives entirely in the mxaccessgw repo (per CLAUDE.md).
**Design:** `git rm -r lib/`. Before deleting, re-confirm nothing references them:
```
grep -rn "aahClient\|ArchestrA\|Historian.CBE\|Historian.DPAPI\|HintPath.*lib" \
--include='*.csproj' --include='*.props' --include='*.targets' .
```
(expected: no hits outside the review docs). Optionally add `lib/` to `.gitignore` to prevent
re-introduction, and note in the commit that the DLLs remain retrievable from history / the AVEVA SDK
if ever needed. Purging them from *history* is a separate, heavier decision (BFG/filter-repo) — out
of scope; the immediate licence-hygiene win is removing them from the tip.
**Files to touch:** delete `lib/*` (7 files); optional `.gitignore` entry.
**Verify:** `git status` clean; `dotnet build ZB.MOM.WW.OtOpcUa.slnx` still succeeds (proves nothing
linked them); `git ls-files lib/` empty.
**Effort:** S. **Risk:** Low — verified zero references; blast radius nil.
---
### U-4 (Medium) — root planning-file sprawl contradicts its own "never stage" rule
**Restatement:** `pending.md`, `current.md`, `looseends.md`, `stillpending.md`,
`HISTORIAN-GATEWAY-INTEGRATION-ISSUES.md` are committed at root; `pending.md:3` itself says never
stage `pending.md`/`current.md`. Content is stale snapshots now contradicting `docs/` + CLAUDE.md.
**Verification:** confirmed — all 5 tracked; the hard-rule text present.
**Root cause:** working-notes files that were staged despite the rule, and never cleaned up as the
backlog shipped (~95% per the memory index).
**Design — make the file's own rule true:**
1. **`pending.md` + `current.md`** — untrack but keep locally (they're active working notes):
`git rm --cached pending.md current.md`, then add both to `.gitignore`. This exactly satisfies
the "never stage" rule.
2. **`looseends.md`, `stillpending.md`, `HISTORIAN-GATEWAY-INTEGRATION-ISSUES.md`** — these are
point-in-time snapshots of shipped work. Preferred: move to `docs/plans/` with dated names (the
established convention), e.g. `docs/plans/2026-05-18-looseends.md`, so the history is preserved
as an archived plan rather than a live-looking root file. Acceptable alternative: `git rm` them
outright since the memory index records the backlog as shipped and `docs/`+CLAUDE.md now carry the
authoritative state. Recommend **archive-move** for `stillpending.md`/`HISTORIAN-GATEWAY-…` (they
have referenced history) and **delete** for `looseends.md` if fully superseded — confirm with a
quick read before choosing per-file.
**Files to touch:** `.gitignore` (+2 entries); `git rm --cached` pending/current; move-or-delete the
3 snapshots; `docs/plans/` if archiving.
**Verify:** `git ls-files | grep -E '^(pending|current|looseends|stillpending|HISTORIAN)'` returns
only any intentionally-archived `docs/plans/*` paths; `git status` shows pending.md/current.md now
untracked (ignored). `pending.md`'s hard-rule is now consistent with reality.
**Effort:** S. **Risk:** Low — pure VCS hygiene; keep local copies (`--cached`) so no working data is
lost.
---
### U-6 (Low) — secrets hygiene + retired-dir husks
**Restatement:** `sql_login.txt` is NOT committed (gitignored) but is a plaintext credential on
Desktop; retired `Driver.Historian.Wonderware*` dirs linger on disk as bin/obj husks.
**Verification:** confirmed — `sql_login.txt` `.gitignore:47`, `git log` empty; the 8
`Driver.Historian.Wonderware*` dirs under `src/`+`tests/` are untracked (only `docs/` stub + 3
`code-reviews/…/findings.md` are tracked residue).
**Root cause:** local dev debris; the credential file is a convenience left in the repo dir.
**Design:**
- **`sql_login.txt`** — no tracking action needed (already ignored). Recommend moving the credential
to `dotnet user-secrets` or an env file *outside* the repo directory, and audit
`export-clean-copy.bat` at root to confirm it excludes `sql_login.txt` (and `lib/`, pki, planning
files) from any export bundle. Low-priority, local-only.
- **Wonderware husks** — delete the untracked `src/Drivers/…Wonderware*` and
`tests/Drivers/…Wonderware*` directories from disk (`rm -rf`, they're pure `bin/obj`). Decide
separately whether to also remove the 3 tracked `code-reviews/Driver.Historian.Wonderware*/`
findings and keep `docs/drivers/Historian.Wonderware.md` as the intentional retired stub (the doc
is deliberately retained per CLAUDE.md; leave it).
**Files to touch:** none tracked for the credential; `rm -rf` the untracked husks; audit
`export-clean-copy.bat`.
**Verify:** `find src tests -type d -iname '*Wonderware*'` returns nothing; `git status` unchanged by
the husk deletion (they were untracked); `export-clean-copy.bat` excludes the sensitive files.
**Effort:** S. **Risk:** Low.
---
### C-5 (Low) — no `.editorconfig`; StyleGuide.md mis-titled for a sister repo
**Restatement:** no mechanical code-style enforcement; `StyleGuide.md:3` still says "for all
**ScadaBridge** documentation" (copy-paste from the sibling repo).
**Verification:** confirmed — no `.editorconfig`; `StyleGuide.md:3` wording present.
**Root cause:** style held by review culture only; StyleGuide copied from ScadaBridge without
re-titling.
**Design — two independent, small changes:**
1. **StyleGuide fix (trivial, do now):** replace "ScadaBridge" with "OtOpcUa" in `StyleGuide.md:3`
(and scan the file for other ScadaBridge leftovers).
2. **`.editorconfig` (optional):** add a root `.editorconfig` codifying the *already-consistent*
conventions — file-scoped namespaces, 4-space indent, `_camelCase` private fields, `var` usage —
set as `suggestion`/`warning` (not `error`) to avoid a big-bang reformat. This is documentation-of-
convention, not a reformatting mandate; a `dotnet format` CI step is a further follow-on (U-7),
deliberately deferred so it doesn't churn the tree.
**Files to touch:** `StyleGuide.md`; optional new `.editorconfig`.
**Verify:** StyleGuide title reads OtOpcUa; `.editorconfig` doesn't flip existing warnings to errors
(build unchanged).
**Effort:** S. **Risk:** Low (keep severities non-error).
---
## TIER C — Documentation
### U-3 (Medium) — `docs/Client.CLI.md` documents 8 of 14 commands
**Restatement:** `ack`/`acknowledge`, `confirm`, `shelve`, `enable`, `disable` have no doc sections,
though all ship and CLAUDE.md points operators at this doc.
**Verification:** confirmed — `confirm`/`shelve`/`disable` = 0 mentions; `ack`/`enable` only
substring hits; command files exist for all 5.
**Root cause:** the inbound operator ack/shelve feature shipped its runtime + CLI but the CLI doc was
never extended.
**Design:** add five command sections to `docs/Client.CLI.md` mirroring the existing section shape
(synopsis, options, example invocation, expected output). Source the exact options/args from the
command classes: `AcknowledgeCommand.cs`, `ConfirmCommand.cs`, `ShelveCommand.cs`, `EnableCommand.cs`,
`DisableCommand.cs` (and their shared `CommandBase` connection options). Group them under an "Alarm
operator commands" heading since they are the operator-facing ack/shelve workflow. Cross-check against
`docs/ScriptedAlarms.md` §"Inbound operator ack/shelve" for terminology consistency.
**Files to touch:** `docs/Client.CLI.md`.
**Verify:** each of the 5 commands has a section; a `-h`/`--help` run of each command matches the
documented options. Optionally add a CI docs-coverage check (U-7 follow-on) asserting every
`Commands/*Command.cs` has a doc section.
**Effort:** S. **Risk:** Low.
---
## TIER D — Engineering debt (as capacity allows)
These are real but lower-leverage than restoring CI meaning. Batched with condensed treatment.
### C-2 (Medium) — TWE never reached the Client/Tooling slice
Add `true` to `Client.CLI.csproj`,
`Client.Shared.csproj`, `Client.UI.csproj`, `Analyzers.csproj` (v2-era code; the legacy xUnit1051
blocker doesn't apply). **Do this AFTER C-1's triage** — with the analyzer wired in, a Warning-level
OTOPCUA0001 would now fail these builds, so sequence C-1 triage → C-2. Verify each builds clean with
TWE on. Effort S; Risk Low (build once locally first). Burn down the 38 test-project TWE gaps per
module as a tracked follow-on.
### P-1 (Medium) — CI does 7 uncached restore+build passes
Add `actions/cache` for `~/.nuget/packages` keyed on a hash of `Directory.Packages.props` +
`NuGet.config`; do one explicit `restore`, then `build --no-restore`, then `test --no-build` in the
unit leg (dovetails with S-1's single-leg design). Effort S; Risk Low; biggest CI wall-clock win.
### U-2 (Medium) — add `Host.Tests` (the one real coverage hole)
Create `tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests` asserting DI composition — e.g. "when
`ServerHistorian:Enabled`, `IHistorianProvisioning` resolves to `GatewayTagProvisioner` and the
applier receives it" — the exact class of dormant-wiring bug (PR #423, DeferredAddressSpaceSink) that
unit tests would have caught. Add to slnx so S-1's whole-solution CI picks it up. Effort M; Risk Low.
### S-5 (Medium) — Client.Shared subscription lock-discipline gaps
In `OpcUaClientService.cs`: (a) move the `_dataSubscription` null-check/create inside an async gate
mirroring `_alarmSubscribeSemaphore` (line ~262); (b) take `_subscriptionLock` in `RunFailoverAsync`
when nulling `_dataSubscription`/`_alarmSubscription` (lines ~695-696); (c) route
`UnsubscribeAlarmsAsync` (line ~335) through `_alarmSubscribeSemaphore`; (d) observe exceptions from
the fire-and-forget keep-alive failover. Near-unhittable for the session-per-command CLI; real for the
long-lived Client.UI service. Effort M; Risk Medium (concurrency code — add targeted tests). Do before
Client.UI grows multi-threaded usage.
### S-6 (Medium) — global `Log.Logger` swap per CLI command
`CommandBase.ConfigureLogging()` (`:120-134`) does `Log.CloseAndFlush()` + replaces static
`Log.Logger` per command — a race under xunit.v3's parallel collections and hostile to embedding.
Give each command an instance `ILogger` from a per-execution `LoggerConfiguration.CreateLogger()`.
The existing `LoggerLifecycleTests` polices this. Effort M; Risk Low-Medium (touches every command's
logging).
### S-4 (Medium) — fixed-sleep timing tests
Replace start-up `await Task.Delay(100)` sleeps in `Client.CLI.Tests` (`SubscribeCommandTests`,
`AlarmsCommandTests`, `EventHandlerLifecycleTests`) with a readiness signal — a `TaskCompletionSource`
completed on first `SubscribeAsync` exposed by `FakeOpcUaClientService`. Currently masked because CI
never runs them (S-1); will flake once S-1 lands, so **pair with S-1**. Effort M; Risk Low.
### C-4 (Low) — finish xunit.v3 migration
Migrate the 3 holdouts (`AdminUI.Tests`, `ControlPlane.Tests`, `Runtime.Tests`) off xunit 2.9.2, then
drop the `xunit`/`xunit.runner.visualstudio` 2.x pins from `Directory.Packages.props:108-109`.
Optionally relocate `GatewayLiveIntegrationTests` to a `*.IntegrationTests` sibling. Effort M; Risk
Low.
### C-6 (Low) — csproj boilerplate duplicates Directory.Build.props
Strip redundant `TargetFramework`/`Nullable`/`ImplicitUsings` from the ~70 csprojs on next sweep so an
SDK bump touches one file. Effort M (mechanical); Risk Low. Low priority.
### S-8 (Low) — first-caller interval wins for shared data subscription
Document the shared-publish-interval behavior in `docs/Client.CLI.md`/`docs/Client.UI.md`, or create
per-interval subscriptions if UI users mix cadences. Effort S; Risk Low.
### U-8 (Low) — CLI security posture is dev-tool-grade
Document that `AutoAcceptCertificates = true` (`CommandBase.cs:91`) means `--security signandencrypt`
encrypts but does not authenticate the server (MITM-able), and that `-P` is process-visible. Optionally
add a `--strict-certs` opt-in. Separately, note the Galaxy-specific alarm attribute names baked into
the generic `OpcUaClientService.cs:851-871` as a documented layering compromise. Effort S; Risk Low.
### P-2 (Low) — recursive browse N+1
`BrowseAsync` issues an extra `HasChildrenAsync` per Object child (`:230-231`) and
`SubscribeCommand.CollectVariablesAsync` walks serially. Batch/bulk-read references only if `subscribe
-r` start-up becomes a soak-test bottleneck. Effort M; Risk Low. Defer.
### U-7 (Low) — missing CI stages inventory
Tracking item: analyzer enforcement (C-1), all-suite coverage (S-1), `dotnet format`/style step,
explicit failing NuGet-audit step, running `scripts/check-code-reviews-readme.ps1` in CI, and a docs
freshness/link check. Also the `ci/ab-server.lock.json` references a non-existent "GitHub Actions
step" in `docs/v2/test-data-sources.md` — either land the AB fixture download step or fix the doc.
Address incrementally as the above land.
---
## No-action (verified positive / accurate — recorded only)
- **P-3** — probe-once collection-fixture pattern is well-engineered; the issue is what CI does with
it (S-2), not the fixtures.
- **P-4** — CLI subscription output uses a channel-serialised writer; allocation-sane. A bounded
`DropOldest` channel is a cosmetic future option.
- **C-3** — CPM discipline exemplary; the `NuGetAuditSuppress` carve-out and the two transitive CVE
pins are still valid and accurately scoped. Keep the removal reminders alive.
- **U-1** — Client.UI is a genuine, adequately-tested product (not vestigial); "maturing", not
"underdeveloped". Its only gap is the same CI/E2E gap (S-1/S-3).
---
## Suggested execution sequence
1. **C-1** (wire analyzer) + its triage — establishes the enforcement mechanism; must precede C-2.
2. **S-1 + P-1** (whole-solution CI + caching) in one workflow edit — biggest coverage win.
3. **S-2** (fail-on-skip guard, then optional `services:`) — closes the false-green.
4. **S-7** (`global.json`) + **S-3** (disable E2E schedule / zero-match guard) — small CI truth fixes.
5. **U-5, U-4, U-6, C-5** (hygiene batch) — one commit; verified zero-reference deletions.
6. **U-3** (Client.CLI doc) — five sections.
7. **C-2** (TWE, after C-1 triage), then Tier-D items as capacity allows, pairing **S-4 with S-1**.
Each of steps 1-4 must be validated by an actual CI run on a branch before merge — per the report's
own theme #4, green CI here has been meaningless, so the fixes must be observed to change the badge's
behavior (a broken build/deliberate skip must now go red).