Merge R2-09 Driver fleet batch (arch-review round 2) [PR #437]

Findings 05/STAB-3/4/5/8/12/16/17 + CONV-4 + onError (6 sub-batches, 29 tasks):
Modbus desync teardown, AbCip runtime lock, FOCAS concurrent caches, new
ConnectionBackoff + PollGroupEngine v2 (S7 poll fork deleted onto it, reconciled
with R2-01), fleet connect throttles, ResolveHost via resolver, TwinCAT replay
hardening. 29/29 tasks. S7 254/254 verified post-merge; build clean.

archreview/plans/R2-09-driver-fleet-batch-plan.md

@@ -569,3 +569,19 @@ Rules: tasks are bite-sized (≤ ~5 min of implementation each — TDD steps spl
 **Overall effort:** ≈ 5.5–7 dev-days across 6 independently shippable PRs (B1/B2/B5/B6 parallelizable from day one; B4 unblocks after B3's first two tasks).
 
 **Explicit hand-offs:** `ConnectionBackoff` → **R2-01** (S7 `EnsureConnectedAsync` throttle + the STAB-14/15 wrapper/classification fixes; B3.6's fork deletion removes the poll-loop half of STAB-14's blast surface in whichever order the two plans land). Parser strictness / `ReadEnum` / remaining equipment-parser parity → **R2-11**. Carried and untouched here: STAB-10, STAB-11, full STAB-12 (FOCAS close-PDU + TwinCAT blocking `Connect`), CONV-3 (`ILoggerFactory` — worth an early ride-along in any of these PRs per the report, but owned elsewhere), PERF-1/2, CONV-5 idioms.
+
+---
+
+## Execution deviations (R2-09)
+
+Executed on branch `r2/09-driver-fleet-batch` off master `1a698cbb` (all 29 tasks completed; none deferred-live). Deviations from the plan, each a best-reversible call:
+
+- **Single branch, no PRs/merges.** The executor owns one isolated branch and never pushes/PRs/merges (house rule). The plan's per-sub-batch PR boundaries are collapsed onto this one branch; every "merge PR N" verification task was satisfied by running that sub-batch's driver unit suite(s) green and committing a status marker. The branch is ready for the parent to slice into PRs or merge as it sees fit.
+- **Pre-existing unrelated failure (not R2-09).** `Core.Abstractions.Tests.InterfaceIndependenceTests.AllPublicTypes_LiveInRootNamespace` fails at base `1a698cbb` — it flags the pre-existing `Historian/` sub-namespace types (`HistorizationCommitMode`, `IHistorianValueWriter`, …), none of which this plan touched. The one new public type added here, `ConnectionBackoff`, correctly lives in the root `ZB.MOM.WW.OtOpcUa.Core.Abstractions` namespace. Left as-is (out of scope).
+- **B3.6 stale handle-prefix lock-in test.** `S7DiscoveryAndSubscribeTests.SubscribeAsync_returns_unique_handles…` asserted the retired fork's `s7-sub-` `DiagnosticId` prefix. The fork retirement moves S7 onto `PollGroupEngine`, whose handle is `poll-sub-<id>`; the assertion was updated to `poll-sub-` (a stale lock-in, per the watch-for-stale-tests note).
+- **B3.5 test 3 reframed.** `SustainedPollFailure_DegradesHealth_WithBackoff` was landed as `SustainedPollFailure_DegradesHealth` — S7's `ReadAsync` already degrades health on comm failure (independent of `onError`), so a sustained-failing poll degrades health on both fork and engine; the timing-sensitive "WithBackoff" cadence assertion is covered deterministically by the engine-level `Backoff_*` tests (B3.2). S7's new `HandlePollError` health wiring is exercised by the migrated poll path.
+- **FOCAS `ResolveHost` test initializes the driver.** FOCAS resolves equipment refs via `ResolveEquipmentTagRef`, which validates against the `_devices` map populated at `InitializeAsync` (the other three multi-device drivers parse directly, no init needed). The FOCAS `ResolveHostTests` init the driver — matching how `ResolveHost` is always called at runtime (post-init).
+- **FOCAS connect backoff is best-effort.** FOCAS's `EnsureConnectedAsync` is not gate-serialized (its read/write/probe/fixed-tree/recycle loops already race the client swap by design), so the per-device `ConnectionBackoff` there is consulted/mutated without a gate — atomic enough on 64-bit for its int/DateTime state, with a benign extra/skipped attempt the worst outcome of a race. Documented in the field's XML doc. (TwinCAT/AbCip consult it under their existing gate / probe discipline.)
+- **B6.5 is a white-box reflection test.** `RecycleClient_HonorsCancellation` invokes the private `RecycleClientAsync(device, ct)` with a cancelled token (via reflection, with the device's `ConnectGate` held) and asserts a prompt `OperationCanceledException` rather than an uncancellable hang. A full behavioral probe-loop wedge is impractical: in the single-probe-loop structure the loop blocks in the *already-cancellable* `EnsureConnectedAsync`, not `RecycleClientAsync`; the blocking-SDK-`Connect` root that makes the real wedge reproducible is carried under STAB-12 (out of scope). The reflection test deterministically pins the `None`→`ct` fix.
+- **Live gates not exercised (offline macOS).** The optional/recommended live gates (B1.3 modbus `exception_injector`, B3.7 s7 fixture bounce) were left offline-unit-verified — every task's core behavior is fully covered by the per-driver unit suites (the authoritative gate for TwinCAT/FOCAS/AbLegacy, which have no fixture at all). No task was blocked on a fixture, so none is marked `deferred-live`.
+- **Warning hygiene.** The five per-driver `PollErrorHealthTests` guard `DriverHealth.LastError` (nullable) with an explicit `ShouldNotBeNull()` before `ShouldContain` to clear a CS8604 nullable-flow warning.

archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json

@@ -3,39 +3,231 @@
   "lastUpdated": "2026-07-12",
   "notes": "Sub-batches B1/B2/B5/B6 are mutually independent PRs; B4 depends on B3 tasks B3.1-B3.2. Ordering inside a sub-batch is encoded via blockedBy. Concurrency tasks are high-risk (see plan Classification fields).",
   "tasks": [
-    { "id": "B1.1", "subject": "B1: failing Modbus transport-desync tests (stall / wrong TxId / truncated header / caller-cancel pin) in ModbusTcpReconnectTests", "status": "pending", "blockedBy": [] },
-    { "id": "B1.2", "subject": "B1: ModbusTransportDesyncException + teardown-before-propagate in SendOnceAsync (timeout-OCE + InvalidDataException) — STAB-3", "status": "pending", "blockedBy": ["B1.1"] },
-    { "id": "B1.3", "subject": "B1: full Modbus suites green + optional exception_injector live gate + merge PR 1", "status": "pending", "blockedBy": ["B1.2"] },
-
-    { "id": "B2.1", "subject": "B2: failing AbCipRuntimeConcurrencyTests (reentrancy-guarded FakeAbCipTag hammer) — STAB-4 repro", "status": "pending", "blockedBy": [] },
-    { "id": "B2.2", "subject": "B2: port _runtimeLocks + GetRuntimeLock + dispose onto AbCip DeviceState (AbLegacy pattern)", "status": "pending", "blockedBy": ["B2.1"] },
-    { "id": "B2.3", "subject": "B2: wrap ReadSingleAsync/ReadGroupAsync/WriteAsync critical sections in the per-runtime lock; AbCip suite green", "status": "pending", "blockedBy": ["B2.2"] },
-    { "id": "B2.4", "subject": "B2: failing FocasFixedTreeConcurrencyTests (writer-vs-reader stress on the four DeviceState caches) — STAB-5 repro", "status": "pending", "blockedBy": [] },
-    { "id": "B2.5", "subject": "B2: convert the four FOCAS caches to ConcurrentDictionary; FOCAS suite green; merge PR 2", "status": "pending", "blockedBy": ["B2.3", "B2.4"] },
-
-    { "id": "B3.1", "subject": "B3: ConnectionBackoff primitive in Core.Abstractions (static ComputeDelay ported from S7 + instance attempt-throttle) + ConnectionBackoffTests — STAB-8 seam, hand-off to R2-01", "status": "pending", "blockedBy": [] },
-    { "id": "B3.2", "subject": "B3: PollGroupEngine v2 — backoffCap + consecutiveFailures + caller-token-filtered OCE catches (STAB-14-class engine guard) + engine tests FAIL->PASS", "status": "pending", "blockedBy": ["B3.1"] },
-    { "id": "B3.3", "subject": "B3: wire onError->health + 30s backoffCap into Modbus/AbCip/AbLegacy engine ctors (tests first per driver) — STAB-9", "status": "pending", "blockedBy": ["B3.2"] },
-    { "id": "B3.4", "subject": "B3: wire onError->health + 30s backoffCap into TwinCAT/FOCAS engine ctors (tests first per driver) — STAB-9", "status": "pending", "blockedBy": ["B3.2"] },
-    { "id": "B3.5", "subject": "B3: failing S7PollEngineMigrationTests (array-cadence PERF-3, rapid sub/unsub STAB-6, sustained-failure health+backoff)", "status": "pending", "blockedBy": ["B3.2"] },
-    { "id": "B3.6", "subject": "B3: retire the S7 poll fork onto PollGroupEngine (delete PollLoopAsync/PollOnceAsync/HandlePollFailure/SubscriptionState; ComputeBackoffDelay -> ConnectionBackoff) — CONV-1; cross-ref R2-01", "status": "pending", "blockedBy": ["B3.3", "B3.4", "B3.5"] },
-    { "id": "B3.7", "subject": "B3: engine + all five driver suites green; s7 fixture bounce live gate; merge PR 3", "status": "pending", "blockedBy": ["B3.6"] },
-
-    { "id": "B4.1", "subject": "B4: TwinCAT per-device connect-attempt backoff (data-path gate, probe bypass + instant reset) tests FAIL->PASS — STAB-8", "status": "pending", "blockedBy": ["B3.2"] },
-    { "id": "B4.2", "subject": "B4: FOCAS per-device connect-attempt backoff across the three loops, tests FAIL->PASS — STAB-8", "status": "pending", "blockedBy": ["B3.2"] },
-    { "id": "B4.3", "subject": "B4: AbCip evict-recreate (Forward Open) create-path throttle, cache-hit unaffected, tests FAIL->PASS — STAB-8", "status": "pending", "blockedBy": ["B3.2"] },
-    { "id": "B4.4", "subject": "B4: S7 seam comment pointing R2-01 at ConnectionBackoff; suites green; merge PR 4", "status": "pending", "blockedBy": ["B4.1", "B4.2", "B4.3"] },
-
-    { "id": "B5.1", "subject": "B5: failing ResolveHost tests in AbCip/AbLegacy/TwinCAT/FOCAS (equipment ref for device B must key B; fallback regression pins) — CONV-4 repro", "status": "pending", "blockedBy": [] },
-    { "id": "B5.2", "subject": "B5: rewrite the four multi-device ResolveHost bodies via EquipmentTagRefResolver + empty-host guard", "status": "pending", "blockedBy": ["B5.1"] },
-    { "id": "B5.3", "subject": "B5: Modbus — optional unitId in ModbusEquipmentTagParser + ResolveHost via resolver (per-unit breaker key); scope-note vs R2-11", "status": "pending", "blockedBy": ["B5.1"] },
-    { "id": "B5.4", "subject": "B5: all five driver suites green; merge PR 5", "status": "pending", "blockedBy": ["B5.2", "B5.3"] },
-
-    { "id": "B6.1", "subject": "B6: failing ReplayFailure_EmitsBadQuality_AndDegradesHealth in TwinCATReconnectReplayTests — STAB-16 repro", "status": "pending", "blockedBy": [] },
-    { "id": "B6.2", "subject": "B6: NativeRegistration HasLiveHandle/MarkHandleDead/OnUnavailable + replay-catch Bad-quality emit + health degrade — STAB-16", "status": "pending", "blockedBy": ["B6.1"] },
-    { "id": "B6.3", "subject": "B6: RetryDeadRegistrationsAsync on successful probe tick (under ConnectGate), tests FAIL->PASS — STAB-16", "status": "pending", "blockedBy": ["B6.2"] },
-    { "id": "B6.4", "subject": "B6: post-SwapHandle ownership re-check retracts the fresh ADS notification when unsubscribe raced the replay, tests FAIL->PASS — STAB-17", "status": "pending", "blockedBy": ["B6.3"] },
-    { "id": "B6.5", "subject": "B6: RecycleClientAsync takes the probe ct (no CancellationToken.None gate wait) + in-catch OCE wrap, tests FAIL->PASS — STAB-12 compounded part", "status": "pending", "blockedBy": ["B6.4"] },
-    { "id": "B6.6", "subject": "B6: full TwinCAT suite green (fake-client authoritative, no TC3 fixture); merge PR 6", "status": "pending", "blockedBy": ["B6.5"] }
+    {
+      "id": "B1.1",
+      "subject": "B1: failing Modbus transport-desync tests (stall / wrong TxId / truncated header / caller-cancel pin) in ModbusTcpReconnectTests",
+      "status": "completed",
+      "blockedBy": []
+    },
+    {
+      "id": "B1.2",
+      "subject": "B1: ModbusTransportDesyncException + teardown-before-propagate in SendOnceAsync (timeout-OCE + InvalidDataException) \u2014 STAB-3",
+      "status": "completed",
+      "blockedBy": [
+        "B1.1"
+      ]
+    },
+    {
+      "id": "B1.3",
+      "subject": "B1: full Modbus suites green + optional exception_injector live gate + merge PR 1",
+      "status": "completed",
+      "blockedBy": [
+        "B1.2"
+      ]
+    },
+    {
+      "id": "B2.1",
+      "subject": "B2: failing AbCipRuntimeConcurrencyTests (reentrancy-guarded FakeAbCipTag hammer) \u2014 STAB-4 repro",
+      "status": "completed",
+      "blockedBy": []
+    },
+    {
+      "id": "B2.2",
+      "subject": "B2: port _runtimeLocks + GetRuntimeLock + dispose onto AbCip DeviceState (AbLegacy pattern)",
+      "status": "completed",
+      "blockedBy": [
+        "B2.1"
+      ]
+    },
+    {
+      "id": "B2.3",
+      "subject": "B2: wrap ReadSingleAsync/ReadGroupAsync/WriteAsync critical sections in the per-runtime lock; AbCip suite green",
+      "status": "completed",
+      "blockedBy": [
+        "B2.2"
+      ]
+    },
+    {
+      "id": "B2.4",
+      "subject": "B2: failing FocasFixedTreeConcurrencyTests (writer-vs-reader stress on the four DeviceState caches) \u2014 STAB-5 repro",
+      "status": "completed",
+      "blockedBy": []
+    },
+    {
+      "id": "B2.5",
+      "subject": "B2: convert the four FOCAS caches to ConcurrentDictionary; FOCAS suite green; merge PR 2",
+      "status": "completed",
+      "blockedBy": [
+        "B2.3",
+        "B2.4"
+      ]
+    },
+    {
+      "id": "B3.1",
+      "subject": "B3: ConnectionBackoff primitive in Core.Abstractions (static ComputeDelay ported from S7 + instance attempt-throttle) + ConnectionBackoffTests \u2014 STAB-8 seam, hand-off to R2-01",
+      "status": "completed",
+      "blockedBy": []
+    },
+    {
+      "id": "B3.2",
+      "subject": "B3: PollGroupEngine v2 \u2014 backoffCap + consecutiveFailures + caller-token-filtered OCE catches (STAB-14-class engine guard) + engine tests FAIL->PASS",
+      "status": "completed",
+      "blockedBy": [
+        "B3.1"
+      ]
+    },
+    {
+      "id": "B3.3",
+      "subject": "B3: wire onError->health + 30s backoffCap into Modbus/AbCip/AbLegacy engine ctors (tests first per driver) \u2014 STAB-9",
+      "status": "completed",
+      "blockedBy": [
+        "B3.2"
+      ]
+    },
+    {
+      "id": "B3.4",
+      "subject": "B3: wire onError->health + 30s backoffCap into TwinCAT/FOCAS engine ctors (tests first per driver) \u2014 STAB-9",
+      "status": "completed",
+      "blockedBy": [
+        "B3.2"
+      ]
+    },
+    {
+      "id": "B3.5",
+      "subject": "B3: failing S7PollEngineMigrationTests (array-cadence PERF-3, rapid sub/unsub STAB-6, sustained-failure health+backoff)",
+      "status": "completed",
+      "blockedBy": [
+        "B3.2"
+      ]
+    },
+    {
+      "id": "B3.6",
+      "subject": "B3: retire the S7 poll fork onto PollGroupEngine (delete PollLoopAsync/PollOnceAsync/HandlePollFailure/SubscriptionState; ComputeBackoffDelay -> ConnectionBackoff) \u2014 CONV-1; cross-ref R2-01",
+      "status": "completed",
+      "blockedBy": [
+        "B3.3",
+        "B3.4",
+        "B3.5"
+      ]
+    },
+    {
+      "id": "B3.7",
+      "subject": "B3: engine + all five driver suites green; s7 fixture bounce live gate; merge PR 3",
+      "status": "completed",
+      "blockedBy": [
+        "B3.6"
+      ]
+    },
+    {
+      "id": "B4.1",
+      "subject": "B4: TwinCAT per-device connect-attempt backoff (data-path gate, probe bypass + instant reset) tests FAIL->PASS \u2014 STAB-8",
+      "status": "completed",
+      "blockedBy": [
+        "B3.2"
+      ]
+    },
+    {
+      "id": "B4.2",
+      "subject": "B4: FOCAS per-device connect-attempt backoff across the three loops, tests FAIL->PASS \u2014 STAB-8",
+      "status": "completed",
+      "blockedBy": [
+        "B3.2"
+      ]
+    },
+    {
+      "id": "B4.3",
+      "subject": "B4: AbCip evict-recreate (Forward Open) create-path throttle, cache-hit unaffected, tests FAIL->PASS \u2014 STAB-8",
+      "status": "completed",
+      "blockedBy": [
+        "B3.2"
+      ]
+    },
+    {
+      "id": "B4.4",
+      "subject": "B4: S7 seam comment pointing R2-01 at ConnectionBackoff; suites green; merge PR 4",
+      "status": "completed",
+      "blockedBy": [
+        "B4.1",
+        "B4.2",
+        "B4.3"
+      ]
+    },
+    {
+      "id": "B5.1",
+      "subject": "B5: failing ResolveHost tests in AbCip/AbLegacy/TwinCAT/FOCAS (equipment ref for device B must key B; fallback regression pins) \u2014 CONV-4 repro",
+      "status": "completed",
+      "blockedBy": []
+    },
+    {
+      "id": "B5.2",
+      "subject": "B5: rewrite the four multi-device ResolveHost bodies via EquipmentTagRefResolver + empty-host guard",
+      "status": "completed",
+      "blockedBy": [
+        "B5.1"
+      ]
+    },
+    {
+      "id": "B5.3",
+      "subject": "B5: Modbus \u2014 optional unitId in ModbusEquipmentTagParser + ResolveHost via resolver (per-unit breaker key); scope-note vs R2-11",
+      "status": "completed",
+      "blockedBy": [
+        "B5.1"
+      ]
+    },
+    {
+      "id": "B5.4",
+      "subject": "B5: all five driver suites green; merge PR 5",
+      "status": "completed",
+      "blockedBy": [
+        "B5.2",
+        "B5.3"
+      ]
+    },
+    {
+      "id": "B6.1",
+      "subject": "B6: failing ReplayFailure_EmitsBadQuality_AndDegradesHealth in TwinCATReconnectReplayTests \u2014 STAB-16 repro",
+      "status": "completed",
+      "blockedBy": []
+    },
+    {
+      "id": "B6.2",
+      "subject": "B6: NativeRegistration HasLiveHandle/MarkHandleDead/OnUnavailable + replay-catch Bad-quality emit + health degrade \u2014 STAB-16",
+      "status": "completed",
+      "blockedBy": [
+        "B6.1"
+      ]
+    },
+    {
+      "id": "B6.3",
+      "subject": "B6: RetryDeadRegistrationsAsync on successful probe tick (under ConnectGate), tests FAIL->PASS \u2014 STAB-16",
+      "status": "completed",
+      "blockedBy": [
+        "B6.2"
+      ]
+    },
+    {
+      "id": "B6.4",
+      "subject": "B6: post-SwapHandle ownership re-check retracts the fresh ADS notification when unsubscribe raced the replay, tests FAIL->PASS \u2014 STAB-17",
+      "status": "completed",
+      "blockedBy": [
+        "B6.3"
+      ]
+    },
+    {
+      "id": "B6.5",
+      "subject": "B6: RecycleClientAsync takes the probe ct (no CancellationToken.None gate wait) + in-catch OCE wrap, tests FAIL->PASS \u2014 STAB-12 compounded part",
+      "status": "completed",
+      "blockedBy": [
+        "B6.4"
+      ]
+    },
+    {
+      "id": "B6.6",
+      "subject": "B6: full TwinCAT suite green (fake-client authoritative, no TC3 fixture); merge PR 6",
+      "status": "completed",
+      "blockedBy": [
+        "B6.5"
+      ]
+    }
   ]
-}
+}