` anywhere; lists are instead bounded by design (`Alerts` cap 200, `Deployments` `Take(50)`, cluster-scoped tables). Adequate. Two nits: `Alerts.razor:63` renders rows without `@key` while prepending (`Insert(0, …)`), forcing whole-table diffs per event (`EquipmentPage` tables do use `@key`); the `UnsTree` filter re-runs the recursive `VisibleUnder` walk per render (self-documented as bounded/cheap — agreed).
-
-### P-6 (Low) — `inlay-hints` round-trips for a documented no-op
-
-`monaco-init.js:99–117` registers an inlay-hints provider that POSTs on every model change; `ScriptAnalysisService.InlayHints` (`ScriptAnalysisService.cs:415`) always returns empty. Skip registering the provider until the feature exists.
-
----
-
-## 3. CONVENTIONS
-
-### C-1 (High) — Three authorization idioms coexist, and the largest mutating surface is effectively ungated
-
-*(Re-verified at f6eaa267 by enumerating every `@attribute [Authorize…]` in `Components/Pages/` — unchanged. STATUS.md confirms no remediation branch targeted this; it was action-list item 5, unscheduled.)*
-
-Observed gates:
-
-| Idiom | Where |
-|---|---|
-| `Roles = "Administrator,Designer"` | `Deployments.razor:12`, `Scripts.razor:2`, `ScriptEdit.razor:5`, `ScriptAnalysisEndpoints.cs:18` |
-| `Policy = "FleetAdmin"` | `RoleGrants.razor:2` (page), `Certificates.razor:90,186` (per-action) |
-| `Policy "DriverOperator"` (imperative) | `Alerts.razor:165`, `DriverStatusPanel.razor:166`, `OpcUaClientAddressPickerBody.razor:75`, `GalaxyAddressPickerBody.razor:124` |
-| **bare `[Authorize]`** | **everything else** — `GlobalUns`, `EquipmentPage`, `ClusterEdit` (+ all cluster sub-pages), `NodeEdit`, `NamespaceEdit`, `AclEdit`, `NewCluster`, all 8 driver pages + `DriverEditRouter`/`DriverTypePicker`, `Reservations`, `Certificates` (page level) |
-
-Consequence: any authenticated user — including a read-only viewer — can create/edit/delete areas, lines, equipment, tags, virtual tags, scripted alarms, driver instances, ACLs, and namespaces; only the *deploy* button is role-gated. Project memory says global-roles/simplest-authz is a deliberate posture, but the current state isn't "simplest" — it's three idioms plus an unguarded majority, which is exactly the drift that produces surprises later. Note the stakes rose since the prior review: driver pages now author `ResilienceConfig` that (post-#13, `75403caa`) directly shapes production retry/breaker behavior — still behind bare `[Authorize]`.
-
-**Recommendation:** centralize policy names as constants (they exist only as string literals today), pick policies over role strings (`FleetAdmin` already means `RequireRole("Administrator")`), and apply one write-gate (`Administrator,Designer` equivalent) to every config-mutating page. This is a one-attribute-per-page change.
-
-### C-3 (Good) — Driver-typed tag-editor pattern is complete and consistent
-
-`Uns/TagEditors/TagConfigEditorMap.cs:10–20` and `TagConfigValidator.cs:12–22` cover **all seven** non-Galaxy drivers (Modbus, S7, AbCip, AbLegacy, TwinCat, Focas, OpcUaClient) with parallel keys; Galaxy intentionally uses the live-browse picker + raw `{"FullName": …}` JSON (`TagModal.razor:96–125`). Every editor follows the same shell-over-pure-model shape (`FromJson`/`ToJson`/`Validate`, preserve-unknown-keys), with the parse-only-on-real-change guard against parent re-renders (`ModbusTagConfigEditor.razor:52–57`). The composable root-key seams (`TagHistorizeConfig`, `TagArrayConfig`, `NativeAlarmModel`) merge onto the same JSON without clobbering — a genuinely good design.
-
-### C-4 (Good) — The numeric-enum serialization bug class is closed
-
-All 8 driver pages carry `JsonStringEnumConverter` in their serializer options; tag editors bind enums as strings via `Enum.GetValues`/`TryParse`; and the fix is pinned by per-driver `*FormSerializationTests` plus `DriverPageJsonConverterTests` in the test project — regression-proofed, not just patched.
-
-### C-5 (Medium) — Two data-access idioms: service seam vs. EF-in-page
-
-UNS pages go through `IUnsTreeService` (testable, RowVersion-aware, 12 test files). But `ScriptEdit.razor:87–156`, `Scripts.razor:60`, `Deployments.razor:83–100`, `Fleet.razor:125`, and `Hosts.razor:273` inject `IDbContextFactory` and run EF (including entity mutation and `SaveChangesAsync`) directly in `@code`. The service-seam pages are the ones with meaningful test coverage; the EF-in-page logic is untestable without a host. New pages copy whichever file is nearest.
-
-**Recommendation:** declare the service-seam the convention; migrate `ScriptEdit`'s save/delete into the existing `UnsTreeService` script methods (`UpdateScriptSourceAsync` already exists at `IUnsTreeService.cs:502`).
-
-### C-6 (Low) — Naming/marker nits
-
-`"GalaxyMxGateway"` is a bare string literal in `TagModal.razor` (comments/branching at lines 98, 262, 275 and the dispatch site); the `DataTypes` array (`TagModal.razor:244`) duplicates type lists that exist elsewhere; casing drifts across `TwinCat` (map key) / `TwinCATTagConfigEditor` / `FOCASAddressPickerBody` / `FocasTagConfigEditor`. CSS is disciplined (one `site.css` + shared theme kit; chip/panel classes reused consistently, `chip-warn` vs `chip-caution` both exist — verify both are defined in the theme).
-
-### C-7 (Medium, NEW) — `ResilienceFormModel` round-trip is lossy: malformed JSON silently discarded on next edit, unknown keys stripped, and the promised raw-text view never landed
-
-The per-driver resilience override editor (`Components/Shared/Drivers/DriverResilienceSection.razor`, embedded in all 8 driver pages via `@bind-ResilienceConfig`) violates the preserve-unknown-keys convention its sibling tag editors (C-3) established:
-
-- `ResilienceFormModel.FromJson` (`ResilienceFormModel.cs:52–74`) maps **only** the known top-level fields and only capability keys present in the fixed `Capabilities` list (line 67 `TryGetValue` drops the rest); `ToJson` (`:78–101`) emits only the model's own fields. Any unknown top-level key or unrecognized capability name in the stored JSON is silently stripped the moment a user edits *any* resilience field (`EmitAsync`, `DriverResilienceSection.razor:60–66`, replaces the bound value with `ToJson()`).
-- Worse for the malformed case: `FromJson` catches `JsonException` and returns an **empty form** (`ResilienceFormModel.cs:59`), with the comment "malformed -> empty form; **raw view (next task) shows the text**". That task never landed — the "Raw JSON (advanced)" pane (`DriverResilienceSection.razor:37–40`) is a read-only `` of `_m.ToJson()`, i.e. the *re-serialized model*, not the stored text. So a malformed/hand-edited `DriverInstance.ResilienceConfig` renders as an apparently-blank form, and the first keystroke in any resilience field overwrites the stored override with the empty model's output — silent data loss with no indication the original existed.
-
-This mattered less when the column was write-only; since #13 (`75403caa`) the artifact read path applies these overrides to production dispatch, so a silently-emptied override silently reverts a driver to tier defaults.
-
-**Recommendation:** adopt the tag-editor round-trip discipline (deserialize to `JsonObject`, overlay known fields, preserve the rest), surface a "stored JSON could not be parsed — editing will replace it" warning when `FromJson` falls back, and make the raw pane show (or edit) the actual stored text. `ResilienceFormModelTests.cs` exists — extend it to pin unknown-key preservation.
-
----
-
-## 4. UNDERDEVELOPED AREAS
-
-### U-1 — Test coverage: strong C# seams, zero component rendering coverage
-
-443 `[Fact]`/`[Theory]` cases (unchanged since the prior review) covering: all tag-config models + validator, `ResilienceFormModel`, `UnsTreeService` (structure/tags/vtags/import/equip-token/script CRUD against a test DB), browse session service/registry/reaper, `CertificateStoreManager`, audit writers, `DeployApiEndpoints` auth matrix, driver-page form serialization (the enum bug class), 8 ScriptAnalysis suites (diagnose/completions/hover/format/tag-path/equip-token), `InProcessBroadcaster`, `HostsDriverView`, address builders. That is a deliberately shaped portfolio around the pure seams.
-
-There is **no bUnit** — 77 razor components have no render/binding coverage, which is the documented repo posture ("no bUnit — live-verify"; the `@`-binding bug class passes all unit tests). The consequence is that everything in `@code` blocks that isn't extracted (e.g. all of `EquipmentPage`'s tab/delete handlers, `GlobalUns`'s modal orchestration, `Alerts`' row-action state machine) is verified only by manual `/run` sessions.
-
-**Recommendation:** either adopt bUnit for the modal/table state machines, or keep extracting page logic into plain classes (the `HostsDriverView`/`VirtualTagModalHelpers` precedent) so the untested residue in `@code` stays trivial.
-
-### U-2 — Stub/known-gap behaviors
-
-- `InlayHints` is a wired no-op endpoint + active client provider (P-6); documented in `ScriptAnalysisService.cs:22–25`.
-- `AlarmsHistorian.razor:73–78` — admits the missing role-gated "no historian on this node" message; page silently shows "Loading…" forever on admin-only nodes.
-- `MonacoEditor.razor:33–37` — `MarkersChanged` is typed `object[]` with a comment that the DTO "is not modelled yet".
-- `GlobalUns.ConfirmDeleteAsync` default branch returns "Delete for this node kind is not yet available." for Tag/VirtualTag nodes (`GlobalUns.razor:~324`) — consistent with equipment-page ownership, but the message leaks a not-implemented posture into the UI.
-- `DriverResilienceSection.razor:59` — the "raw view (next task)" comment in `ResilienceFormModel.cs` documents a follow-up that never shipped (see C-7).
-
-### U-3 — Raw-JSON fallback surfaces
-
-`TagModal.razor:130–136` falls back to a schemaless textarea for unmapped drivers ("Validated server-side at deploy" — i.e., no client validation), and the Galaxy path allows direct JSON editing beside the picker. With all current drivers mapped (C-3), the fallback today only serves Galaxy + future drivers, but it remains the escape hatch through which invalid configs reach deploy time. `_form.TagConfig` is `[Required]` yet never checked for *well-formed JSON* in the fallback path.
-
-**Recommendation:** at minimum, `JsonDocument.Parse` the fallback textarea in `SaveAsync` before submitting.
-
-### U-4 — Accessibility
-
-~31 `aria-*`/`role=` attributes across 77 components, concentrated in the modals (`role="dialog"`, `btn-close` aria-labels). Gaps: text-glyph expander buttons (`▼`/`▶` in `UnsTree.razor:49`) with no `aria-expanded`/label; modals have no focus trap or Escape handling; live tails (`Alerts`) have no `aria-live` region; tables lack captions/scope. Form labeling is decent (most inputs have `for`/`id` pairs). Acceptable for an internal ops tool; worth a targeted pass on the tree and the modals if operator diversity matters.
-
-### U-5 (NEW) — Resilience runtime status has no AdminUI surface; overrides are authored blind — ✅ REMEDIATED (R2-10)
-
-**Remediated** on branch `r2/10-resilience-observability`: `DriverResilienceStatusTracker` now flows to the AdminUI through exactly the recommended idiom — a per-node DPS bridge (`DriverResilienceStatusBridge`) → in-process store (`IDriverResilienceStatusStore`, registered beside `IDriverStatusSnapshotStore` in `AddOtOpcUaDriverStatusServices`) → a **resilience section on `DriverStatusPanel.razor`** (embedded on all 8 driver pages, right where the `ResilienceConfig` overrides are authored — closing the feedback loop). It renders breaker-OPEN / consecutive-failures / in-flight / staleness chips, read in-process (self-HubConnection ban honored). The driver-node feed is a periodic 5 s full-snapshot publisher (`DriverResilienceStatusPublisherService`) with a wiring guard. Live gate (T10) — docker-dev breaker-open on both centrals + staleness — pending. *(Original finding below.)*
-
-The remediation merges made per-driver resilience live in production: #10 (`bacea1a4`) wired `CapabilityInvoker` into every `DriverInstanceActor` dispatch site, and #13 (`75403caa`) applies the AdminUI-authored `ResilienceConfig` overrides from the deploy artifact. But `DriverResilienceStatusTracker` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs`) still has **no reader** — no AdminUI component, hub, or endpoint consumes it (`grep -rn Resilience src/Server/ZB.MOM.WW.OtOpcUa.AdminUI` matches only the authoring form + driver pages). The planned `/hosts` Stream E.2/E.3 status panels never shipped; the interim observability surface is retry/breaker **log lines** added during #10. `archreview/plans/STATUS.md` explicitly records this as "still open as an Admin-UI follow-on."
-
-Consequence: an operator can now author retry counts, breaker thresholds, and bulkhead limits per driver instance (all 8 driver pages) with zero UI feedback on whether a breaker is open, a bulkhead is queuing, or retries are firing — the only diagnosis path is server logs.
-
-**Recommendation:** ship the deferred `/hosts` resilience panel: expose `DriverResilienceStatusTracker` snapshots through the existing `DriverStatusSignalRBridge`/`IDriverStatusSnapshotStore` pattern (the plumbing idiom already exists) and render breaker/bulkhead state chips beside the driver health chips.
+| S-1 (Med) | Three components don't drain in-flight timer callbacks on dispose | **PARTIALLY CLOSED** — `Fleet.razor` fully fixed (`63081d52`/`4b6a93bb`/`445bb855`: `IAsyncDisposable` + page-lifetime CTS + `Interlocked` tick-overlap guard + drain, `Fleet.razor:131–210`; pinned by `PollTimerTeardownOrderingTests`). The other two stragglers unchanged: `AlarmsHistorian.razor:85` is still `public void Dispose() => _timer?.Dispose();` and `DriverTestConnectButton.razor:68–81` still runs `System.Timers.Timer` with an async `Elapsed` lambda + sync dispose. |
+| S-2 (Med) | `Fleet.LoadAsync` no catch; `AlarmsHistorian` bare `catch { }` leaves "Loading…" forever | **PARTIALLY CLOSED** — Fleet now catches, but only `OperationCanceledException`/`ObjectDisposedException` (`Fleet.razor:167–171`); a transient SQL fault still faults the discarded `InvokeAsync` task silently (state no longer wedges — the `finally` at 172–181 resets the guard — but no error surfaces). `AlarmsHistorian.razor:68–74` still bare-catches with the admitted "lands when we add role gating" TODO. |
+| S-3 (Med) | No `` anywhere | **STILL OPEN** — `grep -rn ErrorBoundary` over the project returns nothing, now across 111 components. |
+| S-4 (Med) | Delete flows re-fetch RowVersion at click time, defeating the concurrency guard | **STILL OPEN on the UNS pages, NOT propagated to new code** — `GlobalUns.razor:280–311` ("Loads the entity's RowVersion first") and `EquipmentPage.razor:449–452, 494–497` ("Load … fresh to capture its current RowVersion") unchanged. But every NEW delete carries the *rendered* RowVersion: `RawTree.razor:462/469/498/546/640/659`, `AclEdit.razor:264/288`, and the reference remove `EquipmentPage.razor:407`. The codebase now demonstrates the correct pattern beside the wrong one. |
+| S-5 (Med) | No confirmation on destructive actions | **STILL OPEN — surface grew.** Unchanged: `EquipmentPage.razor:242/297` (vtag/alarm delete), `ScriptEdit.razor:70→142`. New unconfirmed instances: `ApiKeys.razor:102–107` (Revoke + **Delete** of a live data-plane credential, single click), `AclEdit.razor:150` (grant delete), `EquipmentPage.razor:189` (reference Remove). Counter-example now in-tree: `RawTree` routes every delete through `RawConfirmDialog` (`RawTree.razor:466–470` etc.). |
+| S-6 (Low) | `DriverStatusPanel` result-chip timer lacks the stale-fire token guard `Alerts` has | **STILL OPEN** — `Alerts.razor:260–268` has the `ReferenceEquals(_opResultClearToken, token)` guard; `DriverStatusPanel.razor:382–390` clears unconditionally (its comment even says "mirrors DriverStatusPanel" in Alerts — the mirror is one-way). |
+| S-7 (Low) | `AdminOperationsClient.AskAsync` bypasses the 10 s ask timeout | **STILL OPEN** — `Clients/AdminOperationsClient.cs:57–58` forwards only the caller's token; the three typed methods still double-guard (`:33–54`). |
+| P-1 (Med) | Monaco pushes the full document to .NET on every keystroke, undebounced | **STILL OPEN** — `wwwroot/js/monaco-init.js:213–214`. |
+| P-2 (Med) | ScriptAnalysis recompiles from scratch per request | **PARTIALLY CLOSED** — the *catalog* half is fixed: `ScriptTagCatalogSnapshotCache` (`3795899d`, singleton, 5 s TTL, gate-collapsed rebuild — `ScriptAnalysis/ScriptTagCatalogSnapshotCache.cs`) removed the six-table scan per keystroke that tag-path completion used to pay. The Roslyn `(text → tree/compilation)` memoization is still absent (`ScriptAnalysisService.cs:60–65` builds a fresh `CSharpCompilation` per call). |
+| P-3 (Low) | Per-circuit DB polling on dashboard pages | **STILL OPEN (accepted)** — `Hosts` 5 s, `Fleet` 10 s per circuit; the overlap/teardown hazards around those ticks are now fixed (see S-1). |
+| P-4 (Low) | `Deployments` runs a full config snapshot + hash per page load for the drift badge | **CLOSED** — `dbc9a132` lists a projection (`Deployments.razor:94–105`, no artifact blobs) and `684e59d8` added `DeploymentDriftProbe` (`Deployments/DeploymentDriftProbe.cs`, singleton at `EndpointRouteBuilderExtensions.cs:112`): recompose only when the `ConfigEdit` `(MAX(EditedAtUtc), COUNT(*))` watermark or the sealed hash moves, gate-collapsed, staleness trade documented. SQL-Server-backed pins in `c149410b`. |
+| P-5 (Low) | `Alerts` rows lack `@key` while prepending | **STILL OPEN** — `Alerts.razor:281` `_rows.Insert(0, evt)`, row markup carries no `@key` (Hosts/EquipmentPage tables do). |
+| P-6 (Low) | `inlay-hints` round-trips for a documented no-op | **STILL OPEN** — `monaco-init.js:107` posts; `ScriptAnalysisService.cs:472` returns empty. |
+| C-1 (High) | Authorization idioms / ungated mutating surface | **CLOSED AND HELD.** R2-05's one-idiom state survived 177 commits: all 30 routable pages carry an explicit named-policy `@attribute` (only `Login` exempt, `[AllowAnonymous]`); the six pages added since were consciously classified — `ApiKeys` = FleetAdmin ("minting a key hands out a data-plane credential"), `SecretsAdmin` = the Secrets RCL's own `ManagePolicy`, `GlobalRaw`/`AclEdit` = ConfigEditor, `ClusterAcls` = AuthenticatedRead (verified read-only: `ClusterAcls.razor:107–108` is the only DB touch, `AsNoTracking`), `Dev/ContextMenuDemo` = AuthenticatedRead. `PageAuthorizationGuardTests` still reflects over `typeof(Routes).Assembly` and its census includes every new page (`Authorization/PageAuthorizationGuardTests.cs:29–79`). `/api/script-analysis/*` = ConfigEditor (`ScriptAnalysisEndpoints.cs:18`). `Certificates` keeps markup + imperative FleetAdmin defense-in-depth (`Certificates.razor:85, 180–186`). The anti-drift guard did its job. |
+| C-2 (Med) | CLAUDE.md ScriptAnalysis policy drift | **STILL FIXED** — CLAUDE.md now says "gated by the `ConfigEditor` policy (Administrator or Designer)", matching `ScriptAnalysisEndpoints.cs:18` (the policy constant replaced the role string; semantics identical). |
+| C-3 (Good) | Driver-typed tag-editor pattern complete and consistent | **HOLDS, EXTENDED** — `TagConfigEditorMap.cs:17–27` and `TagConfigValidator.cs:19–29` now cover **11** drivers (added Calculation, Sql, Mqtt, MTConnect) with parallel keys, all keyed off `DriverTypeNames` constants (the casing-drift class is structurally closed; #558 consolidated the browse-commit parity guard onto one list, `17130d9c`). Authoring↔runtime parity got a new backstop: `TagConfigParseGate` makes authoring models consult the driver's own runtime parse (#554/#549/#551). |
+| C-4 (Good) | Numeric-enum serialization bug class closed | **HOLDS** — every new enum-carrying driver form carries `JsonStringEnumConverter` (`SqlDriverForm.razor:121`, `MTConnectDriverForm.razor:170`, `CalculationDriverForm.razor:52`, …); `MqttDriverForm` routes through the driver's own shared `MqttJson.Options` (`MqttDriverForm.razor:7–8`) — same discipline, different mechanism. Per-driver `*FormSerializationTests` extended. |
+| C-5 (Med) | Two data-access idioms: service seam vs EF-in-page | **STILL OPEN, GREW** — 15 components now inject `IDbContextFactory` directly (was 5 named): all the prior offenders plus `Reservations`, `ClusterOverview/Redundancy/Edit/Acls/Audit/List`, `NewCluster`, `NodeEdit`, `AclEdit`. Meanwhile the *new* Raw/Acl surfaces went through real seams (`IRawTreeService`, `IAclScopeService`, `IApiKeyAdminService`) — the split is now seam-for-new, EF-for-old. |
+| C-6 (Low) | Naming/marker nits | **STILL OPEN (reduced)** — `"GalaxyMxGateway"` literal survives in `DriverIdentitySection.razor:38` and `GalaxyAddressPickerBody.razor:136` (though `RawDriverTypeDialog.razor:60` now documents + guards the mapping); dispatch maps moved onto `DriverTypeNames`. |
+| C-7 (Med) | `ResilienceFormModel` lossy round-trip; malformed JSON silently overwritten | **CLOSED (R2-02)** — `ResilienceFormModel.cs:13–17/43–48/106–124`: `JsonObject` bag preserves unknown top-level keys, unknown capabilities, unknown per-policy fields; a malformed blob sets `ParseFailed`, `ToJson` returns the stored text **verbatim**, and `DriverResilienceSection.razor:9–14` shows the warning + explicit discard button while every input is `disabled="@_m.ParseFailed"` (`:40–42`) and `EmitAsync` early-returns (`:93`). `ResilienceFormModelTests` pins it. |
+| U-1 | Strong seam tests, zero component rendering coverage | **IMPROVED IN VOLUME, SAME SHAPE** — 978 cases (443 → 978): Acls, Raw (CSV mapper, tree service, modals), Api, Browsing, Grpc, Telemetry, Hosts suites added. Still no bUnit (documented posture) — but a **reflection-driven component-test harness now exists**: `RawBrowseModalStaleOpenTests` drives a real modal class end-to-end (inert `Renderer` subclass for the render handle, reflection for `[Parameter]`s), proving modal `@code` logic *is* testable here without bUnit. |
+| U-2 | Stub/known-gap behaviors | **STILL OPEN** — InlayHints no-op (`ScriptAnalysisService.cs:472` + active provider `monaco-init.js:107`); `AlarmsHistorian.razor:68–74` role-gating TODO; `MonacoEditor.razor:44` `MarkersChanged` still `object[]`; `GlobalUns.razor:327` "Delete for this node kind is not yet available." |
+| U-3 | Raw-JSON fallback unvalidated | **SUBSTANTIALLY CLOSED** — typed client validation covers 11 drivers and is now **service-enforced**, not just modal-side: `RawTreeService.cs:977` (create/update), `:1273` (import), `RawTagCsvMapper.cs:228` (CSV rows), `RawTagModal.razor:242` / `RawManualTagEntryModal.razor:243` (modals) — the #535→#559 authoring-vs-deploy-gate arc. Residual: the schemaless textarea for *unmapped* drivers (Galaxy only) still takes any text ("Validated server-side at deploy", `RawTagModal.razor:90`) — the deploy gate is the only check there. |
+| U-4 | Accessibility gaps | **STILL OPEN** — tree expanders are still text glyphs without `aria-expanded` (`RawTree.razor:176–179`, `UnsTree`), modals have no focus trap/Escape, live tails no `aria-live`. New code at least marks decorative emoji `aria-hidden` (`RawTree.razor:229`). |
+| U-5 | Resilience runtime status has no AdminUI reader | **CLOSED (R2-10 + #546)** — `DriverStatusPanel.razor:15/77–92/203/263–264/292–303` renders breaker-OPEN / consecutive-failures / in-flight / staleness chips off `IDriverResilienceStatusStore` (in-process, self-HubConnection ban honored), and #546 (`d33e4e20` + `e669fb34`) wired the panel into every `/hosts` driver row via the Controls dialog — with the `Enabled`-in-`@key` recycle trick documented in the markup (`Hosts.razor:283–299`) so a live Enabled flip re-mounts rather than orphaning subscriptions. R2-10's live gate ran (per the R2-10 memory/plans register). |
---
## Maturity Ratings
-| Dimension | 2026-07-08 | 2026-07-12 | Justification |
-|---|---|---|---|
-| Stability | **4** | **4** | Unchanged code, unchanged assessment: disciplined disposal/marshaling with documented race reasoning and the self-HubConnection ban honored everywhere; docked for three non-draining timers, `Fleet`'s uncaught refresh path, no `ErrorBoundary`, and the RowVersion-refetch delete pattern — all still open. |
-| Performance | **3** | **3** | Unchanged: event-push where it matters and bounded lists, but keystroke-grain Monaco interop, per-request Roslyn recompiles ×6 endpoints, per-circuit DB polling, and a full config snapshot per Deployments render. |
-| Conventions | **3** | **3** | C-2 (doc drift) fixed, but the dominant C-1 three-idiom/ungated-majority finding is untouched, C-5/C-6 remain, and the new C-7 shows the preserve-unknown-keys convention wasn't carried into the resilience editor. |
-| Underdeveloped | **3** | **3** | 443 well-aimed seam tests but still zero rendering coverage (by policy), the same admitted stubs, the unvalidated raw-JSON escape hatch — and the new U-5: production-live resilience behavior authored with no status surface. |
+| Dimension | 2026-07-08 | 2026-07-12 | **2026-08-19** | Justification |
+|---|---|---|---|---|
+| Stability | 4 | 4 | **4** | The dominant open items moved: Fleet's timer/teardown fully fixed, C-7's silent data loss closed, staleness guards + confirm dialogs are the *norm* in the new Raw surface, and the tick-overlap class got a dedicated test file. Docked at 4 (not higher) because the fixes were applied N−1-wide — five sibling modals still carry the exact superseded-open race that three others were just fixed for (S-8), three EditForms silently no-op on invalid input (S-9), and ErrorBoundary/AlarmsHistorian/DriverTestConnectButton stragglers persist. |
+| Performance | 3 | 3 | **4** | A deliberate, well-engineered perf sweep landed: drift-watermark probe (P-4 closed), `/deployments` and raw-tree projections that stop shipping blobs, the 5 s tag-catalog snapshot cache, server-side rename script-scan with chunked OR-chains, one guard round-trip per bulk reference add. What remains (Monaco keystroke push, per-request Roslyn compile, per-circuit polling) is bounded at single-admin scale and known. |
+| Conventions | 3 | 3 | **4** | C-1 closed *and held under drift pressure* — the reflection guard forced conscious classification of all six new pages. C-7 closed to the tag-editor discipline. The editor/validator/parity pattern extended cleanly to four new drivers, keyed off constants. Docked for the EF-in-page split growing (C-5) and the naming residue (C-6). |
+| Underdeveloped | 3 | 3 | **3.5** | U-5 closed (resilience observability shipped and wired to `/hosts`), U-3 substantially closed (validation now service-enforced), tests 443→978 with a component-harness pattern emerging. Still: no bUnit (by policy) with most new modal orchestration verified only by `/run`, the same four U-2 stubs, and accessibility untouched. |
+
+---
+
+## Architecture Overview (delta since f6eaa267)
+
+- **Page map changes:** `+/apikeys` (FleetAdmin), `+/secrets` (Secrets RCL wrapper), `+/raw` (`GlobalRaw` → `RawTree`, the v3 Batch-2 authoring surface), `+/clusters/{id}/acls` list + `AclEdit` + `AclScopePickerModal`/`AclScopeTree` (#520 stage 7), `+Dev/ContextMenuDemo`. **Retired:** the routed `/clusters/{id}/drivers` flow (`DriverTypePicker`, `DriverEditRouter`, 8 `*DriverPage` shells) — form bodies live on inside the `/raw` modals (`DriverConfigModal`/`DeviceModal` + `Forms/`/`DeviceForms/`); the UNS `TagModal` (Batch 3 made equipment Tags reference-only via `AddReferenceModal`).
+- **Hubs:** 3 (`alerts`, `scriptlog`, `driverstatus`) — `FleetStatusHub` + bridge + broadcaster deleted as a zero-client chain (#542, `f1c18811`). The in-process singleton pattern (DPS bridge → store/broadcaster → component event) unchanged and still honored everywhere.
+- **New service seams:** `IRawTreeService` (1701 lines, RowVersion-aware, warn-on-rename substring scan — now server-side filtered + chunked, `fe8d132c`/`ca87329a`), `IAclScopeService` (mirrors `DraftValidator.ValidateNodeAclScopes` rule-for-rule so page and deploy gate cannot disagree; blocks save on unresolvable scope, re-checked on submit — `AclEdit.razor:238–242`), `IApiKeyAdminService` (server-side validation, hash-only storage), `RawTagCsvExportReader`, `DeploymentDriftProbe` (singleton), `ScriptTagCatalogSnapshotCache` (singleton).
+- **UI sweep held:** the scoped-CSS bundle is linked (`App.razor:23`), chip variants documented against the Theme kit (`site.css:20–22`), empty-string columns guarded (`Hosts.razor` Last-error uses `Abbreviate … is { Length: > 0 } … : "—"`), `#504` slicing discipline held — every `[..N]` in the tree is length-guarded or Guid-sourced.
+
+---
+
+## 1. STABILITY
+
+### S-8 (Medium, NEW) — The superseded-open staleness guard was applied to three modals; five siblings share the unguarded shape
+
+The 2026-08-05/08 fixes (`87637e4e` RawBrowseModal, `2c19a50b`/`4e3a6e84` RawManualTagEntryModal + RawTagModal) established the class: Blazor does not serialize `OnParametersSetAsync` against its own outstanding task, and a bool open-flag cannot tell two opens apart — so a close-and-reopen onto another target while a load is in flight lets the first open's late continuation install its state under the second open's displayed modal. The fixed modals carry generation counters and post-await re-checks (`RawBrowseModal.razor:362–446`, `RawManualTagEntryModal.razor:183–197`, `RawTagModal.razor:202`), and `RawBrowseModalStaleOpenTests` pins one of them.
+
+Five sibling modals have the identical shape with **no** guard:
+
+- **`RawCsvImportModal.razor:209–240`** — the sharpest instance. `ResolveTargetAsync` awaits `ResolveGroupContextAsync` and then writes `_effectiveDeviceId` + `_groupPrefix` unconditionally; `_resolved` is a bool, not a generation. A stale resolve retargets *both* the parse (`_groupPrefix` is baked into every row's `EffectiveGroupPath`, `:250`) and the **commit itself** — `Commit()` calls `Svc.ImportTagsAsync(_effectiveDeviceId, rows)` (`:293`), and `ImportTagsAsync` takes a device id with no RowVersion, so unlike the edit modals nothing downstream catches the mistake: the all-or-nothing import lands **in the wrong device**.
+- **`RawCsvExportModal.razor:88–129`** — same shape, read-only consequence (another device's tags exported under this device's filename).
+- **`DeviceModal.razor:126–171`** — `_openHandled` bool; `LoadAsync` writes `_name`/`_deviceConfigJson`/`_rowVersion`/`_parentDriverConfig` after two awaits with no re-check. A stale install shows device A's config under device B's title; Save is *accidentally* blocked by the RowVersion mismatch, but Test-connect probes the wrong endpoint with no error.
+- **`DriverConfigModal.razor:132–165`** — identical to DeviceModal (`_driverConfigJson`/`_resilienceConfig`/`_rowVersion` installed post-await).
+- **`AddReferenceModal.razor:83–101`** — `_wasVisible` bool; a stale `LoadReferencePickerRootAsync` installs equipment A's picker root under equipment B's open; picks then commit as B's references (`CommitAsync:116` uses the current `EquipmentId`), which the cluster-scope server check will *accept* whenever A and B share a cluster.
+
+**Recommendation:** port the `_openGeneration` + post-await `IsStale` idiom (and its reflection-harness test — the pattern is proven) to all five. This is the "ForTest-seam / N−1 dispatch sites" failure mode in modal form: the fix commits named three modals; the class has eight members.
+
+### S-9 (Medium, NEW) — Three EditForms validate with no validation display: invalid input = silent no-op Save
+
+`NodeEdit.razor:42` and `AclEdit.razor:36` show the house convention (`DataAnnotationsValidator` + ``); the UNS modals render `ValidationMessage`s. Three pages drifted:
+
+- **`ClusterEdit.razor:35–36`** — `DataAnnotationsValidator`, `[Required] Name` (`:195`), no `ValidationSummary`/`ValidationMessage` anywhere in the form.
+- **`NewCluster.razor:18–19`** — worse: `[Required]` ×3 plus a `[RegularExpression(..., ErrorMessage = "Use uppercase letters, digits, dash, underscore.")]` (`:132–136`) whose message **can never render**.
+- **`ScriptEdit.razor:32–33`** — `[Required] Name` + `[Required] SourceCode` (`:167–168`), no display.
+
+Consequence: clear the Name (or type a lowercase cluster id) and click Save/Create — `OnValidSubmit` never fires, no message appears, the button just does nothing. This is precisely the #505-adjacent "EditForm with no ValidationSummary = silent no-op Save" class the project memory documents, and it passes every unit test because there is no rendering coverage.
+
+**Recommendation:** one-line fix per page; add the three `` lines, and consider a guard-style scan (the pages are compiled classes — a reflection test could assert every component that renders `DataAnnotationsValidator` also renders a validation display, mirroring `PageAuthorizationGuardTests`).
+
+### S-1 / S-2 residue (Medium, carried) — Two straggler timers and two swallowed-error paths
+
+Fleet is now the model citizen (`Fleet.razor:113–210`: CTS-before-drain, `Interlocked` overlap guard, tick-holding-CTS dispose reasoning — all commented). Still owed:
+
+- `AlarmsHistorian.razor:85` sync `Dispose`; `:68–74` bare catch that leaves "Loading…" forever on admin-only nodes (the TODO is now over a year of review cycles old).
+- `DriverTestConnectButton.razor:68–81` `System.Timers.Timer` + async `Elapsed` (effectively async-void) + sync dispose.
+- `Fleet.razor:167–171` catches only OCE/ODE — a transient SQL fault is still an unobserved task exception with no error surface (the `Hosts` catch-log-degrade shape was never copied over).
+
+### S-3 (Medium, carried) — No ``; S-4 (Medium, carried) — UNS delete RowVersion refetch; S-5 (Medium, carried) — unconfirmed destructive actions
+
+All three persist as described in the table above. Two aggravations worth noting:
+
+- **S-4 is now an internal inconsistency, not just a defect**: `RawTree` and `AclEdit` prove the rendered-RowVersion delete works fine in this codebase; `GlobalUns`/`EquipmentPage` sit beside them doing the opposite.
+- **S-5's sharpest new instance is `ApiKeys.razor:106–107`**: single-click **Delete** of an API key destroys a live machine credential (revocation is immediate on ConfigDb nodes) with no confirm and no undo — and unlike Revoke it also erases the row. At minimum, Delete here deserves the `RawConfirmDialog` treatment; arguably Delete should be gated behind Revoke-first.
+
+### S-6 / S-7 (Low, carried)
+
+`DriverStatusPanel.razor:382–390` still lacks the stale-fire token guard its own Alerts twin has (`Alerts.razor:260–268` — whose comment claims to "mirror DriverStatusPanel"); `AdminOperationsClient.cs:57–58` still forwards only the caller's token.
+
+### Positive stability notes
+
+- The three fixed modals + `RawBrowseModalStaleOpenTests` show the *harness* problem (no bUnit) is solvable for modal state machines — an inert `Renderer` subclass + reflection drives the real component class.
+- `RawTree` is a well-built coordinator: one modal set for the whole recursive tree, `@key` per row, lazy loads with captured errors, confirm dialogs on every delete, rendered-RowVersion mutations, warnings surfaced through a shared message modal (`RawTree.razor:705–723`).
+- `Hosts`' Controls dialog documents its own `@key`-recycle rationale in markup (`Hosts.razor:283–299`) — the "Enabled flips must recycle the panel or its subscriptions dangle" trap is pinned in words where the next editor will read them.
+- `AlarmsHistorian` aside, every new page reports load errors (`ApiKeys.ReloadAsync` catches and surfaces; `AclEdit` catches `DbUpdateConcurrencyException` with a human message on both save and delete).
+
+---
+
+## 2. PERFORMANCE
+
+### P-1 / P-2 / P-3 / P-5 / P-6 (carried — see table)
+
+The two Mediums still worth scheduling: Monaco's undebounced full-document push (`monaco-init.js:213–214`) and the per-request Roslyn compile (`ScriptAnalysisService.cs:60–65` — a size-1 LRU keyed on normalized text would collapse the completions-after-diagnostics case; the catalog half is done).
+
+### Positive performance notes (the sweep that closed P-4)
+
+- `DeploymentDriftProbe` is the standout: watermark = `(MAX(EditedAtUtc), COUNT(*))` over the append-only `ConfigEdit` audit (the COUNT half defends same-timestamp edits), volatile-published verdict, gate-collapsed recompose, and an honest "accepted staleness" paragraph naming the exact condition under which the chip lies and why that's tolerable (`Deployments/DeploymentDriftProbe.cs`).
+- `/deployments` lists a projection (`Deployments.razor:94–105`) instead of 50 artifact blobs; the raw tree renders tag rows without `TagConfig` (`ba977dbe`); the rename script-scan filters server-side and chunks its OR-chain with SQL-Server-backed pins (`fe8d132c`/`ca87329a`/`c149410b`); bulk reference adds pay one guard round-trip (`fa36d64e`); `ScriptTagCatalogSnapshotCache` bounds editor-assist DB load at one six-table scan per 5 s process-wide.
+- CSV import bounds the upload at 16 MB via `OpenReadStream(maxAllowedSize:)` (`RawCsvImportModal.razor:251`) and stages all-or-nothing review before any write.
+
+---
+
+## 3. CONVENTIONS
+
+### C-1 — CLOSED AND HELD (see table). The reflection guard proved its worth
+
+The census in `PageAuthorizationGuardTests` was *maintained*, not bypassed: each of the six post-R2-05 pages appears with a policy and, for the interesting ones, a rationale comment (ApiKeys→FleetAdmin "same authority level as editing role grants"; SecretsAdmin carrying the RCL's own policy). This is the anti-drift outcome R2-05 was designed for.
+
+### C-5 (Medium, carried) — The seam/EF-in-page split is now generational
+
+New surfaces (Raw, Acls, ApiKeys, drift, catalog) all went through injectable services with real test suites; the older cluster/script/dashboard pages still run EF in `@code` (15 components inject `IDbContextFactory`). The convention argument has effectively been won by the new code — declare it and migrate the old pages opportunistically (`ScriptEdit`'s save/delete still belongs in the existing script service methods).
+
+### C-6 (Low, carried) — reduced to two `"GalaxyMxGateway"` literals (`DriverIdentitySection.razor:38`, `GalaxyAddressPickerBody.razor:136`); the dispatch-map keying moved to `DriverTypeNames` fleet-wide.
+
+### C-8 (Low, NEW) — A dev demo page ships in the production route table
+
+`Components/Pages/Dev/ContextMenuDemo.razor` is routable in every deployment (gated `AuthenticatedRead`, consciously classified in the guard census — so this is a deliberate choice, recorded here as a nit). If it's meant as a pattern gallery, name it so; if not, `#if DEBUG` or delete.
+
+### Positive conventions notes
+
+- The tag-editor discipline scaled to 11 drivers without a single map/validator drift, because the keys are constants and the parity tests are consolidated onto one list (#558).
+- The authoring↔runtime parity arc (#549/#550/#551/#554/#557) gave the AdminUI a genuinely novel convention: authoring models consult the **driver's own runtime parse** (`TagConfigParseGate`) so the editor can never accept a config the runtime rejects — enforced at modal, CSV, and service layers (one authority, service-enforced — the `reference_authoring_vs_deploy_gate_drift` lesson applied).
+- `AclScopeService` mirrors `DraftValidator.ValidateNodeAclScopes` rule-for-rule with better operator wording, and the picker makes invalid grants *unauthorable* (Cluster ⇒ null ScopeId at projection, `AclScopeService.cs:265–267`; UNS tags deliberately unofferable with the rationale in code, `:317–320`).
+
+---
+
+## 4. UNDERDEVELOPED AREAS
+
+### U-1 (carried, improved) — 978 seam tests, no rendering coverage — but the harness excuse is gone
+
+The reflection-driven component harness (`RawBrowseModalStaleOpenTests` — inert `Renderer` subclass for the render handle, reflection past BL0005 for parameters) demonstrates modal `@code` machines are testable without bUnit. S-8's five unguarded modals and S-9's three silent forms are exactly the tests that harness should now grow.
+
+### U-2 (carried) — the same four stubs
+
+InlayHints wired-no-op (client provider still POSTs, `monaco-init.js:107`; `ScriptAnalysisService.cs:472`); `AlarmsHistorian` role-gating TODO; `MonacoEditor.MarkersChanged` untyped `object[]`; `GlobalUns` "not yet available" delete branch. None moved in ~1300 commits — either schedule or annotate as accepted.
+
+### U-3 (carried, substantially closed) — residual: the Galaxy schemaless textarea
+
+Only unmapped drivers (today: Galaxy) reach the raw textarea, and only the deploy gate validates it. A `JsonDocument.Parse` in the service create/update path for unmapped types remains a cheap backstop.
+
+### U-4 (carried) — accessibility unchanged; the new tree components repeated the text-glyph expander pattern without `aria-expanded` (`RawTree.razor:176–179`, `AclScopeTree`).
---
## Top Recommendations (ordered)
-1. **C-1:** Centralize policy constants and put a write-role gate on every config-mutating page (`/uns`, equipment, cluster/driver/ACL/namespace editors, Reservations) — now also guarding production-live resilience overrides.
-2. **C-7:** Make `ResilienceFormModel` round-trips non-lossy (preserve unknown keys, warn on unparseable stored JSON) before an edit silently reverts a live driver to tier defaults.
-3. **S-4 + S-5:** Make deletes carry the rendered RowVersion (or document last-writer-wins) and add confirm dialogs on `EquipmentPage`/`ScriptEdit` deletes.
-4. **S-3:** Add an `ErrorBoundary` around `@Body`.
-5. **U-5:** Ship the deferred `/hosts` resilience status panel via the existing snapshot-store idiom.
-6. **P-1:** Debounce Monaco's value push to .NET.
-7. **S-1/S-2:** Converge the three straggler timers on the async-dispose idiom and give `Fleet.LoadAsync` the `Hosts`-style catch.
-8. **C-5/U-1:** Declare the service-seam pattern canonical and keep extracting `@code` logic into testable classes.
+1. **S-8:** Port the superseded-open generation guard to the five unguarded modals — `RawCsvImportModal` first (its stale `_effectiveDeviceId` retargets a commit that nothing downstream can catch).
+2. **S-9:** Add the missing `ValidationSummary` to `ClusterEdit`/`NewCluster`/`ScriptEdit`; consider a reflection guard for the class.
+3. **S-5 (ApiKeys):** Confirm-dialog the API-key Delete (and AclEdit/EquipmentPage deletes) — a live credential should not die to one click.
+4. **S-4:** Carry rendered RowVersions on the UNS deletes (the codebase's own new code shows how) or document delete as last-writer-wins.
+5. **S-3:** `ErrorBoundary` around `@Body` — still absent at 111 components.
+6. **P-1/P-2:** Debounce Monaco's value push; memoize `(text → compilation)` size-1 in `ScriptAnalysisService`.
+7. **S-1/S-2 stragglers:** `AlarmsHistorian` + `DriverTestConnectButton` timers; give `Fleet` (and `AlarmsHistorian`) a real error surface.
+8. **C-5:** Declare the service-seam convention won; migrate `ScriptEdit` first.
diff --git a/archreview/05-protocol-drivers.md b/archreview/05-protocol-drivers.md
index de67dfa8..241f3597 100644
--- a/archreview/05-protocol-drivers.md
+++ b/archreview/05-protocol-drivers.md
@@ -1,755 +1,418 @@
# Architecture Review 05 — Protocol Drivers
-**Date:** 2026-07-12
-**Commit:** `f6eaa267` (master, clean tree)
-**Updates:** the 2026-07-08 review at `9cad9ed0`. Since then the arch-review
-remediation branches merged to master; the delta touching this domain is
-exactly: Critical 3 — S7 lazy reconnect (`25c0c6f6`, merged `dfc02af3`),
-Critical 4 — TwinCAT native-notification replay (`af318fb4`, merged
-`8f7b0017`), the OTOPCUA0001 analyzer wiring (`f0082af5` — pragma/NoWarn-only
-in this domain: two driver-internal self-call pragmas in
-`AbCipAlarmProjection.cs`, one in `S7Driver.PollOnceAsync`, `NoWarn` in the
-six CLI csprojs), plus the new reconnect test suites under `tests/Drivers/`.
-No other driver file changed.
-**Scope:** the seven cross-platform protocol drivers and their satellites —
-`Driver.Modbus` (+ `.Addressing`, `.Contracts`), `Driver.S7` (+ `.Contracts`),
-`Driver.AbCip` (+ `.Contracts`), `Driver.AbLegacy` (+ `.Contracts`),
-`Driver.TwinCAT` (+ `.Contracts`), `Driver.FOCAS` (+ `.Contracts`),
-`Driver.OpcUaClient` (+ `.Contracts`, `.Browser`), and `src/Drivers/Cli/*`
-(six per-driver CLI harnesses + `Cli.Common`). Test projects under
-`tests/Drivers/` and `tests/Drivers/Cli/` were assessed for coverage only.
-Galaxy and the Historian drivers are out of scope.
+**Date:** 2026-08-19
+**Commit:** `12b0035a` (master, clean tree)
+**Updates:** the second update — round 1 ran 2026-07-08 at `9cad9ed0`, round 2 (re-review)
+2026-07-12 at `f6eaa267`. ~292 commits touched this domain since round 2, including the R2
+remediation waves (R2-01 S7 fault hardening, R2-09 PollGroupEngine v2 + S7 fork retirement,
+R2-11 TagConfig consolidation + #457 strict parsing), the 05/STAB-3/4/5/8/9/16/17 and
+CONV-4 targeted fixes, the #516 config-re-parse sweep, the #519/#517/#497 status-code work,
+the comms-loss-watchdog driver legs (#527), and **four brand-new drivers**: MTConnect
+(+ IWritable), MQTT incl. Sparkplug B (Driver.Mqtt + .Browser/.Contracts/.Transport),
+the SQL poll driver (Driver.Sql + .Browser/.Contracts/.Dialect), and the Calculation
+pseudo-driver.
+**Scope:** the twelve cross-platform protocol drivers and their satellites —
+`Driver.Modbus` (+ `.Addressing`, `.Contracts`, and the new RTU-over-TCP transport),
+`Driver.S7`, `Driver.AbCip`, `Driver.AbLegacy`, `Driver.TwinCAT`, `Driver.FOCAS`,
+`Driver.OpcUaClient` (+ `.Contracts`, `.Browser`, and the new `.Session` leaf),
+`Driver.Mqtt` (+ `.Browser`, `.Contracts`, `.Transport`), `Driver.MTConnect`
+(+ `.Contracts`), `Driver.Sql` (+ `.Browser`, `.Contracts`, `.Dialect`),
+`Driver.Calculation` (+ `.Contracts`), and `src/Drivers/Cli/*`. Galaxy and
+Historian.Gateway are out of scope (report 06).
-**Method:** every prior finding was re-verified against current code (the
-delta above means findings in untouched files carry forward with their line
-references intact; only `S7Driver.cs` and `TwinCATDriver.cs` shifted). The
-two remediation commits were read in full and adversarially — the fresh
-findings STAB-14..17 come from that reading. The original method (Modbus as
-the exhaustively-read reference driver, structural diffing of each sibling)
-stands from the 2026-07-08 pass.
+**Method:** every prior finding — open or claimed-closed — was re-verified against current
+code with file:line evidence; no remediation claim, commit message, or doc was trusted.
+The four new drivers were fresh-reviewed to the same adversarial standard as the
+incumbents (full driver reads, house failure-mode hunt: built-but-never-wired, optimistic
+success, fatal-vs-transient misclassification, teardown leaks on reconnect,
+fixture-shaped verification gaps).
---
-## Prior-finding status (9cad9ed0 → f6eaa267)
+## Prior-finding status (f6eaa267 → 12b0035a)
-| ID | One-liner | Status @ f6eaa267 |
+| ID | One-liner | Status @ 12b0035a |
|---|---|---|
-| STAB-1 | S7 has no reconnect path at all | **FIXED** — `25c0c6f6`: `IS7Plc`/`IS7PlcFactory` seam (`Driver.S7/IS7Plc.cs`), lazy `EnsureConnectedAsync` under `_gate` (`S7Driver.cs:1188-1224`), connection-fatal classification `MarkConnectionDeadIfFatal`/`IsS7ConnectionFatal` (`:1226-1251`), probe loop as reconnect backstop (`:1532-1536`). 4 unit guards (`S7DriverReconnectTests`) + env-gated live bounce test (`S7_1500ReconnectTests`), live-verified against python-snap7. **However, the fix itself introduces new findings STAB-14 (High) and STAB-15 (High)** — see below. |
-| STAB-2 | TwinCAT native subs silently orphaned after reconnect | **FIXED** — `af318fb4`: `NativeRegistration` replayable-intent registry per device (`TwinCATDriver.cs:551-613, 921`), `EnsureConnectedUnderGateAsync` replays every intent onto a fresh client (`:731-773, 784-823`), probe-failure `RecycleClientAsync` closes the `IsConnected`-lies compounding hole (`:631-643, 826-843`). Register and replay both run under `ConnectGate` — verified no register/replay double-registration race (intent stored only after the handle is live, `:508-520`). Unit-only verification (4 tests, fake client) — no TC3 fixture exists, accepted. **Residuals: STAB-16 (Medium) and STAB-17 (Low)** — see below. Data-path reads on a wire-dead-but-port-open client still fail Bad until the next probe tick recycles (bounded by probe interval — acceptable); the wrapper's pre-gate fast path (`:709-710`) can hand a data-path caller a client the probe is concurrently disposing (transient Bad read — acceptable). |
-| STAB-3 | Modbus transport desyncs after response timeout / TxId mismatch | **STILL OPEN** — `ModbusTcpTransport.cs:155-165, 233-237, 257-261` unchanged. Note: STAB-15 shows the new S7 reconnect path reproduces this same failure mode. |
-| STAB-4 | AbCip concurrent ops on shared libplctag handles, no lock | **STILL OPEN** — `AbCipDriver.cs:544-575, 614-637, 708-726` unchanged; alarm-projection read loop now `AbCipAlarmProjection.cs:230-232` (pragma lines shifted it +4). |
-| STAB-5 | FOCAS fixed-tree caches are plain `Dictionary`s mutated cross-thread | **STILL OPEN** — `FocasDriver.cs:1184-1194, 752-839, 912-945` unchanged. |
-| STAB-6 | AbLegacy probe-shutdown race; S7 unsubscribe race | **STILL OPEN** — AbLegacy unchanged (`AbLegacyDriver.cs:118, 156-170`); S7 `UnsubscribeAsync` still cancels + disposes the CTS without draining `PollTask` (now `S7Driver.cs:1350-1354`; the loop's `Task.Delay` still catches only OCE, `:1396-1397`). |
-| STAB-7 | `InitializeAsync` ignores `driverConfigJson` (Modbus, AbLegacy); TwinCAT empty-config quirk | **STILL OPEN** — all three sites unchanged (`ModbusDriver.cs:167-197`, `AbLegacyDriver.cs:78-153`, `TwinCATDriver.cs:89-94`). |
-| STAB-8 | No reconnect backoff fleet-wide (except Modbus transport, S7 poll loop) | **STILL OPEN — and slightly worse on S7**: the new lazy reconnect attempts a full TCP connect (bounded only by `_options.Timeout`) on *every* data call while the PLC is down, with no attempt throttle; poll-loop backoff shields subscriptions but not server-initiated reads/writes or the probe. |
-| STAB-9 | `PollGroupEngine` `onError` sink dead — no driver passes it | **STILL OPEN** — verified all five construct without it (`ModbusDriver.cs:115`, `AbCipDriver.cs:133`, `AbLegacyDriver.cs:65`, `TwinCATDriver.cs:69`, `FocasDriver.cs:72`). |
-| STAB-10 | AbLegacy never evicts failed tag handles on the data path | **STILL OPEN** — `AbLegacyDriver.cs:246-274` unchanged. |
-| STAB-11 | RMW locks don't cover direct parent writes; AbCip DINT / TwinCAT 4-byte assumptions | **STILL OPEN** — all sites unchanged (`AdsTwinCATClient.cs` untouched by the replay commit). |
-| STAB-12 | Blocking/uncancellable teardown paths (FOCAS close-PDU, TwinCAT blocking Connect) | **STILL OPEN — and compounded**: the new `RecycleClientAsync` waits on `ConnectGate` with `CancellationToken.None` (`TwinCATDriver.cs:835`), so a connect wedged under the gate now also wedges the probe loop uncancellably. |
-| STAB-13 | (Positive) write-outcome surfacing universal; OpcUaClient reconnect machine strongest | **STILL HOLDS** — S7 write path now `S7Driver.cs:981-1083` (per-item statuses preserved through the reconnect wrapper); all other cites unchanged. |
-| PERF-1 | Five of seven drivers read one tag per wire round-trip | **STILL OPEN** — no batching work landed. |
-| PERF-2 | AbCip whole-UDT read exists in two never-joined halves | **STILL OPEN** — planner/template-cache unchanged. |
-| PERF-3 | S7 forked poll loop diffs arrays by reference → publishes every tick | **STILL OPEN** — now `S7Driver.cs:1469` (`Equals(lastSeen?.Value, current.Value)`). |
-| PERF-4 | OpcUaClient serializes all traffic on one global gate | **STILL OPEN** — `OpcUaClientDriver.cs` untouched. |
-| PERF-5 | OpcUaClient discovery enrichment un-chunked Read + silent degrade | **STILL OPEN**. |
-| PERF-6 | Assorted per-tick allocation churn (TwinCAT re-parse, etc.) | **STILL OPEN** — TwinCAT symbol re-parse now `TwinCATDriver.cs:220, 284, 468`. |
-| PERF-7 | Deadband only in Modbus | **STILL OPEN**. |
-| CONV-1 | S7 forked the poll loop; fork diverged both directions | **STILL OPEN** — fork now `S7Driver.cs:1373-1412` (backoff `:1447`, cap `:1363`); neither direction merged. The fork's bare OCE-return is now load-bearing for the new STAB-14. |
-| CONV-2 | Three config-parse strictness tiers; silent enum defaulting in all six equipment-tag parsers | **STILL OPEN** — all six `ReadEnum` copies + Modbus probe shape unchanged. |
-| CONV-3 | Four factories take no `ILoggerFactory` → NullLogger in production | **STILL OPEN** — verified `S7DriverFactoryExtensions.cs:19` still `Register(DriverFactoryRegistry)` only; ditto TwinCAT/AbCip/FOCAS. Now more costly: the new S7 reconnect and TwinCAT replay log recovery *only* via `_logger` — invisible in production for S7/TwinCAT until this lands. |
-| CONV-4 | `ResolveHost` mis-keys equipment-tag refs in every multi-device driver | **STILL OPEN** — TwinCAT's now `TwinCATDriver.cs:689-696`; others unchanged. |
-| CONV-5 | Health-field publication idioms drift (plain fields in S7/AbCip/TwinCAT/OpcUaClient) | **STILL OPEN** — S7's new `_plcDead`/`_initialized` are gate-guarded (fine), but `_health` remains a plain field written from data paths, poll loops and the probe. |
-| CONV-6 | OpcUaClient.Contracts drags the OPC UA SDK | **STILL OPEN** (previously-deferred NamespaceMap move). |
-| CONV-7 | CLI harness divergences | **STILL OPEN** — only change is the uniform `NoWarn OTOPCUA0001` (wire-level projects, commented; consistent with the analyzer triage — neutral). |
-| CONV-8 | Dead config knobs + stale scaffold docs; no options `Validate()` | **STILL OPEN**. |
-| UNDER-1 | FOCAS advertises writes it cannot perform | **STILL OPEN**. |
-| UNDER-2 | OpcUaClient `UnsMappingTable` mandatory-but-unconsumed | **STILL OPEN**. |
-| UNDER-3 | OpcUaClient alarm filter compares mismatched NodeId encodings | **STILL OPEN**. |
-| UNDER-4 | S7 authored-array/write/UInt32 gaps | **STILL OPEN** — refs shifted: array-write guard `S7Driver.cs:~1088-1110`, wide-type arrays out of scope `:344, :357`, `UInt32 → DriverDataType.Int32` lossy map `:1315`; `S7TagDto` still has no `ArrayCount`. |
-| UNDER-5 | AbCip alarm-ack outcomes discarded | **STILL OPEN** — `AbCipAlarmProjection.cs:149-151` (fire-and-forget now sits inside an intentional-self-call pragma — the *dispatch* annotation is correct, the discarded per-item outcomes are still the finding). |
-| UNDER-6 | Equipment-tag parsers lag the authored-tag feature set | **STILL OPEN**. |
-| UNDER-7 | Live-risk assumptions documented but ungated | **STILL OPEN** — note the TwinCAT replay path is itself unit-only (no TC3 fixture), joining this list's "needs bench time" set. |
-| UNDER-8 | Test coverage codec-heavy, failure-path-light | **PARTIALLY FIXED** — S7 reconnect gap closed (`S7DriverReconnectTests` ×4 incl. fatal-vs-data-error classification + reopen-fails-then-recovers; live `S7_1500ReconnectTests` double-gated on sim + bounce cmd); TwinCAT gained `TwinCATReconnectReplayTests` ×4 (replay, multi-tag, unsub-after-reconnect, probe-recycle — fake client). Still absent: AbLegacy reconnect tests, S7/AbCip concurrency tests, connect-*timeout* (vs connect-refused) outage coverage — which is exactly where the new STAB-14 lives. |
-| UNDER-9 | OpcUaClient 2,154-line monolith; PKCS#12-only identity | **STILL OPEN**. |
+| STAB-1 | S7 no reconnect path | **REMAINS FIXED** — the R2-01 shape survives the fork retirement: `EnsureConnectedAsync` (`S7Driver.cs:1912-1954`), `MarkConnectionDeadIfFatal`, probe backstop. |
+| STAB-2 | TwinCAT native subs orphaned after reconnect | **REMAINS FIXED** — `NativeRegistration` replay intact; residuals STAB-16/17 now closed too (below). |
+| STAB-3 | Modbus transport desync after timeout / TxId mismatch | **CLOSED** (`98f48601`) — per-op timeout tears the socket down and normalizes as `ModbusTransportDesyncException.DesyncReason.Timeout` (`ModbusTcpTransport.cs:182-191`); TxId mismatch / truncated frame likewise (`:153-161, :176-181`); a well-formed exception PDU correctly does NOT tear down (`:165-172`). The new RTU-over-TCP transport ships the same discipline from day one — CRC/unit-mismatch/truncation → desync + teardown, per-op timeout → teardown (`ModbusRtuOverTcpTransport.cs:160-177`). |
+| STAB-4 | AbCip concurrent ops on shared libplctag handles | **CLOSED** (`e48609b0`+`00dcb547`) — per-runtime `SemaphoreSlim` via `DeviceState.GetRuntimeLock` taken on read (`AbCipDriver.cs:776`), RMW (`:856`), write (`:960`); the alarm projection routes through the driver's own locked `ReadAsync`/`WriteAsync` (`AbCipAlarmProjection.cs:21-24, :150, :231`), so the new bounded-parallel read window (`MaxConcurrentReads`, `:643-645`) composes with the locks instead of bypassing them. |
+| STAB-5 | FOCAS fixed-tree caches plain `Dictionary`s | **CLOSED** (`4231afb2`) — all four are `ConcurrentDictionary` (`FocasDriver.cs:1585, :1591, :1593, :1595`). But two *other* FOCAS dictionaries stayed plain — new STAB-32. |
+| STAB-6 | AbLegacy probe-shutdown race; S7 unsubscribe race | **PARTIAL** — the **S7 half is CLOSED** (fork retired; `PollGroupEngine.StopStateAsync` awaits the loop before disposing the CTS, `PollGroupEngine.cs:230-239`). The **AbLegacy half is STILL OPEN**: the probe task is still discarded at spawn (`_ = Task.Run(...)`, `AbLegacyDriver.cs:190`) and `ShutdownAsync` still cancels + disposes `ProbeCts` and the runtimes without awaiting loop exit (`:231-237`) — a winding-down loop can touch a disposed CTS and raise `OnHostStatusChanged` post-shutdown. AbCip's retained-`ProbeTask` fix still never propagated back. |
+| STAB-7 | `InitializeAsync` ignores `driverConfigJson` (Modbus, AbLegacy); TwinCAT empty-config quirk | **MOSTLY CLOSED** (#516 sweep `d32d89c3`/`5184a2e1`) — Modbus re-parses (`ModbusDriver.cs:326-327`), AbLegacy re-parses (`AbLegacyDriver.cs`, "#516" comment + `ParseOptions` in `InitializeAsync`), Sql re-parses all-or-nothing via `ApplyBinding` (`SqlDriver.cs:362-366`), FOCAS via the `_rebind` seam (`FocasDriver.cs:190-195`), OpcUaClient (`OpcUaClientDriver.cs:204-205`). **The TwinCAT empty-config quirk survives verbatim**: `if (parsed.Devices.Count > 0 \|\| parsed.RawTags.Count > 0) _options = parsed;` (`TwinCATDriver.cs:121-126`) — a contentful zero-device config is silently discarded, so a deploy that deliberately empties a TwinCAT driver keeps serving the old device set (FOCAS fixed the same problem with a shape check; TwinCAT's content check is the bug). |
+| STAB-8 | No reconnect backoff fleet-wide | **MOSTLY CLOSED** — engine-level capped-exponential failure backoff (`backoffCap: 30s`) is passed by all seven poll consumers, and per-device `ConnectionBackoff` connect throttles landed in AbCip (`AbCipDriver.cs:1147-1167`, single-flight cold connect under `ConnectGate` `:1121`), TwinCAT (`TwinCATDriver.cs:876-878, :909-913, :1147`), FOCAS (`FocasDriver.cs:1442-1473, :1548`). **The S7 leg alone is STILL OPEN**: `EnsureConnectedAsync` documents the seam and defers it (`S7Driver.cs:1896-1908` — "Deliberately NOT implemented driver-locally here") — and S7's engine backoff is unreachable anyway (new STAB-21), so a dead PLC still pays a full `_options.Timeout` connect attempt per read/write batch + probe tick. |
+| STAB-9 | `PollGroupEngine` `onError` sink dead — no driver passes it | **CLOSED** (`9213c037`/`b494d09d`) — all seven poll consumers pass `onError: HandlePollError` routing to health + logger (Modbus `ModbusDriver.cs:158+`, S7 `S7Driver.cs:67-79`, AbCip `AbCipDriver.cs:158+`, AbLegacy `AbLegacyDriver.cs:75+`, TwinCAT `TwinCATDriver.cs:77-81`, FOCAS `FocasDriver.cs:118-122`, Sql `SqlDriver.cs:227-232`), plus `backoffCap` and the `onOverrun` meter closure. The parameter remains optional in the engine (`PollGroupEngine.cs:129`) but has no bare consumer left. |
+| STAB-10 | AbLegacy never evicts failed tag handles | **CLOSED** (`a5a0f96d`) — `EvictRuntime` on non-zero status and transport exceptions on both read (`AbLegacyDriver.cs:315-316, :340-341`) and write (`:417-418, :442-443`) paths, mirroring AbCip (`:787-792`). |
+| STAB-11 | RMW locks don't cover direct parent writes; AbCip DINT / TwinCAT 4-byte assumptions | **STILL OPEN, all parts** — AbCip: bit-RMW takes `GetRmwLock(parentName)` (`AbCipDriver.cs:1054`) but a direct write to the parent tag goes through `GetRuntimeLock(def.Name)` only (`:960`) — different lock dictionaries, no mutual exclusion → lost update; RMW is still BOOL-within-**DINT** only (`:942-944, :1043-1044`). TwinCAT: RMW still reads/writes the parent as 4-byte `UDInt` (`TwinCATDriver.cs:341-359`; `AdsTwinCATClient.cs:102` admits the width assumption) — a 4-byte write-back to a 2-byte WORD parent is not harmless — and only the `Bool`+`BitIndex` branch takes `_bitRmwLocks` (`:333-368`), direct parent writes bypass it (`:370-373`). |
+| STAB-12 | Blocking/uncancellable teardown paths | **PARTIAL** — the TwinCAT probe-recycle compounding is **CLOSED** (`502a327f`): `RecycleClientAsync` waits on `ConnectGate` with the probe token (`TwinCATDriver.cs:1049-1052`). Still open: FOCAS's sync `Dispose()` close-PDU exchange on a `NetworkStream` with **no** timeout and no token (`Wire/FocasWireClient.cs:156-182`, `SendPdu`+`ReadPdu` at `:168-169`; zero `ReadTimeout`/`WriteTimeout` sets in the file) — a wedged CNC blocks Dispose indefinitely while holding `_lifetimeGate`; and TwinCAT's blocking SDK `Connect` ignoring token+timeout (`AdsTwinCATClient.cs:73-80`, self-documented as "the un-fixed root, carried" at `TwinCATDriver.cs:1050-1051`). |
+| STAB-13 | (Positive) write-outcome surfacing universal; OpcUaClient reconnect strongest | **STILL HOLDS** — OpcUaClient per-item verbatim passthrough + `BadTimeout` "outcome unknown" on post-dispatch cancellation (`OpcUaClientDriver.cs:813-839`); reconnect machine intact (`:1997-2133`). The four new drivers all return per-item `WriteResult`s with named constants visible to `StatusCodeParityTests` (Sql is structurally read-only — `SqlDriver.cs:17-20`). But see STAB-22 (S7 write-health ordering) and STAB-26 (MQTT QoS-0 optimism). |
+| STAB-14 | S7 connect-timeout OCE kills subscription polls (regression in Crit-3 fix) | **CLOSED** (R2-01 + PR #453) — connect timeout surfaces as `TimeoutException` at both connect sites (`S7Driver.cs:246, :1933-1942`); read/write ensure-wrappers rethrow OCE only on the caller's token (`:565, :1707`); the poll loop is now the shared engine whose OCE catches filter on the loop's own token (`PollGroupEngine.cs:266, :323` — the "STAB-14 engine guard" comment). The read-leg sibling: `S7OperationDeadline.RunAsync` wall-clocks **every** data-plane wire op — string/block/multi-var read, string/block write, probe (`IS7Plc.cs:142-177`) — via `WaitAsync` so control returns even when S7.Net ignores the token (`S7OperationDeadline.cs:48-58`). |
+| STAB-15 | S7 fatal classification misses framing/desync + cancel-mid-PDU; probe never marks dead | **CLOSED** — `IsS7ConnectionFatal` (`S7Driver.cs:1994-2017`) classifies `TimeoutException`, `TPKTInvalidException`/`TPDUInvalidException`/`WrongNumberOfBytesException`, `PlcException{ConnectionError, WrongNumberReceivedBytes}` fatal, walks inners, and deliberately excludes `InvalidDataException`; mid-PDU cancellation marks `_plcDead` unconditionally at all four wire paths (`:652-663, :925-931, :1044-1050, :1750-1758`); the probe classifies + marks dead **under `_gate`** (`:2167-2180`). |
+| STAB-16 | TwinCAT replay failure silent + never retried | **CLOSED** (`a06515ae`+`de297596`) — a failed intent is marked dead, publishes a Bad snapshot (`BadCommunicationError` via `OnUnavailable`, `TwinCATDriver.cs:962-970, :534-536`), degrades health, and is retried on the next successful probe tick (`RetryDeadRegistrationsAsync`, `:730, :989-1037`). Pinned by `TwinCATReconnectReplayTests`. Residual (info): the retry rides the probe loop, so `Probe.Enabled=false` silently disables STAB-16 recovery; default is enabled (`TwinCATDriverFactoryExtensions.cs:68`). |
+| STAB-17 | Unsubscribe racing replay leaks a live ADS notification | **CLOSED** (`082ee99b`) — both replay sites re-check ownership after `SwapHandle` and retract an ownerless fresh handle (`TwinCATDriver.cs:947-952, :1008-1014`); handle transfer is `Interlocked.Exchange` (`:679-697`). All interleavings traced to exactly-once disposal; pinned by `UnsubscribeDuringReplay_DisposesFreshHandle_NoOrphan`. |
+| PERF-1 | Five of seven drivers read one tag per wire round-trip | **PARTIAL** — **S7 closed** (span-grouped block reads, one PDU per contiguous block, `S7ReadSpanPlanner` + `ReadSpanAsync` `S7Driver.cs:608-619, :1037-1042`, on by default with `-1` opt-out validated at parse time; leftovers batch through multi-var PDUs chunked at 12 with a safe whole-chunk-falls-to-per-tag fallback `:932-938`); **AbCip improved** (bounded-parallel reads, `MaxConcurrentReads` default 8, `:643-645`) though still one CIP round-trip per tag; the engine gained a per-cycle batch callback consumed by Modbus/S7/AbCip/FOCAS (`IBatchSubscribable`). Still per-tag: **TwinCAT** (no ADS sum-read — sole occurrence is a doc citation, `AdsTwinCATClient.cs:116`), **FOCAS** (single-flight socket, `pmc_rdpmcrng` still fetches one value's width, `Wire/WireFocasClient.cs:302-317`), **AbLegacy** (no span coalescing). |
+| PERF-2 | AbCip whole-UDT read exists in two never-joined halves | **STILL OPEN** — the planner still relies on declaration-order offsets (`AbCipUdtReadPlanner.cs:27`), the fast path is still opt-in-and-dangerous (`AbCipDriverOptions.cs:72-79`), and the Template-Object true-offset cache still feeds discovery only. |
+| PERF-3 | S7 forked poll loop diffs arrays by reference | **CLOSED** — the fork is deleted (`85ef74ea`); the shared engine compares arrays structurally (`PollGroupEngine.cs:425-431`). |
+| PERF-4 | OpcUaClient serializes all traffic on one global gate | **STILL OPEN** — `_gate = new SemaphoreSlim(1,1)` (`OpcUaClientDriver.cs:120`) taken by every wire path (reads `:685`, writes `:773`, discovery `:907` — held across the *entire* recursive browse —, subs `:1254`, alarms `:1446-1520`, HistoryRead `:1724`, events `:1896`). |
+| PERF-5 | OpcUaClient discovery enrichment un-chunked Read + silent degrade | **STILL OPEN** — `pending.Count * 4` ReadValueIds in one call (`:1097-1109`); the wholesale catch registers everything Int32/ViewOnly with **no log line** (`:1117-1125, :1162-1172`); the ":932 the SDK chunks automatically" comment remains unsubstantiated. |
+| PERF-6 | Per-tick allocation churn (TwinCAT symbol re-parse) | **STILL OPEN** — `TwinCATSymbolPath.TryParse` per hot call (`TwinCATDriver.cs:264, :328, :520`; allocations in `TwinCATSymbolPath.cs:21-96`); FOCAS's `_parsedAddressesByTagName` fix never propagated. |
+| PERF-7 | Deadband only in Modbus | **STILL OPEN** — publish-suppression deadband remains Modbus-only (`ShouldPublish`, honoured on the batch path too — `_cycleBatchItems` buffers only what `ShouldPublish` passed, `ModbusDriver.cs:158-177`); MQTT re-implements unchanged-value suppression locally for Plain mode; the engine itself has none. |
+| CONV-1 | S7 forked the poll loop; fork diverged both ways | **CLOSED** (`85ef74ea`) — the fork is deleted; S7 is a normal `PollGroupEngine` consumer (`S7Driver.cs:67-79`); the engine absorbed the fork's backoff + failure-health (via `onError`) and kept the structural diff + drain-before-dispose it always had. (But see STAB-21: S7's absorbed backoff is unreachable.) |
+| CONV-2 | Three config-parse strictness tiers; silent enum defaulting ×6 | **CLOSED** (R2-11 + #457 + the #549/#551/#556/#559 authoring chain) — the six copy-pasted `ReadEnum`s are replaced by shared `TagConfigJson.TryReadEnumStrict` (present-but-invalid ⇒ reject ⇒ `BadNodeIdUnknown`); the four new drivers use the strict readers from day one (e.g. `SqlEquipmentTagParser.cs:49-50`); the Modbus probe parses the factory DTO shape incl. transport mode (`ModbusDriverProbe.cs:33-63`); Sql/Mqtt/MTConnect probes document DTO parity (`SqlDriverProbe.cs:12-16`). Residual: MTConnect's probe JSON options are *stricter* than its factory's (new CONV-9). |
+| CONV-3 | Four factories take no `ILoggerFactory` → NullLogger in production | **STILL OPEN — same four** — `S7DriverFactoryExtensions.cs:20`, `TwinCATDriverFactoryExtensions.cs:19`, `AbCipDriverFactoryExtensions.cs:21`, `FocasDriverFactoryExtensions.cs:37` all take `(DriverFactoryRegistry)` only; `DriverFactoryBootstrap.cs:171-188` calls them bare while the other **eight** get `loggerFactory`. Costlier every round: S7's reconnect/span logs, TwinCAT's replay-retry warnings, FOCAS's capability-gate + skipped-tag warnings all run blind in production, and FOCAS's `BuildClientFactory` uses the no-logger `WireFocasClientFactory` ctor even though a logger-taking ctor exists precisely because "the previous no-arg factory path discarded them" (`FocasDriverFactoryExtensions.cs:141`, `WireFocasClient.cs:409-414`). |
+| CONV-4 | `ResolveHost` mis-keys equipment-tag refs in every multi-device driver | **CLOSED** (`9262dd25`+`e69d3b3b`) — all resolve through `EquipmentTagRefResolver` to the parsed def's host: Modbus per-unit (`ModbusDriver.cs:226-235`, `{host}:{port}/unit{n}`; equipment tags now carry `unitId`), AbCip (`AbCipDriver.cs:604-610`), AbLegacy (`AbLegacyDriver.cs:621-626`), TwinCAT/FOCAS (same commit, resolver-backed). Per-host breaker isolation is real for equipment tags now. |
+| CONV-5 | Health-field publication idioms drift | **PARTIAL, still drifting** — conforming: Modbus (`ModbusDriver.cs:433-438`), Mqtt (`MqttDriver.cs:1486-1489`), MTConnect (`MTConnectDriver.cs:3205-3208`), FOCAS (converted — `FocasDriver.cs:157, :183` + `Volatile.Read` in `GetHealth`), AbLegacy (`volatile`, `:40-45`), and **Sql sets the new high bar** with a CAS retry loop that a lost-update race motivated (`SqlDriver.cs:836-845`, Driver.Sql-010). Still plain fields: S7 (`S7Driver.cs:181`), AbCip (`AbCipDriver.cs:113`), TwinCAT (`TwinCATDriver.cs:52`), OpcUaClient (`OpcUaClientDriver.cs:147`). |
+| CONV-6 | OpcUaClient.Contracts drags the OPC UA SDK | **CLOSED** (`b4a03b4b`) — `NamespaceMap` moved to the new `Driver.OpcUaClient.Session` leaf which carries the sole SDK ref; Contracts references only Core.Abstractions, pinned by `ContractsIsDependencyLightTests`. |
+| CONV-7 | CLI harness divergences | **STILL OPEN** — six harnesses unchanged: `ParseValue` copy-pasted ×6 (`*/Commands/WriteCommand.cs`), AbLegacy still validation-free, AbCip's `Timeout` init-setter still throws (`AbCipCommandBase.cs:44`). None of the four new drivers (nor OpcUaClient) has a CLI. |
+| CONV-8 | Dead config knobs + stale scaffold docs; no options `Validate()` | **PARTIAL** — `DisableFC23` is still a no-op but now honestly documented "Reserved / no-op" (`ModbusDriverOptions.cs:91-97`); AbCip `ConnectionSize` doc now claims "Honoured by the driver layer" (`AbCipDriverOptions.cs:120-122`) — unverified this round; stale scaffold docs remain (`AbLegacyDriver.cs:12` "ship in PRs 2 and 3", `OpcUaClientDriver.cs:13` "PR 66 ships"). Options validation improved materially where it matters: the Sql factory validates every knob incl. the inverted-deadline gate (`SqlDriverFactoryExtensions.cs:236-278`), S7 validates span options at parse time, R2-02 clamps resilience knobs. |
+| UNDER-1 | FOCAS advertises writes it cannot perform | **CLOSED (reshaped)** — discovery forces `ViewOnly` on every emitter (`FocasDriver.cs:692-702, :751, :771, :810`); the wire still refuses (`WireFocasClient.cs:71-73`) and `FIXED:` writes are refused pre-wire (`FocasDriver.cs:543-547`); deploy inspection warns on authored `writable:true` (`FocasTagDefinitionFactory.cs:250-251`). The internal write gate honours the key by design as the seam for a future writable backend — with the wire refusing, nothing misleads. |
+| UNDER-2 | OpcUaClient `UnsMappingTable` mandatory-but-unconsumed | **STILL OPEN** — validated mandatory (`OpcUaClientDriver.cs:411, :420`), consumed nowhere (repo-wide grep: option + validator + tests only). |
+| UNDER-3 | OpcUaClient alarm filter compares mismatched NodeId encodings | **STILL OPEN** — verbatim ordinal set of caller refs (`:1406`) vs session-relative `ns=N;…` rendering (`:1609-1610`); stored refs are `nsu=` (`:879`); production caller passes v3 refs (`DriverInstanceActor.cs:1146`); no `_namespaceMap` conversion on the path — any non-empty filter still drops every event. |
+| UNDER-4 | S7 authored-array/write/UInt32 gaps | **PARTIAL** — `ArrayCount` **closed** (`S7DriverOptions.cs:178-185`, factory `S7TagDefinitionFactory.ReadArrayCount :255-269`, discovery surfaces `IsArray`/`ArrayDim` `S7Driver.cs:2059-2068`, single block-read array path `:1227-1244`). Array *writes* still unsupported but now loud (init guard `:463-469`; `NotSupportedException` → `BadNotSupported` `:1805-1808`). `UInt32 → Int32` still lossy, now documented (`:2077-2080`). |
+| UNDER-5 | AbCip alarm-ack outcomes discarded | **STILL OPEN** — `_ = await _driver.WriteAsync(requests, …)` verbatim (`AbCipAlarmProjection.cs:150`); a failed ack is still invisible to operator/health/Part 9 caller. The discovery residuals also persist: with `EnableControllerBrowse` on, the multi-device `await foreach` has no per-device try/catch (`AbCipDriver.cs:1265+` — one unreachable PLC faults the whole `DiscoverAsync`). The #468/#539/#534 chain did land (controller-browse gate unblocked, authored EtherNet/IP port honoured — `BuildCreateParams` uses `ParsedAddress.Port`, `AbCipDriver.cs:1617`). |
+| UNDER-6 | Equipment-tag parsers lag the authored-tag feature set | **CLOSED** — R2-11 gave all six incumbents shared readers + the `writable` key (FOCAS forced read-only, see UNDER-1) + `Inspect()` warnings; FOCAS equipment tags run the capability-matrix pre-flight (`FocasDriver.cs:251-258`); the #549/#551 chain made authoring never accept what the runtime rejects, service-enforced. The four new drivers shipped with parser/authoring parity from the start. |
+| UNDER-7 | Live-risk assumptions documented but ungated | **PARTIAL** — FOCAS servo-load scaling and timer endianness are now **live-verified** ("Validated against a live 31i-B 2026-06-25", `WireFocasClient.cs:934-941, :917-924`). Still unverified: TwinCAT array read shape (`AdsTwinCATClient.cs:113-116`), Flat-mode `SubSymbols` (`:270-276`, gated only by `EnableControllerBrowse` defaulting false), FOCAS `cnc_getfigure` (`WireFocasClient.cs:391-395`, soft-degrades), AbLegacy array-decode assumptions, and the TwinCAT replay path (still fake-client-only — no TC3 fixture). |
+| UNDER-8 | Test coverage codec-heavy, failure-path-light | **SUBSTANTIALLY IMPROVED** — the four new drivers arrive with the best failure-path coverage in the subsystem (MQTT ~541 tests/27 files + 5 live-integration suites + a project-owned Sparkplug simulator; MTConnect ~408 tests incl. deterministic teardown/backoff/blocked-subscriber suites + a self-skipping cppagent integration; Sql ~263+54 unit + 3-dialect docker integration incl. a blackhole-timeout suite; Calculation 44). S7 gained deadline/migration/span-parity/live-outage suites. Remaining gaps: still **zero AbLegacy reconnect tests** (STAB-6 lives there), no S7/AbCip concurrency suites, no test pins S7's write-health ordering (STAB-22) or backoff engagement (STAB-21), no DTD-hardening test for MTConnect's probe parser (UNDER-11), and the Sql fixtures cover only text key columns / unbound topByTimestamp on Postgres (the STAB-24 blind spot). |
+| UNDER-9 | OpcUaClient 2,154-line monolith; PKCS#12-only identity | **STILL OPEN, one leg worse** — 2,230 lines now; `TransferSubscriptionsOnReconnect` is still never set (SDK default `false`) while the `OnReconnectComplete` doc-comment (`:2020-2022`) now affirmatively claims it defaults `true` — the reconnect path does not do what the driver believes; `BuildCertificateIdentity` still loads PKCS#12 only (`:559-560`) while its own comment promises a PEM fallback that does not exist (`:556-558`). |
---
-## Architecture Overview
+## Architecture overview — what changed since round 2
-### The common driver shape
+**PollGroupEngine v2** (`src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs`,
+484 lines) is the round's structural win: fixed-**rate** pacing with an overrun counter +
+`onOverrun` sink (`:37-56, :351-358`), capped-exponential failure backoff via
+`ConnectionBackoff` on the failure branch only (`:287-295`), the teardown-token-filtered
+OCE catches that make the S7 fork retirement safe (`:262-266, :321-323`), an optional
+per-cycle `onBatch` callback (raised once, after all per-tag callbacks, never empty,
+`:415-416`), a non-blocking `UnsubscribeAsync` that drains before disposing (`:191-239`),
+and structural array comparison (`:425-431`). Seven drivers consume it uniformly with
+`onError` + `backoffCap: 30s` + a meter-wired `onOverrun`.
-Every protocol driver is expected to follow the same template:
+**The four new drivers** are, on conventions, the best code in the subsystem — each has a
+single config-parse authority shared by factory/probe/driver, strict enum reads, a
+Contracts-side `Inspect()` wired into the deploy gate, named status constants covered by
+the reflection `StatusCodeParityTests` (`tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/StatusCodeParityTests.cs` —
+scans every `Driver.*` dll; note its own warning that inline literals are invisible to it),
+`ISkippedTagReporter` (now implemented by **all twelve** drivers, #541), and health
+surfaces designed around the #527 comms-loss watchdog's conjunction (Sql's
+`IsAnswerEvidence` classification, `SqlDriver.cs:743-749`, is the most carefully reasoned
+`LastSuccessfulRead` semantics in the fleet). The Sql injection boundary is genuinely
+airtight in design: identifiers reach SQL text only through `ISqlDialect.QuoteIdentifier`
+(control/format-char rejection incl. the Trojan-Source class, `SqlServerDialect.cs:129-155`)
+*after* the §8.1 catalog gate has rewritten them to catalog spellings
+(`SqlDriver.cs:953-996` — polls resolve only against the validated table, `:319`), and all
+values are bound parameters (`SqlGroupPlanner.cs:17-26`).
-1. **Factory extension** (`DriverFactoryExtensions.Register`) registers a
- `DriverTypeName → CreateInstance(id, configJson)` closure with the
- `DriverFactoryRegistry` (wired centrally in
- `src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs`).
- Config parses through a string-typed DTO with fail-loud `ParseEnum`
- helpers and `PropertyNameCaseInsensitive` + comment/trailing-comma
- tolerance.
-2. **Driver class** implements `IDriver, ITagDiscovery, IReadable,
- IWritable, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver,
- IDisposable, IAsyncDisposable` (Modbus,
- `Driver.Modbus/ModbusDriver.cs:21-22`), with optional capability
- interfaces (`IAlarmSource`, `IRediscoverable`, `IHistoryProvider`) per
- protocol.
-3. **Reference resolution** goes through the shared
- `EquipmentTagRefResolver`
- (`src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/EquipmentTagRefResolver.cs`),
- bridging authored tag-table names and raw equipment-tag `TagConfig` JSON
- blobs (parsed once by a per-driver `EquipmentTagParser` in the
- Contracts project, cached, cleared on teardown).
-4. **Subscriptions** overlay a polling loop via the shared `PollGroupEngine`
- (`Core.Abstractions/PollGroupEngine.cs`) for protocols without a push
- model; the driver supplies the batch reader + on-change bridge.
-5. **Health** is an immutable `DriverHealth` snapshot published via
- `Volatile.Read/Write`; failures set `Degraded` while retaining
- `LastSuccessfulRead`; `Faulted` is reserved for permanent config faults.
-6. **Writes** return per-item `WriteResult` with protocol-error → OPC UA
- StatusCode mapping (a per-driver `StatusMapper` or switch). Accurate
- per-item statuses are load-bearing: the server's write-outcome
- self-correction (`OtOpcUaNodeManager` reverting the node value on a
- failed device write) only works if the driver reports the failure.
-7. **Probe loop** — a background task issuing a cheap protocol-level
- handshake, raising `OnHostStatusChanged` on Running↔Stopped transitions
- under a probe lock; plus a stateless `DriverProbe : IDriverProbe`
- backing the AdminUI Test Connect button.
-8. **Teardown** — one shared `TeardownAsync` used by both `ShutdownAsync`
- and `DisposeAsync`: cancel loops, await them, dispose CTSs, clear
- caches, dispose the transport.
+### Per-driver conformance matrix (updated)
-Since the prior review the template gained a ninth expectation, enforced
-tree-wide by the OTOPCUA0001 analyzer: **driver capability calls are wrapped
-by `CapabilityInvoker` at the dispatch layer**, and a driver's *internal*
-self-calls (its own poll loop, its own alarm projection reading through its
-own `ReadAsync`) carry an explanatory `#pragma` marking them intentional
-(`S7Driver.cs:1460-1462`, `AbCipAlarmProjection.cs:149-151, 230-232`). This
-matters here because dispatch-level Polly *timeouts* can now cancel driver
-I/O mid-PDU — see STAB-15.
-
-Modbus adds the most advanced read planner in the fleet: block-read
-coalescing (`MaxReadGap`), auto-prohibition of failing ranges with
-bisection re-probing (`ModbusDriver.cs:687-795, 553-677`), chunking over
-per-device caps, per-register bit-RMW locks, deadband publish filtering,
-and `WriteOnChangeOnly` suppression with read-side invalidation.
-
-### Per-driver conformance matrix
-
-| Aspect | Modbus (ref) | S7 | AbCip | AbLegacy | TwinCAT | FOCAS | OpcUaClient |
-|---|---|---|---|---|---|---|---|
-| LOC (driver proj) | 2,374 | ~2,170 | 3,762 | 1,830 | ~2,460 | 5,007 | 2,347 |
-| `IPerCallHostResolver` | yes | **no** | yes | yes | yes | yes | no (single endpoint, defensible) |
-| Extra capabilities | — | — | `IAlarmSource` | — | `IRediscoverable` | `IAlarmSource` | `IAlarmSource`, `IHistoryProvider` |
-| Subscription model | PollGroupEngine | **bespoke forked loop** | PollGroupEngine | PollGroupEngine | native ADS push (default) + PollGroupEngine fallback | PollGroupEngine | **real OPC UA MonitoredItems** |
-| `onError` sink passed to engine | no | n/a | no | no | no | no | n/a |
-| Reconnect story | transport auto-reconnect + geometric backoff | **NEW: lazy reconnect via `IS7Plc` seam + probe backstop** (but see STAB-14/15) | evict + recreate handle, no backoff | **no eviction on data path** | **NEW: lazy reconnect + native-sub intent replay + probe-failure recycle** | lazy reconnect per tick, no backoff | `SessionReconnectHandler` + re-arm (best) |
-| Read batching | block coalescing + bisection | per-tag | per-tag; UDT planner **half-built** | per-tag | per-tag (no ADS sum-read) | per-address, single-flight socket | batched `ReadValueIdCollection` |
-| Deadband / WriteOnChangeOnly | yes / yes | no / no | no / no | no / no | no / no | no / no | n/a (passthrough) |
-| Bit-level writes | RMW + per-register lock | native S7 bit write | RMW + per-parent lock (DINT-only assumption) | RMW + per-parent lock, width-aware | RMW + per-parent lock (**4-byte width assumption unverified**) | n/a (read-only backend) | n/a |
-| Health via `Volatile`/`volatile` | yes | **plain field** | **plain field** | `volatile` | **plain field** | yes | **plain field** |
-| `InitializeAsync` honours configJson | **no** | yes | yes | **no** | yes (empty-config quirk) | yes | yes |
-| Factory `Register` takes `ILoggerFactory` | yes | **no** | **no** | yes | **no** | **no** | yes |
-| Options `Validate()` | no (none in fleet) | no | no | no | no | no | no |
-| Live browse / discovery | declared tags only | declared tags only | declared + opt-in controller browse + UDT fan-out | declared only (correct for PCCC) | declared + opt-in symbol browse + expander | declared + capability-gated fixed tree | full recursive remote browse |
-| CLI harness | yes | yes | yes | yes | yes (+`browse`) | yes | **none** |
+| Aspect | Modbus | S7 | AbCip | AbLegacy | TwinCAT | FOCAS | OpcUaClient | Mqtt | MTConnect | Sql | Calculation |
+|---|---|---|---|---|---|---|---|---|---|---|---|
+| Subscription model | PollGroupEngine | PollGroupEngine (**fork deleted**) | PollGroupEngine | PollGroupEngine | native ADS + engine fallback | PollGroupEngine | MonitoredItems | **broker push** | **/sample stream pump** | PollGroupEngine | dependency-mux push |
+| onError→health / backoffCap | yes/yes | yes/yes (but see STAB-21) | yes/yes | yes/yes | yes/yes | yes/yes | n/a | n/a | n/a | yes/yes | n/a |
+| Batch subscribe (`IBatchSubscribable`) | yes | yes | yes | no | no | yes | no | no | no | no | no |
+| Connect-attempt throttle | transport backoff | **no (STAB-8 residual)** | ConnectionBackoff | evict-parity only | ConnectionBackoff | ConnectionBackoff | SessionReconnectHandler | bounded reconnect loop | bounded backoff | n/a (per-poll open) | n/a |
+| Config re-parse on Initialize (#516) | yes | yes | yes | yes | **empty-config quirk** | yes | yes | yes | yes | yes (all-or-nothing) | fallback bug (STAB-28) |
+| Factory `Register` takes `ILoggerFactory` | yes | **no** | **no** | yes | **no** | **no** | yes | yes | yes | yes | yes |
+| Health idiom | Volatile | **plain** | **plain** | volatile | **plain** | Volatile | **plain** | Volatile | Volatile | **CAS loop (best)** | derived |
+| Writes | per-item + RMW | per-item (health ordering: STAB-22) | per-item + RMW | per-item + RMW | per-item + RMW | refused at wire | verbatim passthrough | QoS-0 publish (STAB-26) | HTTP PUT, double-gated | **structurally read-only** | n/a |
+| `ISkippedTagReporter` | yes | yes | yes | yes | yes | yes | yes | yes | yes | yes (two-stage union) | yes |
+| CLI harness | yes | yes | yes | yes | yes | yes | no | no | no | no | no |
---
## Findings
-Severity scale: **Critical** = live data-loss/unavailability in a realistic
-field scenario; **High** = correctness or stability defect likely to bite;
-**Medium** = architectural debt / divergence with concrete failure modes;
-**Low** = hygiene.
-
-STAB-1 and STAB-2 are fixed (see the status table); their residual defects
-are the new STAB-14..17 below.
+Severity scale unchanged: **Critical** = live data-loss/unavailability in a realistic
+field scenario; **High** = correctness or stability defect likely to bite; **Medium** =
+architectural debt / divergence with concrete failure modes; **Low** = hygiene.
+New IDs continue the existing numbering (STAB-18+, PERF-8+, CONV-9+, UNDER-10+).
### 1. Stability
-> **REMEDIATED by R2-01 (branch `r2/01-s7-fault-hardening`).** Connect-timeout now surfaces as a
-> `TimeoutException` at the `EnsureConnectedAsync`/`InitializeAsync` source (not an escaping OCE), plus
-> `when (token.IsCancellationRequested)` filters on the read/write ensure-wrappers (`:488`/`:1000`) and all
-> three poll-loop OCE catches — so a non-teardown OCE degrades/backs-off instead of killing the loop. Unit
-> guards T1–T4 (`S7DriverReconnectTests`, incl. the poll-loop-survival regression). Live gate
-> `S7_1500ConnectTimeoutOutageTests` **CLOSED 2026-07-15** — running it (blackhole via `docker pause`)
-> exposed that the fix above covered only the *connect* leg: S7.Net's socket `ReadTimeout`/`WriteTimeout`
-> are ignored by its async read/write paths, so a read on an established-but-frozen peer blocked for
-> minutes and wedged the poll loop. Fixed (PR #453, master `4dc937fc`) by a new `S7OperationDeadline`
-> wall-clock ceiling on every data-plane wire op (`S7PlcAdapter` → `TimeoutException`) + `IsS7ConnectionFatal`
-> now treating `TimeoutException` as fatal → mark-dead → reopen (the READ-leg sibling of STAB-14). Live gate
-> now GREEN; Driver.S7.Tests 246→260.
+**STAB-18 — High — NEW — MQTT has no initial-push analogue of the poll engine's
+`forceRaise`, so a ref latched Bad recovers only when the plant value changes.**
+`DriverHostActor.FanCommsLossToDependencyMux` forces `BadNoCommunication` into the mux for
+every ref of a disconnected driver and documents its recovery contract as riding "a fresh
+subscription whose PollGroupEngine initial push runs with `forceRaise: true`"
+(`DriverHostActor.cs:1310-1318`). MQTT has no poll engine: `MqttSubscriptionManager.SubscribeAsync`
+(`MqttSubscriptionManager.cs:325-365`) and `SparkplugIngestor.SubscribeAsync`
+(`SparkplugIngestor.cs:470-506`) record handles and push nothing, nothing in production
+calls `IReadable.ReadAsync`, and Plain-mode delivery then actively suppresses the one
+message that would recover the ref — `Deliver` skips `Raise` when value+status match the
+driver-owned last-value cache (`MqttSubscriptionManager.cs:762-767`), which survives
+reconnects and redeploys by design. Concrete shapes: (a) after a #527 watchdog reconnect,
+the retained-seed replay carries the identical value → suppressed → scripted-alarm /
+virtual-tag inputs latched Bad by the #481 class never recover; (b) after a full
+address-space rebuild, nodes reseed `BadWaitingForInitialData` and an unchanged republish
+never re-raises — the exact "prod-inert after redeploy → Bad forever" class the v3 Batch 4
+live gate already caught once at the host layer. Sparkplug shares shape (b) for quiet
+metrics (a rebuild triggers no rebirth). *Recommendation:* on (re)subscribe, republish the
+cached snapshots for the subscription's refs (the push-model `forceRaise`), or clear
+suppression memory per re-subscribe.
-**STAB-14 — High — NEW — S7's reconnect wrapper rethrows connect-timeout
-cancellation, and the poll loop's bare OCE catch then permanently kills the
-subscription.** `EnsureConnectedAsync` bounds the reopen with a linked
-`CancelAfter(_options.Timeout)` CTS (`S7Driver.cs:1206-1208`). When the PLC
-is unreachable-but-not-refusing — pulled cable, powered-off device on a
-switched network, firewall drop: the *classic* field outage Critical 3
-targeted — `OpenAsync` throws `TaskCanceledException`. `ReadAsync`'s
-ensure-wrapper catches `OperationCanceledException` and rethrows
-**unconditionally** (`:488`, no `when (cancellationToken.IsCancellationRequested)`
-filter; `WriteAsync` same at `:1000`), so the timeout escapes as OCE to the
-caller. In the poll loop that caller is `PollLoopAsync`, whose
-`catch (OperationCanceledException) { return; }` (`:1383, :1404`) treats
-*any* OCE as teardown and exits — silently: no log, no health change, no
-`HandlePollFailure`. Net: after a connect-timeout outage, S7 reads and
-writes self-heal (the very fix), but **every S7 subscription poll loop that
-ticked during the outage is permanently dead until redeploy** — data-loss in
-the exact scenario class Critical 3 was meant to close. The live bounce test
-(`S7_1500ReconnectTests`) cannot catch this: `docker restart` produces
-connection-*refused* (`SocketException` → the degrade-to-Bad path the unit
-tests cover), never connect-*timeout*. *Recommendation:* filter the
-wrapper's OCE rethrow on the caller's token (timeout-OCE joins the
-degrade-to-`BadCommunicationError` path), and filter the poll loop's OCE
-catches on `ct.IsCancellationRequested`; add a fake-factory unit test where
-`OpenAsync` honours a token that only the `CancelAfter` fires.
+**STAB-19 — High — NEW — Calculation launders dependency quality to Good, defeating the
+system's own comms-loss fan-out.** `CalculationDriver.OnDependencyValue` never reads its
+`statusCode` parameter (`CalculationDriver.cs:330-352`), and the feeding seam hardcodes
+`statusCode: 0u` behind a comment — "Quality is stripped at the mux boundary" — that is
+factually wrong (`DependencyConsumerMuxAdapter.cs:71-72`): `DependencyValueChanged` has
+carried `StatusCode` since #478/#519 and `DependencyMuxActor` forwards it
+(`DependencyMuxActor.cs:114, :150`). `DriverHostActor.FanCommsLossToDependencyMux`
+(`DriverHostActor.cs:1334-1359`) deliberately pushes `BadNoCommunication` + null value so
+dependents see comms loss — a calc tag receives that as an *arrived, Good, null*:
+(a) the warm-up gate is defeated (`_arrivedDeps.Add` at `:336` fires for a value that
+never arrived); (b) the common `(double)ctx.GetTag("x").Value` idiom NREs on the null →
+a spurious "script threw" storm — the exact noise class the `a836e0d9` warm-up gate was
+built to eliminate; (c) a null-tolerant script publishes a **Good** output computed from a
+dead input, and a slow-moving input that dies leaves the calc output stale-Good
+indefinitely. `CalculationEvaluator.BuildReadCache` (`:181-187`) additionally stamps every
+dep Good with eval-time timestamps. *Recommendation:* thread the real status through the
+adapter, hold the warm-up gate open for Bad arrivals, and degrade the calc output quality
+when an input is Bad.
-> **REMEDIATED by R2-01 (branch `r2/01-s7-fault-hardening`).** `IsS7ConnectionFatal` broadened to the
-> ISO-on-TCP framing surface (`TPKTInvalidException`/`TPDUInvalidException`/`WrongNumberOfBytesException` +
-> `PlcException{WrongNumberReceivedBytes}`; deliberately NOT `InvalidDataException` — that is the driver's own
-> config-mismatch decode backstop on a healthy socket). Cancellation observed mid-PDU on read + write now
-> marks `_plcDead` before propagating; the probe restructured to classify + mark dead under `_gate` so the next
-> tick reopens a lying/hung handle. Unit guards T5–T9 (`S7DriverReconnectTests`) + negative control
-> `Read_does_not_reopen_on_a_data_address_error`.
+**STAB-20 — Medium — NEW — MTConnect: a Subscribe racing a lifecycle rebuild can orphan
+the /sample pump on a disposed client, permanently.** `SubscribeAsync` deliberately runs
+outside `_lifecycle` and starts the pump under `_subscriptionLock`
+(`MTConnectDriver.cs:1043-1049`); the rebuild path runs stop-pump → `TeardownCoreAsync`
+(disposes the client) → `StartCoreAsync` (`:551-555, :599-607`), and the actor invokes
+initialize on a fire-and-forget task (the driver's own #517 comment, `:2741-2743`) — so a
+Subscribe can land between stop and teardown, bind the old client, and then
+`StartCoreAsync`'s start is a no-op because a live pump exists (`:1809`
+`if (_pumpTask is { IsCompleted: false }) return;`). Every `SampleAsync` on the disposed
+client throws ODE → classified Transient → the stray pump reconnect-loops at max backoff
+against a dead object forever; reads keep succeeding (advancing `LastSuccessfulRead`), so
+the watchdog never trips and nothing self-heals until the next lifecycle call.
+*Recommendation:* generation-stamp the pump with its `AgentSession` and let
+`StartCoreAsync` supersede a pump from an older session.
-**STAB-15 — High — NEW — S7's connection-fatal classification misses
-framing/desync errors and cancellation-during-I/O, reproducing Modbus's
-STAB-3 desync; and the probe never marks the handle dead.**
-`IsS7ConnectionFatal` (`S7Driver.cs:1239-1251`) treats only
-`SocketException`/`IOException`/`ObjectDisposedException` (walking inners)
-and `PlcException{ErrorCode.ConnectionError}` as fatal. Two gaps:
-(a) a caller-token cancellation mid-PDU — server request deadline, or a
-dispatch-layer Polly timeout now that `CapabilityInvoker` wraps driver calls
-— abandons a half-read ISO-on-TCP response on the socket; the connection is
-kept (OCE is not classified), and the next call reads the stale bytes.
-S7.Net then surfaces framing faults (`TPKTInvalidException`,
-`PlcException` `WrongNumberReceivedBytes`, `InvalidDataException`) — none of
-which are classified fatal either — so the single gated connection stays
-**permanently desynchronized**, returning `BadDeviceFailure`/
-`BadCommunicationError` forever on a socket that a reopen would fix. This is
-the exact Modbus STAB-3 failure mode rebuilt in the new S7 path.
-(b) The probe loop swallows every failure (`:1542`) without calling
-`MarkConnectionDeadIfFatal`, so its backstop role only works when
-`IsConnected` flips false on its own; a wire-dead or desynced handle whose
-`TcpClient.Connected` stays true is re-probed and re-fails forever
-(contrast TwinCAT, whose probe now force-recycles on failure —
-`TwinCATDriver.cs:631-643`). *Recommendation:* classify framing exceptions
-and cancellation-observed-during-I/O as connection-fatal (tear down before
-propagating, mirroring the STAB-3 fix recommendation), and mark-dead from
-the probe's catch.
+**STAB-21 — Medium — NEW — S7's poll-failure backoff is unreachable: the engine backs off
+only when the reader throws, and S7's reader never throws for device faults.**
+`consecutiveFailures` increments only in the engine's catch (`PollGroupEngine.cs:287-289,
+:324-329`); `S7Driver.ReadAsync` by design returns a fully-degraded batch on a failed
+(re)connect (`S7Driver.cs:566-571`) and absorbs every per-tag/span/multi-var fault into
+result stamps — its only throws are caller-cancellation OCE. With the PLC down, backoff
+never engages and each cycle immediately follows the previous one after a full
+`_options.Timeout` (default 5 s) connect attempt, per subscription, plus the probe every
+5 s, all serialized on `_gate` — this is what makes the STAB-8 S7 residual bite. The ctor
+comment (`S7Driver.cs:63-66`) claiming the migration brought backoff to S7 overstates what
+is reachable, and `SustainedPollFailure_DegradesHealth` never exercises the branch.
+*Recommendation:* wire `ConnectionBackoff.ShouldAttempt/RecordFailure/RecordSuccess` into
+`EnsureConnectedAsync` under `_gate` (the FOCAS/TwinCAT/AbCip shape) — exactly where the
+code comment already says it belongs.
-**STAB-16 — Medium — NEW — TwinCAT replay failure is silent to subscribers
-and never retried while the client stays up.**
-`ReplayNativeRegistrationsAsync` logs and skips a failed intent
-(`TwinCATDriver.cs:801-816`) — deliberately, so one bad symbol can't abort
-recovery of the rest (good) — but the failed registration keeps its dead old
-handle (`SwapHandle` runs only on success), the `OnChange` callback shape
-(`Action`) has no quality channel so nothing can push a Bad
-snapshot, and nothing schedules a retry: replay runs only when a *new client*
-is installed. If the fresh client is healthy and one `AddNotificationAsync`
-failed transiently (ADS notification-quota blip, symbol mid-redeploy), that
-tag's push feed is dead with only a warning log — the same
-silent-subscription failure STAB-2 was about, now scoped to single tags.
-*Recommendation:* on replay failure degrade driver health and emit a
-Bad-quality `OnDataChange` for the affected reference (the driver holds
-handle + reference even though the closure doesn't), and re-attempt failed
-intents on the next probe tick rather than the next reconnect.
+**STAB-22 — Medium — NEW — S7 write-path health is per-item last-writer-wins, and the next
+read pass erases a write-side Faulted.** `WriteAsync` stamps `_health` inside each item's
+catch (`S7Driver.cs:1770-1772` Faulted PUT/GET-denied, `:1782, :1791` Degraded) with no
+outcome accumulator — item 2's transport Degraded overwrites item 1's operator-actionable
+Faulted in the same batch, the exact ordering bug the read pass was refactored to kill
+(`ReadPassOutcome`, `:703-711`, worst-state-wins with rank Healthy` is undefined behaviour (corruption or throw on resize).
-The neighbouring `_parsedAddressesByTagName` already got the
-`ConcurrentDictionary` treatment (`FocasDriver.cs:40`); these did not.
-*Recommendation:* convert to `ConcurrentDictionary` or swap immutable
-snapshots.
+**STAB-26 — Medium — NEW — MQTT writes are QoS-0 fire-and-forget yet return Good.**
+`MqttWriteEncoder.WriteQos = 0` (`MqttWriteEncoder.cs:77`) for Plain and NCMD/DCMD;
+`WriteAsync` returns `Good` when `PublishAsync` completes (`MqttDriver.cs:731-733`) —
+at QoS 0 that means "bytes reached the socket", so during a half-open TCP session the
+write vanishes, the driver reports success, and the server's write-outcome
+self-correction (#5) never reverts. The optimistic posture is documented
+(`MqttDriver.cs:677-691`) and Sparkplug NCMD arguably wants QoS 0, but for Plain-mode
+`writeTopic` publishes QoS 1 would make Good mean "broker accepted" at trivial cost — and
+the QoS is a non-configurable constant.
-**STAB-6 — High — AbLegacy probe-loop shutdown race; S7 unsubscribe
-race.** AbLegacy discards the probe task at spawn (`_ = Task.Run(...)`,
-`Driver.AbLegacy/AbLegacyDriver.cs:118`) and `ShutdownAsync` cancels then
-immediately disposes the CTS and runtimes without awaiting loop exit
-(`AbLegacyDriver.cs:156-170`) — a winding-down loop can raise
-`OnHostStatusChanged` after shutdown and touch a disposed CTS. AbCip fixed
-this identical bug (retained `ProbeTask` + phased shutdown,
-`Driver.AbCip/AbCipDriver.cs:295-298, 332-365`); the fix never propagated
-back. S7 has the mirror-image bug in `UnsubscribeAsync`: cancel + dispose
-the CTS without draining `PollTask` (`Driver.S7/S7Driver.cs:1350-1354`),
-so `Task.Delay` on the disposed source can throw `ObjectDisposedException`
-that the loop only catches as OCE (`S7Driver.cs:1396-1397`) —
-`PollGroupEngine.StopState` exists precisely to prevent this
-(`PollGroupEngine.cs:99-112, 136`). (S7's `ShutdownAsync` does drain
-`PollTask` bounded — the gap is specifically per-subscription unsubscribe.)
-*Recommendation:* await the loop task (bounded) before disposing the CTS in
-both places.
+**STAB-27 — Medium — NEW — MQTT Sparkplug primary-host claim retries at 1 Hz forever with
+no backoff.** `SparkplugPrimaryHost.RoleSampleInterval` is 1 s (`SparkplugPrimaryHost.cs:67`);
+a broker that accepts CONNECT but refuses the STATE publish (an ACL without
+`spBv1.0/STATE/#` — the driver's own docs call this "the ordinary way this happens",
+`MqttDriver.cs:1357-1359`) produces a 1 Hz connect/teardown loop plus one warning per tick
+(`SparkplugPrimaryHost.cs:335-343`) — ~86k warnings/day, no escalation. The ingest
+connection's supervisor has exponential backoff; the claim loop has none.
-**STAB-7 — High — `InitializeAsync` ignores `driverConfigJson` in Modbus
-and AbLegacy.** AbLegacy's `_options` is readonly ctor state and
-`Initialize/Reinitialize` never parse the parameter
-(`Driver.AbLegacy/AbLegacyDriver.cs:16, 78-153`); Modbus likewise uses only
-ctor options (`Driver.Modbus/ModbusDriver.cs:167-197`). S7, AbCip and
-TwinCAT deliberately re-parse (`Driver.AbCip/AbCipDriver.cs:229-236`
-documents "rather than being silently discarded"). Any config change
-delivered through `ReinitializeAsync` on a live instance is silently
-discarded for these two drivers. TwinCAT has a related quirk: a parsed
-config with zero devices AND zero tags is silently discarded in favour of
-ctor options (`Driver.TwinCAT/TwinCATDriver.cs:89-94`), so an intentional
-"empty the driver" redeploy is ignored. *Recommendation:* make config
-re-parse on initialize a template requirement; add a consuming test per
-driver.
+**STAB-28 — Medium — NEW — Calculation's malformed-reinit fallback reverts to the
+construction-time tag set.** `_options` is `readonly`; `ParseOptions`'s fallback
+(`CalculationDriver.cs:236`) returns `RawTags = _options.RawTags` under the comment
+"keeping the current tag table" — but after one successful delta the ctor set is stale, so
+a second, malformed delta actively rebuilds back to the *spawn-time* tags
+(`RebuildTagTable`, `:286`), silently un-deploying every tag added since. Latent (the host
+emits well-formed merged config), but the fallback exists exactly for the case it
+mishandles.
-**STAB-8 — Medium — No reconnect backoff anywhere except the Modbus
-transport and S7's poll loop.** A dead device pays: AbCip — a full `Tag`
-create + Forward Open per tag per poll tick (evict-recreate,
-`AbCipDriver.cs:891-897`); FOCAS — a full two-socket reconnect attempt per
-tick of *each* of three loops (`Driver.FOCAS/FocasDriver.cs:1089-1120`);
-TwinCAT — a connect attempt per call (`TwinCATDriver.cs:706-721`); **S7
-(new) — a full TCP connect attempt bounded only by `_options.Timeout` on
-every read/write batch and every probe tick while the PLC is down**
-(`S7Driver.cs:1188-1224` retries unconditionally on each call; the poll
-loop's capped backoff `:1393-1397` shields subscriptions only).
-`PollGroupEngine` itself polls at a fixed cadence regardless of failures —
-S7's bespoke loop is the only subscriber-side backoff in the fleet.
-*Recommendation:* add optional failure backoff to `PollGroupEngine` (adopt
-S7's `ComputeBackoffDelay`, `S7Driver.cs:1447`) and a small connect-attempt
-throttle to the lazy-reconnect drivers (S7's `EnsureConnectedAsync` is now
-the natural home for a last-attempt timestamp).
+**STAB-29 — Medium — NEW — Calculation's `_pending` queue is unbounded with no per-tag
+coalescing.** Every dep arrival enqueues one `PendingEvaluation` (with its own snapshot
+dictionary) per change-triggered dependent (`CalculationDriver.cs:344-349`), drained by a
+single pump at worst one per `RunTimeout` (2 s default). A fast-ticking dependency feeding
+a slow script grows memory and publish latency without bound and publishes stale results
+in trigger order. No cap, no coalesce, no queue-depth diagnostic — the timer path has both
+an in-flight skip and a `SkippedTickCount` metric (`CalculationTimerScheduler.cs:62-87`);
+the change path has neither.
-> **SEAM DOCUMENTED by R2-01 (branch `r2/01-s7-fault-hardening`).** The S7 connect-throttle is left to
-> R2-09 (fleet-wide reconnect-backoff), not implemented driver-locally. `EnsureConnectedAsync`'s ``
-> now names itself as the single gate-serialized choke-point where a last-failed-attempt timestamp plugs in.
-> (Note: R2-01's STAB-14/15 fixes already *reduce* dead-PLC cost — poll ticks that used to die now back off to
-> `PollBackoffCap`, and connect-timeouts no longer escape as anomalous OCEs.)
+**STAB-30 — Low — NEW — MQTT lifecycle residuals.** (a) `ShutdownAsync`'s wedged-gate
+fallback lacks the `_disposed`-flag defence `DisposeAsync` has (`MqttDriver.cs:385-416` vs
+`:999-1003`) — a wedged initialize can complete after shutdown returns and publish a live
+connection on a driver the host believes stopped (self-heals on the next lifecycle call).
+(b) A failed `ConnectAsync` in `InitializeCoreAsync` abandons the never-published
+`MqttConnection` undisposed (`:992` vs the disposing sibling paths `:1001, :1021-1025`).
+(c) A later subscription's stronger QoS on an already-subscribed topic is silently
+deferred to the next reconnect (`MqttSubscriptionManager.cs:361, :554-572`). (d) A
+transient failure of the *initial* SUBSCRIBE degrades tags to `BadCommunicationError` with
+no retry until the connection drops (`:311-316, :581-651` — deliberate for permanent ACL
+refusals, but it also covers a broker hiccup; contrast Sparkplug, where a refused group
+filter faults the initialize into the resilience retry loop).
-**STAB-9 — Medium — `PollGroupEngine`'s `onError` sink is dead code: no
-driver passes it.** All five consumers construct the engine without the
-error callback (`ModbusDriver.cs:115`, `AbCipDriver.cs:133`,
-`AbLegacyDriver.cs:65`, `TwinCATDriver.cs:69`, `FocasDriver.cs:72`), so a
-reader exception in a poll tick (e.g. `RequireTransport` after teardown,
-reader contract violation) is silently swallowed — the precise
-"silently-broken subscription" failure mode the engine's own doc comment
-warns about (`PollGroupEngine.cs:21-25, 164-169`). *Recommendation:* pass
-`onError` routing to the driver's health surface + logger in all five;
-consider making the parameter required.
+**STAB-31 — Low — NEW — TwinCAT sync `Dispose` can dispose `ConnectGate` under a live
+probe loop.** `Dispose()` waits at most 2 s for `ProbeTask` (`TwinCATDriver.cs:1104-1107`)
+then unconditionally `DisposeGate()` (`:1111`); a probe wedged in the blocking SDK
+`Connect` (STAB-12c) then throws `ObjectDisposedException` from its next
+`ConnectGate.WaitAsync` (`:1052, :993`), which the loop shields only as OCE (`:739-740`) —
+the ODE escapes as an unobserved task exception and the loop dies silently. `ShutdownAsync`
+(unbounded await) is safe; only sync `Dispose` has the hole.
-**STAB-10 — Medium — AbLegacy never evicts failed tag handles on the data
-path.** Non-zero status and transport exceptions leave the cached runtime
-in place (`AbLegacyDriver.cs:246-259, 269-274`) with no recreate-on-failure
-(contrast `AbCipDriver.cs:891-897` and AbLegacy's own probe loop at
-`AbLegacyDriver.cs:456-461`). A handle invalidated by a PLC state change
-pins the tag Bad until full driver reinit.
+**STAB-32 — Low — NEW — FOCAS: sync-over-async `Dispose` and two plain shared
+dictionaries.** `public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult();`
+(`FocasDriver.cs:1478`) — the exact pattern TwinCAT's own comment forbids for the OPC UA
+stack thread (`TwinCATDriver.cs:1081-1087`), compounded by STAB-12b's unbounded close-PDU.
+And `_devices` / `_tagsByRawPath` are plain `Dictionary`s (`FocasDriver.cs:47, :50`) read
+concurrently by data paths, `GetHostStatuses`, and the alarm scan while
+`ShutdownAsync`/`InitializeAsync` clear/repopulate them — TwinCAT hardened the same two
+fields to `ConcurrentDictionary` with an explicit rationale (`TwinCATDriver.cs:30-37`).
-**STAB-11 — Medium — RMW locks don't cover direct parent writes (AbCip,
-AbLegacy, TwinCAT).** The per-parent semaphore serializes bit-RMW callers
-against each other, but a direct write to the parent word/DINT itself goes
-through the normal write path without taking the same lock
-(`AbCipDriver.cs:708-710` vs `787-818`; same shape in AbLegacy and
-TwinCAT), so it can interleave with an in-flight read-modify-write → lost
-update. Additionally AbCip's RMW hard-codes DINT while `AbCipTagPath`
-accepts `.N` on any parent (`Driver.AbCip/AbCipTagPath.cs:17-19`), and
-TwinCAT's RMW always accesses the parent as 4-byte `UDInt` — explicitly
-flagged as an unverified assumption that will likely fail
-`DeviceInvalidSize` on WORD/BYTE parents on real hardware
-(`Driver.TwinCAT/AdsTwinCATClient.cs:96-107`).
+**STAB-33 — Low — NEW — S7 residuals from the fresh pass.** (a) `InitializeAsync`
+overwrites `Plc` and `_probeCts`/`_probeTask` without disposing existing ones
+(`S7Driver.cs:264, :286-291`) — currently safe because every production path tears down
+first, but it violates the dispose-before-overwrite rule Modbus/MQTT now embody
+(`ModbusDriver.cs:298-319`) and is one refactor from live. (b) The probe loop reports
+`HostState.Stopped` on mere gate contention — an OCE from `_gate.WaitAsync` under the 2 s
+probe deadline falls into the bare catch (`:2185-2187`) and flaps host state while a long
+span read holds the gate. (c) `IsAccessDenied` classifies any `PlcException` whose message
+chain contains "not allowed" as PUT/GET-denied → latched Faulted (`:2028-2039`) — the
+broad arm widens the false-positive surface over the specific
+"Accessing object not allowed" pattern.
-**STAB-12 — Medium — Blocking/uncancellable teardown paths.** FOCAS's sync
-`Dispose()` performs a close-PDU exchange (`SendPdu` + `ReadPdu`) on a
-`NetworkStream` with no ReadTimeout and no token
-(`Driver.FOCAS/Wire/FocasWireClient.cs:164-175`) — a stalled CNC can wedge
-`ShutdownAsync` indefinitely. TwinCAT's `ConnectAsync` calls the blocking
-SDK `Connect` and ignores both its token and timeout parameter for the
-connect itself (`AdsTwinCATClient.cs:73-80`). **New compounding:** the
-probe loop's `RecycleClientAsync` waits on `ConnectGate` with
-`CancellationToken.None` (`TwinCATDriver.cs:835`), so a connect wedged under
-the gate now also wedges the probe loop beyond cancellation.
+**STAB-34 — Low — NEW — Calculation minor residuals.** One-stale-publish race across
+reinit for surviving tags (state-existence guard only, `CalculationDriver.cs:524`);
+`GetHealth` is Healthy whenever `_running` — a driver whose every evaluation fails stays
+Healthy with only Bad quality + script logs as evidence (defensible per CLAUDE.md, but
+there is no Degraded surface at all), and `_lastComputeUtc` (`DateTime?`, `:103`) is
+written under `_lock` but read lock-free (`:319-321`); value dedup uses
+`Equals(st.LastValue, value)` (`:551`) so array results republish every evaluation; the
+deploy-time Tarjan gate is the sole cycle defence (no runtime damping).
-**STAB-13 — Positive.** Write-outcome surfacing — the seam the server's
-value-revert self-correction depends on — is honoured by every protocol
-driver: per-item `WriteResult` with typed exception→status mapping (Modbus
-`ModbusDriver.cs:897-942`; S7 `S7Driver.cs:981-1083` including the
-PUT/GET-denied → `Faulted` escalation, preserved through the reconnect
-rework; AbCip `AbCipDriver.cs:732-775`; AbLegacy
-`AbLegacyDriver.cs:348-366`; TwinCAT `TwinCATDriver.cs:332-349` with a
-numerically-verified ADS error table; OpcUaClient passes upstream codes
-verbatim and distinguishes post-dispatch cancellation as `BadTimeout`
-"outcome unknown" — `OpcUaClientDriver.cs:753-779`, the most careful
-non-idempotency handling in the fleet). Nothing is fire-and-forget except
-the AbCip alarm *acknowledge* path (UNDER-5). OpcUaClient's reconnect state
-machine (double-arm guard, re-arm on exhaustion, session swap under
-`_probeLock` — `OpcUaClientDriver.cs:1908-2058`) remains the strongest
-reconnect implementation in the subsystem. The two new reconnect paths are
-also well-shaped where it counts: S7's dead-handle flag guarantees
-exactly-once disposal under the gate (`S7Driver.cs:1188-1204`), and
-TwinCAT's register/replay share one `ConnectGate` with
-intent-stored-only-after-handle-live ordering (`TwinCATDriver.cs:508-520`)
-— the obvious double-registration race was designed out.
+**STAB-35 — Low — NEW — PollGroupEngine records a change as seen before delivering it, so
+a throwing subscriber loses that change permanently — and only Modbus defends its
+bridge.** `state.LastValues[tagRef] = current;` precedes `_onChange(...)`
+(`PollGroupEngine.cs:404-411`); an `OnDataChange` subscriber that throws unwinds
+`PollOnceAsync` mid-loop — the throwing tag's change is marked seen but never delivered
+(no re-raise until the next value change), later tags in the cycle are skipped, and for
+batch-wired drivers the cycle's `onBatch` never fires. Modbus wraps its bridge in
+try/catch precisely for this (`ModbusDriver.cs:158-177`, with the cross-cycle-contamination
+rationale); the other six consumers invoke `OnDataChange?.Invoke` unguarded.
### 2. Performance
-**PERF-1 — High — Five of seven drivers read one tag per wire round-trip.**
-Only Modbus (block coalescing + auto-prohibition/bisection,
-`ModbusDriver.cs:687-795`) and OpcUaClient (single
-`ReadValueIdCollection` per batch, `OpcUaClientDriver.cs:640-666`) batch.
-The others are O(N) round trips per poll tick on serialized links:
-- S7: one PDU per tag under one `_gate`d connection
- (`S7Driver.cs:466-545`); S7comm multi-var reads (S7.Net
- `ReadMultipleVarsAsync`) unused.
-- TwinCAT: one ADS call per symbol; ADS **sum-read** — Beckhoff's
- documented batching mechanism — unused (`TwinCATDriver.cs:203-248`).
-- FOCAS: one request/response per address on a strictly single-flight
- socket (`SynchronizedFocasClient.cs:34`); `pmc_rdpmcrng` supports ranges
- but each read fetches exactly one value's width
- (`Wire/WireFocasClient.cs:296-334`). Throughput ceiling ≈ tags × RTT.
-- AbLegacy: one PCCC transaction per scalar; no span coalescing of
- adjacent file elements (N7:0..N7:2), the natural Modbus analog.
-*Recommendation:* per-protocol batching is the single biggest scalability
-lever; priority order = TwinCAT sum-read, S7 multi-var, FOCAS PMC range
-reads, AbLegacy spans.
+*(PERF-1/2/4/5/6/7 verdicts in the table above; the substantive change is S7's span +
+multi-var batching and AbCip's bounded-parallel reads.)*
-**PERF-2 — High — AbCip's whole-UDT batched read exists in two halves that
-were never joined.** The read planner
-(`Driver.AbCip/AbCipUdtReadPlanner.cs:32-100`) can collapse N members into
-one parent read, but only via declaration-order layout — off by default
-because it produces "silently-plausible wrong numbers" when member order
-diverges (`AbCipDriverOptions.cs:60-72`). Meanwhile the driver already
-fetches and caches *true* member offsets via the CIP Template Object
-(`AbCipDriver.cs:151-182`, `AbCipTemplateCache.cs`) — used only for
-discovery, never consulted by the planner
-(`AbCipUdtMemberLayout.cs:22-24` even points at the richer path). Net: in
-every safe configuration, N UDT members = N CIP round trips per tick.
-*Recommendation:* feed `AbCipUdtShape` offsets into the planner; retire
-the unsafe declaration-order mode.
-
-**PERF-3 — High — S7's forked poll loop diffs arrays by reference.**
-`PollOnceAsync` compares `Equals(lastSeen?.Value, current.Value)`
-(`S7Driver.cs:1469`); array reads return a fresh CLR array every poll, so
-every subscribed array tag fires `OnDataChange` **every tick**, flooding
-the dependency mux/DPS downstream. `PollGroupEngine` fixed exactly this
-with `StructuralComparisons` (`PollGroupEngine.cs:203-209`); the bespoke
-S7 copy never picked it up. (Also a Conventions finding — see CONV-1.)
-
-**PERF-4 — Medium — OpcUaClient serializes all traffic on one global
-`SemaphoreSlim(1,1)`.** Reads, writes, discovery, subscription creates,
-acks and HistoryReads all queue on `_gate`
-(`OpcUaClientDriver.cs:78, 626, 713, 836, 1186, 1377, 1451, 1653, 1825`).
-OPC UA sessions multiplex service calls natively; a multi-second
-HistoryRead or full re-discovery blocks every live read/write behind it.
-The gate is only needed for session-swap consistency, which `_probeLock` +
-re-read-inside-critical-section already provide.
-
-**PERF-5 — Medium — OpcUaClient discovery enrichment issues one giant
-un-chunked Read and silently degrades on rejection.**
-`EnrichAndRegisterVariablesAsync` sends `pending.Count * 4` ReadValueIds in
-a single call (`OpcUaClientDriver.cs:1029-1046`) — 40k operations for a
-10k-node server. A server enforcing `MaxNodesPerRead` returns
-`BadTooManyOperations`; the blanket catch (`:1049-1057`) then registers
-*every* variable as Int32/ViewOnly/non-historized with no log or health
-signal. *Recommendation:* chunk client-side; log the fallback.
-
-**PERF-6 — Low — Assorted per-tick allocation churn.** TwinCAT re-parses
-`TwinCATSymbolPath` on every read/write/subscribe call — lists +
-StringBuilder per tag per tick (`TwinCATDriver.cs:220, 284, 468`); S7
-caches parses (`_parsedByName`) and is the model. Modbus publishes a fresh
-`DriverHealth` record per successful tag read (`ModbusDriver.cs:285`).
-AbCip's `BuildArray` boxes per element
-(`LibplctagTagRuntime.cs:107-118`). None are hot enough to be urgent.
-
-**PERF-7 — Low — Deadband exists only in Modbus.** The publish-suppression
-deadband (`ModbusDriver.cs:136-159`) and `WriteOnChangeOnly` never
-generalized; noisy analog CNC/PLC signals on the other five poll-based
-drivers publish every jitter. The mechanism is driver-agnostic and belongs
-in or beside `PollGroupEngine`.
+**PERF-8 — Medium — NEW — MQTT's `MaxPayloadBytes` bounds decode work only, and the
+Sparkplug path has no publish suppression.** `MqttClientOptionsFactory.BuildClientOptions`
+(`:63-68`) never calls `WithMaximumPacketSize`, so a multi-MB retained blob is fully
+received and buffered by MQTTnet before the handler's one-comparison refusal — and the
+in-code remark still calls the cap "a constructor knob, not yet an operator-facing one"
+(`MqttSubscriptionManager.cs:184-190`) when the options key has existed for some time,
+hiding that only the protocol-layer half is missing. Separately, Sparkplug ingest bypasses
+the Plain-mode unchanged-value suppression entirely (documented at `:759-761`), so every
+rebirth re-fans full birth-value sets downstream.
### 3. Conventions
-**CONV-1 — High — S7 forked the poll loop instead of using
-`PollGroupEngine`, and the fork has diverged in both directions.**
-`PollGroupEngine`'s own doc claims it serves "Modbus, AB CIP, S7, FOCAS"
-(`PollGroupEngine.cs:8-9`) but S7 re-ships the entire loop
-(`S7Driver.cs:1373-1412`). The fork is *better* in two ways the engine
-never absorbed (capped exponential failure backoff `:1393-1397, 1447`;
-poll failures logged + degrade health `:1421-1437`) and *worse* in three
-ways the engine already fixed or avoided (reference-equality array diff —
-PERF-3; CTS disposed without draining the loop — STAB-6; **and the bare
-`catch (OperationCanceledException) { return; }` that STAB-14 turns into
-permanent subscription death**). This is the textbook cost of template
-divergence: each side owns fixes the other needs, and the fork's OCE
-handling is now load-bearing for a High-severity defect. *Recommendation:*
-extend `PollGroupEngine` with backoff + onError-driven health, then move
-S7 onto it and delete the fork.
+*(CONV-1/2/4/6 closed; CONV-3/7 open; CONV-5/8 partial — table above.)*
-**CONV-2 — High — Three different strictness tiers parse the same config,
-and the most-used one is the most permissive.** For every driver the same
-enum field is parsed three ways:
-- **Factory**: string-typed DTO + fail-loud `ParseEnum` listing valid
- values (`ModbusDriverFactoryExtensions.cs:191-201` and equivalents) —
- deliberate, good.
-- **Probe**: deserializes with `JsonStringEnumConverter`; Modbus's probe
- binds the *runtime options type* (`ModbusDriverOptions`, TimeSpan
- fields) rather than the factory DTO shape
- (`ModbusDriverProbe.cs:19-24, 36`), so a DB config with `timeoutMs`
- silently loses its timeout in the probe and tag-shape mismatches can
- make the probe reject/misread a config the factory accepts. OpcUaClient
- documents probe/factory parse-parity as the rule
- (`OpcUaClientDriverProbe.cs:22`); Modbus violates it.
-- **Equipment-tag parser**: silent fallback — an unknown or typo'd
- `dataType` string quietly becomes the default type (`Int16`/`DInt`/
- `Int`/`Int32` per driver) instead of a rejection
- (`Driver.Modbus.Contracts/ModbusEquipmentTagParser.cs:65-67`,
- `S7EquipmentTagParser.cs:59-61`, `AbCipEquipmentTagParser.cs:86-88`,
- `AbLegacyEquipmentTagParser.cs:60-62`,
- `TwinCATEquipmentTagParser.cs:47-49`,
- `FocasEquipmentTagParser.cs:43-45`). A mis-typed equipment tag reads and
- writes the wrong width with Good status. The identical private
- `ReadEnum` helper is copy-pasted six times instead of living beside
- `EquipmentTagRefResolver` in Core.Abstractions.
-*Recommendation:* hoist a shared strict `ReadEnum` (present-but-invalid ⇒
-parse failure ⇒ `BadNodeIdUnknown`); make probes parse the factory DTO.
-
-**CONV-3 — Medium — Factory registration asymmetry leaves four drivers
-logging to `NullLogger` in production.** S7, TwinCAT, AbCip and FOCAS
-`Register(registry)` take no `ILoggerFactory`
-(`S7DriverFactoryExtensions.cs:19`, `TwinCATDriverFactoryExtensions.cs:18`,
-`AbCipDriverFactoryExtensions.cs:21`, `FocasDriverFactoryExtensions.cs:36`)
-even though all four driver classes accept a logger; the bootstrap
-consequently can't pass one
-(`Host/Drivers/DriverFactoryBootstrap.cs:100-107`). This finding got
-*more* expensive since the prior review: the S7 reconnect path
-("S7Driver reconnected", replay tallies) and the TwinCAT replay path
-("re-registered {Replayed}/{Total} native notifications") log recovery and
-partial-replay failures **only** through `_logger` — in production those
-two brand-new recovery mechanisms run blind. Modbus/AbLegacy/OpcUaClient/
-Galaxy have the logger-aware overload. *Recommendation:* one-line fix per
-factory; align all eight `Register` signatures.
-
-**CONV-4 — Medium — `ResolveHost` mis-keys equipment-tag references in
-every multi-device driver.** The implementations consult only
-`_tagsByName` (authored names); an equipment-tag TagConfig-JSON reference
-never matches and falls back to the driver-level/first-device host
-(`AbCipDriver.cs:483-488`, `AbLegacyDriver.cs:496-501`,
-`TwinCATDriver.cs:689-696`, `FocasDriver.cs:1082-1087`; Modbus
-`ModbusDriver.cs:125-131` has the same shape but per-tag `UnitId` isn't in
-the equipment parser anyway). Per-host Polly bulkhead/circuit-breaker keys
-are therefore wrong for equipment tags on multi-device instances — a
-broken device B can trip device A's breaker and vice versa. Since
-equipment tags are the *primary* authoring model post-Galaxy-standardization,
-and the CapabilityInvoker dispatch wiring (task #10) now actually consumes
-these keys in production, the mis-keying is no longer latent.
-*Recommendation:* route `ResolveHost` through
-`EquipmentTagRefResolver` and derive the host from the parsed def
-(`deviceHostAddress` / `unitId`).
-
-**CONV-5 — Medium — Health-field publication conventions drift.** The
-template documents `Volatile.Read/Write` (`ModbusDriver.cs:217-228`);
-FOCAS conforms (`FocasDriver.cs:43-46`), AbLegacy uses a `volatile` field
-with rationale (`AbLegacyDriver.cs:29-34`), while S7
-(`S7Driver.cs:240`), AbCip (`AbCipDriver.cs:95`), TwinCAT and
-OpcUaClient (`OpcUaClientDriver.cs:96`) use plain fields. Immutable record
-swap means no torn reads — this is a visibility/staleness nit — but four
-different idioms for the same one-line concern is exactly the divergence
-this review is hunting. Related one-offs: TwinCAT declares `Healthy` at
-init with zero wire contact (`TwinCATDriver.cs:115`) where every other
-driver proves the connection first; TwinCAT reads host state without its
-probe lock (`~TwinCATDriver.cs:554`); AbLegacy never refreshes health
-on successful writes (`AbLegacyDriver.cs:344-346` vs
-`AbCipDriver.cs:725`); AbLegacy silently clobbers duplicate tag names
-(`AbLegacyDriver.cs:91`) where AbCip fails fast
-(`AbCipDriver.cs:249-256`).
-
-**CONV-6 — Medium — Contracts layering is uniform except OpcUaClient,
-which drags the whole OPC UA SDK into its Contracts.** Six Contracts
-projects are POCO options + equipment-tag parser (FOCAS's csproj even
-enforces "NO PackageReference. NO ProjectReference."), but
-`Driver.OpcUaClient.Contracts` carries a full OPC UA stack reference
-because `NamespaceMap` lives there
-(`OpcUaClient.Contracts.csproj:9`) — every consumer wanting just the
-options DTO (probe hosts, AdminUI editors, Browser) transitively loads the
-SDK. This is the previously-deferred OpcUaClient.Contracts-002 finding;
-it remains the right call to move `NamespaceMap` out. Minor: all Contracts
-projects use the parent namespace without a `.Contracts` suffix —
-consistent, just surprising.
-
-**CONV-7 — Low/Medium — CLI harness divergences.** The six CLIs share
-`DriverCommandBase` + `SnapshotFormatter` and a uniform
-probe/read/write/subscribe verb set, but: `ParseValue`/`ParseBool` is
-copy-pasted six times (`*/Commands/WriteCommand.cs`), option validation is
-a different shape in every base and **absent entirely in AbLegacy**
-(`AbLegacyCommandBase.cs` — `--timeout-ms 0` reaches the driver), the
-abstract `Timeout` init-setter is a silent no-op in five bases but throws
-`NotSupportedException` in AbCip (`AbCipCommandBase.cs:43`), and only
-TwinCAT exposes a `browse` command despite AbCip also supporting
-controller browse (hardcoded `EnableControllerBrowse=false`,
-`AbCipCommandBase.cs:63`). OpcUaClient has no CLI at all (the server-side
-Client.CLI tests the *server*, not this driver against a third-party
-endpoint). The six csprojs gained a commented `NoWarn OTOPCUA0001` in the
-analyzer wiring — consistent and correct for wire-level harnesses.
-*Recommendation:* hoist value parsing + a standard `Validate()`
-into Cli.Common; add an OpcUaClient CLI or document the gap.
-
-**CONV-8 — Low — Dead/no-op config knobs and stale scaffold docs.**
-`DisableFC23` is a documented no-op (`ModbusDriverOptions.cs:83-89`);
-AbCip `ConnectionSize` is plumbed but never applied
-(`AbCipDriverOptions.cs:97-103`); AbLegacy's class doc still says
-read/write "ship in PRs 2 and 3" (`AbLegacyDriver.cs:9-11`); OpcUaClient's
-header still says "PR 66 ships the scaffold: IDriver only"
-(`OpcUaClientDriver.cs:12-14`). No options class in the fleet has a
-`Validate()` method — validation is scattered across factory throws and
-init guards (S7's init guards are the best of breed,
-`S7Driver.cs:340-420`).
+**CONV-9 — Low — NEW — MTConnect's probe parses a stricter JSON dialect than its
+factory.** The probe's `JsonOptions` (`MTConnectDriverProbe.cs:96-101`) lack
+`ReadCommentHandling.Skip`/`AllowTrailingCommas` that the driver's have
+(`MTConnectDriver.cs:169-174`) — a config carrying a comment or trailing comma deploys
+fine but Test Connect reports "Config JSON is invalid": the inverse of the documented
+"reachable ≠ deployable" gap, and a violation of the probe-parity rule the fleet
+otherwise now follows.
### 4. Underdeveloped areas
-**UNDER-1 — High — FOCAS advertises writes it cannot perform.** The only
-real backend returns `BadNotWritable` for every address
-(`Wire/WireFocasClient.cs:71-73`), yet `FocasTagDefinition.Writable`
-defaults `true` with a doc-comment citing write tests
-(`FOCAS.Contracts/FocasDriverOptions.cs:152-158`) and
-`FocasEquipmentTagParser` hard-codes `Writable: true`
-(`FocasEquipmentTagParser.cs:35`) — so equipment tags can surface as
-Operate-writable OPC UA nodes whose writes always fail. The driver-side
-write status mapping (`FocasDriver.cs:353-381`) is currently dead code.
-*Recommendation:* either implement PMC writes in the wire client or force
-`Writable:false` at the parser/discovery seams until it exists.
+*(UNDER-1/6 closed; UNDER-2/3/5 open; UNDER-4/7/8/9 partial — table above.)*
-**UNDER-2 — High — OpcUaClient's `UnsMappingTable` is validated as
-mandatory but consumed nowhere.** `ValidateNamespaceKind` hard-fails
-Equipment-kind configs lacking a mapping table
-(`OpcUaClientDriver.cs:330-355`), yet no code reads the table —
-`DiscoverAsync` builds the local tree purely from remote browse structure
-(`:824-1105`). Operators are forced to author config with zero runtime
-effect; the promised remote→UNS remapping feature does not exist.
-*Recommendation:* delete the gate or build the feature.
+**UNDER-10 — Medium — NEW — S7's deploy-time `Inspect` does not cover the init-time config
+guards, so a clean-inspecting tag can fault the whole driver.**
+`S7TagDefinitionFactory.Inspect` warns only for blobs `FromTagConfig` rejects
+(`S7TagDefinitionFactory.cs:161-201`), but `FromTagConfig` accepts a `Float64`+`isArray`
+blob, a non-byte-anchored wide scalar, a mis-typed Timer/Counter, and a writable array —
+each of which then throws at driver init in `RejectUnsupportedTagConfigs`
+(`S7Driver.cs:407-469`, guards a–e), faulting the entire instance and taking every other
+tag on it to Bad — contradicting the mapper's own "one bad tag must not fail the whole
+driver init" philosophy three lines up (`:204-205`). This is the authoring-vs-deploy-gate
+drift class the repo has closed elsewhere (#535→#559), reopened for exactly the blobs with
+the worst blast radius.
-**UNDER-3 — High — OpcUaClient alarm source-node filtering compares
-mismatched NodeId encodings.** `OnEventNotification` filters
-session-relative `ns=N;…` renderings against caller-supplied refs by
-ordinal string compare (`OpcUaClientDriver.cs:1337, 1538-1539`), while the
-driver's own persisted references are stable `nsu=…` strings
-(`:808-809`). Any non-empty filter built from stored refs silently drops
-every event; only empty-filter subscriptions work today.
+**UNDER-11 — Low — NEW — MTConnect deferred/ungated edges.** DTD hardening is real in the
+streams parser (`DtdProcessing.Prohibit`, `MTConnectStreamsParser.cs:131-138`) but the
+probe parser inherits it only via `XDocument` defaults and **no test feeds an
+entity-expansion document** — a refactor onto a permissive `XmlReader` passes every test.
+A credentialed `AgentUri` is carefully redacted but non-functional (`HttpClient` never
+converts URI userinfo to an Authorization header) and there are no TLS trust knobs. No
+host-resolution seam: default `SocketsHttpHandler` with infinite
+`PooledConnectionLifetime` means a DNS-repointed agent name is re-resolved only when a
+connection drops — a healthy long-lived `/sample` stream to a stale IP persists
+indefinitely. And an authorable `AccessLevel=ReadWrite` tag on a write-disabled instance
+advertises writable and honestly fails (`BadNotWritable` + revert) with no deploy-gate
+warning. An Agent write *refusal* (401/403) and unreachability also share
+`BadCommunicationError` (`MTConnectDriver.cs:86-89, :787-789`) where
+`BadUserAccessDenied` would tell the operator over the wire.
-**UNDER-4 — Medium — S7 feature gaps make authored array tags
-unreachable.** `S7TagDto` has no `ArrayCount` field and `BuildTag` never
-sets it (`S7DriverFactoryExtensions.cs:80-90, 137-151`) — driver-config
-array tags are silently scalar; arrays only work as equipment tags
-(`S7EquipmentTagParser.cs:73-87`). Array *writes* are unsupported
-(`S7Driver.cs:~1088-1110`), wide-type arrays are a stated follow-up
-(`:344, :357`), and `UInt32` surfaces lossily as `Int32`
-(`S7Driver.cs:1315`).
-
-**UNDER-5 — Medium — AbCip alarm acknowledge outcomes are discarded.**
-`AcknowledgeAsync` fire-and-forgets the write results
-(`AbCipAlarmProjection.cs:149-151`); a failed ack (undeclared `.Acked`
-member, non-writable, comms error) is invisible to the operator, health,
-and the Part 9 caller — conditions stick un-acked with no visible cause.
-(The new OTOPCUA0001 pragma at this site correctly annotates the
-*self-call* as intentional; the discarded outcomes remain the defect.)
-Related discovery weak spots: with `EnableControllerBrowse` on, one
-unreachable PLC faults the entire multi-device `DiscoverAsync`
-(uncaught `await foreach`, `AbCipDriver.cs:970-1037`), and browsed tags
-default to writable/Operate (`CipSymbolObjectDecoder.cs:91`).
-
-**UNDER-6 — Medium — Equipment-tag parsers lag the authored-tag feature
-set.** Modbus's parser ignores `unitId`, `deadband`, `stringByteOrder`,
-`writable`, `coalesceProhibited` and the address-grammar string, and
-forces `Writable: true` (`ModbusEquipmentTagParser.cs:54-57`); AbLegacy
-and FOCAS also hard-code writable-true
-(`AbLegacyEquipmentTagParser.cs:52`, `FocasEquipmentTagParser.cs:35`) —
-read-only equipment tags are unauthorable on three drivers (AbCip honours
-a `writable` key, `AbCipEquipmentTagParser.cs:53-54`). FOCAS equipment
-tags additionally bypass the `FocasCapabilityMatrix` pre-flight gate that
-authored tags get (`FocasDriver.cs:243-247` vs `:115-119`). As equipment
-tags are the primary authoring model, the parsers deserve parity plus a
-shared conformance test.
-
-**UNDER-7 — Medium — Live-risk assumptions documented but ungated.**
-TwinCAT carries two explicit "unverified against real hardware" blocks on
-core paths — array read shape (`AdsTwinCATClient.cs:109-116`) and
-Flat-mode `SubSymbols` population (`:263-277`, struct members could
-silently vanish from discovery) — plus the bit-RMW width assumption
-(STAB-11), **and now the entire native-notification replay path, which is
-fake-client-verified only** (no TC3 docker fixture exists; STATUS.md
-acknowledges the unit tests are authoritative). AbLegacy's array decode
-rests on five explicitly unproven layout assumptions with no PCCC fixture
-(`LibplctagLegacyTagRuntime.cs:64-88`). FOCAS's `cnc_getfigure` command id
-and servo-load scaling are sim-validated only
-(`FocasWireClient.cs:391-395, 943-947`). These are honest, well-documented
-debts — but nothing aggregates them into a "needs bench time" checklist.
-
-**UNDER-8 — Medium — Test coverage is codec-heavy, failure-path-light, and
-uneven — partially improved.** The prior review's worst gap — **zero S7
-reconnect tests** — is closed: `S7DriverReconnectTests` (4 tests: fatal
-fault → reopen, data-address error → no reopen, raw socket exception →
-reopen, reopen-fails-then-recovers) plus the env-gated live
-`S7_1500ReconnectTests` (a real `docker restart` bounce, live-verified per
-STATUS.md). TwinCAT gained `TwinCATReconnectReplayTests` (4 tests: replay
-onto fresh client, all-tags replay, unsubscribe-after-reconnect disposes +
-stops replaying, probe-failure recycle) — fake-client only, per UNDER-7.
-Remaining systemic gaps: **zero reconnect tests for AbLegacy** (STAB-10
-lives there), zero concurrency tests for S7 and AbCip (STAB-4 lives
-there; only AbLegacy has a dedicated runtime-concurrency suite), **no
-connect-timeout (vs connect-refused) outage test anywhere** — precisely
-the STAB-14 blind spot — and OpcUaClient still has the widest feature
-surface on the lowest test density. `Cli.Common`'s `DriverCommandBase` has
-no direct tests. There remains **zero TODO/HACK/FIXME/
-NotImplementedException debt** in the subsystem.
-
-**UNDER-9 — Low — OpcUaClient is a 2,154-line monolith.** Six separable
-responsibilities share one file (config/PKI/identity, failover sweep,
-recursive discovery, live subscriptions, alarms/acks, the full
-IHistoryProvider, plus the reconnect state machine). The reconnect state
-machine is the trickiest concurrency in the subsystem and deserves
-extraction for isolated testability. Also: subscription transfer rests on
-the *unset* SDK default `TransferSubscriptionsOnReconnect`
-(`OpcUaClientDriver.cs:1946-1949`), and `BuildCertificateIdentity` only
-loads PKCS#12 while the options doc promises PEM support
-(`:479-484` vs `OpcUaClientDriverOptions.cs:68-75`).
+**UNDER-12 — Low — NEW — Every valid MQTT Sparkplug tag materializes writable.**
+Sparkplug `CanWrite` is "binding tuple complete" (`MqttDriver.cs:648-650`), which every
+mapped tag satisfies, so read-only sensor metrics all advertise
+`SecurityClassification.Operate`. Defensible under Sparkplug command semantics and gated
+server-side by `WriteOperate`/ACLs — but Plain mode has a per-tag opt-in (`writeTopic`
+absent ⇒ ViewOnly) and Sparkplug has no equivalent opt-out.
---
@@ -757,41 +420,40 @@ loads PKCS#12 while the options doc promises PEM support
| Dimension | Rating (1-5) | Prior | Justification |
|---|---|---|---|
-| Stability | **3** | 3 | Both Criticals are genuinely closed — a routine PLC power-cycle no longer permanently kills S7, and TwinCAT native subs now replay — and the fixes are well-shaped (seam-based, gate-disciplined, guard-tested). Held at 3 rather than promoted because the S7 fix ships a High regression in the same scenario class (STAB-14: connect-*timeout* outages permanently kill S7 subscription polls) and reproduces the Modbus desync defect (STAB-15), while the untouched silent-corruption Highs (STAB-3/4/5) and the shutdown races (STAB-6) all remain. |
-| Performance | **2** | 2 | Unchanged — no batching work landed. Only 2 of 7 drivers batch reads; AbCip's flagship UDT batching is still half-built; deadband exists only in Modbus; the subsystem scales by tag count, not request count. S7's new per-call reconnect attempt marginally worsens dead-device cost (STAB-8). |
-| Conventions | **3** | 3 | Unchanged in substance. The shared seams are real and the analyzer/pragma discipline is a genuine improvement, but the S7 poll-loop fork (now hosting a High defect in its OCE handling), three-tier config-parse strictness, six-way `ReadEnum` copy-paste, logger-registration asymmetry (now hiding the new recovery logs in production), and drifting health idioms all persist. |
-| Underdeveloped areas | **3** | 3 | Slightly improved within the band: the S7 reconnect test gap is closed with both unit and live coverage, TwinCAT replay has fake-client guards. Advertised-but-dead features (FOCAS writes, `UnsMappingTable`, `DisableFC23`, `ConnectionSize`), equipment-tag parser lag, AbLegacy/concurrency test gaps, and the growing unverified-against-hardware set (now including the replay path) keep it at 3. |
+| Stability | **3.5** | 3 | Every carried High from rounds 1–2 is now verifiably closed in code — STAB-3/4/5/14/15 (the desync/locking/classification class), STAB-16/17, STAB-9/10, most of 7/8/12 — with tests that pin the fixes, and the comms-loss-watchdog driver legs (Modbus/MQTT dispose-before-overwrite, honest `LastSuccessfulRead` semantics) landed. Held below 4 because the two new push drivers each ship one High of the classic house type (STAB-18 MQTT no-initial-push recovery gap; STAB-19 Calculation quality laundering — both "the signal exists end-to-end and is discarded at the last seam"), the AbLegacy shutdown race and STAB-11 RMW class persist untouched, and S7's backoff is dead code (STAB-21). |
+| Performance | **3** | 2 | Real batching finally landed where it was cheapest and hottest: S7 span-grouped block reads + multi-var leftovers (on by default, live-gated), AbCip bounded-parallel reads, engine-level per-cycle batch fan-out consumed by four drivers, fixed-rate pacing with an overrun meter. Still open: TwinCAT sum-read / FOCAS PMC ranges / AbLegacy spans (per-tag on serialized links), the AbCip UDT planner⇄template join, the OpcUaClient global gate + un-chunked enrichment, TwinCAT per-call symbol re-parse. |
+| Conventions | **4** | 3 | The template is now enforced rather than aspirational: one config-parse authority per driver (probe parity), strict enum parsing fleet-wide (#457), Contracts-side `Inspect()` deploy gates, reflection-guarded status constants (#497/#519), `ISkippedTagReporter` ×12, `DriverTypeNames` keying, and the four new drivers are exemplary citizens (MQTT and MTConnect arguably now define the template). Docked for the unchanged CONV-3 logger asymmetry (S7/TwinCAT/AbCip/FOCAS still run blind in production — more expensive every round), the four-way health-idiom drift, the CLI divergence, and CONV-9/UNDER-10 gate-drift residuals. |
+| Underdeveloped areas | **3.5** | 3 | UNDER-1 and UNDER-6 closed properly (forced-ViewOnly discovery + capability pre-flight + authoring parity), FOCAS's riskiest scalings live-verified, and the new drivers' deferrals are declared-and-loud rather than silent (MTConnect DATA_SET→`BadNotSupported`, Sql's reserved providers failing at construction with a naming message). Still carried: `UnsMappingTable` (advertised feature that does not exist), the OpcUaClient alarm-filter encoding mismatch, AbCip ack-outcome discard, the unverified-against-hardware TwinCAT set, AbLegacy's zero reconnect tests — and the OpcUaClient reconnect comment now actively misstates `TransferSubscriptionsOnReconnect`'s default. |
---
## Recommended remediation order
-1. **STAB-14 — S7 connect-timeout OCE kills subscription polls** — a
- two-line filter class fix (`when (cancellationToken.IsCancellationRequested)`
- on the ensure-wrapper rethrow + the poll-loop catches) plus a
- fake-factory connect-timeout unit test. This is a regression inside the
- Critical-3 fix and belongs at the top.
-2. **STAB-15 + STAB-3 together — timeout/framing ⇒ connection-fatal** in
- both the new S7 classification and the Modbus transport; same defect
- class, same fix shape (tear down before propagating). Add probe-side
- `MarkConnectionDeadIfFatal` in S7.
-3. **AbCip per-runtime operation lock** (STAB-4, port from AbLegacy) and
- **FOCAS ConcurrentDictionary caches** (STAB-5) — silent-corruption
- class.
-4. **TwinCAT replay hardening** (STAB-16 Bad-quality surfacing + probe-tick
- retry; STAB-17 ownership re-check after `SwapHandle`).
-5. **PollGroupEngine v2**: absorb S7's backoff + failure-health, wire
- `onError` in all five consumers, migrate S7 off its fork — this also
- deletes the fork's dangerous OCE handling for good
- (CONV-1, STAB-9, PERF-3, STAB-6, STAB-8).
-6. **Factory `ILoggerFactory`** (CONV-3) — one line per factory, and now a
- prerequisite for the new reconnect/replay paths being observable in
+1. **STAB-18 + STAB-19 together — the "signal discarded at the last seam" pair.** MQTT
+ needs a re-subscribe initial push (or suppression reset); Calculation needs the real
+ `statusCode` threaded through `DependencyConsumerMuxAdapter` and honoured. Both defeat
+ the host's comms-loss fan-out contract that the rest of the system relies on, and both
+ are invisible to unit tests that never drive the reconnect/rebuild path.
+2. **STAB-21 + STAB-8(S7) — wire `ConnectionBackoff` into `EnsureConnectedAsync`** (the
+ comment already names the seam) so S7 stops paying a 5 s connect per batch against a
+ dead PLC and the engine's backoff branch becomes reachable; add the missing
+ backoff-engagement test.
+3. **STAB-24 — Sql Postgres typed-parameter binding** + integer-key fixture legs; this is
+ a wrong-guidance correctness bug on a shipped dialect, currently masked by the fixture.
+4. **STAB-22 — S7 write-pass `ReadPassOutcome` sibling** (worst-state-wins accumulator on
+ `WriteAsync`, and stop the read pass erasing write-side Faulted).
+5. **STAB-20 — MTConnect pump generation stamp**; STAB-23 — FOCAS loop joins (port
+ TwinCAT's retained-task shape); STAB-6 — the same fix finally applied to AbLegacy's
+ probe loop.
+6. **CONV-3 — one line per factory** (S7/TwinCAT/AbCip/FOCAS `Register(registry,
+ loggerFactory)`): now a prerequisite for *four* recovery mechanisms being observable in
production at all.
-7. **Shared strict equipment-tag enum parsing + writable/capability
- parity** (CONV-2, UNDER-6) — one Core.Abstractions helper, six deletions.
-8. **Per-protocol read batching** (PERF-1/PERF-2), starting with the AbCip
- planner⇄template-cache join (design already exists in-tree).
-9. Sweep the one-liners: `ResolveHost` via resolver (CONV-4, now live via
- CapabilityInvoker dispatch), FOCAS writable-false (UNDER-1), delete
- `UnsMappingTable` gate (UNDER-2), OpcUaClient alarm-filter encoding
- (UNDER-3), AbLegacy reconnect-test template (UNDER-8).
+7. **STAB-25/26/27 — MQTT write honesty + claim backoff**; UNDER-10 — S7 Inspect/init-guard
+ parity (the #535→#559 arc's method applies directly).
+8. **STAB-11 — RMW lock unification** (route direct parent writes through the RMW lock;
+ width-aware TwinCAT RMW) — unchanged three rounds running; either fix it or demote it
+ to a documented limitation with a deploy-gate warning.
+9. Sweep the carried one-liners: UNDER-2 delete the `UnsMappingTable` gate, UNDER-3
+ alarm-filter encoding, UNDER-5 ack outcomes + per-device browse fault isolation,
+ PERF-5 chunk+log, UNDER-9 fix the two misleading comments (TransferSubscriptions, PEM),
+ STAB-7 TwinCAT empty-config quirk, CONV-9 probe JSON parity.
diff --git a/archreview/06-gateway-integrations.md b/archreview/06-gateway-integrations.md
index 6ba8529c..ba842cee 100644
--- a/archreview/06-gateway-integrations.md
+++ b/archreview/06-gateway-integrations.md
@@ -1,263 +1,63 @@
# Architecture Review 06 — Gateway Integrations (Galaxy Driver + Historian Gateway Driver)
-- **Date:** 2026-07-12
-- **Commit:** `f6eaa267` (master, clean tree)
-- **Updates:** the 2026-07-08 review at `9cad9ed0`. Since then the arch-review remediation branches merged to master (none targeted this domain's code), plus two in-domain deltas: the docker-dev `ServerHistorian` stale-keys fix (`7233e2ba`) and the `ZB.MOM.WW.HistorianGateway` Client + Contracts **0.1.0 → 0.2.0** bump (`f6eaa267`). Both `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy*` and `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway` (and their test projects) are **byte-identical** to `9cad9ed0` — all prior file:line citations remain exact.
+- **Date:** 2026-08-19
+- **Commit:** `12b0035a` (master, clean tree)
+- **Updates:** this is the **second update** — round 1 ran 2026-07-08 at `9cad9ed0`, round 2 (re-review) 2026-07-12 at `f6eaa267`. Since `f6eaa267`, **25 commits landed in this domain's source** (plus the Runtime-side historian recorder work), including: the R2-04/06/S-1 Galaxy write-confirmation arc (`a537d29f`, `61030673`, `084c6e3c`, `20a816db`, `205cc4a4`); the continuous-historization gate arc #491/#537/#538 (`ad744734`, `e7fbe977`, `a76b4e07`, `384568b4`, `6ff64ca0`, `73205865`, `451273c8`, `7870c91f`); Boolean→Int2 provisioning #441 (`9f62310b`) + StorageRateMs (`4da2d62b`); v3 RawPath re-keying (`636c755b`); replay-gap + sparse-array handling (`4769044f`); BadTypeMismatch for unrepresentable values (`c51c5998`); numeric alarm severity + provisioner channel dispose (`55b12dc2`); skipped-tags reporting #541 (`39db7508`, `dd900f97`); status-constant parity #497 (`b95b99ba`); `GetMemoryFootprint`/`FlushOptionalCachesAsync` deletion #525 (`b8154469`); secrets `secret:` arm (`1424a214`, `772d3a5f`).
- **Scope:**
- `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy` (+ `.Contracts`, `.Browser`)
- `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway`
- Test coverage: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests`, `.Galaxy.Browser.Tests`, `.Historian.Gateway.Tests`
- - Peripheral (read for context, not reviewed in depth): `Runtime/Historian/*` (recorder, options), `AddressSpaceApplier` provisioning/subscription hooks, `OtOpcUaNodeManager` tie-cluster paging, Host `Program.cs` historian wiring, `docker-dev/docker-compose.yml`.
+ - Peripheral (read for context): `Runtime/Historian/*` (`ContinuousHistorizationRecorder`, options, `ServerHistorianOptions`), Host `Program.cs` + `Host/Configuration/ServerHistorianOptionsValidator.cs`, `Host/Drivers/DriverFactoryBootstrap.cs`.
- **Dimensions:** Stability, Performance, Conventions, Underdeveloped Areas.
---
-## Prior-finding status (9cad9ed0 → f6eaa267)
+## Prior-finding status (f6eaa267 → 12b0035a)
-| ID | One-liner | Status @ f6eaa267 |
+Every verdict below was re-derived from current code, not from remediation claims.
+
+| ID | One-liner | Status @ 12b0035a |
|---|---|---|
-| S-1 (High) | Galaxy write success is optimistic; committed-write failure can never surface | ~~**STILL OPEN** — `GatewayGalaxyDataWriter.cs:281-302` unchanged (empty-statuses ⇒ `Good`; advise-failure path `:234-243` still proceeds). STATUS.md lists "06/S-1 (Galaxy fail-closed)" under *suggested next*, not done.~~ **Update 2026-08-07 (truth sweep): PARTIAL** — the fail-closed advise path and the `galaxy.writes.unconfirmed` meter shipped in `GatewayGalaxyDataWriter.cs` (failure-visibility meters, advise failure no longer proceeds; empty statuses still ⇒ provisional Good but metered). Residual = the gateway-side WriteComplete correlation (cross-repo, mxaccessgw). |
-| S-2 (Med) | EventPump saturation drops the newest events (inverted staleness bias) | **STILL OPEN** — `EventPump.cs:128-140` unchanged. |
-| S-3 (Med) | `ReadViaSubscribeOnceAsync` can wait forever on a non-cancellable token | **STILL OPEN** — `GalaxyDriver.cs:738-755` unchanged. |
-| S-4 (Med) | Transport-failure detection is EventPump-only; subscription-less driver never degrades | **STILL OPEN** — `ReportTransportFailure` still has the single pump-fault caller (`GalaxyDriver.cs:929-949`). |
-| S-5 (Med) | Outbox `RemoveAsync` truncates the FIFO prefix; out-of-order acks silently drop | **STILL OPEN** — `FasterLogHistorizationOutbox.cs:158-182` unchanged. |
-| S-6 (Med) | `RefreshConnectionStateAsync` has no production caller; snapshot flags dormant | **STILL OPEN** — verified by grep: only `GatewayHealthSnapshotTests` (3 call sites) invokes it; no hosted-service was added. |
-| S-7 (Low) | Alarm feed reconnect has no backoff (fixed 5 s) | **STILL OPEN** — `GatewayGalaxyAlarmFeed.cs:102-148` unchanged. |
-| S-8 (Low) | `GalaxyMxSession` state unsynchronized across reopen (accepted, emergent safety) | **STILL OPEN** — unchanged; still un-documented invariant. |
-| S-9 / S-10 (Positive) | Store-and-forward discipline; TLS/API-key posture | **Unchanged** — and S-10 strengthened for free by the 0.2.0 client (PEM-bundle trust anchors; see bump assessment). |
-| P-1 (Med) | Galaxy Read = 3 round-trips + publish wait | **STILL OPEN** — gateway-constrained, unchanged. |
-| P-2 (Med) | Alarm drain is one unary `SendEvent` per event | **STILL OPEN** — 0.2.0 ships no batched `SendEvents`; client surface for events unchanged. |
-| P-3 (Low) | Four channels to the historian sidecar (deliberate) | **Unchanged** — trade-off still argued in-source. |
-| P-4 (Positive) | Fan-in/fan-out indexed, bounded, metered | **Unchanged.** |
-| P-5 (Low) | HistoryRead buffers whole replies; `maxEvents <= 0` unbounded | **STILL OPEN** — 0.2.0 `ReadEventsAsync` signature is identical (no wire-side cap param); adapter still drops `maxEvents` (`HistorianGatewayClientAdapter.cs:76-89`). |
-| P-6 (Low) | Outbox peek holds the state lock across disk I/O | **STILL OPEN** — unchanged, still negligible at 64-entry batches. |
-| C-1 / C-2 (Positive) | Secret handling; seam quality | **Unchanged.** |
-| C-3 (Med) | Historian seam leaks wire types by design; keep contained | **STILL OPEN** — `IHistorianGatewayClient.cs` doc unchanged; no guard note added. The 0.2.0 bump is a live demonstration of the risk: contract-semantics changes now flow straight into driver behavior (see U-7). |
-| C-4 (Low) | Options-validation styles diverge | **STILL OPEN** — unchanged; and the warnings-only style's sharp edge materialized live (see S-11). |
-| C-5 (Low) | Retired Wonderware project directories still on disk | **STILL OPEN** — all five dirs (`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware{,.Client,.Client.Contracts}` + 2 test dirs) still present, still absent from the slnx. |
-| C-6 (Low) | EventPump silently drops unknown event families with no metric | **STILL OPEN** — `EventPump.cs:192-207` unchanged. |
-| U-1 (High, doc-drift) | CLAUDE.md KNOWN LIMITATION 2 stale (ref feed wired but doc said empty-set) | **FIXED** — the docs branch (`9fadead6`, merged to master via `b67bd9e8`) rewrote LIMITATION 2 to "value-capture wired; live end-to-end verification still pending", matching the code. The remaining *live verification* half was never part of the drift finding — it lives on under U-2. |
-| U-2 (Med) | Live-validation gate built but only partially run | **STILL OPEN** — no live run since; the arch-review #12 rig session ran with the historian **disabled** (that's what surfaced `7233e2ba`). The 0.2.0 bump adds fresh reason to re-run it (see U-7). |
-| U-3 (Med) | WriteSecured / VerifiedWrite user identity stubbed at zero | **STILL OPEN** — `GatewayGalaxyDataWriter.cs:263-264` unchanged. |
-| U-4 (Med) | Dormant-paths inventory (refresh, batched replay, footprint, client-side event cap/filter) | **STILL OPEN** — all four items unchanged; 0.2.0 fixes none of the gateway gaps (no wire-side `maxEvents`, no batched replay/`SendEvents`). |
-| U-5 (Low) | Outbox serializer has no version/format byte | **STILL OPEN** — `HistorizationOutboxEntrySerializer.cs:14-16` unchanged. |
-| U-6 | Test-coverage thin spots (advise-failure status, out-of-order `RemoveAsync`, S-3 hang, pump filter) | **STILL OPEN** — zero test changes in the three gateway test projects since `9cad9ed0`. |
+| S-1 (High) | Galaxy write success optimistic; committed-write failure could never surface | **CLOSED** (residual metered, documented). Three legs all verified in code: (1) **fail-closed advise** — a failed `AdviseSupervisory` short-circuits to Bad *without* issuing the raw Write (`GatewayGalaxyDataWriter.cs:174-178`, meter `galaxy.writes.advise_failed` `:48-50`), feeding the #5 node revert; (2) **correlated write statuses** — `TranslateReply` consumes the gateway's `OnWriteComplete` outcome rolled into `statuses[0]` (`:308-332`); an EMPTY statuses array is provisional Good and increments `galaxy.writes.unconfirmed` (`:330-331`); (3) **no truncation on the mapping seam** — `StatusCodeMap.FromMxStatus` routes MxStatus detail codes > 0xFF per-code instead of truncating into the quality-byte table (the 1007→239→Good defect: `StatusCodeMap.cs:120-133`), with an explicit invariant that a failure row can never surface Good (`:129-131`) and a per-code `MxStatusDetail` table (1004-1017, `:152-167`). The borrow path is **deleted by design** — the writer never borrows a subscription's plain-advised handle (live-captured rationale in the class remarks, `:21-27`). Pinned offline via the `IMxWriteOps` seam (`GatewayGalaxyDataWriterFailClosedTests`). Residual: empty-statuses ⇒ provisional Good against a pre-correlation gateway — metered, cross-repo (mxaccessgw ≥ `53f69cd`). But see **NEW S-12**: the advise *cache* has an exception-path poisoning bug. |
+| S-2 (Med) | EventPump saturation drops the *newest* events | **STILL OPEN** — `TryWrite` drop-newest unchanged (`EventPump.cs:164-170`). Metered; bias still inverted for last-value-wins telemetry. |
+| S-3 (Med) | `ReadViaSubscribeOnceAsync` can wait forever on a non-cancellable token | **STILL OPEN** — Bad-fill still only via `cancellationToken.Register` (`GalaxyDriver.cs:899-909`); the awaits at `:914` hang on `CancellationToken.None` + a never-publishing subscribed tag. No internal deadline added; no test covers the case (`GalaxyDriverReadTests` always emits or cancels). |
+| S-4 (Med) | Transport-failure detection is EventPump-only | **OPEN, NARROWED** — `ReportTransportFailure` still has exactly one caller (`GalaxyDriver.cs:1196`, the pump fault callback). Improvement: the pump now treats a **server-completed** stream as a fault too (`EventPump.cs:180-188` — closes the "clean termination evades watchdogs" shape for the streaming path). Residual: a subscription-less driver (writes only, or idle) never starts the pump, so a gateway restart still leaves `GetHealth()==Healthy` with per-write `BadCommunicationError` and no reopen — and the #527 comms-loss watchdog needs a non-Healthy state to trip, so it doesn't cover this either. |
+| S-5 (Med) | Outbox `RemoveAsync` truncates the FIFO prefix; out-of-order acks silently drop | **OPEN → downgraded Low / accepted-by-design.** The truncate-through semantics are unchanged (`FasterLogHistorizationOutbox.cs:203-227`) but are now (a) **pinned by test** (`FasterLogHistorizationOutboxTests.cs:99` — "acks A and B, leaves C"), (b) **explicitly relied on** by the sole caller (`ContinuousHistorizationRecorder.cs:677-680` documents why the ack stays all-or-nothing), and (c) documented in CLAUDE.md (#538 narrative). The prefix drop still doesn't increment `DroppedCount` — residual observability nit only. |
+| S-6 (Med) | Historian health snapshot's connection flags are dormant | **STILL OPEN, and SHARPENED** — `RefreshConnectionStateAsync` (`GatewayHistorianDataSource.cs:219-244`) still has only test callers (`GatewayHealthSnapshotTests.cs:33,59,74`). Worse: grep shows **`GetHealthSnapshot()` itself has no production consumer anywhere** — the whole `HistorianHealthSnapshot` surface (counters + flags) is currently a write-only ledger. Wire a reader (the R2-10 resilience-chip pattern is the template) or delete the flags. |
+| S-7 (Low) | Alarm feed reconnect has no backoff | **STILL OPEN** — fixed 5 s (`GatewayGalaxyAlarmFeed.cs:46,141`); `DeployWatcher` and `ReconnectSupervisor` still do capped exponential. |
+| S-8 (Low) | `GalaxyMxSession` state unsynchronized across reopen | **STILL OPEN / accepted** — plain fields (`GalaxyMxSession.cs:29-31`); safety remains emergent (single-flight supervisor + per-call catch). The `AttachForTest` path at least documents the `_connected` invariant now (`:129-131`). |
+| S-9 / S-10 (Positive) | S&F/outbox crash-safety; TLS/API-key posture | **Stand, strengthened** — batch append is incremental-indexed so a mid-loop fault leaves no untracked prefix (`FasterLogHistorizationOutbox.cs:119-137`); provisioner now disposes its channel (`GatewayTagProvisioner.cs:108-115`); `ApiKey` supports `${secret:}` (`ServerHistorianOptions.cs:43-46`). |
+| S-11 (Med) | Misconfigured `ServerHistorian` crash-loops Host with raw `UriFormatException` | **CLOSED** — verified all three layers: fail-fast `ServerHistorianOptionsValidator` wired via `AddValidatedOptions` (`Host/Program.cs:188-196`), consumer-gated so the alarm-only corner is covered, with the alarm-only startup warning (`Program.cs:205-212`); adapter defense-in-depth `Uri.TryCreate` → named `InvalidOperationException` (`HistorianGatewayClientAdapter.cs:53-58`); warning parity for malformed endpoints in `Validate()` (`ServerHistorianOptions.cs:117-123`). |
+| P-1 (Med) | Galaxy Read = 3 round-trips + publish wait | **STILL OPEN** — gateway-constrained, unchanged (`GalaxyDriver.cs:805-941`). Accepted; "subscribe, don't poll" remains the pattern. |
+| P-2 (Med) | Alarm drain is one unary `SendEvent` per event | **STILL OPEN** — serial per-event loop unchanged (`GatewayAlarmHistorianWriter.cs:66-78`); still no batched `SendEvents` on the client surface. Deliberate poison isolation; accepted ceiling. |
+| P-3 (Low) | Four channels to the historian sidecar (deliberate) | **Unchanged** — and the dispose-ownership rationale is now stronger (each path, provisioner included, owns and disposes its channel). |
+| P-4 (Positive) | Fan-in/fan-out indexed, bounded, metered | **Stands** — plus new counters (`galaxy.events.replay_gaps`, `galaxy.events.undecodable`, `EventPump.cs:50-55`). |
+| P-5 (Low) | HistoryRead buffers whole replies; `maxEvents <= 0` unbounded | **OPEN, NARROWED** — events reads now stop at the cap client-side with a truncation continuation token (`GatewayHistorianDataSource.cs:142-175`); only `maxEvents <= 0` still collects unbounded (`:142-143`), and the adapter still cannot forward the cap on the wire (`HistorianGatewayClientAdapter.cs:100-113` — client signature unchanged). The 03/S3 knobs (`HistoryReadBatchParallelism`/`MaxConcurrentHistoryReadBatches`/`HistoryReadDeadline`, `ServerHistorianOptions.cs:80-97`) bound the server-side fan-out/flood — a real mitigation for the same blast radius. |
+| P-6 (Low) | Outbox peek holds the state lock across disk I/O | **STILL OPEN** — `PeekBatchAsync` scans under `_state` (`FasterLogHistorizationOutbox.cs:190-197`); note the exposure grew 16× with `DrainBatchSize` 64→1024 (`ContinuousHistorizationOptions.cs:78`). Still an in-memory-page scan; still acceptable; revisit if drain batches grow again. |
+| C-1 / C-2 (Positive) | Secret handling; seam quality | **Stand, strengthened** — `GalaxySecretRef` gained the fail-closed `secret:` arm through the shared `ISecretResolver` (`Galaxy.Contracts/GalaxySecretRef.cs:19-21,93-98`); production wires the real resolver (`DriverFactoryBootstrap.cs:175`) while the `NullSecretResolver` default is confined to the test/standalone convenience overload (`GalaxyDriverFactoryExtensions.cs:45-54` — the ForTest-seam trap checked and clear). New `IMxWriteOps` internal seam (`GatewayGalaxyDataWriter.cs:342-405`) made the sealed-SDK write pipeline unit-testable without widening the public contract. |
+| C-3 (Med) | Historian seam leaks wire types by design | **STILL OPEN (Low now)** — `IHistorianGatewayClient.cs:5-11` unchanged, no guard note added. Mitigation exists in practice: the U-7 presence-pinning tests + the 6/6 live gate are the bump-gate the finding asked for. Keep any future consumer above `GatewayHistorianDataSource`. |
+| C-4 (Low) | Options-validation styles diverge; warnings-only style crashes | **CLOSED** — the asked-for fail-fast tier landed (S-11's `ServerHistorianOptionsValidator` + `Validate()` parity). Two idioms still coexist (Galaxy throw-on-construct vs. historian warnings+Host validator) but the sharp edge — correct warning followed by opaque crash — is gone. |
+| C-5 (Low) | Retired Wonderware project dirs on disk | **CLOSED** — glob `src/Drivers/*Wonderware*` matches nothing (R2-12 established they were untracked husks; they are gone from this checkout). |
+| C-6 (Low) | EventPump silently drops unknown event families with no metric | **STILL OPEN (narrowed)** — the default family arm still returns uncounted (`EventPump.cs:297-312`); however undecodable values and replay gaps now have their own counters, and filtered volume is derivable as `received − dispatched − dropped − gaps`. A `galaxy.events.filtered` counter remains the clean fix. |
+| C-7 (Low) | Live-value writer populates a documented-dead `Quality` field | **CLOSED** — keep-populating decision verified with the full rationale in the class remarks (`GatewayHistorianValueWriter.cs:27-37`: proto3 unset transmits 0 = Bad, so populating the correct 192 is forward-safe) + inline note at the mapping (`:74-76`) citing 0.2.0 Contracts C-002, + the explicit "do NOT extend the recorder to capture real node quality" warning. |
+| U-1 (High, doc-drift) | CLAUDE.md KNOWN LIMITATION 2 stale | **CLOSED** (round 2) — and now doubly moot: the live gate ran and #491 is closed; CLAUDE.md reflects it. |
+| U-2 (Med) | Live-validation gate built but not fully run | **CLOSED** — two full documented live runs since: (1) R2-06 T12, 2026-07-13, against a real 0.2.0 gateway + real AVEVA Historian over VPN — **6/6** after finding + fixing the StorageRateMs provisioning bug (`4da2d62b`) and driving the Boolean→Int2 remap (STATUS.md §"R2-06 live gate"); (2) the continuous-historization gate 2026-08-01→04 (#491) — outbox/drain/WriteLiveValues/EnsureTags/read-back all green with distinct non-zero constants (`docs/plans/2026-08-01-continuous-historization-live-gate.md`). The retype-policy question is **answered** (upsert; supported-type retype accepted). |
+| U-3 (Med) | WriteSecured / VerifiedWrite user identity stubbed at zero | **STILL OPEN** — `CurrentUserId = 0, VerifierUserId = 0` (`GatewayGalaxyDataWriter.cs:284-285`). Unchanged since round 1: writes to secured/verified-classification tags remain effectively unsupported, with no session-identity→ArchestrA-user mapping and no fail-fast. The correlated statuses (S-1) at least make a rejection *visible* now (Bad instead of phantom Good). |
+| U-4 (Med) | Dormant-paths inventory | **PARTIALLY CLOSED** — `GetMemoryFootprint`/`FlushOptionalCachesAsync` **deleted** fleet-wide (#525, `b8154469` — grep-zero in `GalaxyDriver.cs`). Still open: per-subscription replay vs. the gateway's batched `ReplaySubscriptionsCommand` (note survives at `GalaxyDriver.cs:446-447`); `maxEvents` never forwarded on the wire + defensive client-side source filter (`GatewayHistorianDataSource.cs:152-157`) — both still gateway gaps to track against the sidecar repo; `RefreshConnectionStateAsync` (S-6); `IHostConnectivityProbe.GetHostStatuses` still unconsumed (Gitea #521, `GalaxyDriver.cs:724`). |
+| U-5 (Low) | Outbox serializer has no version/format byte | **STILL OPEN** — fixed positional layout, no version prefix (`HistorizationOutboxEntrySerializer.cs:14-16,23-40`). The window to add one byte cheaply is closing as deployments accumulate on-disk logs. |
+| U-6 | Test-coverage thin spots | **LARGELY CLOSED** — +2,304 test lines across 30 files this period. The advise-failure status path is now covered offline (`GatewayGalaxyDataWriterFailClosedTests` via `IMxWriteOps`); outbox prefix-removal semantics pinned (`FasterLogHistorizationOutboxTests.cs:55-99,288`); pump fault/replay-gap/bounded-channel suites exist; recorder gates all pinned (capture-gate re-read, quarantine falsification control, batch-append prefix accounting). **Remaining thin spots:** the S-3 read-hang case; a C-6 unknown-family filter assertion. |
+| U-7 (Med) | 0.2.0 `DataType` presence change silently altered Boolean provisioning | **CLOSED** — and resolved *differently than the round-2 remediation pinned*: the live gate exposed Int1 as server-degenerate, so `Boolean → Int2` (`HistorianTypeMapper.cs:31`, issue #441), retyping cleanly from pre-existing Float tags. `StorageRateMs` is stamped (`GatewayTagProvisioner.cs:72` — its absence had silently failed **every** provision, found live). Live gate 6/6; retype policy answered (accepted-retyped). |
-**New findings this pass:** S-11 (Medium), U-7 (Medium), C-7 (Low) — see body. **All three REMEDIATED on `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13); the U-7 live EnsureTags/retype leg is operator-gated pending a VPN run.**
+**New findings this pass:** S-12 (Medium), S-13 (Low), C-8 (Low) — see body.
---
-## What changed since 2026-07-08 (in-domain delta assessment)
+## What changed since 2026-07-12 (delta assessment)
-### The HistorianGateway 0.1.0 → 0.2.0 bump (`f6eaa267`)
-
-One-line change to `Directory.Packages.props`; the consumed contract/client surface changed in five ways (verified by diffing the cached 0.1.0 vs 0.2.0 package XML docs + member lists):
-
-1. **`HistorianTagDefinition.DataType` is now proto3-optional** (`HasDataType`/`ClearDataType` added). Old semantics: wire-0 (`INT1`) was indistinguishable from unset ⇒ the gateway mapped it to the SDK default **Float**. New semantics: presence is explicit, so a set `DataType` — *including* wire-0 `Int1` — is honored. **This directly changes `GatewayTagProvisioner` behavior for Boolean tags** (it maps `Boolean → HistorianDataType.Int1`, `HistorianTypeMapper.cs:28`, and always sets the field, `GatewayTagProvisioner.cs:57`): under 0.1.0 a Boolean tag was silently provisioned as Float; under 0.2.0 it provisions as Int1 (correct). The bump commit's impact analysis covered only the `opc_quality` change — this one was not assessed (see **U-7**).
-2. **`HistorianHistoricalValue.OpcQuality` is now proto3-optional.** The commit's gate check is correct and verified: OtOpcUa never constructs `HistorianHistoricalValue` (no `AddHistoricalValues` caller anywhere in src/tests); all `OpcQuality` uses are the read-side `HistorianSample`/`HistorianAggregateSample`, unaffected.
-3. **`HistorianLiveValue.Quality`/`QualityDetail` documented as dead inputs on `WriteLiveValues`** — the gateway's SQL live path silently discards them (server-managed quality; the SQL login is column-permission-denied). `GatewayHistorianValueWriter.cs:63` populates `Quality` anyway (see **C-7**).
-4. **Client `ToUtcTimestamp` behavior fix:** a `DateTimeKind.Unspecified` timestamp is now interpreted as already-UTC instead of being shifted by the host's UTC offset. Absorbed improvement — OtOpcUa's own conversion sites already `SpecifyKind(..., Utc)` defensively (`GatewayHistorianDataSource.cs:269`, `AlarmEventMapper.cs:20`, `GatewayHistorianValueWriter.cs:70`), and UA-stack DateTimes are Utc-kinded, so no behavior change is expected here; the client-side times passed to `ReadRaw`/`ReadAggregate` are now safe even if an Unspecified kind ever leaks through.
-5. **Client hardening/docs:** `LoadTrustAnchors` now honors a full PEM bundle (root + intermediates) for `CaCertificatePath` pinning (strengthens S-10); the `RawRead`/`RawWrite`/`RawAdmin` escape hatches are documented as bypassing bearer-header injection, typed-exception mapping, and the retry pipeline (the adapter uses only typed methods — unaffected); per-RPC scope requirements (`historian:read` / `historian:tags:write`) are now documented, matching CLAUDE.md's prerequisites; streaming reads documented as *not* retried (transient failures surface — consistent with how `GatewayHistorianDataSource` maps them to health failures); a new `IHistorianGatewayClientTransport` seam exists client-side (unused by OtOpcUa; `IHistorianGatewayClient` remains the repo's seam).
-
-Wire compatibility is preserved (field numbers unchanged; the proto3-optional changes are wire-compatible). No compile-surface break: the solution builds and the gateway-driver suite passed per the bump commit. The residual risk is behavioral, not structural — captured as U-7.
-
-### The docker-dev `ServerHistorian` stale-keys fix (`7233e2ba`)
-
-Config-only: docker-dev compose had `ServerHistorian__Enabled=true` with the retired Wonderware `Host`/`Port`/`SharedSecret` keys, so post-cutover code read an empty `Endpoint`, and `HistorianGatewayClientAdapter.Create` threw an unhandled `UriFormatException` at every bring-up — docker-dev was unbootable against current code. The fix rewrites both centrals to the gateway-shape keys, **disabled by default**, enable via `OTOPCUA_HISTORIAN_ENABLED` + `OTOPCUA_HISTORIAN_ENDPOINT` + env `ServerHistorian__ApiKey`; central-1 verified booting clean. Correct fix for the rig — but it fixed only the config, not the code path that turns a misconfigured section into a crash-loop with a raw `UriFormatException`. That code gap is new finding **S-11**.
-
----
-
-## Architecture Overview
-
-### Galaxy driver data flow (mxaccessgw)
-
-`GalaxyDriver` (`GalaxyDriver.cs`) is a standard Tier-A in-process Equipment-kind driver registered under type name `GalaxyMxGateway`. All Galaxy access flows over gRPC to the external **mxaccessgw** gateway (sibling repo), consumed via the Gitea-feed packages `ZB.MOM.WW.MxGateway.Client` / `.Contracts` (`MxCommand` / `MxEvent` protos — unchanged this period). The driver holds **three logical gRPC clients**:
-
-1. **Worker session** — `GalaxyMxSession` wraps `MxGatewayClient.OpenSessionAsync` + MXAccess `Register`. All data-plane traffic rides it:
- - **Subscribe** — `GatewayGalaxySubscriber.SubscribeBulkAsync` (session-level `SetBufferedUpdateInterval` applied first, cached last-applied value), events consumed by the shared `EventPump` off the bidirectional `StreamEvents` RPC, fanned out through `SubscriptionRegistry`'s handle→subscription reverse map onto `OnDataChange`.
- - **Read** — MxAccess has no one-shot read; `ReadViaSubscribeOnceAsync` synthesises Read as SubscribeBulk → first-event wait per handle → UnsubscribeBulk.
- - **Write** — `GatewayGalaxyDataWriter`: lazy `AddItem` (or borrow a live handle from the subscription registry), `AdviseSupervisory` once per handle (required for commit under WriteUserId=0), then raw `Write` or `WriteSecured` routed on the discovery-captured per-tag `SecurityClassification`.
-2. **Session-less client** (`_ownedMxClient`) — the always-on central alarm monitor: `GatewayGalaxyAlarmFeed` (StreamAlarms: active-alarm snapshot → `snapshot_complete` → live transitions, self-reconnecting) and `GatewayGalaxyAlarmAcknowledger` (unary `AcknowledgeAlarm`).
-3. **Repository client** (`GalaxyRepositoryClient`) — hierarchy browse for `DiscoverAsync` (via `IGalaxyHierarchySource` → `GalaxyDiscoverer`) and the `DeployWatcher` deploy-event stream that raises `IRediscoverable.OnRediscoveryNeeded`.
-
-Recovery is owned by `ReconnectSupervisor` (Healthy → TransportLost → Reopening → Replaying → Healthy, capped exponential backoff, never gives up): the `EventPump` stream fault feeds `ReportTransportFailure`; reopen routes through `GalaxyMxSession.RecreateAsync` (dispose stale session + client, rebuild) and invalidates the writer's handle/advise caches; replay recreates the EventPump then re-issues SubscribeBulk per tracked subscription and `Rebind`s the registry with the fresh handles. Host connectivity is surfaced through `HostStatusAggregator` (transport state from the supervisor + per-platform `ScanState` probes via `PerPlatformProbeWatcher`).
-
-Since `9cad9ed0`, Galaxy capability dispatch from `DriverInstanceActor` is additionally wrapped by the arch-review #10 `IDriverCapabilityInvoker` seam (tier-A resilience pipeline) — a Runtime-side change outside this domain's code, noted here because Galaxy calls now flow through it.
-
-### Historian gateway data flow (HistorianGateway sidecar)
-
-`Driver.Historian.Gateway` is the **sole historian backend**, consuming the `ZB.MOM.WW.HistorianGateway.Client` package **0.2.0** (`historian_gateway.v1`) behind the proto-typed `IHistorianGatewayClient` seam (`HistorianGatewayClientAdapter` is a pure pass-through over the typed client methods; channels are lazy — no I/O at construction, except the eager `new Uri(Endpoint)` at `HistorianGatewayClientAdapter.cs:45`, see S-11). Four independent paths, each with **its own gRPC channel** to the same sidecar (documented deliberate trade-off, `GatewayHistorianServiceCollectionExtensions.cs:57-64`):
-
-1. **Read** — `GatewayHistorianDataSource` (`IHistorianDataSource`): OPC UA HistoryRead Raw/Processed/AtTime/Events → `ReadRaw` / `ReadAggregate` / `ReadAtTime` / `ReadEvents`; at-time replies re-aligned one-snapshot-per-requested-timestamp; health counters under a single lock. Tie-cluster paging (`MaxTieClusterOverfetch`) lives server-side in `OtOpcUaNodeManager` (~line 2200), bound from `ServerHistorianOptions`.
-2. **Alarm history write** — `GatewayAlarmHistorianWriter` (`SendEvent` per event) behind the durable `SqliteStoreAndForwardSink`; maps every outcome (ack, typed client exceptions, raw `RpcException`) to exactly one `Ack` / `RetryPlease` / `PermanentFail` per event and never throws.
-3. **Continuous historization** — `ContinuousHistorizationRecorder` (Runtime actor) taps the dependency-mux value fan-out, appends to the crash-safe `FasterLogHistorizationOutbox` (PerEntry fsync or Periodic commit; bounded drop-oldest capacity; startup recovery scan), drains through `GatewayHistorianValueWriter` (`WriteLiveValues`, non-throwing bool).
-4. **Tag provisioning** — `GatewayTagProvisioner` (`EnsureTags`), dispatched fire-and-forget from `AddressSpaceApplier.Apply` with a tally log; non-historizable data types skipped; can never block or fail a deploy.
-
----
-
-## Findings
-
-### 1. Stability
-
-#### S-1 (High) — Galaxy write success is optimistic; a committed-write failure can never surface
-
-`GatewayGalaxyDataWriter.TranslateReply` (`GatewayGalaxyDataWriter.cs:281-302`) honours the protocol status and the first MXAccess status row, **defaulting to `Good` when the statuses array is empty** — and the gateway's write execution is effectively fire-and-forget past dispatch (the reply reflects command acceptance, not the eventual COM-side commit). Two concrete consequences:
-
-- The supervisory-advise failure path (`GatewayGalaxyDataWriter.cs:234-243`) logs a warning, forgets the handle, and **lets the write proceed anyway** — but the file's own comment (`:163-165`) states a raw Write without supervisory advise "doesn't throw (reply looks OK) but the value never reaches the galaxy". That is a silent write loss returning `Good` to the OPC UA client.
-- The server's write-outcome self-correction (#5, reverts the node on a failed device write) can structurally never trigger for Galaxy, so a lost write leaves a phantom-Good node value indefinitely.
-
-**Blast radius:** operator writes (WriteOperate-gated) to Galaxy attributes that silently don't commit, with no Bad status, no health degradation, no metric. Not scheduled in the remediation pass; STATUS.md carries it under "suggested next" as *06/S-1 (Galaxy fail-closed)*. Recommendation: (a) return `Uncertain` (not proceed-to-Good) when supervisory advise fails; (b) pursue a gateway-side `WriteComplete` correlation (the gw backlog's `OnWriteComplete` event family already exists in the proto — `EventPump.cs:200-206` filters it out) so the reply/statuses row carries the real commit outcome; (c) add a `galaxy.writes.unconfirmed` counter in the interim.
-
-#### S-2 (Medium) — EventPump saturation drops the *newest* events (inverted staleness bias)
-
-`EventPump.RunAsync` (`EventPump.cs:128-140`) uses `TryWrite` against a bounded channel: when the fan-out consumer stalls, the **just-arrived** event is dropped and 50 000 stale queued events are preserved. For last-value-wins OPC UA telemetry this is the wrong bias — after a stall clears, subscribers replay old values and the freshest one may be the dropped one. The drop is at least metered (`galaxy.events.dropped`). Recommendation: per-item-handle conflation (keep newest per handle) or drop-oldest ring semantics; either preserves the no-backpressure requirement while keeping recent data.
-
-#### S-3 (Medium) — `ReadViaSubscribeOnceAsync` can wait forever on a non-cancellable token
-
-The read synthesis (`GalaxyDriver.cs:738-755`) fills pending snapshots with `BadTimeout` **only via `cancellationToken.Register`**. If a caller passes `CancellationToken.None` (or a token that never fires) and a successfully subscribed tag never publishes an initial event (gateway hiccup between SubscribeBulk and first push), the awaits at `:753` never complete and the read hangs, holding the subscription open. Recommendation: race each pending TCS against an internal deadline derived from `DefaultCallTimeoutSeconds` / `PublishingIntervalMs` regardless of the caller's token.
-
-#### S-4 (Medium) — Transport-failure detection is EventPump-only; a subscription-less driver never degrades
-
-`ReconnectSupervisor.ReportTransportFailure` has exactly one production caller: the EventPump stream-fault callback (`GalaxyDriver.cs:929-949`). The pump only starts on the first subscribe/read. A driver doing only writes (or idle after discovery) whose gateway restarts will keep `GetHealth() == Healthy`; each write then fails per-request with `BadCommunicationError`, but no reopen/replay/`Degraded` transition ever runs, and the stale session persists until something subscribes. Failed unary RPCs (SubscribeBulk, Write, AcknowledgeAlarm) do not feed the supervisor either. Recommendation: report classified transport exceptions from the write/subscribe paths into the supervisor (idempotent by design, so this is cheap).
-
-#### S-5 (Medium) — FasterLog outbox `RemoveAsync` truncates the FIFO prefix; out-of-order acks silently drop unacked entries
-
-`FasterLogHistorizationOutbox.RemoveAsync` (`FasterLogHistorizationOutbox.cs:158-182`) removes the target **plus every older live entry** and truncates the log to the target's successor. The contract "recorder acks in FIFO order" is enforced only by comment. If the drain ever acks a mid-batch id first (e.g. per-tag partial write success in a future recorder change), all older unacked values are durably discarded without incrementing `DroppedCount`. Recommendation: count and warn when the prefix removal drops non-target entries, or make the API batch-oriented (`RemoveThroughAsync`) so the semantics are explicit at the call site.
-
-#### S-6 (Medium) — Historian health snapshot's connection flags are dormant in production
-
-`GatewayHistorianDataSource.RefreshConnectionStateAsync` (`GatewayHistorianDataSource.cs:219-244`) is documented as "intended to be driven by a periodic health hosted-service", but **no production caller exists** — re-verified this pass: only `GatewayHealthSnapshotTests` invokes it. `ProcessConnectionOpen` / `EventConnectionOpen` in `GetHealthSnapshot` are therefore permanently `false` in a deployed host, which any dashboard consuming the snapshot will read as "historian down" (or, if ignored, the flags are dead weight). Same shape as the memory's "register-AND-pass-into-consumer" trap that bit `GatewayTagProvisioner` in PR #423. Recommendation: add the periodic refresh hosted-service (or fold a refresh into the existing health probe cadence), or remove the flags from the snapshot.
-
-#### S-7 (Low) — Alarm feed reconnect has no backoff
-
-`GatewayGalaxyAlarmFeed.RunAsync` (`GatewayGalaxyAlarmFeed.cs:102-148`) re-opens on a fixed 5 s delay forever. `DeployWatcher` (capped exponential + jitter, `DeployWatcher.cs:212-233`) and `ReconnectSupervisor` both do this properly. A dead gateway gets hammered every 5 s per driver instance. Cosmetic at this scale, but inconsistent; align on capped exponential.
-
-#### S-8 (Low) — `GalaxyMxSession` state is unsynchronized across reopen
-
-`_session` / `_connected` (`GalaxyMxSession.cs:28-32`) are plain fields; `RecreateAsync` tears down while concurrent writers/subscribers may hold or fetch `Session`. In practice the supervisor's single-flight recovery plus the per-call try/catch (mapped to `BadCommunicationError`) contain it, and the caches are invalidated post-reopen — but the safety is emergent, not designed. Acceptable; document the invariant on `Session`.
-
-#### S-11 (Medium, NEW) — An enabled-but-misconfigured `ServerHistorian` section crash-loops the Host with a raw `UriFormatException`; one corner gets no warning at all
-
-> **REMEDIATED — `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13; live leg operator-gated).** New fail-fast `ServerHistorianOptionsValidator` (Host, wired via `AddValidatedOptions`) consumer-gates on `ServerHistorian:Enabled` **OR** `AlarmHistorian:Enabled` (closing the zero-warning corner) and fails startup with a named, aggregated `OptionsValidationException` on an empty / non-absolute / non-http(s) `Endpoint`. Adapter defense-in-depth replaces the bare `new Uri(...)` with `Uri.TryCreate`→named `InvalidOperationException`. `Validate()` gains malformed-endpoint warning parity; an alarm-only deployment logs a startup warning. Also directly answers C-4's "give the warnings style a fail-fast tier".
-
-`HistorianGatewayClientAdapter.Create` eagerly does `new Uri(options.Endpoint)` (`HistorianGatewayClientAdapter.cs:45`). `ServerHistorianOptions.Validate()` produces exactly the right warning for an empty Endpoint (`ServerHistorianOptions.cs:78-79`) — but it is **warnings-only**: registration proceeds (`Runtime/ServiceCollectionExtensions.cs:135-139`, `Host/Program.cs:123-127`) and the first DI resolution throws an unhandled `UriFormatException`, crashing the Host at bring-up. This is not theoretical — it made docker-dev unbootable until the config-side fix `7233e2ba` (whose commit message documents the exact failure). The code path is untouched.
-
-The sharper corner: **`AddAlarmHistorian` gates only on `AlarmHistorian:Enabled`** (`Runtime/ServiceCollectionExtensions.cs:87`) but sources its connection from the `ServerHistorian` section (`Program.cs:127`). With `AlarmHistorian:Enabled=true` and `ServerHistorian:Enabled=false` (Endpoint legitimately empty), `Validate()` early-returns on `!Enabled` (`ServerHistorianOptions.cs:77`) — so the operator gets **zero warning**, then the same `UriFormatException` out of `CreateAlarmWriter` when the sink singleton is built. Same exposure for the continuous-historization writer (`Program.cs:187`, though that one is co-gated on `ServerHistorian.Enabled`, so only a *malformed* non-empty Endpoint reaches it).
-
-A hard fail at startup on misconfiguration is defensible — but it should be a deliberate, named failure, not an incidental `UriFormatException` from deep inside a DI factory. Recommendation: (a) validate `Endpoint` parses as an absolute URI in `Validate()`; (b) when the section is Enabled (or *consumed* by an enabled AlarmHistorian) and Endpoint is empty/invalid, fail fast with an `InvalidOperationException` naming the configuration key — or force-disable and keep the Null defaults; (c) make `AddAlarmHistorian` also run `ServerHistorianOptions.Validate()`-equivalent checks on the connection section it actually depends on.
-
-#### S-9 (Positive) — Store-and-forward + outbox crash-safety discipline is strong
-
-- `GatewayAlarmHistorianWriter` (`GatewayAlarmHistorianWriter.cs:66-118`) short-circuits remaining batch entries to `RetryPlease` on shutdown, classifies auth failures as retryable (an auth blip never dead-letters), and defaults unknowns to `PermanentFail` so poison events cannot loop.
-- `FasterLogHistorizationOutbox` PerEntry mode fsyncs before append returns; recovery rebuilds the index from `BeginAddress`; the capacity-overflow-after-crash convergence case is analysed in-source (`FasterLogHistorizationOutbox.cs:200-204`).
-- Partial-open recovery in `GalaxyMxSession.ConnectAsync` (`GalaxyMxSession.cs:95-101`) tears down half-open state so retry rebuilds cleanly.
-
-#### S-10 (Positive) — TLS / API-key posture
-
-Both integrations support TLS with CA pinning (`CaCertificatePath`), the historian's `AllowUntrustedServerCertificate` inversion is explicitly documented at the mapping site (`HistorianGatewayClientAdapter.cs:49-52`), and keys are never logged. The 0.2.0 client strengthens this for free: `CaCertificatePath` now honours a full PEM bundle (root + intermediates) as trust anchors, and the raw-client escape hatches are documented as bypassing bearer injection (the adapter uses only typed methods, so no exposure). See C-1 for the resolver.
-
-### 2. Performance
-
-#### P-1 (Medium) — Galaxy Read costs three gateway round-trips plus a publish wait per OPC UA Read
-
-`ReadViaSubscribeOnceAsync` = SubscribeBulk + first-event wait (up to `PublishingIntervalMs`) + UnsubscribeBulk, with server-side AddItem/RemoveItem churn per read (`GalaxyDriver.cs:644-780`). This is forced by MxAccess (no one-shot read RPC) and is correctly batched per request, but a client polling via Read instead of subscribing multiplies gateway/COM load ~3×. Recommendation: document "subscribe, don't poll" as the supported pattern; consider a short-lived read-through cache keyed on full reference if polling clients appear.
-
-#### P-2 (Medium) — Alarm-history drain is one unary `SendEvent` per event
-
-`GatewayAlarmHistorianWriter.WriteBatchAsync` (`GatewayAlarmHistorianWriter.cs:66-80`) serializes the batch — deliberate poison-event isolation, but an alarm storm of N events costs N sequential RPC round-trips out of the SQLite drain worker. The 0.2.0 client still has no batched `SendEvents` with per-event status rows; when the gateway grows one, adopt it. Until then this is an accepted, well-documented ceiling.
-
-#### P-3 (Low) — Four channels to the historian sidecar; two-plus to mxaccessgw
-
-Each historian path owns its channel (read / alarm-write / provisioner / value-writer) and the Galaxy driver holds a session client, a session-less client, and a repository client. All are lazy, HTTP/2-multiplexed, and the trade-off (clean independent dispose ownership over channel sharing) is argued in-source (`GatewayHistorianServiceCollectionExtensions.cs:57-64`). No action; noted so nobody "optimizes" it into a shared-singleton dispose bug.
-
-#### P-4 (Positive) — Fan-in/fan-out paths are indexed, bounded, and metered
-
-- `SubscriptionRegistry` maintains a handle→subscriptions reverse map with a per-entry handle→fullRef index, so `ResolveSubscribers` is O(subscribers), not O(bindings) (`SubscriptionRegistry.cs:97-111`), and `TryResolveItemHandle` lets the writer skip AddItem for subscribed tags with a stale-entry cross-check (`:128-142`).
-- Subscribe/replay correlation uses a one-pass result index (`GalaxyDriver.BuildResultIndex`, fixing a former O(n²) on the 50k-tag path).
-- EventPump decouples network read from dispatch via a bounded channel (default 50 000, configurable) with received/dispatched/dropped counters.
-
-#### P-5 (Low) — HistoryRead buffers whole replies; `maxEvents <= 0` is unbounded
-
-`ReadRawAsync` / `ReadProcessedAsync` / `ReadEventsAsync` (`GatewayHistorianDataSource.cs:59-182`) drain the stream into a list before mapping. Raw reads are capped by `maxValuesPerNode`, but an events read with `maxEvents <= 0` collects without limit (the gateway's `RuntimeDb:EventReadMaxRows` is the only cap, and it's a remote deployment knob). Re-checked against 0.2.0: `ReadEventsAsync`'s client signature is unchanged — still no wire-side cap, so the adapter still cannot forward `maxEvents` (`HistorianGatewayClientAdapter.cs:76-89`). The tie-cluster paging bound (`MaxTieClusterOverfetch`, validated > 0 in `ServerHistorianOptions.Validate`, enforced at `OtOpcUaNodeManager.cs:2200-2214`) is well-designed. Recommendation: clamp events reads to a server-side default cap when the caller passes 0.
-
-#### P-6 (Low) — Outbox peek holds the state lock across disk I/O
-
-`PeekBatchAsync` scans FasterLog from the logical head under `_state` (`FasterLogHistorizationOutbox.cs:140-155`), blocking a concurrent `AppendAsync`'s index update for the duration of a (mostly page-cached) disk scan. At the recorder's 64-entry default batch this is negligible; revisit only if drain batches grow.
-
-### 3. Conventions
-
-#### C-1 (Positive) — Secret handling is a model for the other drivers
-
-`GalaxySecretRef` (`Galaxy.Contracts/GalaxySecretRef.cs`) resolves `env:` / `file:` / `dev:` / literal with fail-fast on unset env vars and a startup warning on unprefixed cleartext; it lives in Contracts so the runtime driver and the AdminUI browser share one implementation (`GalaxyDriverBrowser.cs:125`). The historian side takes `ServerHistorian__ApiKey` via env with a "never commit" doc contract and an empty-key `Validate()` warning; the docker-dev rewrite (`7233e2ba`) follows the same discipline (`${ServerHistorian__ApiKey:-}` pass-through, key never committed). No key value is ever logged on either path.
-
-#### C-2 (Positive) — Seam quality and driver-family consistency
-
-- `IHistorianGatewayClient` is a clean single seam: every consumer (`GatewayHistorianDataSource`, `GatewayAlarmHistorianWriter`, `GatewayTagProvisioner`, `GatewayHistorianValueWriter`) depends only on it; `FakeHistorianGatewayClient` backs the offline suite; the adapter is a zero-translation pass-through. (The 0.2.0 client's new `IHistorianGatewayClientTransport` seam is package-internal plumbing — correctly not adopted; the repo seam stays `IHistorianGatewayClient`.)
-- The Galaxy capability seams (`IGalaxyHierarchySource`, `IGalaxyDataReader/Writer`, `IGalaxySubscriber`, `IGalaxyAlarmFeed/Acknowledger`) plus the internal test ctor mirror the protocol-driver pattern; tracing decorators (`Traced*`) wrap the production seams without an OpenTelemetry dependency.
-- `GalaxyDriverFactoryExtensions.CreateInstance` throws precise, instance-named errors on missing required fields (`GalaxyDriverFactoryExtensions.cs:56-75`), and `GalaxyDriverProbe` uses `JsonStringEnumConverter` (the FB-9/FB-10 enum-serialization lesson applied).
-
-#### C-3 (Medium) — The historian seam leaks wire types by design; keep it contained
-
-`IHistorianGatewayClient` signatures use `HistorianGateway.Contracts.Grpc` types (`HistorianSample`, `RetrievalMode`, `WriteAck`, ...) — documented as deliberate (`IHistorianGatewayClient.cs:5-11`). This is fine while the driver is the only consumer, but the pure `Mapping/` layer is the real anti-corruption boundary: any future consumer must sit **above** `GatewayHistorianDataSource`, never on the seam. The 0.2.0 bump demonstrated the flip side concretely: a *semantics* change in a contract type (`HistorianTagDefinition.DataType` presence, U-7) flowed straight into driver behavior with no compile signal. Worth a one-line guard note in the interface doc, plus a bump-checklist habit of diffing the contract XML (this pass's method) rather than relying on green builds.
-
-#### C-4 (Low) — Options-validation styles diverge
-
-Galaxy: throw-on-construct for required fields; DataAnnotations attributes on the records are decorative (nothing runs `Validator`). Historian: `Validate()` returns operator warnings and never throws (registration logs them). Both are defensible, but the repo now has two idioms for "driver-adjacent options validation" — and the warnings-only historian style's sharp edge materialized live this period (the docker-dev crash, see S-11): the operator got the correct warning immediately followed by an opaque crash. Pick one idiom for new sections, and give the warnings style a fail-fast tier for configurations that will provably crash anyway.
-
-#### C-5 (Low) — Retired Wonderware project directories still on disk
-
-`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware{,.Client,.Client.Contracts}` (and their test dirs) remain in the tree but are absent from `ZB.MOM.WW.OtOpcUa.slnx` (0 references) — re-verified this pass. Dead directories invite grep noise and accidental resurrection; delete them (git history preserves the code).
-
-#### C-6 (Low) — EventPump silently drops unknown event families with no metric
-
-`Dispatch` (`EventPump.cs:192-207`) returns without counting on any non-`OnDataChange` family. The rationale comment is good, but a gateway version emitting `OnBufferedDataChange` (or a new family) would silently discard data with zero observability — the alarm feed counts its decode drops (`AlarmTransitionsDecodingFailures`); the pump should count filtered/unknown families the same way.
-
-#### C-7 (Low, NEW) — The live-value writer populates a documented-dead `Quality` field
-
-> **REMEDIATED — `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13).** Decision: **keep populating** the field (the "stop populating so it's visible" alternative was rejected — proto3 sends `0` = Bad when unset, planting a future bug the instant a gateway version honors the field, whereas the recorder's `192` is correct). Visibility achieved with doc comments instead: `GatewayHistorianValueWriter` `` + inline note at the mapping, `ContinuousHistorizationRecorder.GoodQuality` guard note, and a `docs/Historian.md` sentence — all citing 0.2.0 Contracts C-002.
-
-`GatewayHistorianValueWriter` maps `HistorizationValue.Quality` onto `HistorianLiveValue.Quality` (`GatewayHistorianValueWriter.cs:63`), but the 0.2.0 contract now documents that field (and `QualityDetail`) as **silently discarded on the `WriteLiveValues` SQL path** — the gateway's SQL login is column-permission-denied on the server-managed Quality column, so the value is accepted on the wire and dropped. Harmless today because the recorder hardcodes `GoodQuality = 192` for every captured value (`ContinuousHistorizationRecorder.cs:45,256` — the outbox round-trips it, `:419`), so nothing meaningful is lost. The hazard is forward-looking: if the recorder is ever extended to capture *real* node quality (a natural next step), it will appear to work and silently persist nothing — quality will remain server-stamped. Add a comment at the mapping site citing the 0.2.0 contract note (Contracts C-002), or stop populating the field so the dead input is visible.
-
-### 4. Underdeveloped Areas
-
-#### U-2 (Medium) — KNOWN LIMITATION 1: the live-validation gate is built but still not fully run
-
-The `Category=LiveIntegration` suite is complete and skip-clean: `GatewayLiveFixture` env-gates on `HISTGW_GATEWAY_ENDPOINT`/`APIKEY` (+ per-test `HISTGW_TEST_TAG`, `HISTGW_WRITE_SANDBOX_TAG`, `HISTGW_ALARM_SOURCE`) with a bounded 3 s TCP probe so a down VPN skips instead of hanging (`Live/GatewayLiveFixture.cs`). Four live tests cover read, EnsureTags+write, alarm SendEvent→ReadEvents round-trip, and a SendEvent contract check. The alarm leg has been run live at least once (merge `245316d8`); nothing further has run since the prior review — the arch-review #12 rig session ran with the historian **disabled**. The 0.2.0 bump raises the stakes: the suite has never executed against a 0.2.0 gateway with the 0.2.0 client, and the DataType presence change (U-7) is precisely the kind of semantics shift only the live EnsureTags leg can validate. A full documented run of the suite is the remaining work — the infrastructure is not the gap.
-
-#### U-3 (Medium) — WriteSecured / VerifiedWrite user identity is stubbed at zero
-
-`InvokeWriteSecuredAsync` hardcodes `CurrentUserId = 0, VerifierUserId = 0` (`GatewayGalaxyDataWriter.cs:263-264`). ArchestrA secured/verified writes exist precisely to attribute and dual-authorize the operation; user 0 will be rejected or unattributed by any galaxy that actually classifies tags SecuredWrite/VerifiedWrite. There is no mapping from the OPC UA session identity (the LDAP-authenticated principal is available server-side) to an ArchestrA user id. Until that lands, writes to secured-classification tags are effectively unsupported — document it, or fail fast with a clear status instead of sending user 0.
-
-#### U-4 (Medium) — Dormant paths inventory
-
-- `GatewayHistorianDataSource.RefreshConnectionStateAsync` — no production caller (see S-6).
-- Replay still fans out per-subscription `SubscribeBulk`; the gateway's batched `ReplaySubscriptionsCommand` remains a "PR 6.x can swap this" note (`GalaxyDriver.cs:314-316`).
-- `GalaxyDriver.FlushOptionalCachesAsync` is a no-op and `GetMemoryFootprint` is a constants-based estimate (`GalaxyDriver.cs:561-575`) — fine, but the server's cache-flush heuristic gets synthetic data from this driver.
-- `HistorianGatewayClientAdapter.ReadEventsAsync` never forwards `maxEvents` on the wire (client-side cap only, documented at `IHistorianGatewayClient.cs:58-63`) and the source filter is re-applied client-side defensively (`GatewayHistorianDataSource.cs:152-157`) because the gateway-side filter "may not be present" — re-checked against 0.2.0: neither gap is addressed (the `ReadEventsAsync` client signature is unchanged). Both remain polite workarounds for gateway gaps that should be tracked against the sidecar repo.
-
-#### U-5 (Low) — Outbox serializer has no version/format byte
-
-`HistorizationOutboxEntrySerializer` writes a fixed positional layout with no version prefix (`HistorizationOutboxEntrySerializer.cs:14-16`). Any future field addition breaks recovery of pre-existing on-disk entries with an undiagnosable deserialization failure mid-`RecoverState`. One reserved byte now is free; a migration later is not.
-
-#### U-7 (Medium, NEW) — The 0.2.0 bump's impact analysis missed the `DataType` presence change that alters Boolean tag provisioning
-
-> **REMEDIATED (code + docs) — `r2/06-serverhistorian-failfast` (R2-06, 2026-07-13); LIVE LEG OPERATOR-GATED.** Pinned the 0.2.0 Boolean→Int1 explicit-presence semantics at two levels (`GatewayTagProvisionerTests.Boolean_definition_carries_explicit_Int1_presence` asserts `DataType==Int1` **and** `HasDataType` — the 0.2.0 presence witness; a package downgrade breaks the compile, a dropped-presence contract change breaks the assert). Added a written upsert/retype migration note to `docs/Historian.md` (three retype hypotheses + `failed=N` tally observability). Added the env-gated live Boolean EnsureTags leg + a **retype probe** (`HISTGW_BOOL_SANDBOX_TAG`) that records the deployed historian's in-place-retype policy. **Pending:** the live VPN run on a 0.2.0 gateway (feeds the observed retype answer back into the doc note; also discharges U-2's full documented run).
-
-The bump commit (`f6eaa267`) gates only on the M3 `opc_quality` change ("checked and CLEAR" — correct, verified: OtOpcUa never writes `HistorianHistoricalValue`). But 0.2.0 also made **`HistorianTagDefinition.DataType` proto3-optional** (contracts review M-2), and that one lands squarely on `GatewayTagProvisioner`:
-
-- The provisioner always sets `DataType` explicitly (`GatewayTagProvisioner.cs:57`), and `Boolean → HistorianDataType.Int1` (`HistorianTypeMapper.cs:28`) — where **Int1 is wire value 0**.
-- Under 0.1.0 the gateway could not distinguish wire-0 from unset and mapped it to the SDK default **Float** — i.e. every Boolean tag EnsureTags'd to date was silently provisioned as a Float historian tag.
-- Under 0.2.0 presence is explicit, so the same call now provisions **Int1** — the intended behavior, arriving as a silent semantic change with zero code diff on our side.
-
-Two consequences need closing out: (1) any Boolean tags provisioned under 0.1.0 exist in the historian as Float, and what the gateway's `EnsureTags` does when the existing tag's type mismatches the (now-honored) requested type is unverified — per-tag failure outcomes would surface only in the provisioner's tally log (`failed=N`); (2) the live EnsureTags gate (U-2) has never run under 0.2.0 semantics. Recommendation: re-run the `Category=LiveIntegration` EnsureTags leg including a Boolean tag; check historian-side types for any previously provisioned Boolean tags; and adopt "diff the contract XML between package versions" (this review's method — both versions sit in the local NuGet cache) as the standing bump-gate checklist, since green builds cannot catch presence-semantics changes.
-
-#### U-6 — Test coverage assessment
-
-Zero test changes in the three gateway test projects since `9cad9ed0`; the assessment stands verbatim:
-
-- **Galaxy** (~250 test methods across 34 files): strong unit coverage of the hard parts — reconnect orchestration (`GalaxyMxSessionReconnectTests`, `ReconnectSupervisorTests`), pump fault/bounded-channel behaviour, registry rebind/handle-resolve, writer caches, alarm feed decode, value encode/decode, status mapping, probe, factory. Three skip-gated live smokes (`GatewayGalaxyLiveReopenAndWriteTests` — proven 2/2 against `10.100.0.48:5120` per project memory — and `GatewayGalaxyAlarmFeedLiveTests`) gated on `MXGW_ENDPOINT` + `GALAXY_MXGW_API_KEY`.
-- **Historian.Gateway** (~15 files): mappers fully covered, writer/provisioner/data-source outcome classification covered via `FakeHistorianGatewayClient`, outbox recovery/capacity covered, plus the 4-test live suite.
-- **Thin spots:** no test drives `GalaxyDriver` write-through the supervisory-advise *failure* branch asserting the returned status (S-1's blast radius is untested); no test exercises out-of-order `RemoveAsync` on the outbox (S-5); nothing covers the read-hang case in S-3; `EventPump` unknown-family filtering has no assertion (C-6). New this pass: nothing asserts the misconfigured-Enabled crash shape (S-11), and no test pins the Boolean→Int1 provisioning expectation against a 0.2.0-semantics fake (U-7).
+1. **The Galaxy write path went from optimistic to correlated** (06/S-1 arc, closed). The shape that shipped is stronger than the round-2 partial: the gateway now correlates the worker's `OnWriteComplete` into every unary write reply, so the driver's reply translation carries the real COM-side commit verdict, including refusals — with an explicit per-code `MxStatusDetail` table and an anti-truncation invariant on the mapping seam (the live gate caught `1007 → low-byte 239 → Good`, i.e. a REFUSED write mapping to Good; `StatusCodeMap.cs:120-133` is the fix). The subscription-handle borrow path was deleted outright after live captures showed a plain-advised handle can never carry a no-login write (SECURITY_ERROR 1008) and cannot be re-advised (E_INVALIDARG).
+2. **The continuous-historization pipeline was live-gated and hardened end-to-end** (#491/#537/#538, all verified in code): `Self` captured before the `ConfigureAwait(false)` await (`ContinuousHistorizationRecorder.cs:369-382`); capture gated on the per-cluster Primary via `IHistorizationRoleView` (registered unconditionally on driver nodes, `Runtime/ServiceCollectionExtensions.cs:99` — the #511 registration accident avoided); poison-tag quarantine keyed on "did another tag write this pass" with the falsification control (`:767-806`); shutdown cancellation mid-drain rethrown so it aborts the *pass* instead of counting as a per-tag failure (`:707-716`, `7870c91f`). Perf landed with correctness intact: batch ingest with one group commit per burst (`FasterLogHistorizationOutbox.AppendBatchAsync`, incremental bookkeeping `:119-137`), bounded-parallel per-tag drain (`DrainConcurrency` 8, fold on the single drain flow `:741-758`), `DrainBatchSize` 64→1024.
+3. **v3 re-keyed the Galaxy driver to RawPath identity** (`636c755b`): forward/reverse RawPath⇄attributeRef tables built once from the artifact (`GalaxyDriver.cs:247-277`), identity on a miss, with the skipped-tag ledger (#541) accounting for undialable entries. Alarm ConditionId, ack routing, discovery and write security resolution all translate consistently (`:964-972`, `:1298-1310`, `:1354`).
+4. **Wire robustness grew**: the replay-gap sentinel is handled ahead of channel pressure with a coalesced, non-blocking re-snapshot (`EventPump.cs:153-162`, `GalaxyDriver.cs:1125-1173` — deliberate contract defence for a path the current configuration cannot hit); sparse arrays decode with gateway-mirrored fill semantics (`MxValueDecoder.cs:96-166`); an unrepresentable value now publishes `BadTypeMismatch` instead of a Good null (`EventPump.cs:336-353` — the "good null is indistinguishable from a real null" fix).
---
@@ -265,17 +65,138 @@ Zero test changes in the three gateway test projects since `9cad9ed0`; the asses
| Dimension | Rating (1-5) | Prior | Justification |
|---|---|---|---|
-| Stability | **4** | 4 | Layered recovery (supervisor, self-reconnecting feeds, backoff, partial-open teardown) and disciplined never-throw write sinks are unchanged; still docked for the Galaxy silent-write-loss blast radius (S-1 — explicitly deferred by the remediation pass), EventPump-only fault detection (S-4), the dormant health refresh (S-6), and now the proven-live misconfiguration crash-loop (S-11, fixed only config-side). |
-| Performance | **4** | 4 | No in-domain code change; bounded channels with drop metering, O(1) fan-out indexes, handle borrowing, and a bounded tie-cluster over-fetch stand; still docked for the 3-round-trip read synthesis (P-1) and serial alarm sends (P-2), both documented and still gateway-constrained in 0.2.0. |
-| Conventions | **4** | 4 | Exemplary seams, shared secret resolver, consistent driver-family shape and tracing decorators; still docked for the dual options-validation idiom (whose sharp edge now has a live incident attached), the on-disk retired Wonderware dirs, the unmetered pump filter, and the new dead-Quality-input mapping (C-7). |
-| Underdeveloped areas | **3** | 3 | The U-1 doc drift is fixed (the one prior finding closed this period), but the live gates net widened: the historian live suite has still never fully run, and the 0.2.0 bump added unverified semantics on the provisioning path (U-7) that only the live EnsureTags leg can retire. WriteSecured identity remains stubbed; the gateway-gap workarounds remain. |
+| Stability | **4.5** | (4) | The two headline stability debts both closed and were *live-gated*: Galaxy writes are fail-closed + correlated (S-1), and the historization pipeline survived a real gate that found and fixed three production-fatal defects (#491/#537/#538). The pump now detects server-completed streams. Docked half a point for the surviving cluster: the S-3 read hang, EventPump-only fault detection for subscription-less drivers (S-4), the dormant health snapshot (S-6), and the new advise-cache poisoning edge (S-12). |
+| Performance | **4.5** | (4) | Group-commit batch append (one fsync per burst), bounded-parallel per-tag drain, a 16×-wider drain batch, and the 03/S3 HistoryRead parallelism/limiter/deadline knobs — all with the correctness discipline intact (all-or-nothing ack, outage-vs-poison discrimination). Remaining docks are gateway-constrained and documented: the 3-round-trip read synthesis (P-1) and serial alarm sends (P-2). |
+| Conventions | **4.5** | (4) | C-4, C-5, C-7 closed; the `IMxWriteOps` seam and the `secret:` arm follow the house patterns exactly; status-code constants are named + reflection-guarded (#497); in-source rationale quality (the anti-truncation invariant, the keep-populating Quality decision, the replay-gap unreachability note) is exemplary. Docked for the surviving Low trio: seam wire-type leakage note (C-3), the unmetered pump family filter (C-6), and the unlatched quarantine warning (C-8). |
+| Underdeveloped areas | **4** | (3) | The live gates that defined this dimension's debt have now *run* — twice — and each found real defects offline tests missed (StorageRateMs, Int1 degeneracy, the ConfigureAwait/`Self` restart, dual-capture, poison-tag pinning); U-2/U-7 closed, dead surface pruned (#525). Remaining: WriteSecured identity still stubbed (U-3), the dormant health-snapshot reader (S-6), the outbox version byte (U-5), and the gateway-gap workarounds tracked cross-repo. |
+
+---
+
+## Findings
+
+### 1. Stability
+
+#### S-12 (Medium, NEW) — A transport failure during `AdviseSupervisory` poisons the supervised-handle cache; subsequent writes skip the advise until the next reconnect
+
+`EnsureSupervisoryAdvisedAsync` marks the handle supervised **before** the round-trip (`_supervisedHandles.TryAdd`, `GatewayGalaxyDataWriter.cs:234`) and removes the mark only on the **non-Ok reply** path (`:251-261`). If `ops.InvokeAsync` **throws** instead — transport blip, gateway restart, cancellation — the exception propagates to `WriteOneAsync`'s catch (`:192-197`, this write correctly returns `BadCommunicationError`) but the handle **stays marked supervised**. Every subsequent write to that tag then takes the "already advised" fast path (`:234`) and issues a raw Write against an item that was never supervisory-advised:
+
+- Against a **correlated** gateway (the production posture), each write is refused with SECURITY_ERROR 1008 → `BadUserAccessDenied` — visible, but **never self-healing**: nothing clears the stale cache entry on a 1008 row, so the tag's writes fail until a session reconnect happens to run `InvalidateHandleCaches` (`:67-71`).
+- Against a **pre-correlation** gateway, the reply's statuses are empty → **provisional Good** (`:330-331`) — the exact S-1 phantom-write shape, resurrected for this one edge.
+
+This is the classic one-fault-variant gap: the fail-closed fix was verified against the *reply-failure* variant (which the fail-closed tests cover) but not the *throw* variant. Fix: wrap the invoke in try/catch and `TryRemove` the handle on any failure to complete the advise; optionally also clear the entry when a write's correlated status comes back 1008 (self-heal). One unit test on the throw path via the existing `IMxWriteOps` fake closes it.
+
+#### S-2 (Medium) — EventPump saturation still drops the *newest* events
+
+Unchanged (`EventPump.cs:164-170`): `TryWrite` against the bounded channel preserves 50 000 stale queued events and discards the just-arrived one. For last-value-wins telemetry the bias is inverted. Metered (`galaxy.events.dropped`). Recommendation stands: per-item-handle conflation (keep newest per handle) or drop-oldest ring semantics.
+
+#### S-3 (Medium) — `ReadViaSubscribeOnceAsync` can still wait forever on a non-cancellable token
+
+Unchanged (`GalaxyDriver.cs:899-915`): the Bad-fill for still-pending snapshots runs only from `cancellationToken.Register`. A caller passing `CancellationToken.None` (or a never-firing token) with a successfully-subscribed tag that never publishes its initial event hangs the read and holds the subscription open. `StatusCodeMap.BadTimeout` is now the correct constant (#497) — but nothing arms it without a caller token. Recommendation stands: race each pending TCS against an internal deadline derived from `PublishingIntervalMs`/call-timeout regardless of the caller's token. Still untested (the read suite always emits or cancels).
+
+#### S-4 (Medium) — Transport-failure detection is still EventPump-only; a subscription-less driver never degrades
+
+`ReportTransportFailure` retains its single production caller (`GalaxyDriver.cs:1196`). The pump-side coverage *improved*: a server-completed stream now notifies the supervisor instead of dying silently (`EventPump.cs:180-188` — explicitly written against the #527 watchdog's non-Healthy requirement). But the pump only exists once something subscribes or reads; a write-only or idle-after-discovery driver whose gateway restarts stays `Healthy` with per-request `BadCommunicationError`, and neither the supervisor nor the #527 watchdog (which requires a non-Healthy state) engages. Recommendation stands: report classified transport exceptions from the write/subscribe unary paths into the supervisor (idempotent, cheap).
+
+#### S-5 (Low, was Medium) — Outbox `RemoveAsync` prefix-truncation: now pinned and relied upon; the silent-drop residual is observability only
+
+The truncate-through-the-id semantics (`FasterLogHistorizationOutbox.cs:203-227`) are now an explicit, tested contract (`FasterLogHistorizationOutboxTests.cs:99`) that the recorder's all-or-nothing ack deliberately builds on (`ContinuousHistorizationRecorder.cs:677-680`) — the round-1 worry ("a future recorder change acks mid-batch") is fenced by documentation at both ends plus the #538 design. Residual: entries dropped by a prefix removal still don't count on `DroppedCount`. One counter increment in the prefix loop would make the ledger honest; low priority.
+
+#### S-6 (Medium) — The historian health snapshot is a write-only ledger: no refresh caller, and now provably no snapshot reader either
+
+`RefreshConnectionStateAsync` (`GatewayHistorianDataSource.cs:219-244`) still has zero production callers (only `GatewayHealthSnapshotTests.cs:33,59,74`), so `ProcessConnectionOpen`/`EventConnectionOpen` are permanently `false` in a deployed host. Sharpened this pass: **`GetHealthSnapshot()` itself has no production consumer** — the interface member (`IHistorianDataSource.cs:117`) and the Null impl are the only other references in `src/`. Third review in a row for this built-but-never-wired instance. Either wire a periodic refresh + a reader (the R2-10 resilience-status flow is the exact template: publisher service → DPS/gRPC → AdminUI chip), or delete the snapshot surface. If kept dormant, at least a registration-time guard test (the R2-10 `ResilienceStatusReaderRegistrationTests` pattern) should pin whichever decision is made.
+
+#### S-7 (Low) — Alarm feed reconnect still has no backoff
+
+Fixed 5 s forever (`GatewayGalaxyAlarmFeed.cs:46,141`), still inconsistent with `DeployWatcher`/`ReconnectSupervisor`'s capped exponential. Cosmetic at fleet scale; align when next touched.
+
+#### S-8 (Low) — `GalaxyMxSession` state unsynchronized across reopen (accepted)
+
+`_session`/`_connected` remain plain fields (`GalaxyMxSession.cs:29-31`); the emergent safety (single-flight recovery, per-call catch → `BadCommunicationError`, cache invalidation post-reopen) held through this period's changes, and the batch-scoped `SessionMxWriteOps` binding (`GatewayGalaxyDataWriter.cs:115-122`) keeps a mid-batch reconnect contained to Bad results. Accepted; the invariant is now partially documented (`:46-48`).
+
+#### S-13 (Low, NEW) — Two RawPaths authored to the same Galaxy attributeRef: the second node silently never receives a value
+
+`BuildRawTagTables` is first-wins on the reverse map (`GalaxyDriver.cs:272-273` — comment-only), and `OnPumpDataChange` reverse-translates every event through `ToRawPath` (`:316-317`, `:1394`), so all data-change fan-out for a shared attributeRef surfaces under the **first** RawPath. The second raw tag dials fine (forward map, writes work) but its node never publishes — stuck on `BadWaitingForInitialData` with nothing logged and **no skip-ledger entry** (#541's exact silence, one layer up: the tag is "bound" yet dead). Nothing upstream prevents authoring the duplicate (two folders can each hold a tag dialing the same `tag_name.Attribute`). Minimum fix: log a warning and record the loser in the `SkippedTagLedger` ("duplicate attributeRef — value fan-out surfaces under {winner}"); better: fan the reverse map out to *all* RawPaths sharing the ref (the historian recorder's `_refMap` multi-name pattern, `ContinuousHistorizationRecorder.cs:540-554`, is the in-repo precedent).
+
+#### S-9 (Positive) — Store-and-forward + outbox crash-safety discipline extended under the perf work
+
+The batching arc kept every crash-safety property while removing the per-value fsync: incremental per-entry indexing means a mid-batch fault strands nothing untracked (`FasterLogHistorizationOutbox.cs:119-137` — the follow-up `73205865` fixed exactly the deferred-indexing variant); the commit-residual is documented at the seam (`:145-149`); drop-oldest runs once per batch (`:153-181`). The alarm writer's shutdown short-circuit and never-throw classification stand (`GatewayAlarmHistorianWriter.cs:66-78,125-156`). The recorder's ingest is hardened per-item (a faulting capture-gate delegate cannot drop the burst tail or restart the actor — `ContinuousHistorizationRecorder.cs:385-423`) and the append failure path explicitly refuses to restart-loop under a persistent disk fault (`:446-467`).
+
+#### S-10 (Positive) — TLS / API-key posture unchanged and extended
+
+CA pinning, the documented inverted TLS mapping and the h2c TLS-only-options guard (`HistorianGatewayClientAdapter.cs:60-79`), keys never logged, `${secret:}` support on `ServerHistorian:ApiKey`, and the Galaxy `secret:` arm resolving fail-closed through the shared store with the real resolver wired in production (`DriverFactoryBootstrap.cs:175`).
+
+### 2. Performance
+
+#### P-1 (Medium) — Galaxy Read still costs three gateway round-trips plus a publish wait
+
+Unchanged and gateway-constrained (`GalaxyDriver.cs:805-941`). Correctly batched per request; "subscribe, don't poll" remains the supported pattern.
+
+#### P-2 (Medium) — Alarm-history drain is still one unary `SendEvent` per event
+
+Unchanged (`GatewayAlarmHistorianWriter.cs:66-78`) — deliberate poison-event isolation; still no batched `SendEvents` with per-event status rows on the client surface. Accepted, documented ceiling; adopt a batched RPC when the gateway grows one.
+
+#### P-3 (Low) — Independent channels per path (deliberate)
+
+Unchanged; the trade-off is argued in-source and every owner now disposes its channel (the provisioner joined this period, `GatewayTagProvisioner.cs:108-115`). No action — still noted so nobody "optimizes" it into a shared-singleton dispose bug.
+
+#### P-4 (Positive) — Fan-out paths indexed, bounded, metered — now including the drain
+
+`SubscriptionRegistry` reverse maps, `BuildResultIndex`, and the bounded pump channel stand. New this period: the recorder's per-tag drain groups run under a bounded `SemaphoreSlim` window with the outcome fold deliberately kept on the single logical drain flow so the #538 counters can't be corrupted by concurrent mutation (`ContinuousHistorizationRecorder.cs:696-758`); ingest is one group commit per burst; `Task.WhenAll`'s complete-before-fault semantics are used to make the `using`-scoped semaphore safe on the cancellation path (`:736-739`).
+
+#### P-5 (Low) — Events reads: client-side cap + truncation landed; `maxEvents <= 0` still unbounded; the cap still never reaches the wire
+
+`ReadEventsAsync` now stops draining at the cap and signals truncation with an opaque token (`GatewayHistorianDataSource.cs:142-175`). Remaining: `maxEvents <= 0` disables the cap entirely (`:142-143` — the gateway's remote `RuntimeDb:EventReadMaxRows` is then the only bound), and the adapter still cannot forward the cap on the wire (`HistorianGatewayClientAdapter.cs:100-113`). Clamp `<= 0` to a server-side default; keep tracking the wire-side cap against the sidecar repo. The 03/S3 parallelism/limiter/deadline knobs (`ServerHistorianOptions.cs:80-97`, validated at `:133-138`) meaningfully bound the surrounding fan-out.
+
+#### P-6 (Low) — Outbox peek still holds the state lock across the log scan — and the batch is 16× wider now
+
+`PeekBatchAsync` scans under `_state` (`FasterLogHistorizationOutbox.cs:190-197`); `DrainBatchSize` default rose 64→1024 (`ContinuousHistorizationOptions.cs:78`), so a concurrent `AppendBatchAsync`'s per-entry index updates can now stall behind a 1024-entry scan. Mostly page-cached; fine today. If drain batches grow again, snapshot `_nextScanAddress` under the lock and scan outside it.
+
+### 3. Conventions
+
+#### C-1 (Positive) — Secret handling remains the model
+
+`GalaxySecretRef` now resolves `env:` / `file:` / `dev:` / **`secret:`** / literal, fail-closed on the `secret:` arm through the shared `ZB.MOM.WW.Secrets` resolver (`Galaxy.Contracts/GalaxySecretRef.cs`), shared by driver and browser. `ServerHistorian:ApiKey` documents env-or-`${secret:}` delivery (`ServerHistorianOptions.cs:40-48`). No key value logged on any path.
+
+#### C-2 (Positive) — Seam quality improved again
+
+The `IMxWriteOps` internal seam (`GatewayGalaxyDataWriter.cs:342-405`) is the right-sized answer to a sealed SDK: production binds a zero-translation adapter, tests drive the *real* single-write pipeline (`WriteOneForTestAsync`, `:142-144`) — precisely the design that lets the fail-closed behaviour be pinned offline. `IHistorianGatewayClient` remains the single historian seam with every consumer behind it.
+
+#### C-3 (Low, was Medium) — The historian seam still leaks wire types by design
+
+`IHistorianGatewayClient.cs:5-11` unchanged; no guard note added. Downgraded: the risk the round-2 bump demonstrated is now institutionally covered — presence-semantics pinned by unit tests (`GatewayTagProvisionerTests.Boolean_definition_carries_explicit_Int2_presence`) and the live suite has become a routine bump gate. Keep future consumers above `GatewayHistorianDataSource`; add the one-line guard note when next editing the file.
+
+#### C-6 (Low) — EventPump unknown-family filter still unmetered
+
+`Dispatch`'s default arm returns uncounted (`EventPump.cs:297-312`). Narrowed by the new `replay_gaps`/`undecodable` counters (a sentinel or bad payload is no longer invisible), and filtered volume is derivable from the counter arithmetic — but a first-class `galaxy.events.filtered` counter (mirroring the alarm feed's decode-failure counter) is still the clean close, and no test asserts the filter.
+
+#### C-8 (Low, NEW) — The poison-tag abandonment warning repeats every drain pass with no latch
+
+Once a tag is over its `MaxWriteAttempts` budget, each subsequent pass (2 s default cadence) that captures new entries for it re-abandons and re-logs the full Warning (`ContinuousHistorizationRecorder.cs:808-819` — `_tagWriteFailures` is never reset on abandonment, so `stillBlocking` stays false and the branch re-fires per pass): ~30 warnings/minute per poison tag until it is fixed or de-historized. The recorder already has the right idiom in-file — the ingest-fault Warning-then-Debug latch (`:404-416`). Apply it per tag.
+
+### 4. Underdeveloped Areas
+
+#### U-3 (Medium) — WriteSecured / VerifiedWrite user identity is still stubbed at zero
+
+`GatewayGalaxyDataWriter.cs:284-285` unchanged across all three reviews. Any galaxy that actually classifies tags SecuredWrite/VerifiedWrite will reject or mis-attribute user 0; there is still no mapping from the OPC UA session identity (now a rich, ACL-carrying principal since #520) to an ArchestrA user id, and no fail-fast. The S-1 correlation at least makes the rejection visible as Bad now. Decide: document as unsupported + fail fast with a distinct status, or build the identity mapping. This is the domain's oldest open Medium.
+
+#### U-4 (Medium → Low) — Dormant-paths inventory, pruned
+
+Closed this period: `GetMemoryFootprint`/`FlushOptionalCachesAsync` deleted (#525). Remaining, all tracked or gateway-side: batched `ReplaySubscriptionsCommand` swap (`GalaxyDriver.cs:446-447`), wire-side `maxEvents` + server-side source filter (sidecar backlog), `RefreshConnectionStateAsync`/`GetHealthSnapshot` (S-6), `IHostConnectivityProbe` (Gitea #521).
+
+#### U-5 (Low) — Outbox serializer still has no version/format byte
+
+`HistorizationOutboxEntrySerializer.cs:14-16,23-40` unchanged. Now that `Capacity` defaults non-zero and production deployments accumulate on-disk logs, a layout change would break `RecoverState` mid-scan with an undiagnosable failure. One reserved byte remains free today; a migration later is not. Third listing.
+
+#### U-6 — Test coverage assessment
+
+Substantially improved: +2,304 lines / 30 files across the three gateway test projects since `f6eaa267`. Now covered: the fail-closed advise path and unconfirmed-write metering (offline via `IMxWriteOps`), RawPath keying, replay-gap handling, stream-completion fault, sparse-array/undecodable decode, outbox batch-append prefix accounting + prefix-removal pinning + serializer round-trips, value-writer quality/timestamp mapping, recorder gate/quarantine/falsification controls (Runtime.Tests). Live suites: Galaxy reopen-and-write + alarm feed + write meters (env-gated), historian `LiveIntegration` 6/6. **Remaining thin spots:** S-3's read-hang (no test passes `CancellationToken.None` with a never-publishing subscribed tag); C-6's family filter; and the new S-12 advise-throw path (one `IMxWriteOps` fake test).
---
## Cross-Cutting Themes
-1. **"Wired but never invoked" is this codebase's recurring failure mode** — the provisioner (PR #423), the F10b sink forwarding, and `RefreshConnectionStateAsync` (S-6, still open). Any new capability interface or hook needs an explicit production-caller check plus a live `/run`, not just unit tests. (The arch-review pass built the reflection forwarding guard for the sink family; the health-refresh instance in this domain remains unguarded.)
-2. **Optimistic-Good on fire-and-forget writes** — Galaxy structurally cannot report a failed commit (S-1, deferred by the remediation pass); reviewers of the node-write router / write-outcome self-correction should not assume driver parity here.
-3. **Green builds cannot gate contract-semantics changes** — the 0.2.0 bump was wire-compatible and compile-clean yet silently changed Boolean tag provisioning (U-7) and dead-lettered the live-value Quality field (C-7). The bump gate for gateway packages should include a contract-XML diff (both versions are always in the local NuGet cache) and a live-suite run, not just `dotnet test`.
-4. **Warnings-only validation defers the crash, it doesn't prevent it** — the docker-dev incident (S-11) logged the exact right warning milliseconds before an opaque `UriFormatException` crash-loop. Configurations that will provably fail should fail fast, by name.
-5. **Gateway-gap workarounds accrue client-side** — defensive source filters, client-side event caps, missing batched replay/`SendEvents`; 0.2.0 closed none of them. These belong on the sister repos' backlogs with links from the in-source comments.
+1. **The live gate is now this domain's most productive tool — keep running it.** Every closure this period that mattered was live-gate-driven, and each gate found defects that 100+ green unit tests coexisted with: the StorageRateMs=0 silent-total-provisioning-failure, Int1 server-degeneracy, the `ConfigureAwait(false)`/`Self` actor kill, dual-node double-capture, the 1007→Good truncation, and the plain-`Write`/borrow-path gaps in the original S-1 brief. The offline suites are strong, but the pattern is consistent: mapping seams and fire-and-forget boundaries only reveal themselves against the real peer.
+2. **"Fixed for the reply-failure variant, not the throw variant" is the residual failure mode.** S-12 is exactly this: the fail-closed advise handles a non-Ok *reply* but not a *thrown* invoke, leaving a poisoned cache. When hardening any request/reply seam here, enumerate both failure shapes (and cancellation) explicitly — the house's one-fault-variant trap.
+3. **Built-but-never-wired is down to one instance — but it's on its third review.** The provisioner (PR #423), the sink forwarding (F10b), and the resilience tracker (R2-10) all got readers or guards; `GetHealthSnapshot`/`RefreshConnectionStateAsync` (S-6) is the last holdout, and this pass established the *entire snapshot surface* is unread. Wire it or delete it; either way pin it with a registration guard.
+4. **Silent-partial-binding is the next #541.** The skipped-tag ledger closed the "N authored, fewer bound, nothing says why" gap at the artifact boundary — but S-13 shows the same silence one layer deeper (bound-but-never-publishing on a duplicate attributeRef). When a first-wins/last-wins tiebreak decides which of two authored things works, the loser must be logged and ledgered.
+5. **Gateway-gap workarounds still accrue client-side** — wire-side `maxEvents`, server-side event source filter, batched replay, batched `SendEvents`, and the WriteComplete correlation's completion-wait timeout (the `galaxy.writes.unconfirmed` residual). All are known and metered/documented here; they belong on the sister repos' backlogs with links from the in-source comments.
diff --git a/archreview/07-client-tooling-engineering.md b/archreview/07-client-tooling-engineering.md
index 3bd99eb8..8dfb0416 100644
--- a/archreview/07-client-tooling-engineering.md
+++ b/archreview/07-client-tooling-engineering.md
@@ -2,54 +2,57 @@
| | |
|---|---|
-| **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 |
+| **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 (9cad9ed0 → f6eaa267)
+## Prior-finding status (f6eaa267 → 12b0035a)
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.
+**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-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-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`. |
-| 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. |
+| 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. |
-| 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. |
+| 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:** 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).
+**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`).
---
@@ -57,66 +60,51 @@ unit leg; the dispatch-gap pragmas are genuinely gone, not relocated).
### 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) implements
- `IOpcUaClientService` behind five adapter seams (`IApplicationConfigurationFactory`,
- `IEndpointDiscovery`, `ISessionFactory`, `ISessionAdapter`, `ISubscriptionAdapter` under
- `Adapters/`) 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. `CommandBase` centralises connection options and (still) swaps the global
- Serilog `Log.Logger` per 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).
+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
-`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).
+**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
+### Test architecture map (grew substantially)
-47 test projects (verified: `git ls-files 'tests/**/*.csproj' | wc -l` = 47) mirror `src/` under
-`tests//` in three tiers:
+**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`** — pure unit suites, 40 projects; xunit.v3 + Shouldly except three xunit 2.9.2
- holdouts (`AdminUI.Tests`, `ControlPlane.Tests`, `Runtime.Tests`).
-2. **`*.IntegrationTests`** — 10 projects, probe-once collection fixtures defaulting to the
- shared Docker host `10.100.0.35`, `Assert.Skip` when unreachable.
-3. **`Category=LiveIntegration`** — env-gated live suites (skip-clean offline).
-4. **`Category=E2E`** — still reserved; **still zero tests carry it** (S-3).
+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 (changed since 2026-07-08)
+### Build / CI pipeline shape
-- `Directory.Build.props` — global net10.0/nullable/implicit-usings/latest, the CVE-2025-6965
- `NuGetAuditSuppress` carve-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 by `10b89830`): `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). **New** `scripts/ci/assert-not-all-skipped.sh` parses trx
- `` 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
- Wonderware `ServerHistorian` keys 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.
+- `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).
---
@@ -125,228 +113,225 @@ C-7).
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.
+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-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) — 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-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):
-- `SubscribeAsync` checks/creates the shared `_dataSubscription` outside the lock
- (`OpcUaClientService.cs:262`) — two concurrent first-subscribers can leak an SDK subscription.
-- `RunFailoverAsync` nulls `_dataSubscription`/`_alarmSubscription` without the lock
- (lines 695-696), contradicting `DisconnectAsync`'s stated assumption (lines 138-140).
-- `UnsubscribeAlarmsAsync` does 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
-in `RunFailoverAsync`, route alarm-unsubscribe through the semaphore.
+#### 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` (`Log.CloseAndFlush()`) + `:133` (static `Log.Logger`
-replacement) per command execution. Race for parallel test collections; hostile to embedding.
-**Recommendation:** per-execution instance `ILogger`.
+Unchanged: `CommandBase.cs:123,133`. Per-execution instance `ILogger` remains the fix.
-#### 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-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`). Document or create per-interval subscriptions.
+Unchanged (`OpcUaClientService.cs:262`).
-#### 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.
+#### 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 (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-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`; `SubscribeCommand` serial tree walk). Acceptable for
-a diagnostic tool.
+Unchanged (`OpcUaClientService.cs:230-231`). 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-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 path is allocation-sane
-Unchanged; the unbounded channel remains a cosmetic cap-it-later.
+#### P-4 (Low) — CLI subscription output channel unbounded
+Unchanged; cosmetic.
-#### 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.
+#### 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 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-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) — 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-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) — 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-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) — 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-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; strip `TargetFramework`/`Nullable`/`ImplicitUsings` restatements on next sweep.
+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, 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).
+#### 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 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-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-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-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
-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".
+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 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-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-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.
+#### 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-08 | 2026-07-12 | Justification |
+| Dimension | 2026-07-12 | 2026-08-19 | 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. |
+| 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. 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.
-2. Add the five missing alarm-command sections to `docs/Client.CLI.md` (U-3).
-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-leg `MIN_EXECUTED` floor above 1 (S-9).
-4. Add TWE to the four Client/Tooling src projects (C-2) — this is also what gives OTOPCUA0001 build-breaking teeth there (C-7).
-5. Add `global.json` and fix the workflow comment (S-7); update the stale STATUS.md topology claims (U-9).
-6. Fix the three `OpcUaClientService` lock-discipline gaps before Client.UI grows long-lived multi-thread usage (S-5); land `Host.Tests` (U-2).
-7. 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).
+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).
diff --git a/archreview/plans/STATUS.md b/archreview/plans/STATUS.md
index f728d943..9944f1fb 100644
--- a/archreview/plans/STATUS.md
+++ b/archreview/plans/STATUS.md
@@ -464,3 +464,38 @@ The R2-06 **code** deliverables (S-11 fail-fast + `ServerHistorianOptionsValidat
upsert; C-7 doc) were already unit-covered + merged; this live gate additionally **found + fixed the StorageRateMs
provisioning bug**, **answered the U-7 retype policy**, and surfaced the Int1 histsdk gap that drove the
**Boolean→Int2 remap (issue #441)** — closing the last red test to reach **6/6**.
+
+---
+
+## Re-review 2026-08-19 (round 3, master `12b0035a`) — outcome + new findings
+
+All seven domain reports and [`../00-OVERALL.md`](../00-OVERALL.md) were **rewritten in place** at
+`12b0035a` (~1,274 commits after `f6eaa267`); each carries a `Prior-finding status (f6eaa267 → 12b0035a)`
+table. Every round-2 finding was re-verified against code, not against this file.
+
+**Round-2 outcome:** every top-nine round-2 finding and every carried round-1 High is **CLOSED in code**
+(05/STAB-14/15 via R2-01 + PR #453; 01/S-6/U-6/S-8 via R2-02; 02/S13/S12/P7 via R2-03; 06/S-11/U-7 via
+R2-06 + #441; 04/C-7 + 04/U-5 + 03/U10 via R2-10; 04/C-1 via R2-05 and held under 177 commits by the
+reflection guard; 03/S2 superseded by #520, 03/S3 via R2-08, 03/P1 via surgical deploys; the
+failure-visibility trio 01/S-1, 03/S4, 06/S-1 all closed). Across the seven tables: **73 closed / 31
+partial / 97 carried** of ~201 round-2 findings. Matrix: 19 of 28 cells rose, 8 held, one fell
+(03 Underdeveloped 4 → 3.5).
+
+**New round-3 Critical/High (all in the window's new code):**
+
+| ID | Sev | One-liner | Where |
+|---|---|---|---|
+| 03/S14 | **Critical** | AdminUI cookie `FallbackPolicy` applies to every mesh gRPC map (no `.AllowAnonymous()`); on a login-enabled admin node artifact fetch / `ReportApply` / telemetry / LocalDb sync all 401 before the PSK interceptor. Masked on every rig gate by `DisableLogin=true`. | `Program.cs:757-783` vs `:728`; `Security/ServiceCollectionExtensions.cs:143-148` |
+| 03/S15 | High | Driver comms loss never marks raw/UNS **value variables** Bad — external clients hold stale-Good for the whole outage | `DriverHostActor.cs:1248-1283, 1334-1359` |
+| 05/STAB-19 | High | Calculation launders dependency quality to Good (`statusCode` never read; adapter hardcodes `0u`) | `CalculationDriver.cs:330-352`, `DependencyConsumerMuxAdapter.cs:71-72` |
+| 05/STAB-18 | High | MQTT has no `forceRaise` analogue — a ref latched Bad after reconnect/rebuild recovers only when the plant value next changes | `MqttSubscriptionManager.cs:325-365, 762-767` |
+| 03/S16 | High | Operator "Restart driver" respawns the child without re-sending desired subscriptions (`_lastComposition` write-only) → dead driver until next deploy | `DriverHostActor.cs:2916-2932, 2660-2672` |
+
+Carried Highs: 02/U1 (dormant `Core.VirtualTags` engine, third round — needs a decision) and 02/P1
+(BCL reference set rebuilt per compile). Two-reviewer Mediums: 01/S-12 = 03/C5 (`OTOPCUA_ROLES` vs
+`Cluster:Roles` divergence), 01/U-8 = 03/P6 (raw announce gap). Full ranking, the seven cross-cutting
+themes (headline: the **stale-Good family** at four seams, and the rig's `DisableLogin` blind spot),
+and the 12-item action list are in `00-OVERALL.md`.
+
+**Round-3 plans: NOT yet written.** Next step is one plan per action-list item (R3-01…), house format,
+as round 2 did.