Re-ran all seven domain reviews at master f6eaa267 (reports rewritten in
place, each with a prior-finding status table): all 4 round-1 Criticals
verified closed; new top findings are the S7 connect-timeout OCE regression
(05/STAB-14), the ResilienceConfig operator-authorable brick (01/S-6), and
a batch of resilience-seam Mediums. 00-OVERALL.md carries the updated
maturity matrix + 12-item action list.
Adds R2-01..R2-12 design/implementation plans (one per action item, house
format + bite-sized TDD task breakdowns + co-located .tasks.json; 193 tasks,
~18-24 dev-days). STATUS.md updated: round-1 topology marked historical
(all merged+pushed), re-review findings table + plan pointers added.
30 KiB
Architecture Review 07 — Client Tooling, Analyzers, and the Cross-Cutting Engineering System
| Date | 2026-07-12 |
| Commit | f6eaa267 (master, clean tree) |
| Reviewer | Architecture review agent (deep-review sweep, slice 07 — re-run) |
| Supersedes | The 2026-07-08 review of this domain at 9cad9ed0. All arch-review remediation branches have since merged to master (~40 commits), including the three squarely in this slice: f0082af5 (07/C-1 analyzer wiring + triage), 10b89830 (07/S-1,S-2,S-4 CI overhaul), and bacea1a4 (#10 dispatch-gap closure, which removed the C-1 triage pragmas). Also landed: the /fixdocs XML-doc sweep 3b5ef439 and the HistorianGateway client 0.1.0→0.2.0 bump f6eaa267. |
| 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/, docker-dev/, overall test architecture, docs freshness, repo-root hygiene |
Prior-finding status (9cad9ed0 → f6eaa267)
Verified against actual code/tree state, not remediation claims. src/Client production code is
byte-identical to the prior baseline (zero commits touched it), so every client-code finding
carries over verbatim.
| ID | One-liner (prior) | Verified status |
|---|---|---|
| S-1 | CI gated ~15% of the test matrix (5-project hand-maintained matrix) | FIXED — v2-ci.yml:58-62 runs one whole-solution leg dotnet test ZB.MOM.WW.OtOpcUa.slnx --filter "Category!=E2E&Category!=LiveIntegration" (10b89830, merged 83fdf5fa). Self-maintaining; all 47 test csprojs in the slnx are covered, including Client (388) + Analyzers (32). Residual: the first actual CI run of this pipeline post-push was not observable from this review — YAML is statically well-formed; the mixed xunit v3/2.9.2 whole-solution run was verified locally per the commit, not on a runner. |
| S-2 | Green CI == skipped integration tests, indistinguishable from pass | PARTIALLY FIXED — new scripts/ci/assert-not-all-skipped.sh gate: strict (MIN_EXECUTED=1) on the unit leg (v2-ci.yml:66-68), report-only (MIN_EXECUTED=0) on the fixtureless integration leg (v2-ci.yml:102-106) with a documented follow-up to run opc-plc as a workflow services: and flip it strict. All-skipped can no longer masquerade as green on the unit leg; the integration tier is still green-while-skipped by explicit, commented design. See new S-9 (threshold coarseness) and P-5 (the leg is now redundant). |
| S-3 | Nightly E2E workflow is a permanent green no-op | STILL OPEN — v2-e2e.yml unchanged since baseline (git diff empty); zero tests carry Category=E2E (grep re-verified). |
| S-4 | Fixed-sleep timing tests are the dominant wait style | PARTIALLY FIXED — the 11 Client.CLI.Tests startup sleeps were replaced with SubscribeInvoked/SubscribeAlarmsInvoked TaskCompletionSource readiness signals (FakeOpcUaClientService.cs:57,60, 10b89830); the one remaining Task.Delay(25) (SubscribeCommandSummaryTests.cs:262) is a bounded poll-until-predicate helper, not a fixed startup sleep — fine. Driver-suite fixed sleeps remain (see body) and are now CI-exercised by S-1's fix, so their flake risk is live. |
| 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 routed through the semaphore. File untouched since baseline. |
| S-6 | Global Log.Logger swap per CLI command |
STILL OPEN — CommandBase.cs:123,133 unchanged. |
| S-7 | No global.json, but v2-ci.yml claims one pins the SDK |
STILL OPEN — no global.json at root; notably the false claim survived the workflow rewrite (v2-ci.yml:10-12 still says "pinned via global.json"). The Roslyn 5.0.0 pin rationale in Directory.Packages.props still depends on an exact SDK band. |
| S-8 | First-caller interval wins for the shared data subscription | STILL OPEN — OpcUaClientService.cs:262. |
| P-1 | 7 uncached restore+build passes per CI push | PARTIALLY FIXED — matrix collapse reduced it to 4 passes (build + unit + 2 integration legs); still no actions/cache for NuGet, and the build job's output is still discarded (every downstream job restores + rebuilds). |
| P-2 | Recursive browse is N+1 over HasChildrenAsync |
STILL OPEN — OpcUaClientService.cs:230-231, SubscribeCommand.cs serial walk; unchanged. |
| P-3 | (positive) probe-once integration-fixture pattern | STILL VALID — unchanged; now doubly relevant since these fixtures probe (and skip) inside the CI unit leg. |
| P-4 | Unbounded channel in CLI subscribe output | STILL OPEN (cosmetic) — unchanged. |
| C-1 | Custom analyzer wired into zero projects | FIXED — Directory.Build.props:45-51 injects the analyzer as OutputItemType="Analyzer" into every project except the analyzer + its tests (f0082af5). Triage verified scoped and honest: 12 wire-level test/CLI csprojs carry commented NoWarn (6 driver CLIs, AbCip/S7 unit+integration suites, ScriptedAlarms/VirtualTags tests); 3 driver-internal-self-call pragmas + 1 test-only-scaffolding pragma (GenericDriverNodeManager.cs:71) remain, each with rationale. The 6 RESILIENCE-DISPATCH-GAP pragmas the wiring surfaced were removed by the follow-on bacea1a4, which actually wired CapabilityInvoker into dispatch via a Polly-free IDriverCapabilityInvoker seam — grep for RESILIENCE-DISPATCH-GAP now matches only the Runtime guard test. Negative control (unwrap → build error in Runtime) documented in both commits. Residual enforcement caveat: see new C-7. |
| C-2 | TWE never reached Client/Tooling src (or most tests) | STILL OPEN — zero TreatWarningsAsErrors in src/Client/*/*.csproj or src/Tooling/*/*.csproj; still 9/47 test projects opted in. Now also weakens C-1's enforcement (C-7). |
| C-3 | (positive) CPM discipline + valid audit carve-out | STILL VALID — NuGetAuditSuppress for GHSA-2m69-gcr7-jv3q still present and still accurate (Directory.Build.props:19-32); only package change since baseline is HistorianGateway Client/Contracts 0.1.0→0.2.0 (f6eaa267), pinned as before. |
| C-4 | 3 legacy xunit 2.9.2 holdouts | STILL OPEN — AdminUI.Tests, ControlPlane.Tests, Runtime.Tests still reference xunit 2.9.2 (Directory.Packages.props:108). |
| C-5 | No .editorconfig; StyleGuide.md says "ScadaBridge" |
STILL OPEN — no .editorconfig; StyleGuide.md:3 still reads "for all ScadaBridge documentation". |
| C-6 | csproj boilerplate duplicates Directory.Build.props | STILL OPEN — unchanged. |
| U-1 | (positive) Client.UI is real and maturing | STILL VALID — unchanged (126 headless tests); now actually run by CI. |
| U-2 | Coverage matrix: Host has no unit tier | STILL OPEN, partially mitigated — no Host.Tests project exists. However the wiring-bug class U-2 worried about gained two targeted guards on master: ResilienceInvokerFactoryRegistrationTests (Host.IntegrationTests, 8a8b9ec5 — asserts prod DI binds the real invoker factory) and the reflection-exhaustive DeferredSinkForwardingReflectionTests (Commons.Tests, a65c2ced). A general Host DI-composition unit tier is still the right ask. |
| U-3 | docs/Client.CLI.md documents 8 of 14 commands | STILL OPEN — ack/confirm/shelve/enable/disable still have zero sections in docs/Client.CLI.md (grep re-verified; the /fixdocs sweep 3b5ef439 touched only XML doc comments in S7/TwinCAT/Core, not this). |
| U-4 | Root planning-file sprawl contradicts its own rules | STILL OPEN — looseends.md, pending.md, current.md, stillpending.md, HISTORIAN-GATEWAY-INTEGRATION-ISSUES.md all still tracked (git ls-files re-verified), including the two pending.md itself forbids staging. |
| U-5 | 7 orphaned proprietary AVEVA DLLs tracked in lib/ |
STILL OPEN — all 7 DLLs still in git ls-files lib/; still zero HintPath references. |
| U-6 | sql_login.txt local-only; Wonderware husk dirs on disk | STILL OPEN (Low, local-only) — sql_login.txt still untracked+gitignored but present on disk; the 5 retired Driver.Historian.Wonderware* dirs still exist as ignored husks under src/Drivers/ + tests/Drivers/; export-clean-copy.bat still has no explicit sql_login exclusion. |
| U-7 | Missing CI stages inventory | PARTIALLY FIXED — the two biggest items (analyzer enforcement, all unit suites) are now CI-gated via C-1 + S-1. Still absent: dotnet format/style, explicit NuGet-audit failing step, scripts/check-code-reviews-readme.ps1, docs link/freshness check, and the ci/ab-server.lock.json download step referenced by docs/v2/test-data-sources.md. |
| U-8 | CLI security posture is dev-tool-grade | STILL OPEN — ConnectionSettings.AutoAcceptCertificates still defaults true (Models/ConnectionSettings.cs:41), -P still argv-visible; undocumented in Client.CLI.md. |
Net: 2 FIXED (S-1, C-1 — the two Highs), 4 PARTIALLY FIXED (S-2, S-4, P-1, U-7), 17 still
open/unchanged (all Medium/Low), 3 positives still valid. Every remediation claim in
archreview/plans/STATUS.md that touches this slice checked out against the code, including the
adversarial checks (suppressions are scoped and commented; the skip-gate is genuinely strict on the
unit leg; the dispatch-gap pragmas are genuinely gone, not relocated).
Architecture Overview
Client stack
Three projects form a clean layered client stack, all .NET 10 — unchanged since the prior review (zero src/Client commits in the window):
Client.Shared— the OPC UA client library.OpcUaClientService(src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs, 975 lines) implementsIOpcUaClientServicebehind five adapter seams (IApplicationConfigurationFactory,IEndpointDiscovery,ISessionFactory,ISessionAdapter,ISubscriptionAdapterunderAdapters/) so the OPC Foundation SDK is fully fakeable. The service owns connect/disconnect, read/write with string→typed coercion, browse with continuation points, data + alarm subscriptions with failover replay, Part 9 alarm method calls, HistoryRead (raw + aggregate), and redundancy-info reads.Client.CLI— CliFx-based terminal client (otopcua-cli), 14 commands, session-per- invocation.CommandBasecentralises connection options and (still) swaps the global SerilogLog.Loggerper execution (S-6).Client.UI— Avalonia 11.2 desktop app (CommunityToolkit.Mvvm), ~2.9 k LOC C# + 551 lines AXAML, 126 headless tests. Real, not vestigial (U-1).
Tooling
src/Tooling/ZB.MOM.WW.OtOpcUa.Analyzers ships OTOPCUA0001 UnwrappedCapabilityCallAnalyzer
— and it is now wired into every project in the tree via Directory.Build.props:45-51
(OutputItemType="Analyzer", excluding only the analyzer and its own test project). The rule it
enforces is also now true: bacea1a4 wired CapabilityInvoker into production dispatch through
a new IDriverCapabilityInvoker/IDriverCapabilityInvokerFactory seam in Core.Abstractions
(mirroring IDriverFactory, because Runtime is deliberately Polly-free), routed all 6
DriverInstanceActor dispatch sites through it, taught the analyzer that the interface is a valid
wrapper home, and deleted the 6 tracked-gap pragmas. 32 analyzer tests. The prior review's
headline failure mode for this slice — "built and tested but plugged into nothing" — is closed,
with the analyzer itself as the standing regression guard (error-severity where TWE is on; see
C-7).
Test architecture map
47 test projects (verified: git ls-files 'tests/**/*.csproj' | wc -l = 47) mirror src/ under
tests/<module>/ in three tiers:
*.Tests— pure unit suites, 40 projects; xunit.v3 + Shouldly except three xunit 2.9.2 holdouts (AdminUI.Tests,ControlPlane.Tests,Runtime.Tests).*.IntegrationTests— 10 projects, probe-once collection fixtures defaulting to the shared Docker host10.100.0.35,Assert.Skipwhen unreachable.Category=LiveIntegration— env-gated live suites (skip-clean offline).Category=E2E— still reserved; still zero tests carry it (S-3).
Build / CI pipeline shape (changed since 2026-07-08)
Directory.Build.props— global net10.0/nullable/implicit-usings/latest, the CVE-2025-6965NuGetAuditSuppresscarve-out (still valid, C-3), and new: the tree-wide OTOPCUA0001 analyzer injection (lines 34-51). TWE remains opt-in per project.Directory.Packages.props— CPM, all versions pinned; only delta since baseline is the HistorianGateway Client/Contracts 0.1.0→0.2.0 bump (f6eaa267).- CI (
v2-ci.yml, rewritten by10b89830):build→ one whole-solution unit leg (dotnet test ZB.MOM.WW.OtOpcUa.slnx --filter "Category!=E2E&Category!=LiveIntegration", trx logger, strict fail-on-skip gate) → the old 2-project integration matrix (same filter, report-only skip gate). Newscripts/ci/assert-not-all-skipped.shparses trx<Counters>and fails when executed <MIN_EXECUTED. v2-e2e.yml— untouched; still the documented nightly green no-op (S-3).docker-dev/— the compose gained a real fix in the window (7233e2ba): the retired WonderwareServerHistoriankeys were replaced with the gateway shape (Endpoint/ApiKey/UseTls, disabled by default) after they were found to crash the Host at bring-up — a live-gate catch, exactly the failure mode this review series exists for.
Findings
Severity scale: Critical (broken now, corrupts trust) / High (material gap, fix soon) / Medium (real debt) / Low (polish).
Fixed prior findings (S-1, C-1) live only in the status table above. Still-open findings keep their IDs; new findings continue each scheme's numbering.
1. STABILITY
S-3 (Medium) — the nightly E2E workflow is still a permanent green no-op
v2-e2e.yml:6-10 unchanged: the E2E test project "does not yet exist… this workflow is a green
no-op", and no test in the tree carries Category=E2E. Now that the unit tier is genuinely gated
(S-1 fixed), this is the largest remaining "always-green" surface. Recommendation: land the
minimal E2E round-trip project (the scripts/e2e/test-*.ps1 harnesses show what to assert) or
disable the schedule until it exists.
S-4 (Medium, narrowed) — driver-suite fixed sleeps are now CI-exposed flake risk
The Client.CLI half of this finding is fixed (readiness signals, 10b89830). What remains:
Driver.Galaxy.Tests (30 Task.Delay/Thread.Sleep occurrences), Driver.Modbus.Tests (22),
Driver.AbCip.Tests (20), Driver.FOCAS.Tests (18), Host.IntegrationTests (15), Driver.TwinCAT.Tests
(7). Before S-1's fix these never ran in CI; now they run on every push, so their fixed-delay
assumptions will meet loaded shared runners for the first time. Recommendation: watch the
first weeks of whole-solution CI for flaky suites and apply the same readiness-signal pattern
(the FakeOpcUaClientService.SubscribeInvoked TCS is the template) to whichever driver suites
actually flake — don't pre-emptively rewrite all ~112 sites.
S-5 (Medium) — Client.Shared subscription bookkeeping still has lock-discipline gaps
Unchanged from the prior review (src/Client had zero commits in the window):
SubscribeAsyncchecks/creates the shared_dataSubscriptionoutside the lock (OpcUaClientService.cs:262) — two concurrent first-subscribers can leak an SDK subscription.RunFailoverAsyncnulls_dataSubscription/_alarmSubscriptionwithout the lock (lines 695-696), contradictingDisconnectAsync's stated assumption (lines 138-140).UnsubscribeAlarmsAsyncdoes not take_alarmSubscribeSemaphore.- Keep-alive failover remains fire-and-forget with an unobserved exception path out of
TransitionState. Near-unhittable for the session-per-command CLI; real for long-lived Client.UI usage. Recommendation unchanged: small async gate around the data-subscription create, take the lock inRunFailoverAsync, route alarm-unsubscribe through the semaphore.
S-6 (Medium) — global Log.Logger swap per CLI command
Unchanged: CommandBase.cs:123 (Log.CloseAndFlush()) + :133 (static Log.Logger
replacement) per command execution. Race for parallel test collections; hostile to embedding.
Recommendation: per-execution instance ILogger.
S-7 (Low) — no global.json, and the workflow's claim survived its own rewrite
Still no global.json; v2-ci.yml:10-12 — a file otherwise rewritten in this window — still
asserts "The .NET 10 SDK is pinned via global.json at the repo root". The
Microsoft.CodeAnalysis.CSharp 5.0.0 pin (Directory.Packages.props) still assumes an exact SDK
band. Recommendation: add global.json (rollForward: latestFeature) or fix the comment.
S-8 (Low) — first-caller interval wins for the shared data subscription
Unchanged (OpcUaClientService.cs:262). Document or create per-interval subscriptions.
S-9 (Low, NEW) — the strict skip-gate threshold is MIN_EXECUTED=1, which only catches total outage
scripts/ci/assert-not-all-skipped.sh:19 defaults the floor to 1 executed test, and the unit leg
uses that default (v2-ci.yml:66-68). A whole-solution run currently executes thousands of
tests; a partial discovery outage (e.g. an adapter regression that silently drops the xunit
2.9.2 or the xunit.v3 population, or a filter typo that matches one project) would still pass the
gate while losing 95%+ of coverage. The gate as shipped converts "green == nothing ran" into red
— genuinely valuable — but "green == almost nothing ran" is still possible.
Recommendation: raise the unit-leg floor to a conservative constant (e.g. MIN_EXECUTED=1500
today, revised occasionally), which costs nothing and catches the partial-outage class too.
2. PERFORMANCE
P-1 (Medium, narrowed) — CI still does 4 independent uncached restore+build passes per push
Improved from 7 legs to 4 (build, unit-tests, 2× integration), but each job still checks
out, restores, and rebuilds from scratch; the dedicated build job's output is still discarded;
there is still no actions/cache for ~/.nuget/packages. Recommendation: cache the NuGet
folder keyed on Directory.Packages.props; consider dropping the redundant integration matrix
(P-5), which removes half the remaining waste for free.
P-2 (Low) — recursive browse is N+1 over HasChildrenAsync
Unchanged (OpcUaClientService.cs:230-231; SubscribeCommand serial tree walk). Acceptable for
a diagnostic tool.
P-3 (Low, positive) — integration-fixture cost is well-engineered
Unchanged and now more load-bearing: the probe-once collection-fixture pattern is what makes the
whole-solution CI leg viable at all (10 IntegrationTests projects probe 10.100.0.35 once each
and skip, rather than timing out per test).
P-4 (Low) — CLI subscription output path is allocation-sane
Unchanged; the unbounded channel remains a cosmetic cap-it-later.
P-5 (Low, NEW) — the CI integration matrix is now fully redundant with the unit leg
The whole-solution unit leg (v2-ci.yml:58-62) already builds and runs
Host.IntegrationTests and OpcUaServer.IntegrationTests under the identical filter — so the
integration job (v2-ci.yml:77-106) re-restores, re-builds, and re-runs the same two projects
in the same fixtureless environment, producing the same skips, twice per push. Its only unique
output is a per-project skip tally that the unit leg's trx already contains. (Note
Host.IntegrationTests is not pure skip — its in-process 2-node Akka harness tests, e.g.
HardKillFailoverTests and ResilienceInvokerFactoryRegistrationTests, actually execute — but
they execute in both jobs.) Recommendation: either delete the matrix, or make it earn its
keep by becoming the S-2 option-#2 leg: add the mcr.microsoft.com/iotedge/opc-plc services:
block, set OPCUA_SIM_ENDPOINT=localhost:50000, and flip its gate to strict — then it covers
something the unit leg cannot.
3. CONVENTIONS
C-2 (Medium) — TreatWarningsAsErrors still never reached the Client/Tooling slice (or most tests)
Unchanged: Client.CLI.csproj, Client.Shared.csproj, Client.UI.csproj, and
Analyzers.csproj still lack TWE; still 9/47 test projects opted in. The prior argument stands
(the client stack is v2-era code with no xUnit1051 legacy), and the stakes rose slightly: TWE is
now also the mechanism that gives OTOPCUA0001 teeth (C-7). Recommendation: add TWE to the
four src projects now; burn down test debt per module.
C-3 (Low, positive) — central package management discipline remains exemplary
Re-verified at f6eaa267: CPM pinned throughout, packageSourceMapping intact, the
GHSA-2m69-gcr7-jv3q NuGetAuditSuppress still present/scoped/accurate with its removal condition
(Directory.Build.props:19-32), the OpenTelemetry + Tmds.DBus.Protocol surgical pins unchanged.
The only version delta in the window (HistorianGateway 0.1.0→0.2.0) followed the pattern. Keep
the removal reminders alive.
C-4 (Low) — three xunit 2.9.2 holdouts remain
Unchanged: AdminUI.Tests, ControlPlane.Tests, Runtime.Tests still on xunit 2.9.2
(Directory.Packages.props:108). Now mildly riskier than before: the whole-solution CI leg runs
a mixed v3/2.x population through one dotnet test invocation, so the holdouts are a moving part
in the new pipeline. Finish the migration and drop the pin.
C-5 (Low) — still no mechanical code-style enforcement; StyleGuide.md still says "ScadaBridge"
Unchanged: no .editorconfig, no dotnet format CI step, StyleGuide.md:3 still carries the
sister-repo product name.
C-6 (Low) — csproj boilerplate still duplicates Directory.Build.props
Unchanged; strip TargetFramework/Nullable/ImplicitUsings restatements on next sweep.
C-7 (Low, NEW) — OTOPCUA0001 enforcement strength is TWE-dependent, so the analyzer is warning-only exactly where TWE is missing
The analyzer ships OTOPCUA0001 at Warning severity; it fails builds only in projects with
TreatWarningsAsErrors (e.g. Runtime, where the bacea1a4 negative control proved
unwrap→build-error). That covers the real dispatch surface today — Core/Server/Runtime/Driver src
projects are TWE'd (63 src csprojs opted in). But in the C-2 gap set (Client/Tooling src, 38 test
projects) a new unwrapped capability call surfaces as a non-failing warning that whole-solution
CI output will bury. Low because no plausible dispatch code lives in those projects today;
listed so the interaction is on record: closing C-2 is also what hardens C-1's guard, and if
TWE promotion stalls, an alternative is escalating OTOPCUA0001 to error-by-default in the
analyzer itself (its intentional-call story is now mature enough — scoped pragmas + documented
NoWarn projects).
4. UNDERDEVELOPED AREAS
U-1 — Client.UI maturity: real product, adequately tested, current docs
Verdict unchanged: not vestigial (~2.9 k LOC, 126 headless tests, accurate
docs/Client.UI.md). Upgrade since the prior review: those 126 tests now actually run in CI on
every push. The AXAML-binding blind spot (headless tests can't catch binding regressions — the
AdminUI "live-verify" lesson) still applies.
U-2 (Medium) — source → test coverage: Host still has no unit tier
The matrix is otherwise unchanged and nearly complete (Contracts projects remain DTO-only with
consuming-suite coverage; Core.Scripting.Abstractions remains incidental). Host is still the
one real hole — no tests/Server/*Host.Tests exists. Two mitigations landed in the window and
narrow the risk without closing it: ResilienceInvokerFactoryRegistrationTests
(tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/ResilienceInvokerFactoryRegistrationTests.cs,
asserts production DI binds the real invoker factory — precisely the PR-#423-class dormancy
check this finding asked for, albeit living in the integration project) and the
reflection-exhaustive deferred-sink forwarding guard (Commons.Tests). Recommendation stands:
a lightweight Host.Tests asserting DI composition claims (ServerHistorian:Enabled ⇒
GatewayTagProvisioner, etc.) at unit speed.
U-3 (Medium) — docs drift: docs/Client.CLI.md still documents 8 of 14 commands
Re-verified: ack, confirm, shelve, enable, disable still have zero sections (the only
grep hits are incidental words in the alarms section), while CLAUDE.md still points readers at
this doc for those commands. The window's doc work (3b5ef439) was XML-comment-only and did not
touch this. The "104 unit tests" claim remains accurate (re-counted: 104). Recommendation:
add the five missing command sections — they are the operator-facing alarm workflow.
U-4 (Medium) — repo-root planning-file sprawl still contradicts its own rules
Unchanged: looseends.md, pending.md, current.md, stillpending.md,
HISTORIAN-GATEWAY-INTEGRATION-ISSUES.md all still tracked at the root, and pending.md still
declares the hard rule against staging pending.md/current.md — which remain tracked. No
remediation branch addressed this (as expected — it had none). Recommendation unchanged:
move under docs/plans/ with dated names, or delete + gitignore.
U-5 (Medium) — orphaned proprietary AVEVA DLLs still tracked in lib/
Unchanged: all 7 vendor binaries (aahClient*.dll, ArchestrA.MXAccess.dll,
ArchestrA.CloudHistorian.Contract.dll, Historian.CBE.dll, Historian.DPAPI.dll) still in
git ls-files lib/, still zero HintPath references repo-wide. Redistribution/licence risk in
every clone. Recommendation: git rm -r lib/.
U-6 (Low) — local-only hygiene: sql_login.txt and Wonderware husk dirs
Unchanged: sql_login.txt remains untracked + gitignored (.gitignore:47) but is still a
plaintext credential on disk at the root; export-clean-copy.bat still lacks an explicit
exclusion for it. The five retired Driver.Historian.Wonderware* directories still exist on disk
as ignored bin/obj husks — safe to delete locally.
U-7 (Low, narrowed) — missing CI stages inventory
The two biggest gaps closed (analyzer enforcement via C-1's fix; all unit suites via S-1's).
Still not gated anywhere: dotnet format/style (blocked on C-5's missing .editorconfig), an
explicit failing NuGet-audit step, scripts/check-code-reviews-readme.ps1, docs link/freshness
checks, and the ci/ab-server.lock.json fixture-download step that
docs/v2/test-data-sources.md still describes as an existing "GitHub Actions step".
U-8 (Low) — CLI security posture is dev-tool-grade by construction
Unchanged: ConnectionSettings.AutoAcceptCertificates defaults true
(Models/ConnectionSettings.cs:41, flowed through
DefaultApplicationConfigurationFactory.cs:53,63), -P is argv-visible, and the Galaxy-specific
alarm attribute names remain baked into the generic Client.Shared fallback path. Document the
posture; add a --strict-certs opt-in.
U-9 (Low, NEW) — archreview/plans/STATUS.md merged to master still claims nothing is merged
The remediation handoff doc now lives on pushed master, but its topology header still reads
"Branch topology (all LOCAL — nothing pushed, nothing on master)" (STATUS.md:6) and the resume
facts still assert "Nothing is pushed. 8 local branches" (STATUS.md:109) — both now false
(all listed branches are merged; master is pushed). Same class as the S-7 comment drift: a
bookkeeping doc that contradicts observable reality erodes trust in the parts that are still
accurate (e.g. the genuinely-pending #10 live behavioral gate). Recommendation: one-paragraph
update to STATUS.md marking the topology section historical and recording the merge date.
Maturity Ratings (1 = ad hoc, 5 = exemplary)
| Dimension | 2026-07-08 | 2026-07-12 | Justification |
|---|---|---|---|
| Stability | 2.5 | 3.5 | The two structural holes are closed: CI now runs the whole solution (S-1) and all-skipped can't masquerade as green on the unit leg (S-2 strict gate); the newly-CI'd CLI suite was deflaked in the same change. Held below 4 by the still-no-op nightly E2E (S-3), the now-CI-exposed driver-suite fixed sleeps (S-4), the untouched Client.Shared lock gaps (S-5), and a first post-push CI run not yet observed. |
| Performance | 3.5 | 3.5 | Net wash: matrix collapse cut CI legs 7→4, but restore/build remains uncached, the build job's output is still discarded, and the integration matrix now duplicates work the unit leg already does (P-5). Client-side perf posture unchanged. |
| Conventions | 3 | 4 | The flagship convention tool went from wired-into-nothing to enforced tree-wide with a proven negative control — and the gap it surfaced (RESILIENCE-DISPATCH-GAP) was actually fixed rather than pragma'd away, with the analyzer left as the standing guard. Suppressions are scoped and commented. Held below 4.5 by the TWE gap that leaves the analyzer warning-only outside the TWE'd projects (C-2/C-7), three xunit 2.x holdouts, and still no .editorconfig. |
| Underdeveloped areas | 3 | 3 | Nothing in this bucket moved: lib/ DLLs, root planning-file sprawl, the 5-command docs drift, and the missing Host unit tier are all exactly as found (none had a remediation branch), and the merged STATUS.md added a new small drift (U-9). Partial credit for the two Host-wiring guard tests that landed en route. |
Top recommendations (ordered)
- Delete the tracked
lib/vendor DLLs (U-5) and resolve the root planning-file contradiction (U-4) — the two remaining findings with licence/trust implications, both zero-risk removals. - Add the five missing alarm-command sections to
docs/Client.CLI.md(U-3). - Make the CI integration matrix earn its keep or delete it: add the opc-plc
services:fixture + flip its skip-gate strict (P-5, completing S-2), and raise the unit-legMIN_EXECUTEDfloor above 1 (S-9). - Add TWE to the four Client/Tooling src projects (C-2) — this is also what gives OTOPCUA0001 build-breaking teeth there (C-7).
- Add
global.jsonand fix the workflow comment (S-7); update the stale STATUS.md topology claims (U-9). - Fix the three
OpcUaClientServicelock-discipline gaps before Client.UI grows long-lived multi-thread usage (S-5); landHost.Tests(U-2). - Watch the first weeks of whole-solution CI for driver-suite sleep flakes (S-4) and either land a real E2E project or disable the nightly no-op (S-3).