docs(code-reviews): regenerate index — 24 Low findings resolved

Batch 4 cleared Open findings in Driver.TwinCAT, Driver.Modbus, Driver.OpcUaClient, Driver.Historian.Wonderware, and Driver.Modbus.Addressing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(driver-modbus-addressing): resolve Low code-review findings (Driver.Modbus.Addressing-006,007,009)
2026-05-23 08:18:21 -04:00 · 2026-05-23 08:18:15 -04:00 · 2026-05-23 08:18:10 -04:00 · 2026-05-23 08:17:55 -04:00 · 2026-05-23 08:17:51 -04:00 · 2026-05-23 08:17:42 -04:00
42 changed files with 2440 additions and 277 deletions
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 7 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -115,7 +115,7 @@ analog/integer tags.
 | Severity | Low |
 | Category | Correctness and logic bugs |
 | Location | `Backend/SdkAlarmHistorianWriteBackend.cs:198-201` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `ToHistorianEvent` only assigns `historianEvent.Id` when
 `Guid.TryParse(dto.EventId, ...)` succeeds. If `EventId` is not a parseable GUID
@@ -128,7 +128,7 @@ The non-parseable case is never logged.
 the event as `PermanentFail` (malformed input) or synthesize a fresh
 `Guid.NewGuid()` so each event still gets a unique id.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `ToHistorianEvent` now synthesizes a fresh `Guid.NewGuid()` when the dto's `EventId` fails `Guid.TryParse`, and logs a warning carrying both the original (unparseable) id and the synthesized id so collisions stop happening silently. Regression tests `ToHistorianEvent_parseable_event_id_is_used_verbatim` and `ToHistorianEvent_unparseable_event_id_synthesizes_unique_non_empty_Guid` in `SdkAlarmHistorianWriteBackendTests`.
 ### Driver.Historian.Wonderware-005
@@ -137,7 +137,7 @@ the event as `PermanentFail` (malformed input) or synthesize a fresh
 | Severity | Low |
 | Category | Concurrency and thread safety |
 | Location | `Backend/HistorianDataSource.cs:124`, `:126-127` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `GetHealthSnapshot` reads `_activeProcessNode` and
 `_activeEventNode` inside `_healthLock`, but those two fields are written under
@@ -152,7 +152,7 @@ a momentarily inconsistent health snapshot.
 `_healthLock` on every connection state change, or read them under the connection
 lock), so the snapshot is internally consistent.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `GetHealthSnapshot` now derives the `ProcessConnectionOpen` / `EventConnectionOpen` booleans from the active-node strings (`_activeProcessNode != null` / `_activeEventNode != null`) which all live under `_healthLock`, instead of reading `_connection`/`_eventConnection` via `Volatile.Read` outside the lock those fields are published under. The snapshot is now self-consistent by construction: open ↔ active node populated. Regression tests in `HistorianDataSourceHealthSnapshotTests` cover the three half-published states plus the steady-state cases.
 ### Driver.Historian.Wonderware-006
@@ -184,7 +184,7 @@ restart the sidecar cleanly.
 | Severity | Low |
 | Category | Error handling and resilience |
 | Location | `Ipc/PipeServer.cs:70-75` |
-| Status | Open |
+| Status | Resolved |
 **Description:** When `VerifyCaller` rejects the peer SID, the server logs the
 reason and calls `_current.Disconnect()` with no `HelloAck` frame sent. The
@@ -198,7 +198,7 @@ harder to test from the client.
 `caller-sid-mismatch` reject reason before disconnecting, consistent with the
 other two rejection paths.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — the SID rejection path now writes a `HelloAck { Accepted=false, RejectReason="caller-sid-mismatch: ..." }` before disconnecting, symmetric with the shared-secret-mismatch and major-version-mismatch paths. The caller-verification function was also extracted into a `CallerVerifier` delegate so tests can override it (the pipe ACL would otherwise block the test client itself). End-to-end regression `PipeServerSidRejectTests.Caller_SID_mismatch_sends_HelloAck_with_reject_reason_before_disconnect` connects a real named-pipe client and asserts the rejecting ack frame arrives.
 ### Driver.Historian.Wonderware-008
@@ -207,7 +207,7 @@ other two rejection paths.
 | Severity | Low |
 | Category | Error handling and resilience |
 | Location | `Backend/HistorianDataSource.cs:301-307`, `:374-380` |
-| Status | Open |
+| Status | Resolved |
 **Description:** When `query.StartQuery` returns `false`, `ReadRawAsync` and
 `ReadAggregateAsync` call `HandleConnectionError()` and return an empty result
@@ -226,7 +226,7 @@ connection intact, surface the error). Consider returning a failed reply
 (`Success = false`) for query-class `StartQuery` failures so the client does not
 treat an SDK error as an empty history.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — extracted a static `ConnectionErrorCodes` set + `IsConnectionClassError` classifier (mirroring the alarm-write side) and centralised the failure handling in a new `HandleStartQueryFailure` helper. Connection-class codes still drop the connection and mark the node failed; query-class codes throw a new `QueryClassStartQueryException` that the outer catch re-throws WITHOUT touching the connection. All four read paths (raw / aggregate / at-time / events) also re-throw caught exceptions so the IPC frame handler surfaces `Success=false` instead of returning an empty list with `Success=true`. Regression tests `HistorianDataSourceStartQueryClassificationTests` pin the connection-class vs query-class classification per error code; the connect-failover suite (`HistorianDataSourceConnectFailoverTests`) verifies the read paths now propagate the exception.
 ### Driver.Historian.Wonderware-009
@@ -261,7 +261,7 @@ cap with an explicit error reply rather than letting `WriteAsync` throw.
 | Severity | Low |
 | Category | Performance and resource management |
 | Location | `Backend/HistorianConfiguration.cs:32-36`, `Backend/HistorianDataSource.cs` (all read methods) |
-| Status | Open |
+| Status | Resolved |
 **Description:** `HistorianConfiguration.RequestTimeoutSeconds` is documented as
 the "outer safety timeout applied to sync-over-async Historian operations" and is
@@ -278,7 +278,7 @@ timeout, but the query path does not). The documented safety net does not exist.
 worker with a bounded wait), or remove the property and its XML doc so the code
 does not advertise a guarantee it does not provide.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added an internal `BuildRequestCts` helper that returns a `CancellationTokenSource` linked into the caller's `ct` with `CancelAfter(RequestTimeoutSeconds)` applied when positive. Each read method (`ReadRawAsync`, `ReadAggregateAsync`, `ReadAtTimeAsync`, `ReadEventsAsync`) now wraps its work with the linked CTS and feeds the linked token into the `ThrowIfCancellationRequested` checks between `MoveNext` iterations, so a hung SDK call cancels at the configured deadline instead of blocking the connection thread indefinitely. Regression tests `HistorianDataSourceRequestTimeoutTests` pin the helper: positive value enforces `CancelAfter`, zero/negative means no timeout, caller cancellation propagates, default is 60s.
 ### Driver.Historian.Wonderware-011
@@ -287,7 +287,7 @@ does not advertise a guarantee it does not provide.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `Backend/HistorianDataSource.cs:9-12`, `Backend/IHistorianDataSource.cs:9-11`, `Backend/HistorianSample.cs:7-9`, `Backend/HistorianConfiguration.cs:7-9` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Several XML doc comments reference the retired v1 architecture as
 if it were current: "inside Galaxy.Host", "the Proxy maps returned samples", "the
@@ -303,7 +303,7 @@ review checklist.
 architecture (sidecar talking to `WonderwareHistorianClient` over the named pipe),
 dropping the `Galaxy.Host` / `Proxy` / `GalaxyDataValue` references.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — refreshed the XML doc comments on `HistorianDataSource`, `IHistorianDataSource`, `HistorianSample` / `HistorianAggregateSample`, and `HistorianConfiguration` to describe the current sidecar / named-pipe / .NET 10 `WonderwareHistorianClient` architecture. References to `Galaxy.Host` / `Galaxy.Proxy` / `GalaxyDataValue` are now framed as historical context tied to the PR 7.2 retirement rather than as current behaviour.
 ### Driver.Historian.Wonderware-012
@@ -312,7 +312,7 @@ dropping the `Galaxy.Host` / `Proxy` / `GalaxyDataValue` references.
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `Backend/HistorianDataSource.cs`, `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The unit-test suite covers `HistorianQualityMapper`,
 `HistorianClusterEndpointPicker`, `SdkAlarmHistorianWriteBackend`,
@@ -334,4 +334,4 @@ removed to avoid confusion.
 cancellation, and the value-type selection — and delete the stale empty
 `tests/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/` directory.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added four new `HistorianDataSource`-targeted test files: `HistorianDataSourceHealthSnapshotTests` (snapshot consistency under half-published state, see also -005), `HistorianDataSourceStartQueryClassificationTests` (connection-class vs query-class error-code table, see also -008), `HistorianDataSourceRequestTimeoutTests` (the request-timeout helper, see also -010), `HistorianDataSourceConnectFailoverTests` (cluster failover order + cooldown via the `IHistorianConnectionFactory` fake), and `HistorianDataSourceValueAndAggregateTests` (the string-vs-numeric heuristic via the new SDK-independent `SelectValueFromPair` overload + the `ExtractAggregateValue` column dispatch). Stale empty `tests/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/` directory deleted. Unit count rose from 80 to 125 (+45 new tests).
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 3 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -157,7 +157,7 @@ overwrite it.
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `ModbusAddressParser.cs:297-301` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `TryParseFamilyNative` catches only `ArgumentException` and `OverflowException`.
 The current helpers throw only those (including `ArgumentOutOfRangeException`, which derives from
@@ -171,7 +171,13 @@ depend on.
 narrow catch, or broaden to a general catch-all that records the message — a try-parse method
 should never throw.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — broadened the `catch` filter in
 `ModbusAddressParser.TryParseFamilyNative` from `ArgumentException or OverflowException` to a
 general `catch (Exception ex)` so any future helper exception type is converted to a structured
 `(false, error)` rather than escaping the `TryParse` method. Added `DL205_TryParse_NeverThrows`
 and `MELSEC_TryParse_NeverThrows` parameterised regression tests in
 `ModbusAddressEdgeCaseTests` covering ~20 pathological inputs (empty prefixes, octal/hex digit
 violations, overflow inputs, unknown prefixes) to pin the defensive contract.
 ### Driver.Modbus.Addressing-007
@@ -180,7 +186,7 @@ should never throw.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `ModbusDataType.cs:91-95`, `docs/v2/dl205.md` section Strings |
-| Status | Open |
+| Status | Resolved |
 **Description:** `ModbusStringByteOrder` (HighByteFirst / LowByteFirst) is defined in this
 assembly and documented as the DL205 low-byte-first string-packing knob, but `ParsedModbusAddress`
@@ -193,7 +199,18 @@ unreachable from the parser, so the grammar cannot represent a known, documented
 token for it, or document explicitly that DL205 string byte order is only configurable via the
 structured tag form and is intentionally out of grammar scope.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — chose the "document the limitation" branch of the
 recommendation rather than adding a grammar token: the 3rd field slot is the multi-register
 word/byte order and the 4th is the array count, so a 5th `:<order>` suffix would conflict with
 the existing count-shape disambiguation; `ModbusStringByteOrder` is already plumbed through the
 structured tag form (`ModbusDriverFactoryExtensions.ModbusTagDto.StringByteOrder` →
 `ModbusTagDefinition.StringByteOrder`) which is the canonical config path. Added an explicit
 "Grammar scope" remarks block to `ModbusStringByteOrder` and to the `ModbusAddressParser`
 `<remarks>` block stating that string byte order is configurable only via the structured tag
 form. Added a corresponding bullet to `docs/v2/dl205.md` §Strings. Added two regression tests
 (`Parser_STR_grammar_does_not_carry_StringByteOrder` reflecting on `ParsedModbusAddress`, and
 `Parser_rejects_unknown_string_byte_order_token_in_grammar`) pinning the contract so a future
 grammar change can't quietly add a conflicting token.
 ### Driver.Modbus.Addressing-008
@@ -226,7 +243,7 @@ finding -001.
 | Severity | Low |
 | Category | Documentation & comments |
 | Location | `ModbusModiconAddress.cs:55-64`, `ModbusModiconAddress.cs:104-110` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The comments on `ModbusModiconAddress.TryParse` are slightly inaccurate. The
 remark that 5-digit Modicon is always exactly 5 chars (40001..49999) and 6-digit is exactly 6
@@ -238,4 +255,11 @@ says the 5-digit form caps at 9999 by construction while the adjacent code path
 **Recommendation:** Reword the range examples to cover all four region digits and drop the
 caps-at-9999 aside or restate it as a precise statement about trailing-digit count.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — reworded the up-front range-check comment to describe all
 four region digits (0/1/3/4) and give examples covering each region (coils 00001..09999 /
 000001..065536, holding registers 40001..49999 / 400001..465536). Reworded the lower
 `> 65536` comment to drop the misleading "5-digit form caps at 9999 by construction" framing and
 state precisely that the check is reached only by the 6-digit form in practice, but applied to
 both for safety rather than relying on the digit-count invariant. Pure documentation change —
 no behavioural change; the existing `ModbusModiconAddressTests` already pin the cross-region
 5-digit ranges (00001..09999 / 10001..19999 / 30001..39999 / 40001..49999).
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 7 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -63,13 +63,13 @@
 | Severity | Low |
 | Category | Concurrency & thread safety |
 | Location | `ModbusDriver.cs:59,188,241,259,266,726,745,759` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `_health` is a non-`volatile` reference field written from multiple threads (concurrent `ReadAsync` callers, the coalesced-read path, `WriteAsync` indirectly, and `ProbeLoopAsync`) and read by `GetHealth()`. Reference assignment is atomic on .NET so a torn read cannot occur, but there is no happens-before ordering: a stale `DriverHealth` can be observed on another core, and concurrent writers race so "last write wins" is non-deterministic (a `Degraded` write from a failed read can clobber a just-published `Healthy`, or vice versa).
 **Recommendation:** Mark `_health` `volatile`, or assign via `Volatile.Write` and read with `Volatile.Read`, to give `GetHealth()` a defined ordering guarantee.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — routed every `_health` access through new `ReadHealth()` (`Volatile.Read`) and `WriteHealth()` (`Volatile.Write`) helpers, giving `GetHealth()` a defined ordering guarantee on every core. Stress-test (`ModbusLifecycleHygieneTests.GetHealth_under_concurrent_pressure_always_returns_a_complete_snapshot`) confirms the read path never sees a torn / half-constructed snapshot under concurrent reader + writer pressure.
 ### Driver.Modbus-004
@@ -123,13 +123,13 @@
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `ModbusDriver.cs:1392`, `ModbusDriverOptions.cs:74-80` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Two design-vs-code drifts. (1) `MapDataType` maps `Int64`/`UInt64` to `DriverDataType.Int32` with the inline comment "widening to Int32 loses precision; PR 25 adds Int64 to DriverDataType". The address-space node for a 64-bit Modbus tag is declared `Int32`, misrepresenting the OPC UA variable's `DataType` even though `DecodeRegister` produces a correct `long`/`ulong` value — clients see a type/value mismatch. (2) `DisableFC23` is documented and bound from JSON but is a confirmed no-op ("The driver does not currently emit FC23"). Both are acknowledged-but-unfinished items worth tracking.
 **Recommendation:** Track the PR 25 `DriverDataType.Int64` follow-up; until then document the Int32 surfacing limitation in `docs/v2/modbus-addressing.md` so operators configuring `I_64`/`UI_64` tags understand the node type. Mark `DisableFC23` clearly as reserved/unimplemented or gate it once FC23 ships.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — promoted the inline Int64/UInt64 caveat into a full `<remarks>` block on `MapDataType` calling out the surfacing limitation and the tracked follow-up, and rewrote the `DisableFC23` XML doc to flag the option as "Reserved / no-op" with a Driver.Modbus-007 tracking reference. (The cross-module doc update in `docs/v2/modbus-addressing.md` is out of scope for this module's edits — code is now self-documenting.)
 ### Driver.Modbus-008
@@ -138,13 +138,13 @@
 | Severity | Low |
 | Category | Documentation & comments |
 | Location | `ModbusDriver.cs:411-417,700-703,737-744` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Stale/misleading comments. (1) The `<summary>` block at `ModbusDriver.cs:411-417` says auto-prohibited ranges are "Cleared by ReinitializeAsync ... or by an explicit re-probe API (not yet shipped)" — the re-probe loop has shipped (#151, `ReprobeLoopAsync`), so the parenthetical is wrong. (2) The comment at `ModbusDriver.cs:700-703` ("On block-level failure mark every member Bad — caller's per-tag fallback won't re-try since handled-set already includes them; auto-split-on-failure is a follow-up") contradicts the actual `catch (ModbusException)` arm below it, which deliberately does not add members to `handled` and does defer to per-tag fallback (and auto-split has shipped via bisection). The empty `foreach (var (idx, _) in block.Members) { }` loop at `ModbusDriver.cs:737-744`, with only a comment body, is dead code from that superseded design.
 **Recommendation:** Update the two comments to match the shipped #148/#150/#151 behaviour and delete the empty `foreach` loop in the `catch (ModbusException)` arm.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — deleted the duplicate `<summary>` orphaned at the top of the prohibition block, rewrote the surviving one to credit the shipped #151 re-probe loop, replaced the "auto-split-on-failure is a follow-up" comment above the block loop with the actual #148/#150 behaviour (per-tag fallback + bisection), and removed the empty `foreach (var (idx, _) in block.Members) { }` plus its unused `status` local from the `catch (ModbusException)` arm.
 ### Driver.Modbus-009
@@ -153,13 +153,13 @@
 | Severity | Low |
 | Category | Correctness & logic bugs |
 | Location | `ModbusDriver.cs:1160-1167`, `ModbusTcpTransport.cs:94-95` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Two edge cases. (1) `RegisterCount` for `ModbusDataType.String` computes `(tag.StringLength + 1) / 2`; a tag configured with `StringLength = 0` yields a register count of 0, flowing into `ReadOneAsync` as `totalRegs = 0` and producing an FC03/FC04 with quantity 0 — a spec-illegal request the PLC rejects with exception 03. The factory does not reject `StringLength = 0` for String tags. (2) `EnableKeepAlive` casts `opts.Time.TotalSeconds`/`opts.Interval.TotalSeconds` to `int`; a sub-second configured `TimeSpan` (e.g. 500 ms) truncates to 0, which most OSes reject or interpret as "use default", silently defeating the configured keep-alive timing.
 **Recommendation:** Validate `StringLength >= 1` for `String` tags in `ModbusDriverFactoryExtensions.BuildTag`. For keep-alive, round up to a minimum of 1 second or validate the configured `TimeSpan` is a whole number of seconds.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added `ValidateStringLength` in `ModbusDriverFactoryExtensions.BuildTag` so String-typed tags with `StringLength < 1` throw at bind time with a clear diagnostic (both AddressString + structured DTO paths), and introduced `ModbusTcpTransport.ClampToWholeSeconds` which rounds the configured keep-alive `TimeSpan` up to a minimum of 1 second so sub-second values no longer truncate to 0. Regression coverage in `ModbusEdgeCaseValidationTests`.
 ### Driver.Modbus-010
@@ -168,13 +168,13 @@
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `ModbusDriver.cs:864-868`, `ModbusDriverOptions.cs:116-125` |
-| Status | Open |
+| Status | Resolved |
 **Description:** When `WriteOnChangeOnly` is enabled and `IsRedundantWrite` returns true, `WriteAsync` returns `WriteResult(0u)` (Good) without touching the wire. The suppression baseline (`_lastWrittenByRef`) is only invalidated by a *read* that returns a divergent value. If a driver instance has `WriteOnChangeOnly = true` but a tag is never subscribed/read (write-only setpoint), a value the operator believes was re-asserted is silently suppressed forever after the first write — no time- or count-based expiry exists. The option XML doc describes the read-invalidation path but does not warn about write-only tags.
 **Recommendation:** Document the write-only-tag caveat on the `WriteOnChangeOnly` option, or add an optional TTL to the suppression cache so a periodic re-write still reaches the PLC.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added a `<remarks>` block on `ModbusDriverOptions.WriteOnChangeOnly` that calls out the write-only-tag caveat explicitly: the cache is only invalidated by reads, so a tag that is never subscribed/polled stays suppressed forever after the first write. Operators choosing this option are directed to either subscribe every affected tag or leave `WriteOnChangeOnly = false`. Adding a TTL was considered but the safer option for an OPC UA driver is to make the limitation discoverable in the documentation surface (no behaviour change for existing deployments).
 ### Driver.Modbus-011
@@ -183,13 +183,13 @@
 | Severity | Low |
 | Category | Code organization & conventions |
 | Location | `ModbusDriver.cs:23-43,89-97,408-432` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Field and member declarations are interleaved with methods throughout `ModbusDriver`. `ResolveHost` (a public method) is the first member of the class, followed by `BuildSlaveHostName`, then a block of fields; `_lastPublishedByRef`/`_lastWrittenByRef` are declared after the constructor; `ProhibitionState`, `_autoProhibited`, and `_reprobeCts` are declared mid-file between `DecodeRegisterArray` and `RangeIsAutoProhibited`. There are also two near-identical `<summary>` blocks stacked back-to-back at `ModbusDriver.cs:411-423`. This hurts readability of a 1400-line file and makes the field inventory hard to audit (relevant to the thread-safety findings above).
 **Recommendation:** Group all instance fields at the top of the class, move nested types together, and remove the orphaned first `<summary>` at lines 411-417 that no longer precedes a member.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — reorganized `ModbusDriver` so every instance field (including the `_autoProhibited` / `_autoProhibitedLock` / `_reprobeCts` / `_rmwLocks` / `_lastPublishedByRef` / `_lastWrittenByRef` fields that were declared mid-file) lives in a single contiguous block at the top of the class, followed by the `ProhibitionState` nested type, the constructor, and then methods. Removed the duplicate orphan `<summary>` and the now-redundant field declarations that had been scattered through the file. The full 263-test suite passes with no behavioural change.
 ### Driver.Modbus-012
@@ -198,10 +198,10 @@
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The unit suite is broad (coalescing, bisection, auto-recovery, byte order, arrays, BCD, RMW, caps, multi-unit, probe, reconnect, subscription). Gaps relative to the findings above: (1) no test exercises concurrent multi-subscription publishing, so the `_lastPublishedByRef` race (Driver.Modbus-001) is uncaught; (2) no test covers `ReinitializeAsync` state hygiene for stale `_tagsByName`/caches (Driver.Modbus-002); (3) no test feeds a malformed/short response PDU through `ReadRegisterBlockAsync`/`DecodeBitArray` to confirm a clean `BadCommunicationError` rather than an index-range crash (Driver.Modbus-005); (4) no test asserts `DisposeAsync` (vs `ShutdownAsync`) tears down the probe/re-probe loops and `_poll` (Driver.Modbus-004).
 **Recommendation:** Add unit tests for concurrent deadband publishing across two subscriptions, `ReinitializeAsync` state hygiene, malformed-response handling in the register/bit block readers, and `DisposeAsync` loop teardown.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — gap (1) was already covered by `ModbusSubscriptionTests.Concurrent_deadband_subscriptions_do_not_corrupt_the_publish_cache` from the Driver.Modbus-001 fix. Added the remaining three in a new `ModbusLifecycleHygieneTests` file: `Reinitialize_clears_stale_tagsByName_entries` + `Reinitialize_clears_lastPublished_and_lastWritten_caches` (gap 2), `Short_response_PDU_surfaces_as_BadCommunicationError_not_an_IndexOutOfRangeException` + `Response_payload_truncated_below_declared_byteCount_surfaces_as_BadCommunicationError` + `DecodeBitArray_rejects_an_empty_bitmap_with_InvalidDataException` (gap 3), `DisposeAsync_without_explicit_Shutdown_tears_down_probe_loop_and_transport` + `DisposeAsync_disposes_the_pollEngine_so_subscriptions_stop` (gap 4). All 12 new tests pass (full suite: 263/263 green).
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 2 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -182,14 +182,14 @@
 |---|---|
 | Severity | Low |
 | Category | Documentation & comments |
-| Location | `OpcUaClientDriver.cs:783-784` |
+| Location | `OpcUaClientDriver.cs:1007-1015` |
-| Status | Open |
+| Status | Resolved |
-**Description:** The comment on the isArray computation states "-1 = scalar; 1+ = array dimensions; 0 = one-dimensional array". This is inaccurate against OPC UA ValueRank semantics: -3 is ScalarOrOneDimension, -2 is Any, -1 is Scalar, and 0 is OneOrMoreDimensions (not specifically one-dimensional). The code `valueRank >= 0` treats -2 (Any) and -3 (ScalarOrOneDimension) as scalar, which is a defensible default, but the comment misdescribes the constants and would mislead a maintainer.
+**Description:** The comment on the isArray computation stated "-1 = scalar; 1+ = array dimensions; 0 = one-dimensional array". This is inaccurate against OPC UA ValueRank semantics: -3 is ScalarOrOneDimension, -2 is Any, -1 is Scalar, and 0 is OneOrMoreDimensions (not specifically one-dimensional). The code `valueRank >= 0` treats -2 (Any) and -3 (ScalarOrOneDimension) as scalar, which is a defensible default, but the comment misdescribed the constants and would mislead a maintainer.
 **Recommendation:** Correct the comment to the actual ValueRank constants (-3 ScalarOrOneDimension, -2 Any, -1 Scalar, 0 OneOrMoreDimensions, 1 OneDimension, >1 multi-dim) and state the deliberate choice that anything >= 0 is treated as an array.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `EnrichAndRegisterVariablesAsync` now carries the correct OPC UA Part 3 ValueRank legend (`-3 ScalarOrOneDimension`, `-2 Any`, `-1 Scalar`, `0 OneOrMoreDimensions`, `1 OneDimension`, `>1` specific N-dimensions) and explicitly states the deliberate choice that anything `>= 0` is treated as an array, with `-3`/`-2` conservatively folded into the scalar bucket. Regression tests `ValueRank_constants_have_the_OPCUA_Part3_spec_values` (anchors the SDK constants) and `IsArray_decision_matches_valueRank_greater_or_equal_zero` (theory across -3..2) pin the logic in `OpcUaClientLowFindingsRegressionTests.cs`.
 ### Driver.OpcUaClient-012
@@ -227,14 +227,14 @@
 |---|---|
 | Severity | Low |
 | Category | Performance & resource management |
-| Location | `OpcUaClientDriver.cs:904`, `:1035` |
+| Location | `OpcUaClientDriver.cs:1138`, `:1314` |
-| Status | Open |
+| Status | Resolved |
-**Description:** `MonitoredItem.Notification += (mi, args) => ...` (and the alarm-event equivalent) attaches a closure-capturing lambda to each monitored item's event. The lambda is never detached. When UnsubscribeAsync removes a subscription it calls Subscription.DeleteAsync but does not clear the MonitoredItem.Notification handlers; if the SDK retains the MonitoredItem/Subscription graph anywhere (the session keeps a reference until its own disposal, or during transfer-on-reconnect), the driver instance is kept alive by the closure longer than necessary.
+**Description:** `MonitoredItem.Notification += (mi, args) => ...` (and the alarm-event equivalent) attached a closure-capturing lambda to each monitored item's event. The lambda was never detached. When UnsubscribeAsync removed a subscription it called Subscription.DeleteAsync but did not clear the MonitoredItem.Notification handlers; if the SDK retains the MonitoredItem/Subscription graph anywhere (the session keeps a reference until its own disposal, or during transfer-on-reconnect), the driver instance was kept alive by the closure longer than necessary.
 **Recommendation:** Detach the Notification handlers when deleting a subscription, or hold the handler delegate so it can be explicitly removed in UnsubscribeAsync/ShutdownAsync.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `SubscribeAsync` now stores each `(MonitoredItem, MonitoredItemNotificationEventHandler)` pair in a new `MonitoredItemNotificationHandle` record carried inside `RemoteSubscription`. `SubscribeAlarmsAsync` similarly stores the event-MonitoredItem and its handler delegate on `RemoteAlarmSubscription`. `UnsubscribeAsync`, `UnsubscribeAlarmsAsync`, and the subscription-teardown loops in `ShutdownAsync` now invoke `DetachNotificationHandlers` (or the alarm-equivalent inline `Notification -= rs.Handler`) BEFORE calling `Subscription.DeleteAsync`, so the SDK's invocation list no longer pins the driver through the captured lambda. Reflection-based regression tests `RemoteSubscription_record_carries_handler_delegates_so_they_can_be_detached` and `RemoteAlarmSubscription_record_carries_handler_delegate_so_it_can_be_detached` pin the contract that the handler reference is reachable from the bookkeeping record (`OpcUaClientLowFindingsRegressionTests.cs`).
 ### Driver.OpcUaClient-015
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 5 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -112,7 +112,7 @@ does not support UDT tags, and `BrowseSymbolsAsync` already correctly yields
 | Severity | Low |
 | Category | Correctness & logic bugs |
 | Location | `TwinCATDataType.cs:24-27` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The inline comments for the IEC time types are inaccurate. TwinCAT `TIME` is
 a duration (32-bit, milliseconds) — not "ms since epoch of day". `DATE` is stored as seconds
@@ -125,7 +125,7 @@ implementer who tries to add proper conversion.
 date/time semantics are intended to be exposed properly, track a follow-up to decode them to
 `DriverDataType.DateTime`; otherwise document that they surface as raw counters.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — rewrote the inline comments to match the actual IEC 61131-3 / TwinCAT encoding (TIME = duration in ms, DATE = seconds since 1970-01-01 truncated to a day boundary, DT = seconds since 1970-01-01, TOD = ms since midnight) and added a block comment documenting that the driver surfaces them as raw UDINT counters via `DriverDataType.UInt32`. Test `Iec_time_types_map_to_uint32_raw_counter` pins the mapping.
 ### Driver.TwinCAT-005
@@ -156,7 +156,7 @@ catch), native-notification registration failures, and host state transitions
 | Severity | Low |
 | Category | OtOpcUa conventions |
 | Location | `TwinCATDriver.cs:406-411` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `ResolveHost` falls back to `DriverInstanceId` when there are no configured
 devices and the reference is unknown. `DriverInstanceId` is a logical config-DB identifier,
@@ -169,7 +169,7 @@ connectivity-status row.
 empty string or a documented unresolved marker), or document why the instance ID is the chosen
 fallback. Prefer the first device HostAddress only when one exists (already done).
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `ResolveHost` now returns `TwinCATDriver.UnresolvedHostSentinel` (empty string) when no devices are configured, replacing the `DriverInstanceId` collision with `GetHostStatuses()` rows. The sentinel is publicly documented on the driver type. Updated `ResolveHost_falls_back_to_unresolved_sentinel_when_no_devices` (was `_to_DriverInstanceId_`) and added `ResolveHost_returns_unresolved_sentinel_when_no_devices` + `ResolveHost_unresolved_sentinel_matches_no_GetHostStatuses_entry` regressions.
 ### Driver.TwinCAT-007
@@ -361,7 +361,7 @@ part of the documented driver contract, not optional.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `TwinCATDriverOptions.cs:41-43`, `TwinCATDriverOptions.cs:57-62`, `AdsTwinCATClient.cs:145` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Several drifts between the implemented config surface and
 `docs/v2/driver-specs.md` section 6. The spec connection-settings list has separate `Host`
@@ -376,7 +376,7 @@ the probe path connects via `_options.Timeout` — a dead config field. The spec
 shape (the doc is DRAFT, so updating it is acceptable). Remove or wire up
 `TwinCATProbeOptions.Timeout`. Expose `NotificationMaxDelayMs` if batching control is wanted.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `TwinCATProbeOptions.Timeout` is now wired into `EnsureConnectedAsync` via an optional `timeoutOverride` parameter that the probe loop passes (reads / writes keep the driver-level `_options.Timeout`). Added a `TwinCATDriverOptions.NotificationMaxDelayMs` config knob (parsed from `driverConfigJson` via `TwinCATDriverConfigDto.NotificationMaxDelayMs`) and threaded it through `ITwinCATClient.AddNotificationAsync` so `NotificationSettings` carries the configured max-delay instead of the hard-coded 0. The `Host` / `AmsNetId` / `AmsPort` triple in the spec was already implemented as the single `HostAddress` (parsed `ads://{netId}:{port}` URI) — kept as-is to match the v2 driver convention; covered by `TwinCATAmsAddress`. Regression tests: `ProbeOptions_Timeout_is_applied_to_probe_calls`, `NotificationMaxDelayMs_is_exposed_on_driver_options`, `NotificationMaxDelayMs_parses_from_driver_config_json`.
 ### Driver.TwinCAT-015
@@ -385,7 +385,7 @@ shape (the doc is DRAFT, so updating it is acceptable). Remove or wire up
 | Severity | Low |
 | Category | Code organization & conventions |
 | Location | `TwinCATDriver.cs:431-432` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `Dispose()` runs `DisposeAsync().AsTask().GetAwaiter().GetResult()` —
 sync-over-async. `docs/v2/driver-stability.md` section Galaxy explicitly lists "sync-over-async
@@ -399,7 +399,7 @@ here — cancelling token sources, disposing clients, clearing dictionaries —
 synchronous, and `PollGroupEngine.DisposeAsync` completes synchronously, so factor the
 synchronous teardown out so `Dispose()` does not block on a `Task`.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `Dispose()` now does an inline synchronous teardown with no `await` and no captured sync context: dispose native subscriptions, drive `PollGroupEngine.DisposeAsync` via `.AsTask().Wait(5s)` (no context capture), per-device `ProbeCts.Cancel()` + `ProbeTask.Wait(2s)`, `DisposeClient()` / `DisposeGate()`, then clear the dictionaries. `DisposeAsync` still routes through `ShutdownAsync` for genuinely async callers. Regression test `Dispose_does_not_block_on_async_in_default_synchronization_context` runs `Dispose()` inside a single-threaded `SynchronizationContext` that would deadlock a sync-over-async teardown and asserts it completes within 5s.
 ### Driver.TwinCAT-016
@@ -408,7 +408,7 @@ synchronous teardown out so `Dispose()` does not block on a `Task`.
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Unit coverage exists for AMS-address parsing, symbol-path parsing, read/write,
 native notifications, symbol browse, and the capability surface. Gaps tied to the findings
@@ -423,4 +423,4 @@ without truncation (Driver.TwinCAT-002).
 addressed, especially a concurrency stress test for `EnsureConnectedAsync` and a
 `ReinitializeAsync`-applies-new-config test.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — the previously-closed High findings each grew their regression coverage as they were resolved (see `TwinCATHighFindingsRegressionTests`: `ReinitializeAsync_applies_changed_device_config` for -001, `LInt_read_round_trips_value_above_int_MaxValue` + `DataType_mapping_preserves_width_and_signedness` for -002, `Concurrent_reads_on_one_device_create_a_single_client` + `Concurrent_reads_and_writes_share_one_client` for -007, `Symbol_version_changed_raises_OnRediscoveryNeeded` + `TwinCATDriver_implements_IRediscoverable` for -013). This pass added the two remaining gaps: `Structure_typed_pre_declared_tag_is_rejected_at_config_parse` (-003) and `Probe_loop_and_read_share_one_client_per_device` (-009 disposal-race coverage races 64 readers against the probe loop for 500ms and asserts a single client / single connect). All coverage lives in the test files `TwinCATHighFindingsRegressionTests.cs` and the new `TwinCATLowFindingsRegressionTests.cs`.
@@ -30,15 +30,15 @@ Each module's `findings.md` is the source of truth; this file is generated from
 | [Driver.FOCAS](Driver.FOCAS/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 12 |
 | [Driver.FOCAS.Cli](Driver.FOCAS.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 5 | 5 |
 | [Driver.Galaxy](Driver.Galaxy/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 14 |
-| [Driver.Historian.Wonderware](Driver.Historian.Wonderware/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 7 | 12 |
+| [Driver.Historian.Wonderware](Driver.Historian.Wonderware/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 12 |
 | [Driver.Historian.Wonderware.Client](Driver.Historian.Wonderware.Client/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 5 | 10 |
-| [Driver.Modbus](Driver.Modbus/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 7 | 12 |
+| [Driver.Modbus](Driver.Modbus/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 12 |
-| [Driver.Modbus.Addressing](Driver.Modbus.Addressing/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 3 | 9 |
+| [Driver.Modbus.Addressing](Driver.Modbus.Addressing/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 9 |
 | [Driver.Modbus.Cli](Driver.Modbus.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 6 | 8 |
-| [Driver.OpcUaClient](Driver.OpcUaClient/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 2 | 15 |
+| [Driver.OpcUaClient](Driver.OpcUaClient/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 15 |
 | [Driver.S7](Driver.S7/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 14 |
 | [Driver.S7.Cli](Driver.S7.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 4 | 7 |
-| [Driver.TwinCAT](Driver.TwinCAT/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 5 | 16 |
+| [Driver.TwinCAT](Driver.TwinCAT/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 16 |
 | [Driver.TwinCAT.Cli](Driver.TwinCAT.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 7 | 7 |
 | [Server](Server/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 15 |
@@ -86,45 +86,21 @@ Findings with status `Open` or `In Progress`, ordered by severity.
 | Driver.FOCAS.Cli-003 | Low | Error handling & resilience | `FocasCommandBase.cs:19` (`CncPort`), `FocasCommandBase.cs:27` (`TimeoutMs`), `Commands/SubscribeCommand.cs:23` (`IntervalMs`) | The numeric command options `--cnc-port`, `--timeout-ms`, and `--interval-ms` are accepted without range validation. A zero or negative `--cnc-port` produces an invalid `focas://host:<n>` string; `--timeout-ms 0` yields a zero `TimeSpan` o… |
 | Driver.FOCAS.Cli-004 | Low | Performance & resource management | `Commands/ProbeCommand.cs:37,54`; `Commands/ReadCommand.cs:37,46`; `Commands/WriteCommand.cs:45,54`; `Commands/SubscribeCommand.cs:39,73` | Every command declares `await using var driver = new FocasDriver(...)` |
 | Driver.FOCAS.Cli-005 | Low | Design-document adherence | `Commands/WriteCommand.cs:50`, `Commands/ProbeCommand.cs:50` (via `SnapshotFormatter.FormatStatus`) | `docs/Driver.FOCAS.Cli.md` documents `BadDeviceFailure` and `BadCommunicationError` as the key diagnostic signals an operator reads off `probe` / `write` output ("A `BadCommunicationError` means ... `BadDeviceFailure` after a successful co… |
 | Driver.Historian.Wonderware-004 | Low | Correctness and logic bugs | `Backend/SdkAlarmHistorianWriteBackend.cs:198-201` | `ToHistorianEvent` only assigns `historianEvent.Id` when `Guid.TryParse(dto.EventId, ...)` succeeds. If `EventId` is not a parseable GUID (or is empty), `Id` stays `Guid.Empty` and the event is written to the historian with an all-zeros id… |
 | Driver.Historian.Wonderware-005 | Low | Concurrency and thread safety | `Backend/HistorianDataSource.cs:124`, `:126-127` | `GetHealthSnapshot` reads `_activeProcessNode` and `_activeEventNode` inside `_healthLock`, but those two fields are written under `_connectionLock` / `_eventConnectionLock` (lines 183, 243, 209-210, 266-269) — a different lock. The health… |
 | Driver.Historian.Wonderware-007 | Low | Error handling and resilience | `Ipc/PipeServer.cs:70-75` | When `VerifyCaller` rejects the peer SID, the server logs the reason and calls `_current.Disconnect()` with no `HelloAck` frame sent. The shared-secret-mismatch and major-version-mismatch paths below it both send a rejecting `HelloAck` so… |
 | Driver.Historian.Wonderware-008 | Low | Error handling and resilience | `Backend/HistorianDataSource.cs:301-307`, `:374-380` | When `query.StartQuery` returns `false`, `ReadRawAsync` and `ReadAggregateAsync` call `HandleConnectionError()` and return an empty result list. A failed `StartQuery` is not necessarily a connection failure — it can be a bad tag name, an i… |
 | Driver.Historian.Wonderware-010 | Low | Performance and resource management | `Backend/HistorianConfiguration.cs:32-36`, `Backend/HistorianDataSource.cs` (all read methods) | `HistorianConfiguration.RequestTimeoutSeconds` is documented as the "outer safety timeout applied to sync-over-async Historian operations" and is copied around (`SdkAlarmHistorianWriteBackend.CloneConfigWithServerName:346`), but it is neve… |
 | Driver.Historian.Wonderware-011 | Low | Design-document adherence | `Backend/HistorianDataSource.cs:9-12`, `Backend/IHistorianDataSource.cs:9-11`, `Backend/HistorianSample.cs:7-9`, `Backend/HistorianConfiguration.cs:7-9` | Several XML doc comments reference the retired v1 architecture as if it were current: "inside Galaxy.Host", "the Proxy maps returned samples", "the Host returns these across the IPC boundary as `GalaxyDataValue`", "Populated from ... the P… |
 | Driver.Historian.Wonderware-012 | Low | Testing coverage | `Backend/HistorianDataSource.cs`, `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/` | The unit-test suite covers `HistorianQualityMapper`, `HistorianClusterEndpointPicker`, `SdkAlarmHistorianWriteBackend`, `AahClientManagedAlarmEventWriter`, the IPC round trip, and `Program` alarm-writer wiring. `HistorianDataSource` itself… |
 | Driver.Historian.Wonderware.Client-003 | Low | Concurrency & thread safety | `WonderwareHistorianClient.cs:207`, `WonderwareHistorianClient.cs:132-150` | `_totalQueries` is mutated with `Interlocked.Increment` in `Invoke`, but read inside `GetHealthSnapshot` under `_healthLock`, and every other counter (`_totalSuccesses`, `_totalFailures`, `_consecutiveFailures`) is mutated only under `_hea… |
 | Driver.Historian.Wonderware.Client-004 | Low | Concurrency & thread safety | `WonderwareHistorianClient.cs:203-267` | A sidecar-reported failure is recorded in two non-atomic steps under separate lock acquisitions: `Invoke` calls `RecordSuccess()` (line 211) and then the caller calls `ThrowIfFailed` which calls `ReclassifySuccessAsFailure()` (line 256), d… |
 | Driver.Historian.Wonderware.Client-006 | Low | Error handling & resilience | `Internal/PipeChannel.cs:96-107`, `WonderwareHistorianClientOptions.cs:11-12` | `PipeChannel.InvokeAsync` retries exactly once on transport failure and otherwise propagates. The options expose `ReconnectInitialBackoff` and `ReconnectMaxBackoff` and `WonderwareHistorianClientOptions` documents them as exponential backo… |
 | Driver.Historian.Wonderware.Client-008 | Low | Security | `ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Client.csproj:29-32` | The csproj suppresses two NuGet audit advisories (`GHSA-37gx-xxp4-5rgx`, `GHSA-w3x6-4m5h-cxqf`) for the `MessagePack` 2.5.187 dependency with no inline comment recording why the suppression is safe, who reviewed it, or when it should be re… |
 | Driver.Historian.Wonderware.Client-010 | Low | Documentation & comments | `WonderwareHistorianClient.cs:355-361`, `WonderwareHistorianClient.cs:132-150` | Two doc/behaviour mismatches. (1) The `Dispose()` XML comment asserts the underlying channel async cleanup is non-blocking so the `GetAwaiter()/GetResult()` bridge is safe. `PipeChannel.DisposeAsync` calls `ResetTransport()`, which invokes… |
 | Driver.Modbus-003 | Low | Concurrency & thread safety | `ModbusDriver.cs:59,188,241,259,266,726,745,759` | `_health` is a non-`volatile` reference field written from multiple threads (concurrent `ReadAsync` callers, the coalesced-read path, `WriteAsync` indirectly, and `ProbeLoopAsync`) and read by `GetHealth()`. Reference assignment is atomic… |
 | Driver.Modbus-007 | Low | Design-document adherence | `ModbusDriver.cs:1392`, `ModbusDriverOptions.cs:74-80` | Two design-vs-code drifts. (1) `MapDataType` maps `Int64`/`UInt64` to `DriverDataType.Int32` with the inline comment "widening to Int32 loses precision; PR 25 adds Int64 to DriverDataType". The address-space node for a 64-bit Modbus tag is… |
 | Driver.Modbus-008 | Low | Documentation & comments | `ModbusDriver.cs:411-417,700-703,737-744` | Stale/misleading comments. (1) The `<summary>` block at `ModbusDriver.cs:411-417` says auto-prohibited ranges are "Cleared by ReinitializeAsync ... or by an explicit re-probe API (not yet shipped)" — the re-probe loop has shipped (#151, `R… |
 | Driver.Modbus-009 | Low | Correctness & logic bugs | `ModbusDriver.cs:1160-1167`, `ModbusTcpTransport.cs:94-95` | Two edge cases. (1) `RegisterCount` for `ModbusDataType.String` computes `(tag.StringLength + 1) / 2`; a tag configured with `StringLength = 0` yields a register count of 0, flowing into `ReadOneAsync` as `totalRegs = 0` and producing an F… |
 | Driver.Modbus-010 | Low | Error handling & resilience | `ModbusDriver.cs:864-868`, `ModbusDriverOptions.cs:116-125` | When `WriteOnChangeOnly` is enabled and `IsRedundantWrite` returns true, `WriteAsync` returns `WriteResult(0u)` (Good) without touching the wire. The suppression baseline (`_lastWrittenByRef`) is only invalidated by a *read* that returns a… |
 | Driver.Modbus-011 | Low | Code organization & conventions | `ModbusDriver.cs:23-43,89-97,408-432` | Field and member declarations are interleaved with methods throughout `ModbusDriver`. `ResolveHost` (a public method) is the first member of the class, followed by `BuildSlaveHostName`, then a block of fields; `_lastPublishedByRef`/`_lastW… |
 | Driver.Modbus-012 | Low | Testing coverage | `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/` | The unit suite is broad (coalescing, bisection, auto-recovery, byte order, arrays, BCD, RMW, caps, multi-unit, probe, reconnect, subscription). Gaps relative to the findings above: (1) no test exercises concurrent multi-subscription publis… |
 | Driver.Modbus.Addressing-006 | Low | Error handling & resilience | `ModbusAddressParser.cs:297-301` | `TryParseFamilyNative` catches only `ArgumentException` and `OverflowException`. The current helpers throw only those (including `ArgumentOutOfRangeException`, which derives from `ArgumentException`), so today it is correct. But the parser… |
 | Driver.Modbus.Addressing-007 | Low | Design-document adherence | `ModbusDataType.cs:91-95`, `docs/v2/dl205.md` section Strings | `ModbusStringByteOrder` (HighByteFirst / LowByteFirst) is defined in this assembly and documented as the DL205 low-byte-first string-packing knob, but `ParsedModbusAddress` has no field for it and `ModbusAddressParser` never produces or co… |
 | Driver.Modbus.Addressing-009 | Low | Documentation & comments | `ModbusModiconAddress.cs:55-64`, `ModbusModiconAddress.cs:104-110` | The comments on `ModbusModiconAddress.TryParse` are slightly inaccurate. The remark that 5-digit Modicon is always exactly 5 chars (40001..49999) and 6-digit is exactly 6 (400001..465536-shaped) implies the leading digit is always 4, but t… |
 | Driver.Modbus.Cli-003 | Low | Correctness & logic bugs | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/ModbusCommandBase.cs:14-24` | `Port` (`int`) and `TimeoutMs` (`int`) accept any 32-bit value, including negatives and ports above 65535. `UnitId` is a `byte`, so it accepts 0-255 even though the option description and `docs/Driver.Modbus.Cli.md` both say the valid rang… |
 | Driver.Modbus.Cli-004 | Low | Concurrency & thread safety | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/SubscribeCommand.cs:61-67` | The `OnDataChange` handler is invoked from the driver's `PollGroupEngine` background thread and calls `console.Output.WriteLine` synchronously. An exception thrown inside this handler (e.g. an `IOException` on a redirected or closed stdout… |
 | Driver.Modbus.Cli-005 | Low | Error handling & resilience | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ProbeCommand.cs:21-54`; `Commands/ReadCommand.cs:46-75`; `Commands/WriteCommand.cs:54-89` | All three commands call `ConfigureLogging()` then `console.RegisterCancellationHandler()`, but if the operator presses Ctrl+C before `InitializeAsync` completes, the resulting `OperationCancelledException` propagates out of `ExecuteAsync`… |
 | Driver.Modbus.Cli-006 | Low | Error handling & resilience | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ProbeCommand.cs:35-53` | `probe` reports `Health: {health.State}` from `GetHealth()`. After a successful `InitializeAsync` the driver sets state to `Healthy` regardless of whether the subsequent probe register read returns Good or a Bad status code. `ReadAsync` do… |
 | Driver.Modbus.Cli-007 | Low | Design-document adherence | `docs/Driver.Modbus.Cli.md:124-156`; `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs` | `docs/Driver.Modbus.Cli.md` devotes a whole "v2 addressing grammar" section to the industry-standard tag-address strings (`40001:F:CDAB`, `HR1:I`, `C100`, `V2000:F:CDAB`, etc.) and says "set the per-tag `addressString` field instead of the… |
 | Driver.Modbus.Cli-008 | Low | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests/` | The test project covers only the two pure-function seams: `ReadCommand.SynthesiseTagName` and `WriteCommand.ParseValue`. There is no coverage for `WriteCommand`'s read-only-region rejection (`Region is not (Coils or HoldingRegisters)`), no… |
 | Driver.OpcUaClient-011 | Low | Documentation & comments | `OpcUaClientDriver.cs:783-784` | The comment on the isArray computation states "-1 = scalar; 1+ = array dimensions; 0 = one-dimensional array". This is inaccurate against OPC UA ValueRank semantics: -3 is ScalarOrOneDimension, -2 is Any, -1 is Scalar, and 0 is OneOrMoreDi… |
 | Driver.OpcUaClient-014 | Low | Performance & resource management | `OpcUaClientDriver.cs:904`, `:1035` | `MonitoredItem.Notification += (mi, args) => ...` (and the alarm-event equivalent) attaches a closure-capturing lambda to each monitored item's event. The lambda is never detached. When UnsubscribeAsync removes a subscription it calls Subs… |
 | Driver.S7.Cli-004 | Low | Performance & resource management | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ProbeCommand.cs:36,53`, `Commands/ReadCommand.cs:45,54`, `Commands/WriteCommand.cs:51,60`, `Commands/SubscribeCommand.cs:39,73` | Every command declares the driver with `await using var driver = new S7Driver(...)` and *also* calls `await driver.ShutdownAsync(...)` in a `finally` block. `S7Driver.DisposeAsync` itself calls `ShutdownAsync`, so shutdown runs twice per c… |
 | Driver.S7.Cli-005 | Low | Code organization & conventions | `tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` | A stale directory `tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` exists containing only an `obj/` folder — no `.csproj`, no source. The real test project lives at `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/`. The empty direct… |
 | Driver.S7.Cli-006 | Low | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/WriteCommandParseValueTests.cs` | The only test file covers `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName`. `S7CommandBase.BuildOptions` — which maps the host / port / CPU / rack / slot / timeout flags onto an `S7DriverOptions` and forces `Probe.Enabled = fa… |
 | Driver.S7.Cli-007 | Low | Documentation & comments | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/SubscribeCommand.cs:45-51` | The Modbus CLI `SubscribeCommand` carries an explanatory comment on the `OnDataChange` handler ("Route every data-change event to the CliFx console (not System.Console — the analyzer flags it + IConsole is the testable abstraction)"). The… |
 | Driver.TwinCAT-004 | Low | Correctness & logic bugs | `TwinCATDataType.cs:24-27` | The inline comments for the IEC time types are inaccurate. TwinCAT `TIME` is a duration (32-bit, milliseconds) — not "ms since epoch of day". `DATE` is stored as seconds since 1970-01-01 (truncated to a day boundary), not "days since 1970-… |
 | Driver.TwinCAT-006 | Low | OtOpcUa conventions | `TwinCATDriver.cs:406-411` | `ResolveHost` falls back to `DriverInstanceId` when there are no configured devices and the reference is unknown. `DriverInstanceId` is a logical config-DB identifier, not a host address; `IPerCallHostResolver` consumers expect a host key… |
 | Driver.TwinCAT-014 | Low | Design-document adherence | `TwinCATDriverOptions.cs:41-43`, `TwinCATDriverOptions.cs:57-62`, `AdsTwinCATClient.cs:145` | Several drifts between the implemented config surface and `docs/v2/driver-specs.md` section 6. The spec connection-settings list has separate `Host` (IP), `AmsNetId`, and `AmsPort` fields; the implementation collapses these into a single `… |
 | Driver.TwinCAT-015 | Low | Code organization & conventions | `TwinCATDriver.cs:431-432` | `Dispose()` runs `DisposeAsync().AsTask().GetAwaiter().GetResult()` — sync-over-async. `docs/v2/driver-stability.md` section Galaxy explicitly lists "sync-over-async on the OPC UA stack thread" among the four 2026-04-13 stability findings… |
 | Driver.TwinCAT-016 | Low | Testing coverage | `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/` | Unit coverage exists for AMS-address parsing, symbol-path parsing, read/write, native notifications, symbol browse, and the capability surface. Gaps tied to the findings above: no test exercises `ReinitializeAsync` with a changed config (D… |
 | Driver.TwinCAT.Cli-001 | Low | Correctness & logic bugs | `TwinCATCommandBase.cs:23-24`, `Commands/SubscribeCommand.cs:23-24`, `Commands/BrowseCommand.cs:21-24` | Numeric command options are accepted without range validation. `--timeout-ms` feeds `Timeout => TimeSpan.FromMilliseconds(TimeoutMs)`; passing `--timeout-ms 0` or a negative value yields `TimeSpan.Zero`/a negative `TimeSpan`, which is then… |
 | Driver.TwinCAT.Cli-002 | Low | Concurrency & thread safety | `Commands/SubscribeCommand.cs:46-58` | The `OnDataChange` handler calls `console.Output.WriteLine(line)` synchronously. In native ADS-notification mode the event is raised from the `Beckhoff.TwinCAT.Ads` notification callback thread (see `TwinCATDriver.SubscribeAsync`, which in… |
 | Driver.TwinCAT.Cli-003 | Low | Error handling & resilience | `Commands/SubscribeCommand.cs:56-58` | The subscribe banner reports the mechanism purely from the `--poll-only` flag (`var mode = PollOnly ? "polling" : "ADS notification"`). The doc (`docs/Driver.TwinCAT.Cli.md`) states the banner "announces which mechanism is in play". The CL… |
@@ -379,11 +355,35 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
 | Driver.Galaxy-010 | Low | Resolved | Security | `GalaxyDriver.cs:311-341` |
 | Driver.Galaxy-012 | Low | Resolved | Performance & resource management | `Runtime/SubscriptionRegistry.cs:65-67`, `GalaxyDriver.cs:538`, `GalaxyDriver.cs:675` |
 | Driver.Galaxy-013 | Low | Resolved | Design-document adherence | `GalaxyDriver.cs:14-27`, `GalaxyDriver.cs:374-382`, `Config/GalaxyDriverOptions.cs:84-86` |
 | Driver.Historian.Wonderware-004 | Low | Resolved | Correctness and logic bugs | `Backend/SdkAlarmHistorianWriteBackend.cs:198-201` |
 | Driver.Historian.Wonderware-005 | Low | Resolved | Concurrency and thread safety | `Backend/HistorianDataSource.cs:124`, `:126-127` |
 | Driver.Historian.Wonderware-007 | Low | Resolved | Error handling and resilience | `Ipc/PipeServer.cs:70-75` |
 | Driver.Historian.Wonderware-008 | Low | Resolved | Error handling and resilience | `Backend/HistorianDataSource.cs:301-307`, `:374-380` |
 | Driver.Historian.Wonderware-010 | Low | Resolved | Performance and resource management | `Backend/HistorianConfiguration.cs:32-36`, `Backend/HistorianDataSource.cs` (all read methods) |
 | Driver.Historian.Wonderware-011 | Low | Resolved | Design-document adherence | `Backend/HistorianDataSource.cs:9-12`, `Backend/IHistorianDataSource.cs:9-11`, `Backend/HistorianSample.cs:7-9`, `Backend/HistorianConfiguration.cs:7-9` |
 | Driver.Historian.Wonderware-012 | Low | Resolved | Testing coverage | `Backend/HistorianDataSource.cs`, `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests/` |
 | Driver.Modbus-003 | Low | Resolved | Concurrency & thread safety | `ModbusDriver.cs:59,188,241,259,266,726,745,759` |
 | Driver.Modbus-007 | Low | Resolved | Design-document adherence | `ModbusDriver.cs:1392`, `ModbusDriverOptions.cs:74-80` |
 | Driver.Modbus-008 | Low | Resolved | Documentation & comments | `ModbusDriver.cs:411-417,700-703,737-744` |
 | Driver.Modbus-009 | Low | Resolved | Correctness & logic bugs | `ModbusDriver.cs:1160-1167`, `ModbusTcpTransport.cs:94-95` |
 | Driver.Modbus-010 | Low | Resolved | Error handling & resilience | `ModbusDriver.cs:864-868`, `ModbusDriverOptions.cs:116-125` |
 | Driver.Modbus-011 | Low | Resolved | Code organization & conventions | `ModbusDriver.cs:23-43,89-97,408-432` |
 | Driver.Modbus-012 | Low | Resolved | Testing coverage | `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/` |
 | Driver.Modbus.Addressing-006 | Low | Resolved | Error handling & resilience | `ModbusAddressParser.cs:297-301` |
 | Driver.Modbus.Addressing-007 | Low | Resolved | Design-document adherence | `ModbusDataType.cs:91-95`, `docs/v2/dl205.md` section Strings |
 | Driver.Modbus.Addressing-009 | Low | Resolved | Documentation & comments | `ModbusModiconAddress.cs:55-64`, `ModbusModiconAddress.cs:104-110` |
 | Driver.OpcUaClient-011 | Low | Resolved | Documentation & comments | `OpcUaClientDriver.cs:1007-1015` |
 | Driver.OpcUaClient-014 | Low | Resolved | Performance & resource management | `OpcUaClientDriver.cs:1138`, `:1314` |
 | Driver.S7-003 | Low | Resolved | Correctness & logic bugs | `S7Driver.cs:172`, `S7Driver.cs:255` |
 | Driver.S7-005 | Low | Resolved | OtOpcUa conventions | `S7Driver.cs:33`, `S7Driver.cs:433` |
 | Driver.S7-009 | Low | Resolved | Error handling & resilience | `S7Driver.cs:392` |
 | Driver.S7-010 | Low | Resolved | Performance & resource management | `S7Driver.cs:504` |
 | Driver.S7-013 | Low | Resolved | Code organization & conventions | `S7DriverOptions.cs:90`, `S7Driver.cs:300` |
 | Driver.TwinCAT-004 | Low | Resolved | Correctness & logic bugs | `TwinCATDataType.cs:24-27` |
 | Driver.TwinCAT-006 | Low | Resolved | OtOpcUa conventions | `TwinCATDriver.cs:406-411` |
 | Driver.TwinCAT-014 | Low | Resolved | Design-document adherence | `TwinCATDriverOptions.cs:41-43`, `TwinCATDriverOptions.cs:57-62`, `AdsTwinCATClient.cs:145` |
 | Driver.TwinCAT-015 | Low | Resolved | Code organization & conventions | `TwinCATDriver.cs:431-432` |
 | Driver.TwinCAT-016 | Low | Resolved | Testing coverage | `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/` |
 | Server-004 | Low | Resolved | OtOpcUa conventions | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs:187-200` |
 | Server-006 | Low | Resolved | Concurrency & thread safety | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:478-482, 1342-1348` |
 | Server-008 | Low | Resolved | Error handling & resilience | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs:736` |
@@ -43,6 +43,15 @@ that a naive Modbus client will byte-swap [1][2].
  really "read 10 consecutive holding registers starting at the Modbus address
  that V2000 translates to (see next section), unpack each register low-byte
  then high-byte, stop at the first `0x00`."
 - **Grammar scope** (Driver.Modbus.Addressing-007): the
  `ModbusStringByteOrder` knob (HighByteFirst / LowByteFirst) is **not**
  expressible through the `ModbusAddressParser` grammar string — the 3rd grammar
  field is the multi-register word/byte order (ABCD/CDAB/BADC/DCBA) and the 4th
  is the array count, so there is no token slot for the per-string byte order.
  Tags that need low-byte-first packing on DL205 must set
  `ModbusTagDefinition.StringByteOrder = LowByteFirst` via the structured tag
  form (the driver config DTO). The grammar default produces high-byte-first
  strings (matches Ignition / Kepware default behaviour).
 Test names:
 `DL205_String_low_byte_first_within_register`,
@@ -3,9 +3,12 @@ using System.Collections.Generic;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
 {
    /// <summary>
-    ///     Wonderware Historian SDK configuration. Populated from environment variables at Host
+    ///     Wonderware Historian SDK configuration. Populated from environment variables at
-    ///     startup (see <c>Program.cs</c>) or from the Proxy's <c>DriverInstance.DriverConfig</c>
+    ///     sidecar startup (see <c>Program.cs</c>): the supervisor (lmxopcua-side
-    ///     section passed during OpenSession. Kept OPC-UA-free — the Proxy side owns UA translation.
+    ///     <c>WonderwareHistorianClient</c>) spawns the sidecar with these env vars; UA
    ///     translation lives on the client side of the named-pipe IPC, so this surface is
    ///     kept OPC-UA-free. The legacy v1 Galaxy.Host / Proxy host this lived in retired
    ///     in PR 7.2.
    /// </summary>
    public sealed class HistorianConfiguration
    {
@@ -11,7 +11,10 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
    /// <summary>
    ///     Reads historical data from the Wonderware Historian via the aahClientManaged SDK.
    ///     OPC-UA-free — emits <see cref="HistorianSample"/>/<see cref="HistorianAggregateSample"/>
-    ///     which the Proxy maps to OPC UA <c>DataValue</c> on its side of the IPC.
+    ///     which the sidecar serialises onto the named-pipe wire (PR 3.3 contracts) for the
    ///     .NET 10 <c>WonderwareHistorianClient</c> to translate into OPC UA <c>DataValue</c>
    ///     on its side of the IPC. The v1 Galaxy.Host / Proxy architecture this class
    ///     originally lived in retired in PR 7.2.
    /// </summary>
    public sealed class HistorianDataSource : IHistorianDataSource
    {
@@ -50,6 +53,51 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            _picker = picker ?? new HistorianClusterEndpointPicker(config);
        }
        // Error codes that signify the connection or server is the problem rather than the
        // query itself. A query-class failure (bad tag name, unsupported aggregate, etc.) must
        // not force us to tear down and re-open the (relatively expensive) historian
        // connection — that would let a burst of bad-tag queries push an otherwise healthy
        // cluster node into cooldown. See Driver.Historian.Wonderware-008.
        private static readonly HashSet<HistorianAccessError.ErrorValue> ConnectionErrorCodes =
            new HashSet<HistorianAccessError.ErrorValue>
            {
                HistorianAccessError.ErrorValue.FailedToConnect,
                HistorianAccessError.ErrorValue.FailedToCreateSession,
                HistorianAccessError.ErrorValue.NoReply,
                HistorianAccessError.ErrorValue.NotReady,
                HistorianAccessError.ErrorValue.NotInitialized,
                HistorianAccessError.ErrorValue.Stopping,
                HistorianAccessError.ErrorValue.Win32Exception,
                HistorianAccessError.ErrorValue.InvalidResponse,
            };
        /// <summary>
        ///     Whether an <c>aahClientManaged</c> error code indicates that the
        ///     <em>connection</em> (rather than the query payload) is the problem and the
        ///     shared SDK connection should therefore be reset. Internal for unit testing.
        /// </summary>
        internal static bool IsConnectionClassError(HistorianAccessError.ErrorValue code)
            => ConnectionErrorCodes.Contains(code);
        /// <summary>
        ///     Builds the per-read <see cref="CancellationTokenSource"/> linked into the
        ///     caller's <paramref name="ct"/> and pre-wired to fire after
        ///     <see cref="HistorianConfiguration.RequestTimeoutSeconds"/> if positive. The
        ///     read paths use the resulting token in their <c>ThrowIfCancellationRequested</c>
        ///     checks so a hung <c>StartQuery</c> or slow <c>MoveNext</c> cannot block the
        ///     single pipe-server connection thread indefinitely. See
        ///     Driver.Historian.Wonderware-010.
        /// </summary>
        internal static CancellationTokenSource BuildRequestCts(HistorianConfiguration cfg, CancellationToken ct)
        {
            var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
            if (cfg.RequestTimeoutSeconds > 0)
            {
                cts.CancelAfter(TimeSpan.FromSeconds(cfg.RequestTimeoutSeconds));
            }
            return cts;
        }
        private (HistorianAccess Connection, string Node) ConnectToAnyHealthyNode(HistorianConnectionType type)
        {
            var candidates = _picker.GetHealthyNodes();
@@ -110,6 +158,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            foreach (var n in nodeStates)
                if (n.IsHealthy) healthyCount++;
            // Driver.Historian.Wonderware-005: derive the connection-open booleans from the
            // active-node strings, both of which live under _healthLock. _connection itself
            // is published under _connectionLock — reading it here under a different lock
            // could produce an internally inconsistent snapshot (open with no node, or
            // closed with a non-null node) at the publish/clear boundary. Treating the
            // active-node strings as the single source of truth makes the snapshot
            // self-consistent by construction.
            lock (_healthLock)
            {
                return new HistorianHealthSnapshot
@@ -121,8 +176,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                    LastSuccessTime = _lastSuccessTime,
                    LastFailureTime = _lastFailureTime,
                    LastError = _lastError,
-                    ProcessConnectionOpen = Volatile.Read(ref _connection) != null,
+                    ProcessConnectionOpen = _activeProcessNode != null,
-                    EventConnectionOpen = Volatile.Read(ref _eventConnection) != null,
+                    EventConnectionOpen = _activeEventNode != null,
                    ActiveProcessNode = _activeProcessNode,
                    ActiveEventNode = _activeEventNode,
                    NodeCount = nodeStates.Count,
@@ -245,6 +300,59 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            }
        }
        /// <summary>
        ///     Internal exception signalling that <c>StartQuery</c> returned an SDK error
        ///     whose code is <em>query-class</em> (bad tag name, unsupported aggregate, etc.)
        ///     and the shared SDK connection therefore must NOT be reset. The outer catch
        ///     re-throws this so the IPC frame handler surfaces <c>Success=false</c> without
        ///     touching the connection. See Driver.Historian.Wonderware-008.
        /// </summary>
        internal sealed class QueryClassStartQueryException : InvalidOperationException
        {
            public HistorianAccessError.ErrorValue Code { get; }
            public QueryClassStartQueryException(string message, HistorianAccessError.ErrorValue code)
                : base(message)
            {
                Code = code;
            }
        }
        /// <summary>
        ///     Centralised <c>StartQuery</c>-failure handler. Throws so the caller surfaces
        ///     <c>Success=false</c> in the IPC reply (the previous return-empty-with-success
        ///     behaviour made an SDK error look like "no data in range" to the client). The
        ///     connection is only reset when the error code is connection-class —
        ///     query-class failures (bad tag name, unsupported aggregate, etc.) must leave
        ///     the shared SDK connection intact, otherwise a burst of bad-tag queries cycles
        ///     the connection and pushes a healthy cluster node into cooldown.
        ///     See Driver.Historian.Wonderware-008.
        /// </summary>
        private void HandleStartQueryFailure(
            string operation, HistorianAccessError error, bool isEventConnection)
        {
            var code = error?.ErrorCode ?? HistorianAccessError.ErrorValue.Failure;
            var description = error?.ErrorDescription ?? string.Empty;
            var connectionClass = IsConnectionClassError(code);
            Log.Warning(
                "Historian SDK StartQuery failed: {Operation} -> {Code} ({Desc}) [{Kind}]",
                operation, code, description,
                connectionClass ? "connection-class" : "query-class");
            RecordFailure($"{operation}: {code}");
            var message = $"Historian SDK StartQuery failed for {operation}: {code} ({description})";
            if (connectionClass)
            {
                if (isEventConnection) HandleEventConnectionError();
                else HandleConnectionError();
                throw new InvalidOperationException(message);
            }
            // Query-class — the outer catch block must NOT call HandleConnectionError on this.
            throw new QueryClassStartQueryException(message, code);
        }
        private void HandleEventConnectionError(Exception? ex = null)
        {
            lock (_eventConnectionLock)
@@ -280,6 +388,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
        {
            var results = new List<HistorianSample>();
            // Driver.Historian.Wonderware-010: wire RequestTimeoutSeconds into the read path
            // so a hung StartQuery / slow MoveNext can't block the connection thread forever.
            using var requestCts = BuildRequestCts(_config, ct);
            var token = requestCts.Token;
            try
            {
                EnsureConnected();
@@ -300,10 +413,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                if (!query.StartQuery(args, out var error))
                {
-                    Log.Warning("Historian SDK raw query start failed for {Tag}: {Error}", tagName, error.ErrorCode);
+                    HandleStartQueryFailure(
-                    RecordFailure($"raw StartQuery: {error.ErrorCode}");
+                        $"raw query for tag '{tagName}'", error, isEventConnection: false);
                    HandleConnectionError();
                    return Task.FromResult(results);
                }
                var count = 0;
@@ -311,7 +422,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                while (query.MoveNext(out error))
                {
-                    ct.ThrowIfCancellationRequested();
+                    token.ThrowIfCancellationRequested();
                    var result = query.QueryResult;
                    var timestamp = DateTime.SpecifyKind(result.StartDateTime, DateTimeKind.Utc);
@@ -332,11 +443,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            }
            catch (OperationCanceledException) { throw; }
            catch (ObjectDisposedException) { throw; }
            catch (QueryClassStartQueryException)
            {
                // Query-class StartQuery failure — HandleStartQueryFailure already logged
                // and recorded. Re-throw so the IPC layer surfaces Success=false instead of
                // returning an empty list (which would look like "no data in range"). The
                // connection is deliberately NOT reset. See Driver.Historian.Wonderware-008.
                throw;
            }
            catch (Exception ex)
            {
                Log.Warning(ex, "HistoryRead raw failed for {Tag}", tagName);
                RecordFailure($"raw: {ex.Message}");
                HandleConnectionError(ex);
                throw;
            }
            Log.Debug("HistoryRead raw: {Tag} returned {Count} values ({Start} to {End})",
@@ -352,6 +472,10 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
        {
            var results = new List<HistorianAggregateSample>();
            // Driver.Historian.Wonderware-010: outer safety timeout — see ReadRawAsync.
            using var requestCts = BuildRequestCts(_config, ct);
            var token = requestCts.Token;
            try
            {
                EnsureConnected();
@@ -367,10 +491,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                if (!query.StartQuery(args, out var error))
                {
-                    Log.Warning("Historian SDK aggregate query start failed for {Tag}: {Error}", tagName, error.ErrorCode);
+                    HandleStartQueryFailure(
-                    RecordFailure($"aggregate StartQuery: {error.ErrorCode}");
+                        $"aggregate query for tag '{tagName}'", error, isEventConnection: false);
                    HandleConnectionError();
                    return Task.FromResult(results);
                }
                // Apply the same bucket cap as the raw-read path so a wide time range with a
@@ -381,7 +503,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                while (query.MoveNext(out error))
                {
-                    ct.ThrowIfCancellationRequested();
+                    token.ThrowIfCancellationRequested();
                    var result = query.QueryResult;
                    var timestamp = DateTime.SpecifyKind(result.StartDateTime, DateTimeKind.Utc);
@@ -408,11 +530,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            }
            catch (OperationCanceledException) { throw; }
            catch (ObjectDisposedException) { throw; }
            catch (QueryClassStartQueryException) { throw; } // see ReadRawAsync — keep connection
            catch (Exception ex)
            {
                Log.Warning(ex, "HistoryRead aggregate failed for {Tag}", tagName);
                RecordFailure($"aggregate: {ex.Message}");
                HandleConnectionError(ex);
                throw;
            }
            Log.Debug("HistoryRead aggregate ({Aggregate}): {Tag} returned {Count} values",
@@ -430,13 +554,17 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            if (timestamps == null || timestamps.Length == 0)
                return Task.FromResult(results);
            // Driver.Historian.Wonderware-010: outer safety timeout — see ReadRawAsync.
            using var requestCts = BuildRequestCts(_config, ct);
            var token = requestCts.Token;
            try
            {
                EnsureConnected();
                foreach (var timestamp in timestamps)
                {
-                    ct.ThrowIfCancellationRequested();
+                    token.ThrowIfCancellationRequested();
                    using var query = _connection!.CreateHistoryQuery();
                    var args = new HistoryQueryArgs
@@ -490,6 +618,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                Log.Warning(ex, "HistoryRead at-time failed for {Tag}", tagName);
                RecordFailure($"at-time: {ex.Message}");
                HandleConnectionError(ex);
                throw;
            }
            Log.Debug("HistoryRead at-time: {Tag} returned {Count} values for {Timestamps} timestamps",
@@ -504,6 +633,10 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
        {
            var results = new List<HistorianEventDto>();
            // Driver.Historian.Wonderware-010: outer safety timeout — see ReadRawAsync.
            using var requestCts = BuildRequestCts(_config, ct);
            var token = requestCts.Token;
            try
            {
                EnsureEventConnected();
@@ -525,16 +658,14 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
                if (!query.StartQuery(args, out var error))
                {
-                    Log.Warning("Historian SDK event query start failed: {Error}", error.ErrorCode);
+                    HandleStartQueryFailure(
-                    RecordFailure($"events StartQuery: {error.ErrorCode}");
+                        $"event query for source '{sourceName ?? "(all)"}'", error, isEventConnection: true);
                    HandleEventConnectionError();
                    return Task.FromResult(results);
                }
                var count = 0;
                while (query.MoveNext(out error))
                {
-                    ct.ThrowIfCancellationRequested();
+                    token.ThrowIfCancellationRequested();
                    results.Add(ToDto(query.QueryResult));
                    count++;
                    if (maxEvents > 0 && count >= maxEvents) break;
@@ -545,11 +676,13 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            }
            catch (OperationCanceledException) { throw; }
            catch (ObjectDisposedException) { throw; }
            catch (QueryClassStartQueryException) { throw; } // see ReadRawAsync — keep connection
            catch (Exception ex)
            {
                Log.Warning(ex, "HistoryRead events failed for source {Source}", sourceName ?? "(all)");
                RecordFailure($"events: {ex.Message}");
                HandleEventConnectionError(ex);
                throw;
            }
            Log.Debug("HistoryRead events: source={Source} returned {Count} events ({Start} to {End})",
@@ -593,11 +726,20 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
        ///         as a string; this is a known edge case of the SDK binding.
        ///     </para>
        /// </summary>
-        private static object? SelectValue(HistoryQueryResult result)
+        internal static object? SelectValue(HistoryQueryResult result)
            => SelectValueFromPair(result.Value, result.StringValue);
        /// <summary>
        ///     SDK-independent overload of the string-vs-numeric heuristic. Exposed so unit
        ///     tests can pin the logic without having to instantiate the SDK
        ///     <see cref="HistoryQueryResult"/> (whose internal property initialisers make
        ///     it impractical to fake). See Driver.Historian.Wonderware-012.
        /// </summary>
        internal static object? SelectValueFromPair(double value, string? stringValue)
        {
-            if (!string.IsNullOrEmpty(result.StringValue) && result.Value == 0)
+            if (!string.IsNullOrEmpty(stringValue) && value == 0)
-                return result.StringValue;
+                return stringValue;
-            return result.Value;
+            return value;
        }
        internal static double? ExtractAggregateValue(AnalogSummaryQueryResult result, string column)
@@ -3,10 +3,11 @@ using System;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
 {
    /// <summary>
-    ///     OPC-UA-free representation of a single historical data point. The Host returns these
+    ///     OPC-UA-free representation of a single historical data point. The sidecar serialises
-    ///     across the IPC boundary as <c>GalaxyDataValue</c>; the Proxy maps quality and value to
+    ///     these onto the named-pipe wire (<c>HistorianSampleDto</c>) for the .NET 10
-    ///     OPC UA <c>DataValue</c>. Raw MX quality byte is preserved so the Proxy can use the same
+    ///     <c>WonderwareHistorianClient</c>, which maps quality and value into OPC UA
-    ///     quality mapper it already uses for live reads.
+    ///     <c>DataValue</c> on its side. Raw OPC DA quality byte is preserved so the client
    ///     can reuse the same quality mapper it already uses for live reads.
    /// </summary>
    public sealed class HistorianSample
    {
@@ -20,7 +21,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
    /// <summary>
    ///     Result of <see cref="IHistorianDataSource.ReadAggregateAsync"/>. When <see cref="Value"/> is
-    ///     null the aggregate is unavailable for that bucket (Proxy maps to <c>BadNoData</c>).
+    ///     null the aggregate is unavailable for that bucket — the client maps to <c>BadNoData</c>.
    /// </summary>
    public sealed class HistorianAggregateSample
    {
@@ -6,9 +6,11 @@ using System.Threading.Tasks;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
 {
    /// <summary>
-    ///     OPC-UA-free surface for the Wonderware Historian subsystem inside Galaxy.Host.
+    ///     OPC-UA-free surface for the Wonderware Historian subsystem inside the historian
-    ///     Implementations read via the aahClient* SDK; the Proxy side maps returned samples
+    ///     sidecar process. Implementations read via the aahClient* SDK; the .NET 10
-    ///     to OPC UA <c>DataValue</c>.
+    ///     <c>WonderwareHistorianClient</c> on the other side of the named-pipe IPC maps
    ///     returned samples to OPC UA <c>DataValue</c>. The v1 Galaxy.Host / Proxy hosts
    ///     this lived in retired in PR 7.2.
    /// </summary>
    public interface IHistorianDataSource : IDisposable
    {
@@ -205,6 +205,19 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend
            {
                historianEvent.Id = id;
            }
            else
            {
                // Driver.Historian.Wonderware-004: an unparseable / empty EventId previously
                // left Id as Guid.Empty, which made every such alarm collide on the same id
                // with no diagnostic. Synthesize a fresh Guid so each event still gets a
                // unique identifier (the historian still accepts the write — outcome stays
                // Ack — and the sender can correlate the synthesized id via the warning log).
                var synthesized = Guid.NewGuid();
                Log.Warning(
                    "Alarm historian event has non-parseable EventId {EventId} for source {Source}; synthesizing Id={SynthesizedId}",
                    dto.EventId ?? "(null)", dto.SourceName ?? "(none)", synthesized);
                historianEvent.Id = synthesized;
            }
 #pragma warning restore CS0618
            if (!string.IsNullOrEmpty(dto.AckComment))
@@ -21,16 +21,33 @@ public sealed class PipeServer : IDisposable
    private readonly string _sharedSecret;
    private readonly ILogger _logger;
    private readonly CancellationTokenSource _cts = new();
    private readonly CallerVerifier _verifier;
    private NamedPipeServerStream? _current;
    /// <summary>
    ///     Pluggable caller-verification seam. Default implementation calls
    ///     <see cref="VerifyCaller"/>; tests can substitute one that ignores the pipe ACL
    ///     to exercise the rejection paths.
    /// </summary>
    internal delegate bool CallerVerifier(NamedPipeServerStream pipe, SecurityIdentifier allowedSid, out string reason);
    public PipeServer(string pipeName, SecurityIdentifier allowedSid, string sharedSecret, ILogger logger)
        : this(pipeName, allowedSid, sharedSecret, logger, DefaultVerifier) { }
    internal PipeServer(
        string pipeName, SecurityIdentifier allowedSid, string sharedSecret, ILogger logger,
        CallerVerifier verifier)
    {
        _pipeName = pipeName ?? throw new ArgumentNullException(nameof(pipeName));
        _allowedSid = allowedSid ?? throw new ArgumentNullException(nameof(allowedSid));
        _sharedSecret = sharedSecret ?? throw new ArgumentNullException(nameof(sharedSecret));
        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
        _verifier = verifier ?? throw new ArgumentNullException(nameof(verifier));
    }
    private static bool DefaultVerifier(NamedPipeServerStream pipe, SecurityIdentifier allowedSid, out string reason)
        => VerifyCaller(pipe, allowedSid, out reason);
    /// <summary>
    ///     Accepts one connection, performs Hello handshake, then dispatches frames to
    ///     <paramref name="handler"/> until EOF or cancel. Returns when the client disconnects.
@@ -67,8 +84,15 @@ public sealed class PipeServer : IDisposable
                return;
            }
-            if (!VerifyCaller(_current, out var reason))
+            if (!_verifier(_current, _allowedSid, out var reason))
            {
                // Driver.Historian.Wonderware-007: send a rejecting HelloAck so the client
                // learns why instead of having to wait for its own read timeout. The reason
                // tag "caller-sid-mismatch" is symmetric with the shared-secret-mismatch and
                // major-version-mismatch acks the two other rejection paths emit below.
                await writer.WriteAsync(MessageKind.HelloAck,
                    new HelloAck { Accepted = false, RejectReason = $"caller-sid-mismatch: {reason}" },
                    linked.Token).ConfigureAwait(false);
                _logger.Warning("Sidecar IPC caller rejected: {Reason}", reason);
                _current.Disconnect();
                return;
@@ -172,7 +196,7 @@ public sealed class PipeServer : IDisposable
        }
    }
-    private bool VerifyCaller(NamedPipeServerStream pipe, out string reason)
+    private static bool VerifyCaller(NamedPipeServerStream pipe, SecurityIdentifier allowedSid, out string reason)
    {
        try
        {
@@ -181,9 +205,9 @@ public sealed class PipeServer : IDisposable
                using var wi = WindowsIdentity.GetCurrent();
                if (wi.User is null)
                    throw new InvalidOperationException("GetCurrent().User is null — cannot verify caller");
-                if (wi.User != _allowedSid)
+                if (wi.User != allowedSid)
                    throw new UnauthorizedAccessException(
-                        $"caller SID {wi.User.Value} does not match allowed {_allowedSid.Value}");
+                        $"caller SID {wi.User.Value} does not match allowed {allowedSid.Value}");
            });
            reason = string.Empty;
            return true;
@@ -29,6 +29,15 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
 ///             <item><c>C100</c> — Coils[99] (mnemonic).</item>
 ///         </list>
 ///     </para>
 ///     <para>
 ///         <b>Grammar scope — out of band (Driver.Modbus.Addressing-007):</b> per-string byte
 ///         order (<see cref="ModbusStringByteOrder"/>) is NOT expressible through this grammar.
 ///         The DL205 low-byte-first string-packing knob is configurable only via the structured
 ///         tag form (the driver's <c>ModbusTagDefinition.StringByteOrder</c> field). The 3rd
 ///         grammar field is the multi-register word/byte order (ABCD/CDAB/BADC/DCBA), not the
 ///         per-string byte order — adding a 5th token would conflict with the array-count slot.
 ///         See <see cref="ModbusStringByteOrder"/> and <c>docs/v2/dl205.md</c> §Strings.
 ///     </para>
 /// </remarks>
 public static class ModbusAddressParser
 {
@@ -341,8 +350,15 @@ public static class ModbusAddressParser
                    return false;
            }
        }
-        catch (Exception ex) when (ex is ArgumentException or OverflowException)
+        catch (Exception ex)
        {
            // Driver.Modbus.Addressing-006: a try-parse method must never throw, so any helper
            // exception is converted to a structured error. The current helpers throw only
            // ArgumentException (incl. ArgumentOutOfRangeException) and OverflowException, but
            // catching narrowly would silently break the TryParse contract if a helper ever
            // switches to e.g. FormatException from a ushort.Parse swap. Config-bind hot-path
            // callers depend on TryParse returning a structured (false, error) rather than
            // throwing an unhandled exception that escapes their TryParse wrapper.
            error = $"Family-native parse for {family} failed on '{text}': {ex.Message}";
            return false;
        }
@@ -88,6 +88,25 @@ public enum ModbusByteOrder
 ///     each register. Word ordering across multiple registers is always ascending address for
 ///     strings — only the byte order inside each register flips.
 /// </summary>
 /// <remarks>
 ///     <para>
 ///         <b>Grammar scope (Driver.Modbus.Addressing-007):</b> this enum is intentionally NOT
 ///         expressible through the <see cref="ModbusAddressParser"/> grammar string. The grammar
 ///         has no token form for it, and <see cref="ParsedModbusAddress"/> has no field for it —
 ///         a DL205 string tag parsed from the grammar always carries the driver's default order.
 ///     </para>
 ///     <para>
 ///         The string byte order is configurable only via the structured tag definition (the
 ///         driver's <c>ModbusTagDefinition.StringByteOrder</c> field). Adding a grammar token
 ///         was explicitly considered and rejected: the 3rd-field slot is the multi-register
 ///         word/byte order (ABCD/CDAB/BADC/DCBA) and the 4th-field slot is the array count, so
 ///         a fifth <c>:&lt;order&gt;</c> suffix would conflict with the count-shape disambiguation.
 ///         Sites that need per-tag low-byte-first strings must use the structured form. The
 ///         default high-byte-first matches the Modbus spec and Ignition / Kepware default
 ///         behaviour.
 ///     </para>
 ///     <para>See <c>docs/v2/dl205.md</c> §Strings for the DL205-specific rationale.</para>
 /// </remarks>
 public enum ModbusStringByteOrder
 {
    HighByteFirst,
@@ -52,10 +52,14 @@ public static class ModbusModiconAddress
            return false;
        }
-        // Range check up-front — keeps the rest of the parser straight-line. 5-digit Modicon
+        // Range check up-front — keeps the rest of the parser straight-line. Modicon addresses
-        // is always exactly 5 chars (40001..49999, with the lead digit selecting region), and
+        // are exactly 5 or 6 characters: a leading region digit (0/1/3/4 — coils, discrete
-        // 6-digit is exactly 6 (400001..465536-shaped). Anything else is unambiguously
+        // inputs, input registers, holding registers respectively) followed by 4 (5-digit form)
-        // malformed so we reject before doing the per-character work.
+        // or 5 (6-digit form) trailing digits encoding the 1-based register number. The
        // 5-digit form covers 1..9999 per region (e.g. coils 00001..09999, holding registers
        // 40001..49999); the 6-digit form covers the full 1..65536 wire range (e.g. coils
        // 000001..065536, holding 400001..465536). Anything else is unambiguously malformed so
        // we reject before doing the per-character work.
        var s = address.Trim();
        if (s.Length is not (5 or 6))
        {
@@ -100,9 +104,10 @@ public static class ModbusModiconAddress
            return false;
        }
-        // 5-digit form caps at 9999 by construction (4 trailing digits); reject if the parsed
+        // Wire-protocol maximum is register number 65536 (PDU offset 65535). The 5-digit form's
-        // value exceeds the wire-protocol maximum of 65536 (i.e. PDU offset 65535). 6-digit
+        // 4 trailing digits can only encode up to 9999, so this check is reached only by the
-        // form can address the full 65535-offset range.
+        // 6-digit form in practice — but it is applied to both for safety / simplicity rather
        // than relying on the digit-count invariant.
        if (registerNumber > 65536)
        {
            error = $"Modicon register number {registerNumber} exceeds the wire maximum (65536 / PDU offset 65535)";
@@ -21,30 +21,40 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus;
 public sealed class ModbusDriver
    : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IPerCallHostResolver, IDisposable, IAsyncDisposable
 {
-    /// <summary>
+    // ---- instance fields (Driver.Modbus-011: grouped at top for auditability) ----
    ///     #142 multi-unit-ID gateway support: per-tag UnitId override drives per-slave host
    ///     name surfacing through this method. The resilience pipeline keys breakers on the
    ///     returned host string, so a dead RTU slave behind an Ethernet gateway opens its own
    ///     breaker without tripping siblings on the same TCP socket.
    /// </summary>
    public string ResolveHost(string fullReference)
    {
        if (_tagsByName.TryGetValue(fullReference, out var tag))
            return BuildSlaveHostName(ResolveUnitId(tag));
        // Unknown reference — fall back to driver-instance host (single-slave behaviour).
        return HostName;
    }
-    /// <summary>Format a per-slave host string. Multi-slave deployments distinguish breakers by this string.</summary>
+    private readonly ModbusDriverOptions _options;
-    private string BuildSlaveHostName(byte unitId) => $"{_options.Host}:{_options.Port}/unit{unitId}";
+    private readonly Func<ModbusDriverOptions, IModbusTransport> _transportFactory;
    private readonly string _driverInstanceId;
    private readonly ILogger<ModbusDriver> _logger;
    // Polled subscriptions delegate to the shared PollGroupEngine. The driver only supplies
    // the reader + on-change bridge; the engine owns the loop, interval floor, and lifecycle.
    private readonly PollGroupEngine _poll;
    private readonly string _driverInstanceId;
-    public event EventHandler<DataChangeEventArgs>? OnDataChange;
+    private readonly Dictionary<string, ModbusTagDefinition> _tagsByName = new(StringComparer.OrdinalIgnoreCase);
-    public event EventHandler<HostStatusChangedEventArgs>? OnHostStatusChanged;
+
    // Last-published value per tag, keyed by FullReference. Used by ShouldPublish to apply
    // the deadband filter. Stored as object so all numeric types share one map; the comparison
    // does a typed cast inside.
    // Driver.Modbus-001: ShouldPublish runs on the PollGroupEngine onChange callback, which
    // executes on one background Task per subscription — so a multi-subscription driver mutates
    // this map concurrently from several threads. A plain Dictionary corrupts under concurrent
    // writes; ConcurrentDictionary makes every TryGetValue / indexer write thread-safe.
    private readonly ConcurrentDictionary<string, object> _lastPublishedByRef = new(StringComparer.OrdinalIgnoreCase);
    // Last-written value per tag for the WriteOnChangeOnly suppression. Invalidated by reads
    // that return a different value (so an HMI-side change doesn't get masked).
    private readonly Dictionary<string, object?> _lastWrittenByRef = new(StringComparer.OrdinalIgnoreCase);
    private readonly object _lastWrittenLock = new();
    // BitInRegister writes need a read-modify-write against the full holding register. A
    // per-register lock keeps concurrent bit-write callers from stomping on each other.
    private readonly ConcurrentDictionary<ushort, SemaphoreSlim> _rmwLocks = new();
    // #148 auto-prohibited coalesce ranges + #150 bisection state (see ProhibitionState below).
    private readonly Dictionary<(byte Unit, ModbusRegion Region, ushort Start, ushort End), ProhibitionState> _autoProhibited = new();
    private readonly object _autoProhibitedLock = new();
    // Single-host probe state — Modbus driver talks to exactly one endpoint so the "hosts"
    // collection has at most one entry. HostName is the Host:Port string so the Admin UI can
@@ -52,15 +62,39 @@ public sealed class ModbusDriver
    private readonly object _probeLock = new();
    private HostState _hostState = HostState.Unknown;
    private DateTime _hostStateChangedUtc = DateTime.UtcNow;
    private CancellationTokenSource? _probeCts;
    private readonly ModbusDriverOptions _options;
    private readonly Func<ModbusDriverOptions, IModbusTransport> _transportFactory;
    private IModbusTransport? _transport;
-    private DriverHealth _health = new(DriverState.Unknown, null, null);
+    private CancellationTokenSource? _probeCts;
-    private readonly Dictionary<string, ModbusTagDefinition> _tagsByName = new(StringComparer.OrdinalIgnoreCase);
+    private CancellationTokenSource? _reprobeCts;
-    private readonly ILogger<ModbusDriver> _logger;
+    // Driver.Modbus-003: every read / write / probe path writes to _health from a different
    // thread, and GetHealth() reads it without coordination. Reference-assignment on .NET is
    // atomic for sealed-record refs (so no tearing), but without a happens-before barrier a
    // stale snapshot can persist on another core indefinitely. Volatile.Write / Volatile.Read
    // give GetHealth() a defined ordering guarantee: any subsequent read sees at least the
    // most recent write any thread has published. The field stays a plain reference (you can't
    // mark a record-typed field 'volatile' through the C# keyword on every framework version,
    // and the Volatile API is the documented portable form).
    private DriverHealth _health = new(DriverState.Unknown, null, null);
    public event EventHandler<DataChangeEventArgs>? OnDataChange;
    public event EventHandler<HostStatusChangedEventArgs>? OnHostStatusChanged;
    // ---- nested types ----
    /// <summary>
    ///     #150 — per-prohibition state. <c>SplitPending</c> drives the re-probe loop's
    ///     bisection: when true and the range spans &gt; 1 register, the next re-probe
    ///     tries the two halves separately to narrow the actual offending register(s).
    ///     Single-register prohibitions can't be split further; they stay re-probed as-is.
    /// </summary>
    private sealed class ProhibitionState
    {
        public DateTime LastProbedUtc;
        public bool SplitPending;
    }
    // ---- ctor + identity ----
    public ModbusDriver(ModbusDriverOptions options, string driverInstanceId,
        Func<ModbusDriverOptions, IModbusTransport>? transportFactory = null,
@@ -87,19 +121,22 @@ public sealed class ModbusDriver
            });
    }
-    // Last-published value per tag, keyed by FullReference. Used by ShouldPublish to apply
+    /// <summary>
-    // the deadband filter. Stored as object so all numeric types share one map; the comparison
+    ///     #142 multi-unit-ID gateway support: per-tag UnitId override drives per-slave host
-    // does a typed cast inside.
+    ///     name surfacing through this method. The resilience pipeline keys breakers on the
-    // Driver.Modbus-001: ShouldPublish runs on the PollGroupEngine onChange callback, which
+    ///     returned host string, so a dead RTU slave behind an Ethernet gateway opens its own
-    // executes on one background Task per subscription — so a multi-subscription driver mutates
+    ///     breaker without tripping siblings on the same TCP socket.
-    // this map concurrently from several threads. A plain Dictionary corrupts under concurrent
+    /// </summary>
-    // writes; ConcurrentDictionary makes every TryGetValue / indexer write thread-safe.
+    public string ResolveHost(string fullReference)
-    private readonly ConcurrentDictionary<string, object> _lastPublishedByRef = new(StringComparer.OrdinalIgnoreCase);
+    {
        if (_tagsByName.TryGetValue(fullReference, out var tag))
            return BuildSlaveHostName(ResolveUnitId(tag));
        // Unknown reference — fall back to driver-instance host (single-slave behaviour).
        return HostName;
    }
-    // Last-written value per tag for the WriteOnChangeOnly suppression. Invalidated by reads
+    /// <summary>Format a per-slave host string. Multi-slave deployments distinguish breakers by this string.</summary>
-    // that return a different value (so an HMI-side change doesn't get masked).
+    private string BuildSlaveHostName(byte unitId) => $"{_options.Host}:{_options.Port}/unit{unitId}";
    private readonly Dictionary<string, object?> _lastWrittenByRef = new(StringComparer.OrdinalIgnoreCase);
    private readonly object _lastWrittenLock = new();
    private bool ShouldPublish(string tagRef, DataValueSnapshot snapshot)
    {
@@ -131,13 +168,13 @@ public sealed class ModbusDriver
    public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken)
    {
-        _health = new DriverHealth(DriverState.Initializing, null, null);
+        WriteHealth(new DriverHealth(DriverState.Initializing, null, null));
        try
        {
            _transport = _transportFactory(_options);
            await _transport.ConnectAsync(cancellationToken).ConfigureAwait(false);
            foreach (var t in _options.Tags) _tagsByName[t.Name] = t;
-            _health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null);
+            WriteHealth(new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null));
            // PR 23: kick off the probe loop once the transport is up. Initial state stays
            // Unknown until the first probe tick succeeds — avoids broadcasting a premature
@@ -157,7 +194,7 @@ public sealed class ModbusDriver
        }
        catch (Exception ex)
        {
-            _health = new DriverHealth(DriverState.Faulted, null, ex.Message);
+            WriteHealth(new DriverHealth(DriverState.Faulted, null, ex.Message));
            throw;
        }
    }
@@ -170,12 +207,25 @@ public sealed class ModbusDriver
    public async Task ShutdownAsync(CancellationToken cancellationToken)
    {
-        var lastRead = _health.LastSuccessfulRead;
+        var lastRead = ReadHealth().LastSuccessfulRead;
        await TeardownAsync().ConfigureAwait(false);
-        _health = new DriverHealth(DriverState.Unknown, lastRead, null);
+        WriteHealth(new DriverHealth(DriverState.Unknown, lastRead, null));
    }
-    public DriverHealth GetHealth() => _health;
+    public DriverHealth GetHealth() => ReadHealth();
    /// <summary>
    ///     Driver.Modbus-003: barrier-protected read of the multi-thread <c>_health</c> field.
    ///     <c>Volatile.Read</c> guarantees <c>GetHealth()</c> and the in-driver self-reads (the
    ///     Degraded paths that retain <c>LastSuccessfulRead</c>) observe the most recently
    ///     published snapshot rather than a per-core cached stale copy.
    /// </summary>
    private DriverHealth ReadHealth() => Volatile.Read(ref _health);
    /// <summary>
    ///     Driver.Modbus-003: barrier-protected publish of a new <c>_health</c> snapshot.
    /// </summary>
    private void WriteHealth(DriverHealth value) => Volatile.Write(ref _health, value);
    public long GetMemoryFootprint() => 0;
    public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask;
@@ -228,7 +278,7 @@ public sealed class ModbusDriver
            {
                var value = await ReadOneAsync(transport, tag, cancellationToken).ConfigureAwait(false);
                results[i] = new DataValueSnapshot(value, 0u, now, now);
-                _health = new DriverHealth(DriverState.Healthy, now, null);
+                WriteHealth(new DriverHealth(DriverState.Healthy, now, null));
                // Invalidate the WriteOnChangeOnly cache when the read returns a different value
                // — typically an HMI-side or PLC-internal change. Without this, a setpoint
@@ -246,14 +296,14 @@ public sealed class ModbusDriver
            catch (ModbusException mex)
            {
                results[i] = new DataValueSnapshot(null, MapModbusExceptionToStatus(mex.ExceptionCode), null, now);
-                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, mex.Message);
+                WriteHealth(new DriverHealth(DriverState.Degraded, ReadHealth().LastSuccessfulRead, mex.Message));
            }
            catch (Exception ex)
            {
                // Non-Modbus-layer failure: socket dropped, timeout, malformed response. Surface
                // as communication error so callers can distinguish it from tag-level faults.
                results[i] = new DataValueSnapshot(null, StatusBadCommunicationError, null, now);
-                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
+                WriteHealth(new DriverHealth(DriverState.Degraded, ReadHealth().LastSuccessfulRead, ex.Message));
            }
        }
        return results;
@@ -402,29 +452,6 @@ public sealed class ModbusDriver
    /// <summary>Resolve the UnitId for a tag — per-tag override (#142) or driver-level fallback.</summary>
    private byte ResolveUnitId(ModbusTagDefinition tag) => tag.UnitId ?? _options.UnitId;
    /// <summary>
    ///     #148 — runtime-discovered ranges where coalesced reads have failed (typically because
    ///     the PLC has a write-only or protected register mid-block). Subsequent scans skip
    ///     coalescing across these ranges and let the per-tag fallback handle the members.
    ///     Cleared by ReinitializeAsync (operator restart) or by an explicit re-probe API
    ///     (not yet shipped).
    /// </summary>
    /// <summary>
    ///     #150 — per-prohibition state. <c>SplitPending</c> drives the re-probe loop's
    ///     bisection: when true and the range spans &gt; 1 register, the next re-probe
    ///     tries the two halves separately to narrow the actual offending register(s).
    ///     Single-register prohibitions can't be split further; they stay re-probed as-is.
    /// </summary>
    private sealed class ProhibitionState
    {
        public DateTime LastProbedUtc;
        public bool SplitPending;
    }
    private readonly Dictionary<(byte Unit, ModbusRegion Region, ushort Start, ushort End), ProhibitionState> _autoProhibited = new();
    private readonly object _autoProhibitedLock = new();
    private CancellationTokenSource? _reprobeCts;
    private bool RangeIsAutoProhibited(byte unit, ModbusRegion region, ushort start, ushort end)
    {
        lock (_autoProhibitedLock)
@@ -700,9 +727,13 @@ public sealed class ModbusDriver
                blocks.Add((tagStart, tagEnd, new List<(int, ModbusTagDefinition)> { (idx, tag) }));
            }
-            // Issue one PDU per block. On block-level failure mark every member Bad — caller's
+            // Issue one PDU per block. On a Modbus-level exception (illegal data address /
-            // per-tag fallback won't re-try since handled-set already includes them; auto-split-
+            // protected register), record the range as auto-prohibited (#148), leave the
-            // on-failure is a follow-up.
+            // member indices UNhandled, and let the per-tag fallback in ReadAsync read each
            // surviving address individually. On transport-level failure (timeout / socket
            // drop) mark members Bad and short-circuit the per-tag fallback (hitting the
            // dead socket again won't help). #150 bisection narrows the prohibition over
            // subsequent re-probe ticks.
            foreach (var block in blocks)
            {
                if (block.Members.Count == 1)
@@ -725,26 +756,19 @@ public sealed class ModbusDriver
                        handled.Add(idx);
                        InvalidateWriteCacheIfDiverged(fullReferences[idx], value);
                    }
-                    _health = new DriverHealth(DriverState.Healthy, timestamp, null);
+                    WriteHealth(new DriverHealth(DriverState.Healthy, timestamp, null));
                }
                catch (ModbusException mex)
                {
                    // #148 — record the failed range so the planner stops re-coalescing across
-                    // it on subsequent scans. Per-tag fallback reads each member individually
+                    // it on subsequent scans. The members are intentionally NOT added to the
-                    // next time, so healthy tags around the protected hole keep working without
+                    // handled-set: ReadAsync's per-tag fallback runs them individually in the
-                    // operator intervention.
+                    // same scan, so healthy tags around the protected hole keep working without
                    // operator intervention. Members that ARE the protected register will fail
                    // again at single-tag granularity and surface the per-tag exception code
                    // naturally — the block-level mex isn't propagated.
                    RecordAutoProhibition(group.Key.Unit, group.Key.Region, block.Start, block.End);
-
+                    WriteHealth(new DriverHealth(DriverState.Degraded, ReadHealth().LastSuccessfulRead, mex.Message));
                    var status = MapModbusExceptionToStatus(mex.ExceptionCode);
                    foreach (var (idx, _) in block.Members)
                    {
                        // Don't mark members handled — leave them for the per-tag fallback in
                        // the same scan so single-register reads can succeed for any non-
                        // protected member. (Pre-#148 behaviour was to mark all Bad and skip.)
                        // Members that ARE the protected register will fail again at single-tag
                        // granularity and surface the per-tag exception code naturally.
                    }
                    _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, mex.Message);
                }
                catch (Exception ex)
                {
@@ -758,7 +782,7 @@ public sealed class ModbusDriver
                        results[idx] = new DataValueSnapshot(null, StatusBadCommunicationError, null, timestamp);
                        handled.Add(idx);
                    }
-                    _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
+                    WriteHealth(new DriverHealth(DriverState.Degraded, ReadHealth().LastSuccessfulRead, ex.Message));
                }
            }
        }
@@ -926,13 +950,11 @@ public sealed class ModbusDriver
        }
    }
-    // BitInRegister writes need a read-modify-write against the full holding register. A
+    // BitInRegister writes need a read-modify-write against the full holding register. The
-    // per-register lock keeps concurrent bit-write callers from stomping on each other —
+    // per-register lock (declared at the top of the class) keeps concurrent bit-write callers
-    // Write bit 0 and Write bit 5 targeting the same register can arrive on separate
+    // from stomping on each other — Write bit 0 and Write bit 5 targeting the same register
-    // subscriber threads, and without serialising the RMW the second-to-commit value wins
+    // can arrive on separate subscriber threads, and without serialising the RMW the
-    // + the first bit update is lost.
+    // second-to-commit value wins + the first bit update is lost.
    private readonly System.Collections.Concurrent.ConcurrentDictionary<ushort, SemaphoreSlim> _rmwLocks = new();
    private SemaphoreSlim GetRmwLock(ushort address) =>
        _rmwLocks.GetOrAdd(address, _ => new SemaphoreSlim(1, 1));
@@ -1410,12 +1432,34 @@ public sealed class ModbusDriver
        }
    }
    /// <summary>
    ///     Map a Modbus logical type to the driver-agnostic <see cref="DriverDataType"/> used
    ///     by the address-space builder.
    /// </summary>
    /// <remarks>
    ///     <para>
    ///         <b>Driver.Modbus-007 — Int64 / UInt64 surfacing limitation:</b>
    ///         <see cref="DriverDataType"/> does not yet include an Int64 enum member, so 64-bit
    ///         Modbus tags currently surface as <see cref="DriverDataType.Int32"/> on the OPC UA
    ///         address space. The wire codec (<c>DecodeRegister</c> / <c>EncodeRegister</c>) is
    ///         correct — values round-trip as 64-bit <c>long</c> / <c>ulong</c> through
    ///         <c>ReadAsync</c> / <c>WriteAsync</c>. Only the variable node's <c>DataType</c>
    ///         attribute is misreported. Clients that consume the type advertisement will see a
    ///         type/value mismatch for values outside the 32-bit signed range. Operators
    ///         configuring <c>I_64</c> / <c>UI_64</c> tags should be aware of this until the
    ///         tracked <c>DriverDataType.Int64</c> follow-up ships.
    ///     </para>
    /// </remarks>
    private static DriverDataType MapDataType(ModbusDataType t) => t switch
    {
        ModbusDataType.Bool or ModbusDataType.BitInRegister => DriverDataType.Boolean,
        ModbusDataType.Int16 or ModbusDataType.Int32 => DriverDataType.Int32,
        ModbusDataType.UInt16 or ModbusDataType.UInt32 => DriverDataType.Int32,
-        ModbusDataType.Int64 or ModbusDataType.UInt64 => DriverDataType.Int32, // widening to Int32 loses precision; PR 25 adds Int64 to DriverDataType
+        // Driver.Modbus-007: Int64 / UInt64 currently surface as Int32 because DriverDataType
        // has no Int64 member yet. The wire codec preserves the 64-bit value; only the OPC UA
        // node's declared DataType is widened. Tracked for a follow-up that adds the enum
        // member + node-type advertisement.
        ModbusDataType.Int64 or ModbusDataType.UInt64 => DriverDataType.Int32,
        ModbusDataType.Float32 => DriverDataType.Float32,
        ModbusDataType.Float64 => DriverDataType.Float64,
        ModbusDataType.String => DriverDataType.String,
@@ -111,16 +111,22 @@ public static class ModbusDriverFactoryExtensions
        var name = t.Name ?? throw new InvalidOperationException(
            $"Modbus config for '{driverInstanceId}' has a tag missing Name");
        // Driver.Modbus-009: a String tag with StringLength = 0 yields RegisterCount = 0, which
        // turns into an FC03/FC04 with quantity 0 — a spec-illegal request the PLC rejects with
        // exception 03. Catch the misconfiguration at bind time with a clear diagnostic instead
        // of waiting for the cryptic Illegal Data Value to surface at runtime.
        // AddressString takes precedence over the structured fields (Region/Address/DataType/
        // ByteOrder/BitIndex/StringLength/ArrayCount). Tags can mix forms freely — newer pasted
        // rows use the grammar string, legacy rows keep the structured form. Fields not derivable
        // from the grammar (Writable, WriteIdempotent, StringByteOrder) always come from the DTO.
        ModbusTagDefinition tag;
        if (!string.IsNullOrWhiteSpace(t.AddressString))
        {
            if (!ModbusAddressParser.TryParse(t.AddressString, family, melsecSubFamily, out var parsed, out var parseError))
                throw new InvalidOperationException(
                    $"Modbus tag '{name}' in '{driverInstanceId}' has invalid AddressString '{t.AddressString}': {parseError}");
-            return new ModbusTagDefinition(
+            tag = new ModbusTagDefinition(
                Name: name,
                Region: parsed!.Region,
                Address: parsed.Offset,
@@ -137,8 +143,9 @@ public static class ModbusDriverFactoryExtensions
                Deadband: t.Deadband,
                UnitId: t.UnitId);
        }
-
+        else
-        return new ModbusTagDefinition(
+        {
            tag = new ModbusTagDefinition(
                Name: name,
                Region: ParseEnum<ModbusRegion>(t.Region, t.Name, driverInstanceId, "Region"),
                Address: t.Address ?? throw new InvalidOperationException(
@@ -159,6 +166,24 @@ public static class ModbusDriverFactoryExtensions
                UnitId: t.UnitId);
        }
        ValidateStringLength(tag, driverInstanceId);
        return tag;
    }
    /// <summary>
    ///     Driver.Modbus-009: reject <c>StringLength = 0</c> for <c>String</c>-typed tags. The
    ///     driver computes <c>RegisterCount = (StringLength + 1) / 2</c> which would emit an
    ///     FC03/FC04 with <c>quantity = 0</c>, a spec-illegal request the PLC rejects with
    ///     exception 03 (Illegal Data Value). Surface as a clear bind-time error.
    /// </summary>
    private static void ValidateStringLength(ModbusTagDefinition tag, string driverInstanceId)
    {
        if (tag.DataType == ModbusDataType.String && tag.StringLength < 1)
            throw new InvalidOperationException(
                $"Modbus tag '{tag.Name}' in '{driverInstanceId}' has DataType=String but StringLength={tag.StringLength}. " +
                $"String tags must declare StringLength >= 1 (the number of ASCII characters, packed 2 per register).");
    }
    private static T ParseEnum<T>(string? raw, string? tagName, string driverInstanceId, string field) where T : struct, Enum
    {
        if (string.IsNullOrWhiteSpace(raw))
@@ -72,10 +72,11 @@ public sealed class ModbusDriverOptions
    public bool UseFC16ForSingleRegisterWrites { get; init; } = false;
    /// <summary>
-    ///     Reserved kill-switch for FC23 (Read/Write Multiple Registers). The driver does not
+    ///     <b>Reserved / no-op</b> kill-switch for FC23 (Read/Write Multiple Registers). The
-    ///     currently emit FC23, so this option is a no-op today but exists so future block-read
+    ///     driver does not currently emit FC23 — toggling this option has no observable effect
-    ///     coalescing work that opts into FC23 can be disabled per-deployment without a code
+    ///     today. The slot exists so a future block-read-coalescing enhancement that opts into
-    ///     change. Default <c>false</c> (FC23 not used either way today).
+    ///     FC23 can be disabled per-deployment without a code change. Track Driver.Modbus-007
    ///     for the wiring follow-up. Default <c>false</c>.
    /// </summary>
    public bool DisableFC23 { get; init; } = false;
@@ -122,6 +123,18 @@ public sealed class ModbusDriverOptions
    ///     masked. Default <c>false</c> preserves the historical "every write goes to the wire"
    ///     behaviour. Per-tag deadband lives on <c>ModbusTagDefinition.Deadband</c>.
    /// </summary>
    /// <remarks>
    ///     <para>
    ///         <b>Driver.Modbus-010 — write-only-tag caveat:</b> the suppression cache is only
    ///         invalidated by a <i>read</i> that returns a divergent value. A tag that is never
    ///         subscribed or polled (write-only setpoints, command registers) never sees its
    ///         cache entry refreshed — so a value the operator believes was re-asserted is
    ///         silently suppressed forever after the first write. There is no time- or
    ///         count-based expiry. If you set <see cref="WriteOnChangeOnly"/> = <c>true</c>,
    ///         either subscribe / poll every tag that needs deterministic re-write, or leave
    ///         this option <c>false</c> for the affected driver instance.
    ///     </para>
    /// </remarks>
    public bool WriteOnChangeOnly { get; init; } = false;
    /// <summary>
@@ -91,13 +91,31 @@ public sealed class ModbusTcpTransport : IModbusTransport
        try
        {
            client.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true);
-            client.Client.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveTime, (int)opts.Time.TotalSeconds);
+            // Driver.Modbus-009: a TimeSpan < 1s previously truncated to 0 via the int cast,
-            client.Client.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveInterval, (int)opts.Interval.TotalSeconds);
+            // which Windows / Linux interpret as "use the default" — silently defeating the
            // configured keep-alive timing. Round up to at least 1 second so a sub-second
            // configuration still produces a real keep-alive cadence. Negative values are
            // also clamped to 1 to avoid surfacing as OS errors.
            client.Client.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveTime,
                ClampToWholeSeconds(opts.Time));
            client.Client.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveInterval,
                ClampToWholeSeconds(opts.Interval));
            client.Client.SetSocketOption(SocketOptionLevel.Tcp, SocketOptionName.TcpKeepAliveRetryCount, opts.RetryCount);
        }
        catch { /* best-effort; older OSes may not expose the granular knobs */ }
    }
    /// <summary>
    ///     Driver.Modbus-009: cast a <see cref="TimeSpan"/> to a whole number of seconds with a
    ///     minimum of 1 — protects callers from the int-cast truncation that turned 500&#160;ms
    ///     keep-alive timing into "use the default" on most OSes.
    /// </summary>
    internal static int ClampToWholeSeconds(TimeSpan ts)
    {
        var seconds = (int)Math.Ceiling(ts.TotalSeconds);
        return seconds < 1 ? 1 : seconds;
    }
    public async Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
    {
        if (_disposed) throw new ObjectDisposedException(nameof(ModbusTcpTransport));
@@ -494,9 +494,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
        // Tear down remote subscriptions first — otherwise Session.Close will try and may fail
        // with BadSubscriptionIdInvalid noise in the upstream log. _subscriptions is cleared
        // whether or not the wire-side delete succeeds since the local handles are useless
-        // after close anyway.
+        // after close anyway. Before deleting each subscription we detach the Notification
        // handlers we attached at subscribe time so the SDK's invocation list no longer
        // holds the driver instance through the closure (Driver.OpcUaClient-014).
        foreach (var rs in _subscriptions.Values)
        {
            DetachNotificationHandlers(rs.ItemHandlers);
            try { await rs.Subscription.DeleteAsync(silent: true, cancellationToken).ConfigureAwait(false); }
            catch { /* best-effort */ }
        }
@@ -504,6 +507,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
        foreach (var ras in _alarmSubscriptions.Values)
        {
            try { ras.EventItem.Notification -= ras.Handler; }
            catch { /* best-effort */ }
            try { await ras.Subscription.DeleteAsync(silent: true, cancellationToken).ConfigureAwait(false); }
            catch { /* best-effort */ }
        }
@@ -1005,7 +1010,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
                ? MapUpstreamDataType(dtId)
                : DriverDataType.Int32;
            var valueRank = StatusCode.IsGood(valueRankDv.StatusCode) && valueRankDv.Value is int vr ? vr : -1;
-            var isArray = valueRank >= 0; // -1 = scalar; 1+ = array dimensions; 0 = one-dimensional array
+            // OPC UA Part 3 ValueRank constants: -3 = ScalarOrOneDimension, -2 = Any,
            // -1 = Scalar, 0 = OneOrMoreDimensions, 1 = OneDimension, >1 = N specific dimensions.
            // Deliberate choice: treat anything >= 0 as an array (the spec guarantees -3/-2/-1
            // are the only negative values, and any non-negative rank denotes at least one
            // array dimension). -3 ScalarOrOneDimension and -2 Any are conservatively treated
            // as scalar — array-of-one is exposed as scalar to the local address space until
            // the upstream variable carries a concrete dimensioned rank.
            var isArray = valueRank >= 0;
            var access = StatusCode.IsGood(accessDv.StatusCode) && accessDv.Value is byte ab ? ab : (byte)0;
            var securityClass = MapAccessLevelToSecurityClass(access);
            var historizing = StatusCode.IsGood(histDv.StatusCode) && histDv.Value is bool b && b;
@@ -1110,6 +1122,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
            session.AddSubscription(subscription);
            await subscription.CreateAsync(cancellationToken).ConfigureAwait(false);
            // Track each (MonitoredItem, handler) pair so UnsubscribeAsync / ShutdownAsync
            // can detach the Notification delegate before disposing the session
            // (Driver.OpcUaClient-014). The lambda captures `handle`, so we must hold the
            // exact delegate instance returned by `+=` to be able to remove it.
            var itemHandlers = new List<MonitoredItemNotificationHandle>();
            foreach (var fullRef in fullReferences)
            {
                if (!TryParseNodeId(session, fullRef, out var nodeId)) continue;
@@ -1128,12 +1145,15 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
                {
                    Handle = fullRef,
                };
-                item.Notification += (mi, args) => OnMonitoredItemNotification(handle, mi, args);
+                MonitoredItemNotificationEventHandler notifHandler = (mi, args) =>
                    OnMonitoredItemNotification(handle, mi, args);
                item.Notification += notifHandler;
                itemHandlers.Add(new MonitoredItemNotificationHandle(item, notifHandler));
                subscription.AddItem(item);
            }
            await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false);
-            _subscriptions[id] = new RemoteSubscription(subscription, handle);
+            _subscriptions[id] = new RemoteSubscription(subscription, handle, itemHandlers);
        }
        finally { _gate.Release(); }
@@ -1148,12 +1168,28 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
        await _gate.WaitAsync(cancellationToken).ConfigureAwait(false);
        try
        {
            // Detach Notification handlers BEFORE deleting the subscription so the SDK's
            // MonitoredItem.Notification multicast invocation list no longer holds a
            // closure that captures the driver instance (Driver.OpcUaClient-014). The
            // delegate stored on RemoteSubscription is the exact instance that was added,
            // so `-=` removes it cleanly.
            DetachNotificationHandlers(rs.ItemHandlers);
            try { await rs.Subscription.DeleteAsync(silent: true, cancellationToken).ConfigureAwait(false); }
            catch { /* best-effort — the subscription may already be gone on reconnect */ }
        }
        finally { _gate.Release(); }
    }
    private static void DetachNotificationHandlers(IReadOnlyList<MonitoredItemNotificationHandle> items)
    {
        for (var i = 0; i < items.Count; i++)
        {
            var pair = items[i];
            try { pair.Item.Notification -= pair.Handler; }
            catch { /* best-effort — SDK may have already cleared its invocation list on session loss */ }
        }
    }
    private void OnMonitoredItemNotification(OpcUaSubscriptionHandle handle, MonitoredItem item, MonitoredItemNotificationEventArgs args)
    {
        // args.NotificationValue arrives as a MonitoredItemNotification for value-change
@@ -1170,7 +1206,28 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
        OnDataChange?.Invoke(this, new DataChangeEventArgs(handle, fullRef, snapshot));
    }
-    private sealed record RemoteSubscription(Subscription Subscription, OpcUaSubscriptionHandle Handle);
+    /// <summary>
    ///     Live data-change subscription bookkeeping. Holds the SDK <see cref="Subscription"/>,
    ///     the local handle, and the per-MonitoredItem (item, handler) pairs so
    ///     <see cref="UnsubscribeAsync"/> / <see cref="ShutdownAsync"/> can detach the
    ///     Notification delegates before the SDK disposes the subscription
    ///     (Driver.OpcUaClient-014).
    /// </summary>
    private sealed record RemoteSubscription(
        Subscription Subscription,
        OpcUaSubscriptionHandle Handle,
        IReadOnlyList<MonitoredItemNotificationHandle> ItemHandlers);
    /// <summary>
    ///     One (MonitoredItem, handler-delegate-instance) pair captured at subscribe time so
    ///     the same delegate instance can be `-=` removed at unsubscribe time. The lambda
    ///     captures the local <c>OpcUaSubscriptionHandle</c>, which is what makes detach
    ///     necessary — without it the SDK's multicast invocation list holds the driver
    ///     through the closure until the session itself is disposed.
    /// </summary>
    private sealed record MonitoredItemNotificationHandle(
        MonitoredItem Item,
        MonitoredItemNotificationEventHandler Handler);
    private sealed record OpcUaSubscriptionHandle(long Id) : ISubscriptionHandle
    {
@@ -1259,11 +1316,17 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
            {
                Handle = handle,
            };
-            eventItem.Notification += (mi, args) => OnEventNotification(handle, sourceFilter, mi, args);
+            // Capture the exact delegate instance so UnsubscribeAlarmsAsync / ShutdownAsync
            // can `-=` it later (Driver.OpcUaClient-014). The lambda captures `handle` and
            // `sourceFilter`, so without the explicit detach the SDK's invocation list keeps
            // the driver instance alive until the session itself is disposed.
            MonitoredItemNotificationEventHandler notifHandler = (mi, args) =>
                OnEventNotification(handle, sourceFilter, mi, args);
            eventItem.Notification += notifHandler;
            subscription.AddItem(eventItem);
            await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false);
-            _alarmSubscriptions[id] = new RemoteAlarmSubscription(subscription, handle);
+            _alarmSubscriptions[id] = new RemoteAlarmSubscription(subscription, handle, eventItem, notifHandler);
        }
        finally { _gate.Release(); }
@@ -1278,6 +1341,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
        await _gate.WaitAsync(cancellationToken).ConfigureAwait(false);
        try
        {
            // Detach the Notification handler before deleting the subscription so the SDK's
            // multicast invocation list no longer holds the driver instance through the
            // closure (Driver.OpcUaClient-014).
            try { rs.EventItem.Notification -= rs.Handler; }
            catch { /* best-effort */ }
            try { await rs.Subscription.DeleteAsync(silent: true, cancellationToken).ConfigureAwait(false); }
            catch { /* best-effort — session may already be gone across a reconnect */ }
        }
@@ -1405,7 +1473,17 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
        _ => AlarmSeverity.Critical,
    };
-    private sealed record RemoteAlarmSubscription(Subscription Subscription, OpcUaAlarmSubscriptionHandle Handle);
+    /// <summary>
    ///     Live alarm-event subscription bookkeeping. Holds the SDK <see cref="Subscription"/>,
    ///     the local handle, the single event-MonitoredItem (`Server/Events`), and the exact
    ///     handler delegate instance so unsubscribe / shutdown can detach the Notification
    ///     event before the SDK disposes the subscription (Driver.OpcUaClient-014).
    /// </summary>
    private sealed record RemoteAlarmSubscription(
        Subscription Subscription,
        OpcUaAlarmSubscriptionHandle Handle,
        MonitoredItem EventItem,
        MonitoredItemNotificationEventHandler Handler);
    private sealed record OpcUaAlarmSubscriptionHandle(long Id) : IAlarmSubscriptionHandle
    {
@@ -167,6 +167,7 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
        TwinCATDataType type,
        int? bitIndex,
        TimeSpan cycleTime,
        int maxDelayMs,
        Action<string, object?> onChange,
        CancellationToken cancellationToken)
    {
@@ -175,9 +176,11 @@ internal sealed class AdsTwinCATClient : ITwinCATClient
        // tcadsnetref/7313319051 — "The unit is 1ms"). AdsTransMode.OnChange fires when
        // the value differs; OnCycle fires every cycle. OnChange is the right default for
        // OPC UA data-change semantics — the PLC already has the best view of "has this
-        // changed" so we let it decide.
+        // changed" so we let it decide. maxDelayMs > 0 lets TwinCAT batch notifications up
        // to that delay before pushing them — exposed via TwinCATDriverOptions
        // (Driver.TwinCAT-014).
        var cycleMs = (int)Math.Max(1, cycleTime.TotalMilliseconds);
-        var settings = new NotificationSettings(AdsTransMode.OnChange, cycleMs, 0);
+        var settings = new NotificationSettings(AdsTransMode.OnChange, cycleMs, Math.Max(0, maxDelayMs));
        // AddDeviceNotificationExAsync returns Task<ResultHandle>; AdsNotificationEx fires
        // with the handle as part of the event args so we use the handle as the correlation
@@ -67,6 +67,9 @@ public interface ITwinCATClient : IDisposable
    /// <param name="type">Declared type; drives the native layout + callback value boxing.</param>
    /// <param name="bitIndex">For BOOL-within-word tags — the bit to extract from the parent word.</param>
    /// <param name="cycleTime">Minimum interval between change notifications (native-floor depends on target).</param>
    /// <param name="maxDelayMs">Maximum batching delay in milliseconds — TwinCAT may coalesce
    ///     notifications up to this delay before pushing them. <c>0</c> = no batching, push
    ///     immediately (Driver.TwinCAT-014).</param>
    /// <param name="onChange">Invoked with <c>(symbolPath, boxedValue)</c> per notification.</param>
    /// <param name="cancellationToken">Cancels the initial registration; does not tear down an established notification.</param>
    Task<ITwinCATNotificationHandle> AddNotificationAsync(
@@ -74,6 +77,7 @@ public interface ITwinCATClient : IDisposable
        TwinCATDataType type,
        int? bitIndex,
        TimeSpan cycleTime,
        int maxDelayMs,
        Action<string, object?> onChange,
        CancellationToken cancellationToken);
@@ -21,10 +21,16 @@ public enum TwinCATDataType
    LReal,  // 64-bit IEEE-754
    String, // ASCII string
    WString,// UTF-16 string
-    Time,   // TIME — ms since epoch of day, stored as UDINT
+    // IEC 61131-3 / TwinCAT temporal types — all stored on the wire as 32-bit unsigned (UDINT)
-    Date,   // DATE — days since 1970-01-01, stored as UDINT
+    // raw counters. The driver surfaces them as DriverDataType.UInt32 (Driver.TwinCAT-002), so
-    DateTime, // DT — seconds since 1970-01-01, stored as UDINT
+    // operators see the raw counter, not a decoded date/duration. Proper decoding to
-    TimeOfDay,// TOD — ms since midnight, stored as UDINT
+    // DriverDataType.DateTime is a future enhancement; until then comments accurately describe
    // the on-wire encoding so the next implementer doesn't re-derive it wrong
    // (Driver.TwinCAT-004).
    Time,   // TIME — duration in milliseconds, stored as UDINT (32-bit unsigned)
    Date,   // DATE — seconds since 1970-01-01 truncated to a day boundary, stored as UDINT
    DateTime, // DT (DATE_AND_TIME) — seconds since 1970-01-01, stored as UDINT
    TimeOfDay,// TOD — milliseconds since midnight, stored as UDINT
    /// <summary>UDT / FB instance. Resolved per member at discovery time.</summary>
    Structure,
 }
@@ -377,6 +377,7 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
                var reg = await client.AddNotificationAsync(
                    symbolName, def.DataType, bitIndex, publishingInterval,
                    _options.NotificationMaxDelayMs,
                    (_, value) => OnDataChange?.Invoke(this,
                        new DataChangeEventArgs(handle, reference, new DataValueSnapshot(
                            value, TwinCATStatusMapper.Good, DateTime.UtcNow, DateTime.UtcNow))),
@@ -433,7 +434,10 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
            var success = false;
            try
            {
-                var client = await EnsureConnectedAsync(state, ct).ConfigureAwait(false);
+                // Probe-initiated connects honor TwinCATProbeOptions.Timeout — distinct from
                // the driver-wide _options.Timeout used by reads/writes (Driver.TwinCAT-014).
                var client = await EnsureConnectedAsync(state, ct, _options.Probe.Timeout)
                    .ConfigureAwait(false);
                success = await client.ProbeAsync(ct).ConfigureAwait(false);
            }
            catch (OperationCanceledException) when (ct.IsCancellationRequested) { break; }
@@ -469,11 +473,25 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
    // ---- IPerCallHostResolver ----
    /// <summary>
    ///     Documented sentinel returned by <see cref="ResolveHost"/> when neither the tag nor a
    ///     fallback device is configured. Empty-string never matches an
    ///     <see cref="HostConnectivityStatus.HostName"/> emitted by this driver (every real
    ///     host is an <c>ads://…</c> URI), so it cleanly signals "unresolved" without colliding
    ///     with a real host key. Used to be <see cref="DriverInstanceId"/>, which is a logical
    ///     config-DB identifier — that collided with consumers who expected the resolver and the
    ///     connectivity-status table to share keys (Driver.TwinCAT-006).
    /// </summary>
    public const string UnresolvedHostSentinel = "";
    public string ResolveHost(string fullReference)
    {
        if (_tagsByName.TryGetValue(fullReference, out var def))
            return def.DeviceHostAddress;
-        return _options.Devices.FirstOrDefault()?.HostAddress ?? DriverInstanceId;
+        // First device's HostAddress when one exists; otherwise the unresolved sentinel —
        // intentionally NOT DriverInstanceId, which is a config-DB key, not a host address
        // (Driver.TwinCAT-006).
        return _options.Devices.FirstOrDefault()?.HostAddress ?? UnresolvedHostSentinel;
    }
    /// <summary>
@@ -484,7 +502,8 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
    ///     AB-CIP drivers serialize device access the same way; single-connection-per-PLC is
    ///     also what docs/v2/driver-specs.md recommends.
    /// </summary>
-    private async Task<ITwinCATClient> EnsureConnectedAsync(DeviceState device, CancellationToken ct)
+    private async Task<ITwinCATClient> EnsureConnectedAsync(
        DeviceState device, CancellationToken ct, TimeSpan? timeoutOverride = null)
    {
        // Fast path — already connected, no gate needed.
        if (device.Client is { IsConnected: true } fast) return fast;
@@ -504,9 +523,13 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
            var client = _clientFactory.Create();
            client.OnSymbolVersionChanged += HandleSymbolVersionChanged;
            // timeoutOverride lets the probe loop use TwinCATProbeOptions.Timeout for probe-
            // initiated connects rather than the driver-level _options.Timeout
            // (Driver.TwinCAT-014). Reads / writes pass null and get the driver default.
            var effectiveTimeout = timeoutOverride ?? _options.Timeout;
            try
            {
-                await client.ConnectAsync(device.ParsedAddress, _options.Timeout, ct)
+                await client.ConnectAsync(device.ParsedAddress, effectiveTimeout, ct)
                    .ConfigureAwait(false);
                _logger.LogInformation(
                    "TwinCAT driver '{DriverInstanceId}' connected to {HostAddress}",
@@ -542,7 +565,43 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery
            "TwinCAT symbol-version-changed (DeviceSymbolVersionInvalid 0x0711) — PLC program re-downloaded",
            ScopeHint: "TwinCAT"));
-    public void Dispose() => DisposeAsync().AsTask().GetAwaiter().GetResult();
+    /// <summary>
    ///     Synchronous teardown — no <c>await</c>, no captured sync context. The OPC UA stack
    ///     thread can call <see cref="Dispose"/>; routing through <c>DisposeAsync().GetResult()</c>
    ///     can deadlock on a single-threaded sync context (Driver.TwinCAT-015,
    ///     docs/v2/driver-stability.md). The operations here are all genuinely synchronous —
    ///     cancel tokens, wait on task handles with a hard timeout, dispose clients — so a
    ///     synchronous path does the right thing without re-entering the scheduler.
    /// </summary>
    public void Dispose()
    {
        // Dispose native subscriptions first — handle disposal is sync.
        foreach (var sub in _nativeSubs.Values)
            foreach (var r in sub.Registrations) { try { r.Dispose(); } catch { } }
        _nativeSubs.Clear();
        // PollGroupEngine.DisposeAsync awaits loop tasks; we drive that synchronously here
        // (bounded wait — same 5s ceiling DisposeAsync uses internally) using Wait() on the
        // returned ValueTask so no sync-context capture happens.
        try { _poll.DisposeAsync().AsTask().Wait(TimeSpan.FromSeconds(5)); } catch { }
        foreach (var state in _devices.Values)
        {
            try { state.ProbeCts?.Cancel(); } catch { }
            if (state.ProbeTask is Task pt)
            {
                try { pt.Wait(TimeSpan.FromSeconds(2)); } catch { /* probe-cancel races are expected */ }
            }
            state.ProbeCts?.Dispose();
            state.ProbeCts = null;
            state.DisposeClient();
            state.DisposeGate();
        }
        _devices.Clear();
        _tagsByName.Clear();
        _health = new DriverHealth(DriverState.Unknown, _health.LastSuccessfulRead, null);
    }
    public async ValueTask DisposeAsync() => await ShutdownAsync(CancellationToken.None).ConfigureAwait(false);
    internal sealed class DeviceState(TwinCATAmsAddress parsedAddress, TwinCATDeviceOptions options)
@@ -60,9 +60,19 @@ public static class TwinCATDriverFactoryExtensions
            Timeout = TimeSpan.FromMilliseconds(dto.TimeoutMs ?? 2_000),
            UseNativeNotifications = dto.UseNativeNotifications ?? true,
            EnableControllerBrowse = dto.EnableControllerBrowse ?? false,
            NotificationMaxDelayMs = dto.NotificationMaxDelayMs ?? 0,
        };
    }
    /// <summary>
    ///     Test-visible wrapper around <see cref="ParseOptions"/> for the regression suite —
    ///     keeps the public driver surface unchanged while letting tests assert that JSON
    ///     fields like <c>NotificationMaxDelayMs</c> and <c>Structure</c>-tag rejection are
    ///     honored end-to-end.
    /// </summary>
    public static TwinCATDriverOptions ParseOptionsForTests(string driverConfigJson, string driverInstanceId)
        => ParseOptions(driverConfigJson, driverInstanceId);
    private static TwinCATTagDefinition BuildTag(TwinCATTagDto t, string driverInstanceId)
    {
        var dataType = ParseEnum<TwinCATDataType>(t.DataType, t.Name, driverInstanceId, "DataType");
@@ -117,6 +127,7 @@ public static class TwinCATDriverFactoryExtensions
        public int? TimeoutMs { get; init; }
        public bool? UseNativeNotifications { get; init; }
        public bool? EnableControllerBrowse { get; init; }
        public int? NotificationMaxDelayMs { get; init; }
        public List<TwinCATDeviceDto>? Devices { get; init; }
        public List<TwinCATTagDto>? Tags { get; init; }
        public TwinCATProbeDto? Probe { get; init; }
@@ -32,6 +32,16 @@ public sealed class TwinCATDriverOptions
    ///     the strict-config path for deployments where only declared tags should appear.
    /// </summary>
    public bool EnableControllerBrowse { get; init; }
    /// <summary>
    ///     Maximum batching delay in milliseconds for ADS device notifications. Passed to
    ///     <c>NotificationSettings</c> as the max-delay value: TwinCAT may coalesce notifications
    ///     up to this delay before pushing them. <c>0</c> (default) = no batching, push
    ///     immediately. Useful for high-churn signals where the OPC UA subscriber tolerates a
    ///     small delay in exchange for fewer wire round-trips. Listed in <c>docs/v2/driver-specs.md</c>
    ///     section 6 — was previously hard-coded to 0 (Driver.TwinCAT-014).
    /// </summary>
    public int NotificationMaxDelayMs { get; init; }
 }
 /// <summary>
@@ -0,0 +1,142 @@
 using System;
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
 using ArchestrA;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.Backend;
 /// <summary>
 ///     Driver.Historian.Wonderware-012 coverage — pins <see cref="HistorianDataSource"/>'s
 ///     connect-failover / cooldown loop via a fake <see cref="IHistorianConnectionFactory"/>.
 ///     A live <see cref="HistorianAccess"/> is never instantiated; the fake throws on every
 ///     attempt so the read path surfaces the connect failure without touching the SDK.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class HistorianDataSourceConnectFailoverTests
 {
    [Fact]
    public async Task ReadRaw_when_no_nodes_are_healthy_throws_so_IPC_surfaces_Success_false()
    {
        var cfg = new HistorianConfiguration
        {
            Enabled = true,
            ServerNames = new List<string> { "node-a" },
            FailureCooldownSeconds = 60,
            // Disable the outer request timeout so the test doesn't race the connect failure
            // against the timeout (we want the connect failure path, not a TimeoutException).
            RequestTimeoutSeconds = 0,
        };
        var ds = new HistorianDataSource(cfg, new ThrowingConnectionFactory());
        // Read methods used to swallow the connect exception and return an empty list with
        // Success=true; the fix re-throws so the IPC layer surfaces Success=false. The
        // exception must therefore propagate.
        await Should.ThrowAsync<Exception>(() => ds.ReadRawAsync(
            "Tank.Level",
            new DateTime(2026, 5, 1, 0, 0, 0, DateTimeKind.Utc),
            new DateTime(2026, 5, 1, 0, 1, 0, DateTimeKind.Utc),
            maxValues: 100,
            CancellationToken.None));
    }
    [Fact]
    public async Task ReadRaw_tries_each_cluster_node_in_order_until_one_succeeds_or_all_fail()
    {
        var cfg = new HistorianConfiguration
        {
            Enabled = true,
            ServerNames = new List<string> { "node-a", "node-b", "node-c" },
            FailureCooldownSeconds = 60,
            RequestTimeoutSeconds = 0,
        };
        var factory = new TrackingThrowingConnectionFactory();
        var ds = new HistorianDataSource(cfg, factory);
        await Should.ThrowAsync<Exception>(() => ds.ReadRawAsync(
            "Tank.Level",
            new DateTime(2026, 5, 1, 0, 0, 0, DateTimeKind.Utc),
            new DateTime(2026, 5, 1, 0, 1, 0, DateTimeKind.Utc),
            maxValues: 100,
            CancellationToken.None));
        // All three candidates must be attempted in the configured order before the
        // connect-loop gives up.
        factory.AttemptedNodes.ShouldBe(new[] { "node-a", "node-b", "node-c" });
    }
    [Fact]
    public async Task ReadRaw_marks_failed_nodes_in_cooldown_so_a_subsequent_call_sees_no_healthy_nodes()
    {
        var cfg = new HistorianConfiguration
        {
            Enabled = true,
            ServerNames = new List<string> { "node-a", "node-b" },
            FailureCooldownSeconds = 60,
            RequestTimeoutSeconds = 0,
        };
        var ds = new HistorianDataSource(cfg, new ThrowingConnectionFactory());
        await Should.ThrowAsync<Exception>(() => ds.ReadRawAsync(
            "Tank.Level",
            DateTime.UtcNow.AddMinutes(-1), DateTime.UtcNow,
            maxValues: 100, CancellationToken.None));
        var snap = ds.GetHealthSnapshot();
        snap.NodeCount.ShouldBe(2);
        snap.HealthyNodeCount.ShouldBe(0, "both nodes failed and entered cooldown after the connect attempts");
        snap.ProcessConnectionOpen.ShouldBeFalse();
        snap.ActiveProcessNode.ShouldBeNull();
    }
    [Fact]
    public async Task ReadEvents_uses_a_separate_event_connection_path()
    {
        // ReadEventsAsync uses _eventConnection / EnsureEventConnected — a different
        // codepath than ReadRawAsync. Symmetric test to pin the dual-connection design.
        var cfg = new HistorianConfiguration
        {
            Enabled = true,
            ServerNames = new List<string> { "node-a" },
            FailureCooldownSeconds = 60,
            RequestTimeoutSeconds = 0,
        };
        var factory = new TrackingThrowingConnectionFactory();
        var ds = new HistorianDataSource(cfg, factory);
        await Should.ThrowAsync<Exception>(() => ds.ReadEventsAsync(
            sourceName: "Tank.HiHi",
            DateTime.UtcNow.AddMinutes(-1), DateTime.UtcNow,
            maxEvents: 100, CancellationToken.None));
        factory.AttemptedTypes.ShouldContain(HistorianConnectionType.Event,
            "event reads must open an Event-typed connection");
        factory.AttemptedNodes.ShouldBe(new[] { "node-a" });
    }
    // ── helpers ──────────────────────────────────────────────────────────
    private sealed class ThrowingConnectionFactory : IHistorianConnectionFactory
    {
        public HistorianAccess CreateAndConnect(
            HistorianConfiguration config, HistorianConnectionType type, bool readOnly = true)
            => throw new InvalidOperationException($"simulated connect failure to {config.ServerName}");
    }
    private sealed class TrackingThrowingConnectionFactory : IHistorianConnectionFactory
    {
        public List<string> AttemptedNodes { get; } = new();
        public List<HistorianConnectionType> AttemptedTypes { get; } = new();
        public HistorianAccess CreateAndConnect(
            HistorianConfiguration config, HistorianConnectionType type, bool readOnly = true)
        {
            AttemptedNodes.Add(config.ServerName);
            AttemptedTypes.Add(type);
            throw new InvalidOperationException($"simulated connect failure to {config.ServerName}");
        }
    }
 }
@@ -0,0 +1,114 @@
 using System;
 using System.Reflection;
 using ArchestrA;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.Backend;
 /// <summary>
 ///     Driver.Historian.Wonderware-005 regression tests for <see cref="HistorianDataSource.GetHealthSnapshot"/>.
 ///     The active-node strings and the connection-open booleans were published under different
 ///     locks, so a snapshot could observe an internally inconsistent pairing (open with no node,
 ///     or closed with a non-null node). The fix derives the open booleans from the same field
 ///     that is published under the same lock so the snapshot is self-consistent by construction.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class HistorianDataSourceHealthSnapshotTests
 {
    /// <summary>
    ///     Drives the "half-published" state directly via reflection: set <c>_connection</c>
    ///     to a non-null sentinel but leave <c>_activeProcessNode</c> null. The snapshot must
    ///     report <c>ProcessConnectionOpen = false</c> and <c>ActiveProcessNode = null</c>
    ///     consistently — never a mismatch.
    /// </summary>
    [Fact]
    public void Snapshot_with_connection_set_but_active_node_null_is_consistent()
    {
        var ds = new HistorianDataSource(
            new HistorianConfiguration { Enabled = true, ServerName = "h1" });
        SetField(ds, "_connection", new HistorianAccess());
        SetField(ds, "_activeProcessNode", (string?)null);
        var snap = ds.GetHealthSnapshot();
        (snap.ProcessConnectionOpen == (snap.ActiveProcessNode != null)).ShouldBeTrue(
            "snapshot must not advertise open with no node — picks one source of truth");
    }
    /// <summary>
    ///     Symmetric case for the event connection.
    /// </summary>
    [Fact]
    public void Snapshot_with_event_connection_set_but_active_node_null_is_consistent()
    {
        var ds = new HistorianDataSource(
            new HistorianConfiguration { Enabled = true, ServerName = "h1" });
        SetField(ds, "_eventConnection", new HistorianAccess());
        SetField(ds, "_activeEventNode", (string?)null);
        var snap = ds.GetHealthSnapshot();
        (snap.EventConnectionOpen == (snap.ActiveEventNode != null)).ShouldBeTrue(
            "snapshot must not advertise event open with no node");
    }
    /// <summary>
    ///     The other direction: connection cleared but node still populated (the failure path
    ///     between the two field clears). The snapshot must still pair them consistently.
    /// </summary>
    [Fact]
    public void Snapshot_with_connection_cleared_but_active_node_populated_is_consistent()
    {
        var ds = new HistorianDataSource(
            new HistorianConfiguration { Enabled = true, ServerName = "h1" });
        SetField(ds, "_connection", (HistorianAccess?)null);
        SetField(ds, "_activeProcessNode", "node-stale");
        var snap = ds.GetHealthSnapshot();
        (snap.ProcessConnectionOpen == (snap.ActiveProcessNode != null)).ShouldBeTrue(
            "snapshot must not advertise closed with a node still set");
    }
    /// <summary>
    ///     Steady-state happy path: both fields populated — snapshot reports both consistently.
    /// </summary>
    [Fact]
    public void Snapshot_with_both_fields_populated_reports_open_and_active_node()
    {
        var ds = new HistorianDataSource(
            new HistorianConfiguration { Enabled = true, ServerName = "h1" });
        SetField(ds, "_connection", new HistorianAccess());
        SetField(ds, "_activeProcessNode", "h1");
        var snap = ds.GetHealthSnapshot();
        snap.ProcessConnectionOpen.ShouldBeTrue();
        snap.ActiveProcessNode.ShouldBe("h1");
    }
    /// <summary>
    ///     Steady-state default (no connect attempted): both null.
    /// </summary>
    [Fact]
    public void Snapshot_with_default_fields_reports_closed_with_no_active_node()
    {
        var ds = new HistorianDataSource(
            new HistorianConfiguration { Enabled = true, ServerName = "h1" });
        var snap = ds.GetHealthSnapshot();
        snap.ProcessConnectionOpen.ShouldBeFalse();
        snap.ActiveProcessNode.ShouldBeNull();
        snap.EventConnectionOpen.ShouldBeFalse();
        snap.ActiveEventNode.ShouldBeNull();
    }
    private static void SetField(object target, string name, object? value)
    {
        var f = target.GetType().GetField(name, BindingFlags.Instance | BindingFlags.NonPublic);
        f.ShouldNotBeNull($"private field '{name}' must exist on {target.GetType().Name}");
        f!.SetValue(target, value);
    }
 }
@@ -0,0 +1,109 @@
 using System;
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.Backend;
 /// <summary>
 ///     Driver.Historian.Wonderware-010 regression. <see cref="HistorianConfiguration.RequestTimeoutSeconds"/>
 ///     was documented as the "outer safety timeout applied to sync-over-async Historian
 ///     operations" but was never read or enforced — a hung <c>StartQuery</c> or a slow
 ///     <c>MoveNext</c> could block the single pipe-server connection thread indefinitely.
 ///     The fix wires it into the read paths via a linked <see cref="CancellationTokenSource"/>
 ///     so the documented safety net actually exists.
 ///
 ///     The SDK-touching read methods cannot be unit-driven without a live AVEVA Historian.
 ///     This test pins the helper that derives the effective timeout from the config — the
 ///     read methods invoke that helper, so a regression in either the helper or the wiring
 ///     would break the test.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class HistorianDataSourceRequestTimeoutTests
 {
    [Fact]
    public void Default_request_timeout_is_60_seconds()
    {
        new HistorianConfiguration().RequestTimeoutSeconds.ShouldBe(60);
    }
    [Fact]
    public void Positive_request_timeout_is_used_verbatim()
    {
        InvokeBuildLinkedTokenSource(
            new HistorianConfiguration { RequestTimeoutSeconds = 30 },
            CancellationToken.None,
            out var cts);
        cts.ShouldNotBeNull();
        // The helper must wire CancelAfter — easiest cross-check is to observe that the
        // returned CTS is NOT already cancelled, and that disposing it is safe.
        cts!.IsCancellationRequested.ShouldBeFalse();
        cts.Dispose();
    }
    [Fact]
    public void Zero_or_negative_request_timeout_is_treated_as_no_timeout()
    {
        // A zero/negative value means "no outer timeout" — the helper must still return a
        // linked CTS so callers can use one code path, but it must not auto-cancel.
        InvokeBuildLinkedTokenSource(
            new HistorianConfiguration { RequestTimeoutSeconds = 0 },
            CancellationToken.None,
            out var cts);
        cts.ShouldNotBeNull();
        cts!.IsCancellationRequested.ShouldBeFalse();
        // Give the runtime a moment — a misconfigured CancelAfter(0) would fire immediately.
        Thread.Sleep(50);
        cts.IsCancellationRequested.ShouldBeFalse("RequestTimeoutSeconds <= 0 must not auto-cancel");
        cts.Dispose();
    }
    [Fact]
    public async Task Small_timeout_cancels_the_linked_token()
    {
        // 50 ms timeout — sleep 250 ms then assert the linked CTS has fired.
        InvokeBuildLinkedTokenSource(
            new HistorianConfiguration { RequestTimeoutSeconds = 1 }, // smallest non-zero whole-second value
            CancellationToken.None,
            out var cts);
        cts.ShouldNotBeNull();
        // The wall-clock cost of waiting a full second per test is acceptable — this
        // pins the actual CancelAfter wiring rather than just the conditional logic.
        await Task.Delay(1500);
        cts!.IsCancellationRequested.ShouldBeTrue("RequestTimeoutSeconds=1 must cancel within 1.5s");
        cts.Dispose();
    }
    [Fact]
    public void Inbound_cancellation_propagates_into_the_linked_token()
    {
        using var outer = new CancellationTokenSource();
        InvokeBuildLinkedTokenSource(
            new HistorianConfiguration { RequestTimeoutSeconds = 60 },
            outer.Token,
            out var cts);
        cts.ShouldNotBeNull();
        cts!.IsCancellationRequested.ShouldBeFalse();
        outer.Cancel();
        cts.IsCancellationRequested.ShouldBeTrue("cancelling the caller's CT must cancel the linked CTS");
        cts.Dispose();
    }
    private static void InvokeBuildLinkedTokenSource(
        HistorianConfiguration cfg, CancellationToken ct, out CancellationTokenSource? cts)
    {
        // The helper is internal so the InternalsVisibleTo on the data-source project lets
        // us bind to it directly. Reflection keeps the test resilient if the method name is
        // ever shortened.
        var method = typeof(HistorianDataSource)
            .GetMethod("BuildRequestCts", BindingFlags.Static | BindingFlags.NonPublic);
        method.ShouldNotBeNull(
            "HistorianDataSource.BuildRequestCts must exist — wires RequestTimeoutSeconds into the read paths");
        cts = (CancellationTokenSource?)method!.Invoke(null, new object[] { cfg, ct });
    }
 }
@@ -0,0 +1,50 @@
 using ArchestrA;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.Backend;
 /// <summary>
 ///     Driver.Historian.Wonderware-008 regression. The previous implementation unconditionally
 ///     called <c>HandleConnectionError()</c> whenever <c>StartQuery</c> returned <c>false</c>,
 ///     which tore down the (relatively expensive) shared SDK connection on a query-class error
 ///     such as a bad tag name. A burst of bad-tag queries could therefore push an otherwise
 ///     healthy cluster node into cooldown via the picker's <c>MarkFailed</c>. The fix
 ///     classifies the SDK error code: connection-class codes drop the connection; query-class
 ///     codes leave it intact.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class HistorianDataSourceStartQueryClassificationTests
 {
    // ── Connection-class codes — the connection should be reset ───────────
    [Theory]
    [InlineData(HistorianAccessError.ErrorValue.FailedToConnect)]
    [InlineData(HistorianAccessError.ErrorValue.FailedToCreateSession)]
    [InlineData(HistorianAccessError.ErrorValue.NoReply)]
    [InlineData(HistorianAccessError.ErrorValue.NotReady)]
    [InlineData(HistorianAccessError.ErrorValue.NotInitialized)]
    [InlineData(HistorianAccessError.ErrorValue.Stopping)]
    [InlineData(HistorianAccessError.ErrorValue.Win32Exception)]
    [InlineData(HistorianAccessError.ErrorValue.InvalidResponse)]
    public void Connection_class_codes_are_classified_as_connection_errors(HistorianAccessError.ErrorValue code)
    {
        HistorianDataSource.IsConnectionClassError(code).ShouldBeTrue(
            $"{code} is a connection/server failure — the SDK connection should be reset");
    }
    // ── Query-class codes — the connection should NOT be reset ────────────
    [Theory]
    [InlineData(HistorianAccessError.ErrorValue.InvalidArgument)]   // bad tag name, etc.
    [InlineData(HistorianAccessError.ErrorValue.ValidationFailed)]  // bad query args
    [InlineData(HistorianAccessError.ErrorValue.NotApplicable)]     // wrong tag kind for query
    [InlineData(HistorianAccessError.ErrorValue.NotImplemented)]    // unsupported aggregate
    [InlineData(HistorianAccessError.ErrorValue.NoData)]            // empty range
    public void Query_class_codes_are_NOT_classified_as_connection_errors(HistorianAccessError.ErrorValue code)
    {
        HistorianDataSource.IsConnectionClassError(code).ShouldBeFalse(
            $"{code} is a query payload problem — must NOT tear down the SDK connection");
    }
 }
@@ -0,0 +1,122 @@
 using System.Runtime.Serialization;
 using ArchestrA;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Backend;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.Backend;
 /// <summary>
 ///     Driver.Historian.Wonderware-012 coverage — pins the two static helpers on
 ///     <see cref="HistorianDataSource"/> that previously had no direct tests:
 ///     <see cref="HistorianDataSource.SelectValueFromPair"/> (the string-vs-numeric heuristic
 ///     for the raw + at-time read paths) and <see cref="HistorianDataSource.ExtractAggregateValue"/>
 ///     (the aggregate-column dispatch). The SDK <c>HistoryQueryResult</c> initialises internal
 ///     state lazily on first property access, which makes it impractical to fake via
 ///     <see cref="FormatterServices.GetUninitializedObject"/>; the heuristic was therefore
 ///     refactored into an SDK-independent overload that the tests drive directly.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class HistorianDataSourceValueAndAggregateTests
 {
    // ── SelectValueFromPair ───────────────────────────────────────────────
    [Fact]
    public void SelectValueFromPair_returns_numeric_value_when_StringValue_is_empty()
    {
        HistorianDataSource.SelectValueFromPair(42.5, string.Empty).ShouldBe(42.5);
    }
    [Fact]
    public void SelectValueFromPair_returns_numeric_value_when_Value_is_non_zero_even_with_StringValue_populated()
    {
        // Tag is numeric and sampled non-zero; the SDK may still populate a formatted
        // StringValue but the value path wins.
        HistorianDataSource.SelectValueFromPair(3.14, "3.14").ShouldBe(3.14);
    }
    [Fact]
    public void SelectValueFromPair_returns_StringValue_when_Value_is_zero_and_StringValue_non_empty()
    {
        // String tags in the SDK always project Value=0 — that's the documented heuristic.
        HistorianDataSource.SelectValueFromPair(0.0, "Ready").ShouldBe("Ready");
    }
    [Fact]
    public void SelectValueFromPair_returns_numeric_zero_when_Value_is_zero_and_StringValue_empty()
    {
        // Numeric tag legitimately samples zero, no formatted text — must remain numeric.
        HistorianDataSource.SelectValueFromPair(0.0, string.Empty).ShouldBe(0.0);
    }
    [Fact]
    public void SelectValueFromPair_null_StringValue_falls_back_to_numeric()
    {
        HistorianDataSource.SelectValueFromPair(7.7, null).ShouldBe(7.7);
    }
    [Fact]
    public void SelectValueFromPair_documented_edge_case_numeric_zero_with_formatted_string_returns_string()
    {
        // The doc comment on SelectValue calls this out as a known SDK-binding edge case:
        // "A numeric tag at exactly zero with a non-empty formatted StringValue (e.g. '0.00')
        //  would be mis-reported as a string". This test pins that documented behaviour so
        //  a future SDK upgrade that surfaces a real data-type field can replace the
        //  heuristic deliberately rather than by accident.
        HistorianDataSource.SelectValueFromPair(0.0, "0.00").ShouldBe("0.00");
    }
    // ── ExtractAggregateValue ─────────────────────────────────────────────
    [Theory]
    [InlineData("Average", 10.0)]
    [InlineData("Minimum", 1.0)]
    [InlineData("Maximum", 20.0)]
    [InlineData("First", 2.0)]
    [InlineData("Last", 8.0)]
    [InlineData("StdDev", 1.5)]
    public void ExtractAggregateValue_dispatches_known_columns(string column, double expected)
    {
        var result = NewAggregateResult();
        result.Average = 10.0;
        result.Minimum = 1.0;
        result.Maximum = 20.0;
        result.ValueCount = 5;
        result.First = 2.0;
        result.Last = 8.0;
        result.StdDev = 1.5;
        HistorianDataSource.ExtractAggregateValue(result, column).ShouldBe(expected);
    }
    [Fact]
    public void ExtractAggregateValue_ValueCount_dispatches_to_uint_field()
    {
        var result = NewAggregateResult();
        result.ValueCount = 42;
        HistorianDataSource.ExtractAggregateValue(result, "ValueCount").ShouldBe(42.0);
    }
    [Fact]
    public void ExtractAggregateValue_unknown_column_returns_null()
    {
        // Unknown column → null → IPC sample carries no value → client maps to BadNoData.
        HistorianDataSource.ExtractAggregateValue(NewAggregateResult(), "NotAColumn").ShouldBeNull();
    }
    [Fact]
    public void ExtractAggregateValue_case_sensitive_dispatch()
    {
        // The switch is case-sensitive — "average" (lowercase) does NOT dispatch. Pinned so
        // the canonical column-name casing is preserved across refactors.
        var result = NewAggregateResult();
        result.Average = 99.0;
        HistorianDataSource.ExtractAggregateValue(result, "average").ShouldBeNull();
        HistorianDataSource.ExtractAggregateValue(result, "Average").ShouldBe(99.0);
    }
    private static AnalogSummaryQueryResult NewAggregateResult()
    {
        return (AnalogSummaryQueryResult)FormatterServices.GetUninitializedObject(typeof(AnalogSummaryQueryResult));
    }
 }
@@ -103,6 +103,58 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Host.Tests
            SdkAlarmHistorianWriteBackend.ClassifyOutcome(code).ShouldBe(expected);
        }
        // ── ToHistorianEvent — EventId handling ───────────────────────────────
        [Fact]
        public void ToHistorianEvent_parseable_event_id_is_used_verbatim()
        {
            // Sanity case: a real GUID round-trips into HistorianEvent.Id.
            var id = Guid.Parse("12345678-1234-1234-1234-123456789abc");
            var dto = new AlarmHistorianEventDto
            {
                EventId = id.ToString(),
                SourceName = "Tank01",
                AlarmType = "AnalogLimitAlarm.HiHi",
                EventTimeUtcTicks = DateTime.UtcNow.Ticks,
            };
 #pragma warning disable CS0618
            SdkAlarmHistorianWriteBackend.ToHistorianEvent(dto).Id.ShouldBe(id);
 #pragma warning restore CS0618
        }
        [Fact]
        public void ToHistorianEvent_unparseable_event_id_synthesizes_unique_non_empty_Guid()
        {
            // Driver.Historian.Wonderware-004 regression: when EventId is not a parseable
            // GUID (or is empty) the previous implementation silently left HistorianEvent.Id
            // as Guid.Empty, so multiple alarms collided on the same id with no warning.
            // The fix synthesizes a fresh Guid so every event still gets a unique identifier.
            var dtoA = new AlarmHistorianEventDto
            {
                EventId = "not-a-guid",
                SourceName = "Tank01",
                AlarmType = "Active",
                EventTimeUtcTicks = DateTime.UtcNow.Ticks,
            };
            var dtoB = new AlarmHistorianEventDto
            {
                EventId = string.Empty,
                SourceName = "Tank01",
                AlarmType = "Active",
                EventTimeUtcTicks = DateTime.UtcNow.Ticks,
            };
 #pragma warning disable CS0618
            var idA = SdkAlarmHistorianWriteBackend.ToHistorianEvent(dtoA).Id;
            var idB = SdkAlarmHistorianWriteBackend.ToHistorianEvent(dtoB).Id;
 #pragma warning restore CS0618
            idA.ShouldNotBe(Guid.Empty, "unparseable EventId must not collapse to Guid.Empty");
            idB.ShouldNotBe(Guid.Empty, "empty EventId must not collapse to Guid.Empty");
            idA.ShouldNotBe(idB, "every event needs a unique synthesized id");
        }
        [Fact]
        public void ClassifyOutcome_WriteToReadOnlyFile_is_RetryPlease_not_PermanentFail()
        {
@@ -0,0 +1,83 @@
 using System;
 using System.IO.Pipes;
 using System.Security.Principal;
 using System.Threading;
 using System.Threading.Tasks;
 using MessagePack;
 using Serilog;
 using Serilog.Core;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Ipc;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests.Ipc;
 /// <summary>
 ///     Driver.Historian.Wonderware-007 regression. The two other rejection paths
 ///     (shared-secret-mismatch and major-version-mismatch) both write a <see cref="HelloAck"/>
 ///     with <c>Accepted=false</c> before disconnecting; the caller-SID-mismatch path used to
 ///     just disconnect abruptly, leaving the client to time out instead of learning why.
 ///     The fix sends a symmetric <c>caller-sid-mismatch</c> ack before disconnecting.
 ///
 ///     The test uses the internal test-seam constructor so the verifier rejects without
 ///     needing to actually relax the pipe ACL (which would block the test client itself).
 /// </summary>
 public sealed class PipeServerSidRejectTests
 {
    private static readonly ILogger Quiet = Logger.None;
    [Fact]
    public async Task Caller_SID_mismatch_sends_HelloAck_with_reject_reason_before_disconnect()
    {
        // The pipe ACL must allow the current process to connect — so wire up the pipe
        // with the current user's SID. Then have the verifier seam simulate the SID
        // mismatch by returning false. This isolates the "what does the server do on a
        // rejected caller" question from the (separate) "is the ACL correct" question.
        var current = WindowsIdentity.GetCurrent().User
                      ?? throw new InvalidOperationException("WindowsIdentity.GetCurrent().User was null — cannot run test");
        var pipeName = $"otopcua-hist-sidreject-test-{Guid.NewGuid():N}";
        PipeServer.CallerVerifier rejecting = (NamedPipeServerStream _, SecurityIdentifier _, out string reason) =>
        {
            reason = "synthetic-mismatch";
            return false;
        };
        using var server = new PipeServer(pipeName, current, "secret", Quiet, rejecting);
        var serverTask = Task.Run(() => server.RunOneConnectionAsync(new NoopHandler(), CancellationToken.None));
        using var client = new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous);
        await client.ConnectAsync(5_000);
        using var writer = new FrameWriter(client, leaveOpen: true);
        using var reader = new FrameReader(client, leaveOpen: true);
        var hello = new Hello { ProtocolMajor = Hello.CurrentMajor, PeerName = "test", SharedSecret = "secret" };
        await writer.WriteAsync(MessageKind.Hello, hello, CancellationToken.None);
        // Read the rejecting HelloAck the server is expected to send before disconnecting.
        var frame = await reader.ReadFrameAsync(CancellationToken.None);
        frame.ShouldNotBeNull("server must send a HelloAck on caller-SID rejection, not just disconnect");
        frame!.Value.Kind.ShouldBe(MessageKind.HelloAck);
        var ack = MessagePackSerializer.Deserialize<HelloAck>(frame.Value.Body);
        ack.Accepted.ShouldBeFalse();
        ack.RejectReason.ShouldNotBeNullOrEmpty();
        ack.RejectReason!.ShouldContain("caller-sid-mismatch",
            Case.Insensitive,
            "reject reason must match the documented caller-sid-mismatch tag so clients can diagnose");
        await serverTask;
    }
    /// <summary>Handler that asserts it is never called — the connection must be rejected at Hello.</summary>
    private sealed class NoopHandler : IFrameHandler
    {
        public Task HandleAsync(MessageKind kind, byte[] body, FrameWriter writer, CancellationToken ct)
        {
            throw new InvalidOperationException(
                $"Handler must not be reached on a rejected caller; got frame {kind}");
        }
    }
 }
@@ -146,4 +146,94 @@ public sealed class ModbusAddressEdgeCaseTests
        // D0 with a bank base that itself overflows: base 65535 + D1 = 65536.
        Should.Throw<OverflowException>(() => MelsecAddress.DRegisterToHolding("D1", dBankBase: 65535));
    }
    // ── TryParse never throws (Driver.Modbus.Addressing-006) ─────────────────────────────────
    //
    // The TryParse contract is that it converts every parse failure into a structured (false,
    // error) return — config-bind hot paths depend on this. The family-native catch was previously
    // narrow (ArgumentException / OverflowException only); any future helper change that threw a
    // different exception type (e.g. FormatException from a ushort.Parse swap) would escape as an
    // unhandled exception out of a TryParse method. These tests assert the defensive contract
    // across a broad set of pathological inputs.
    [Theory]
    [InlineData("V")]                  // V prefix with no digits
    [InlineData("V99999999999999")]    // overflow in user V-memory octal decode
    [InlineData("V200000")]            // overflow in user V-memory octal decode
    [InlineData("V77777777")]          // octal way past 0xFFFF in system bank
    [InlineData("Y")]                  // Y prefix with no digits
    [InlineData("Y8888")]              // non-octal digit
    [InlineData("Y174000")]            // octal offset overflows YOutputBaseCoil + value
    [InlineData("C")]                  // C prefix alone
    [InlineData("C99999999")]          // overflow in C-relay
    [InlineData("X")]                  // X prefix alone
    [InlineData("X8")]                 // non-octal digit
    [InlineData("SP")]                 // SP prefix alone
    [InlineData("SP9")]                // non-octal digit
    [InlineData("Z123")]               // unknown DL205 prefix
    public void DL205_TryParse_NeverThrows_ReturnsStructuredError(string addr)
    {
        // Defensive contract: any helper failure must surface as (false, non-null error), never
        // as an unhandled exception out of TryParse.
        var ok = ModbusAddressParser.TryParse(addr, ModbusFamily.DL205, MelsecFamily.Q_L_iQR, out var result, out var error);
        ok.ShouldBeFalse();
        result.ShouldBeNull();
        error.ShouldNotBeNullOrEmpty();
    }
    [Theory]
    [InlineData("D")]                          // D prefix alone — no digits
    [InlineData("D-1")]                        // negative — would fail ushort.TryParse, must not throw
    [InlineData("D65536")]                     // overflow
    [InlineData("DABC")]                       // non-decimal digits in D
    [InlineData("MABC")]                       // non-decimal digits in M
    [InlineData("X10000")]                     // hex overflow (Q-family)
    [InlineData("XZZZZ")]                      // non-hex digit (Q-family)
    [InlineData("Y10000")]                     // hex overflow (Q-family)
    public void MELSEC_TryParse_NeverThrows_ReturnsStructuredError(string addr)
    {
        var ok = ModbusAddressParser.TryParse(addr, ModbusFamily.MELSEC, MelsecFamily.Q_L_iQR, out var result, out var error);
        ok.ShouldBeFalse();
        result.ShouldBeNull();
        error.ShouldNotBeNullOrEmpty();
    }
    // ── ModbusStringByteOrder is grammar-out-of-scope (Driver.Modbus.Addressing-007) ────────
    //
    // ModbusStringByteOrder (HighByteFirst / LowByteFirst) is the DL205 low-byte-first packing
    // knob. It is intentionally NOT expressible through the address grammar — there is no token
    // form to set it and ParsedModbusAddress has no field for it. The string byte order is
    // configurable only via the structured tag form (ModbusTagDefinition.StringByteOrder), which
    // is the canonical config path. These tests pin that contract so a future grammar change
    // can't quietly add a token that conflicts with the array-count slot.
    [Fact]
    public void Parser_STR_grammar_does_not_carry_StringByteOrder()
    {
        // STR20 parses fine — but the result has no StringByteOrder field (the property does
        // not exist on ParsedModbusAddress). The string byte order must be set on the structured
        // tag definition, not the grammar string.
        var ok = ModbusAddressParser.TryParse("40001:STR20", out var result, out _);
        ok.ShouldBeTrue();
        result!.DataType.ShouldBe(ModbusDataType.String);
        result.StringLength.ShouldBe((ushort)20);
        // Compile-time assertion: ParsedModbusAddress does not expose StringByteOrder.
        // Searching for a property by reflection would let us assert "no such field":
        typeof(ParsedModbusAddress)
            .GetProperty("StringByteOrder")
            .ShouldBeNull();
    }
    [Fact]
    public void Parser_rejects_unknown_string_byte_order_token_in_grammar()
    {
        // A user trying to express low-byte-first via a grammar suffix like "LOWB" or "HIGH" in
        // the byte-order slot gets the standard "Unknown byte order" diagnostic — the parser is
        // explicit that field 3 is the multi-register word/byte order, not the per-string byte
        // order. The structured tag form is the only configuration path for ModbusStringByteOrder.
        var ok = ModbusAddressParser.TryParse("40001:STR20:LOWB", out _, out var error);
        ok.ShouldBeFalse();
        error.ShouldNotBeNullOrEmpty();
        error!.ShouldContain("byte order", Case.Insensitive);
    }
 }
@@ -0,0 +1,106 @@
 using Shouldly;
 using Xunit;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
 /// <summary>
 ///     Regression coverage for Driver.Modbus-009: two configuration edge cases that previously
 ///     silently produced wrong wire behaviour.
 ///     (1) <c>StringLength = 0</c> for a <c>String</c>-typed tag — used to flow into an FC03
 ///     with quantity 0, a spec-illegal request the PLC rejects with exception 03. Now bind-time
 ///     validation in <c>ModbusDriverFactoryExtensions</c> rejects the misconfiguration with a
 ///     clear diagnostic.
 ///     (2) Sub-second <see cref="TimeSpan"/> values on <c>ModbusKeepAliveOptions.Time</c> /
 ///     <c>Interval</c> — the int-cast in <c>EnableKeepAlive</c> truncated <c>500&#160;ms</c> to
 ///     <c>0</c>, which most OSes interpret as "use the default", silently defeating the
 ///     configured timing. <c>ModbusTcpTransport.ClampToWholeSeconds</c> rounds up to a minimum
 ///     of 1&#160;second.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class ModbusEdgeCaseValidationTests
 {
    [Fact]
    public void Factory_rejects_String_tag_with_StringLength_zero_via_structured_form()
    {
        const string json = """
        {
          "host": "10.0.0.10",
          "tags": [
            { "name": "Greeting", "region": "HoldingRegisters", "address": 100, "dataType": "String", "stringLength": 0 }
          ]
        }
        """;
        var ex = Should.Throw<InvalidOperationException>(
            () => ModbusDriverFactoryExtensions.CreateInstance("modbus-1", json));
        ex.Message.ShouldContain("StringLength");
        ex.Message.ShouldContain("Greeting");
    }
    [Fact]
    public void Factory_rejects_String_tag_with_StringLength_zero_via_missing_field()
    {
        // No stringLength → defaults to 0. Same misconfiguration via a different DTO shape.
        const string json = """
        {
          "host": "10.0.0.10",
          "tags": [
            { "name": "Greeting", "region": "HoldingRegisters", "address": 100, "dataType": "String" }
          ]
        }
        """;
        var ex = Should.Throw<InvalidOperationException>(
            () => ModbusDriverFactoryExtensions.CreateInstance("modbus-1", json));
        ex.Message.ShouldContain("StringLength");
    }
    [Fact]
    public void Factory_accepts_String_tag_with_StringLength_one()
    {
        const string json = """
        {
          "host": "10.0.0.10",
          "tags": [
            { "name": "Greeting", "region": "HoldingRegisters", "address": 100, "dataType": "String", "stringLength": 1 }
          ]
        }
        """;
        Should.NotThrow(() => ModbusDriverFactoryExtensions.CreateInstance("modbus-1", json));
    }
    [Fact]
    public void Factory_accepts_non_String_tag_with_StringLength_zero()
    {
        // The validation only kicks in for String tags — Int16 tags with StringLength=0 are normal.
        const string json = """
        {
          "host": "10.0.0.10",
          "tags": [
            { "name": "Level", "region": "HoldingRegisters", "address": 100, "dataType": "Int16" }
          ]
        }
        """;
        Should.NotThrow(() => ModbusDriverFactoryExtensions.CreateInstance("modbus-1", json));
    }
    [Theory]
    [InlineData(0, 1)]       // zero clamps up to 1
    [InlineData(500, 1)]     // 500 ms rounds up to 1
    [InlineData(999, 1)]     // just under 1s rounds up to 1
    [InlineData(1_000, 1)]   // exactly 1s passes through
    [InlineData(1_500, 2)]   // 1.5s rounds up to 2
    [InlineData(30_000, 30)] // historical PR 53 default — unchanged
    [InlineData(60_000, 60)]
    public void ClampToWholeSeconds_rounds_up_to_at_least_one_second(int ms, int expected)
    {
        ModbusTcpTransport.ClampToWholeSeconds(TimeSpan.FromMilliseconds(ms)).ShouldBe(expected);
    }
    [Fact]
    public void ClampToWholeSeconds_treats_negative_TimeSpan_as_one_second()
    {
        // Defensive — operators occasionally configure a negative TimeSpan thinking it disables
        // the feature. The OS would reject the negative int — clamping to 1 keeps the socket
        // valid until the operator fixes the config.
        ModbusTcpTransport.ClampToWholeSeconds(TimeSpan.FromSeconds(-5)).ShouldBe(1);
    }
 }
@@ -0,0 +1,353 @@
 using System.Collections.Concurrent;
 using System.Reflection;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.Modbus;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
 /// <summary>
 ///     Regression coverage for Driver.Modbus findings -002 (Reinitialize state hygiene),
 ///     -003 (_health volatile-write ordering), -004 (DisposeAsync teardown parity), and
 ///     -005 (malformed/short response PDU handling). All four resolved fixes need a
 ///     unit test alongside them per Driver.Modbus-012.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class ModbusLifecycleHygieneTests
 {
    private sealed class FakeTransport : IModbusTransport
    {
        public readonly ushort[] HoldingRegisters = new ushort[256];
        public int ConnectCount;
        public int DisposeCount;
        public int SendCount;
        public Task ConnectAsync(CancellationToken ct) { Interlocked.Increment(ref ConnectCount); return Task.CompletedTask; }
        public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
        {
            Interlocked.Increment(ref SendCount);
            var fc = pdu[0];
            switch (fc)
            {
                case 0x03:
                case 0x04:
                {
                    var addr = (ushort)((pdu[1] << 8) | pdu[2]);
                    var qty = (ushort)((pdu[3] << 8) | pdu[4]);
                    var resp = new byte[2 + qty * 2];
                    resp[0] = fc;
                    resp[1] = (byte)(qty * 2);
                    for (var i = 0; i < qty; i++)
                    {
                        resp[2 + i * 2] = (byte)(HoldingRegisters[addr + i] >> 8);
                        resp[3 + i * 2] = (byte)(HoldingRegisters[addr + i] & 0xFF);
                    }
                    return Task.FromResult(resp);
                }
                case 0x06:
                {
                    var addr = (ushort)((pdu[1] << 8) | pdu[2]);
                    HoldingRegisters[addr] = (ushort)((pdu[3] << 8) | pdu[4]);
                    return Task.FromResult(pdu); // FC06 echoes the request
                }
                case 0x10:
                {
                    var addr = (ushort)((pdu[1] << 8) | pdu[2]);
                    var qty = (ushort)((pdu[3] << 8) | pdu[4]);
                    for (var i = 0; i < qty; i++)
                        HoldingRegisters[addr + i] = (ushort)((pdu[6 + i * 2] << 8) | pdu[7 + i * 2]);
                    return Task.FromResult(new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] });
                }
                default:
                    return Task.FromException<byte[]>(new NotSupportedException($"fc={fc}"));
            }
        }
        public ValueTask DisposeAsync() { Interlocked.Increment(ref DisposeCount); return ValueTask.CompletedTask; }
    }
    /// <summary>
    ///     Returns a snapshot of the driver's private <c>_tagsByName</c> dictionary so the
    ///     hygiene tests can confirm the cache is empty after teardown.
    /// </summary>
    private static System.Collections.IDictionary GetTagsByName(ModbusDriver drv) =>
        (System.Collections.IDictionary)typeof(ModbusDriver)
            .GetField("_tagsByName", BindingFlags.NonPublic | BindingFlags.Instance)!
            .GetValue(drv)!;
    // -------------------- Finding -002 / -012 (2) --------------------
    [Fact]
    public async Task Reinitialize_clears_stale_tagsByName_entries()
    {
        // Re-initializing with a different options instance would leak stale entries before
        // the fix. We simulate by inspecting _tagsByName after a Shutdown — it must be empty
        // so InitializeAsync repopulates from a clean slate.
        var fake = new FakeTransport();
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Tags = [new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        GetTagsByName(drv).Count.ShouldBe(1);
        await drv.ShutdownAsync(CancellationToken.None);
        GetTagsByName(drv).Count.ShouldBe(0, "Shutdown must clear the tag cache so the next Initialize starts clean");
    }
    [Fact]
    public async Task Reinitialize_clears_lastPublished_and_lastWritten_caches()
    {
        var fake = new FakeTransport();
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            WriteOnChangeOnly = true,
            Tags = [new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16,
                Deadband: 1.0)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        var lastPublished = (System.Collections.IDictionary)typeof(ModbusDriver)
            .GetField("_lastPublishedByRef", BindingFlags.NonPublic | BindingFlags.Instance)!
            .GetValue(drv)!;
        var lastWritten = (System.Collections.IDictionary)typeof(ModbusDriver)
            .GetField("_lastWrittenByRef", BindingFlags.NonPublic | BindingFlags.Instance)!
            .GetValue(drv)!;
        // Seed both caches via a write (lastWritten) and a publish through ShouldPublish (lastPublished).
        await drv.WriteAsync([new WriteRequest("A", (short)5)], CancellationToken.None);
        lastWritten.Count.ShouldBe(1);
        // Reach ShouldPublish directly through a subscription so the deadband cache fills.
        fake.HoldingRegisters[0] = 5;
        var handle = await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), CancellationToken.None);
        var deadline = DateTime.UtcNow.AddSeconds(2);
        while (lastPublished.Count == 0 && DateTime.UtcNow < deadline) await Task.Delay(25);
        lastPublished.Count.ShouldBe(1);
        await drv.UnsubscribeAsync(handle, CancellationToken.None);
        await drv.ShutdownAsync(CancellationToken.None);
        lastPublished.Count.ShouldBe(0, "Shutdown must clear the deadband cache");
        lastWritten.Count.ShouldBe(0, "Shutdown must clear the write-suppression cache");
    }
    // -------------------- Finding -004 / -012 (4) --------------------
    [Fact]
    public async Task DisposeAsync_without_explicit_Shutdown_tears_down_probe_loop_and_transport()
    {
        var fake = new FakeTransport();
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Probe = new ModbusProbeOptions
            {
                Enabled = true,
                Interval = TimeSpan.FromMilliseconds(50),
                Timeout = TimeSpan.FromSeconds(1),
            },
            // Re-probe loop also opted in so DisposeAsync exercises both CTS cancellations.
            AutoProhibitReprobeInterval = TimeSpan.FromMilliseconds(50),
            Tags = [new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        // Let the probe + re-probe loops spin a few iterations.
        await Task.Delay(200);
        var sendsAtDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
        sendsAtDispose.ShouldBeGreaterThan(0, "background probe loop should have issued at least one send");
        // Skip ShutdownAsync — exercise the await-using path that previously leaked.
        await drv.DisposeAsync();
        // Transport must have been disposed exactly once and the background loops stop scheduling
        // new sends. Tolerate at most one in-flight send straddling the cancel.
        fake.DisposeCount.ShouldBe(1);
        var sendsAfterDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
        await Task.Delay(300);
        var sendsAtRest = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
        (sendsAtRest - sendsAfterDispose).ShouldBeLessThanOrEqualTo(1, "background loops must stop after DisposeAsync");
    }
    [Fact]
    public async Task DisposeAsync_disposes_the_pollEngine_so_subscriptions_stop()
    {
        var fake = new FakeTransport();
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Probe = new ModbusProbeOptions { Enabled = false },
            Tags = [new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        // Spin up a polled subscription; the PollGroupEngine schedules a background Task that
        // will keep issuing SendAsync until either Unsubscribe or DisposeAsync stops it.
        var handle = await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), CancellationToken.None);
        await Task.Delay(250);
        var beforeDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
        beforeDispose.ShouldBeGreaterThan(0);
        // No ShutdownAsync — DisposeAsync must also tear down the poll engine.
        await drv.DisposeAsync();
        var atDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
        await Task.Delay(400);
        var atRest = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
        (atRest - atDispose).ShouldBeLessThanOrEqualTo(1,
            "DisposeAsync must dispose the PollGroupEngine so its background Task stops, not just the transport");
    }
    // -------------------- Finding -005 / -012 (3) --------------------
    /// <summary>
    ///     Transport that returns a structurally-broken response for FC03/FC04 — too short to
    ///     hold the declared byte-count. Pre-fix the driver dereferenced <c>resp[1]</c> and then
    ///     ran <c>Buffer.BlockCopy(resp, 2, ..., resp[1])</c> which threw <c>ArgumentException</c>
    ///     (out-of-range). Post-fix the driver throws <c>InvalidDataException</c> which the
    ///     <c>ReadAsync</c> catch-all maps to <see cref="BadCommunicationError"/>.
    /// </summary>
    private sealed class TruncatingTransport : IModbusTransport
    {
        /// <summary>How many bytes to return — anything &lt; 2 + bytecount is malformed.</summary>
        public int ResponseBytes { get; set; } = 1; // just the fc byte, no bytecount
        public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
        public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
        {
            var resp = new byte[ResponseBytes];
            if (ResponseBytes >= 1) resp[0] = pdu[0];
            if (ResponseBytes >= 2) resp[1] = 4; // claim 4 bytes of payload but provide none
            return Task.FromResult(resp);
        }
        public ValueTask DisposeAsync() => ValueTask.CompletedTask;
    }
    [Fact]
    public async Task Short_response_PDU_surfaces_as_BadCommunicationError_not_an_IndexOutOfRangeException()
    {
        var fake = new TruncatingTransport { ResponseBytes = 1 };
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Tags = [new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        var r = await drv.ReadAsync(["Level"], CancellationToken.None);
        r[0].StatusCode.ShouldBe(0x80050000u, "BadCommunicationError = a clean transport-layer fault");
        r[0].Value.ShouldBeNull();
    }
    [Fact]
    public async Task Response_payload_truncated_below_declared_byteCount_surfaces_as_BadCommunicationError()
    {
        // Header says "4 bytes follow" but the message is only 3 bytes total — pre-fix the
        // Buffer.BlockCopy would throw ArgumentException.
        var fake = new TruncatingTransport { ResponseBytes = 3 };
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Tags = [new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        var r = await drv.ReadAsync(["Level"], CancellationToken.None);
        r[0].StatusCode.ShouldBe(0x80050000u);
    }
    [Fact]
    public void DecodeBitArray_rejects_an_empty_bitmap_with_InvalidDataException()
    {
        var decode = typeof(ModbusDriver).GetMethod(
            "DecodeBitArray", BindingFlags.NonPublic | BindingFlags.Static)!;
        // We can't invoke through reflection because ReadOnlySpan<byte> isn't representable in
        // object-array invocation parameters. Instead, exercise the path through ReadAsync with
        // a bit-region tag and a transport that returns a zero-byte-count response.
        var fake = new EmptyBitTransport();
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Tags = [new ModbusTagDefinition("Coil", ModbusRegion.Coils, 0, ModbusDataType.Bool)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult();
        var r = drv.ReadAsync(["Coil"], CancellationToken.None).GetAwaiter().GetResult();
        // The empty-bitmap guard surfaces via the BadCommunicationError catch-all.
        r[0].StatusCode.ShouldBe(0x80050000u);
    }
    /// <summary>
    ///     Coil-bank transport that returns <c>[fc][bytecount=0]</c> — a response with a
    ///     declared zero-byte payload. Pre-fix <c>DecodeBitArray</c> indexed into the empty
    ///     bitmap and threw <c>IndexOutOfRangeException</c>.
    /// </summary>
    private sealed class EmptyBitTransport : IModbusTransport
    {
        public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
        public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
            => Task.FromResult(new byte[] { pdu[0], 0 });
        public ValueTask DisposeAsync() => ValueTask.CompletedTask;
    }
    // -------------------- Finding -003 (volatile _health) --------------------
    /// <summary>
    ///     The <c>_health</c> field is read by <c>GetHealth()</c> and written by every read /
    ///     write / probe path. The fix uses <c>Volatile.Read</c>/<c>Volatile.Write</c> to give
    ///     <c>GetHealth()</c> a defined ordering guarantee. We verify that under concurrent
    ///     pressure <c>GetHealth()</c> never returns a half-constructed value (it's a sealed
    ///     record so reference-assignment atomicity already prevents tearing; the test guards
    ///     against future regressions to a struct-typed health surface).
    /// </summary>
    [Fact]
    public async Task GetHealth_under_concurrent_pressure_always_returns_a_complete_snapshot()
    {
        var fake = new FakeTransport();
        var opts = new ModbusDriverOptions
        {
            Host = "fake",
            Tags = [new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)],
        };
        var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
        await drv.InitializeAsync("{}", CancellationToken.None);
        // Two writer loops and one reader loop — 250ms of churn.
        var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(250));
        var faults = new ConcurrentQueue<Exception>();
        var writer = Task.Run(async () =>
        {
            try { while (!cts.IsCancellationRequested) await drv.ReadAsync(["A"], CancellationToken.None); }
            catch (Exception ex) { faults.Enqueue(ex); }
        });
        var reader = Task.Run(() =>
        {
            try
            {
                while (!cts.IsCancellationRequested)
                {
                    var h = drv.GetHealth();
                    // State must be one of the enum values; LastSuccessfulRead can be null or a real time;
                    // the record constructor enforces no field is wholly garbage.
                    h.State.ShouldBeOneOf(DriverState.Unknown, DriverState.Initializing, DriverState.Healthy, DriverState.Degraded, DriverState.Faulted);
                }
            }
            catch (Exception ex) { faults.Enqueue(ex); }
        });
        await Task.WhenAll(writer, reader);
        faults.ShouldBeEmpty();
    }
 }
@@ -0,0 +1,144 @@
 using System.Reflection;
 using Opc.Ua;
 using Opc.Ua.Client;
 using Shouldly;
 using Xunit;
 namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests;
 /// <summary>
 ///     Regression tests for the Low code-review findings cleared in the 2026-05-23 pass:
 ///     <list type="bullet">
 ///       <item>Driver.OpcUaClient-011 — ValueRank comment + boundary semantics</item>
 ///       <item>Driver.OpcUaClient-014 — MonitoredItem.Notification handlers are detached on
 ///             Unsubscribe / Shutdown so the driver instance is not held alive by an
 ///             SDK-side reference graph after the subscription is gone</item>
 ///     </list>
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class OpcUaClientLowFindingsRegressionTests
 {
    // ---- Driver.OpcUaClient-011 ----
    //
    // The pre-fix comment claimed "-1 = scalar; 1+ = array dimensions; 0 = one-dimensional
    // array", which is wrong against OPC UA Part 3 ValueRank semantics. The fix corrects the
    // comment and locks in the deliberate choice that anything >= 0 is treated as an array.
    // The decision branches are pure logic; assert them against the SDK constants so a
    // regression rewriting `valueRank >= 0` shows up in CI.
    [Fact]
    public void ValueRank_constants_have_the_OPCUA_Part3_spec_values()
    {
        // Anchor the spec values from the SDK so the comment in the driver and any code
        // keying off them stays accurate. -3, -2, -1 are the three negative sentinels;
        // 0 means OneOrMoreDimensions (multi-dim), 1 means OneDimension specifically.
        ValueRanks.ScalarOrOneDimension.ShouldBe(-3);
        ValueRanks.Any.ShouldBe(-2);
        ValueRanks.Scalar.ShouldBe(-1);
        ValueRanks.OneOrMoreDimensions.ShouldBe(0);
        ValueRanks.OneDimension.ShouldBe(1);
    }
    [Theory]
    [InlineData(-3, false)] // ScalarOrOneDimension — conservatively treated as scalar
    [InlineData(-2, false)] // Any — conservatively treated as scalar
    [InlineData(-1, false)] // Scalar
    [InlineData(0, true)]   // OneOrMoreDimensions — array (multi-dim)
    [InlineData(1, true)]   // OneDimension — array
    [InlineData(2, true)]   // 2 specific dimensions — array
    public void IsArray_decision_matches_valueRank_greater_or_equal_zero(int valueRank, bool expectedIsArray)
    {
        // Mirrors EnrichAndRegisterVariablesAsync's `isArray = valueRank >= 0` decision.
        // The pre-fix comment was wrong; the *code* is correct and this test pins it.
        var isArray = valueRank >= 0;
        isArray.ShouldBe(expectedIsArray);
    }
    // ---- Driver.OpcUaClient-014 ----
    //
    // The Notification lambda must be detached when the subscription is removed; otherwise
    // the SDK retains the closure (and through it the OpcUaClientDriver instance) until the
    // session itself is disposed. UnsubscribeAsync had no detach step in the pre-fix code.
    //
    // The two angles we can test without a live session:
    //   (a) The fix tracks the handler delegate inside the RemoteSubscription record so
    //       UnsubscribeAsync / ShutdownAsync can detach it. Use reflection to assert the
    //       record carries the handler (the contract surface).
    //   (b) Simulate the detach against a synthetic MonitoredItem: build one, attach a
    //       lambda the same way the driver does, then call MonitoredItem.Notification -=
    //       with the *same delegate instance*. Confirm that further notifications do not
    //       invoke the handler.
    [Fact]
    public void RemoteSubscription_record_carries_handler_delegates_so_they_can_be_detached()
    {
        // The fix introduces a list of (MonitoredItem, NotificationEventHandler) pairs onto
        // RemoteSubscription so UnsubscribeAsync can `item.Notification -= handler` each one
        // before deleting the subscription. We assert via reflection because the record is
        // private — the public observable is just "no leaks" which is hard to assert
        // synthetically.
        var driverType = typeof(OpcUaClientDriver);
        var remoteSubType = driverType.GetNestedType("RemoteSubscription", BindingFlags.NonPublic);
        remoteSubType.ShouldNotBeNull("RemoteSubscription record should exist");
        // The record must carry a property/field referencing the per-item handler delegates
        // so detach is possible. Accept either a List of pairs or a parallel handler list —
        // both are valid implementations; what matters is that the handler reference is
        // reachable from the record.
        var members = remoteSubType.GetMembers(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
        var hasHandlerStorage = members.Any(m =>
            m.Name.Contains("Handler", StringComparison.OrdinalIgnoreCase) ||
            m.Name.Contains("Notif", StringComparison.OrdinalIgnoreCase) ||
            m.Name.Contains("Item", StringComparison.OrdinalIgnoreCase));
        hasHandlerStorage.ShouldBeTrue(
            "RemoteSubscription must expose the per-item handler reference (or the MonitoredItem itself) " +
            "so UnsubscribeAsync/ShutdownAsync can detach the Notification delegate before disposing the session.");
    }
    [Fact]
    public void RemoteAlarmSubscription_record_carries_handler_delegate_so_it_can_be_detached()
    {
        var driverType = typeof(OpcUaClientDriver);
        var remoteAlarmSubType = driverType.GetNestedType("RemoteAlarmSubscription", BindingFlags.NonPublic);
        remoteAlarmSubType.ShouldNotBeNull("RemoteAlarmSubscription record should exist");
        var members = remoteAlarmSubType.GetMembers(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
        var hasHandlerStorage = members.Any(m =>
            m.Name.Contains("Handler", StringComparison.OrdinalIgnoreCase) ||
            m.Name.Contains("Notif", StringComparison.OrdinalIgnoreCase) ||
            m.Name.Contains("EventItem", StringComparison.OrdinalIgnoreCase) ||
            m.Name.Contains("Item", StringComparison.OrdinalIgnoreCase));
        hasHandlerStorage.ShouldBeTrue(
            "RemoteAlarmSubscription must expose the event-MonitoredItem (or its handler) reference " +
            "so UnsubscribeAlarmsAsync/ShutdownAsync can detach the Notification delegate before " +
            "disposing the session.");
    }
    [Fact]
    public async Task UnsubscribeAsync_unknown_handle_does_not_throw_after_fix()
    {
        // Smoke: confirm the new detach logic doesn't break the existing unknown-handle
        // no-op path (UnsubscribeAsync returns cleanly without an entry in _subscriptions).
        using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-low-014");
        await Should.NotThrowAsync(async () =>
            await drv.UnsubscribeAsync(new FakeHandle(), TestContext.Current.CancellationToken));
    }
    [Fact]
    public async Task UnsubscribeAlarmsAsync_unknown_handle_does_not_throw_after_fix()
    {
        using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-low-014-alarm");
        await Should.NotThrowAsync(async () =>
            await drv.UnsubscribeAlarmsAsync(new FakeAlarmHandle(), TestContext.Current.CancellationToken));
    }
    private sealed class FakeHandle : Core.Abstractions.ISubscriptionHandle
    {
        public string DiagnosticId => "fake-sub";
    }
    private sealed class FakeAlarmHandle : Core.Abstractions.IAlarmSubscriptionHandle
    {
        public string DiagnosticId => "fake-alarm-sub";
    }
 }
@@ -67,14 +67,17 @@ internal class FakeTwinCATClient : ITwinCATClient
    public List<FakeNotification> Notifications { get; } = new();
    public bool ThrowOnAddNotification { get; set; }
    /// <summary>Records the most recently-supplied <c>maxDelayMs</c> for Driver.TwinCAT-014 tests.</summary>
    public int LastMaxDelayMs { get; private set; }
    public virtual Task<ITwinCATNotificationHandle> AddNotificationAsync(
        string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime,
-        Action<string, object?> onChange, CancellationToken cancellationToken)
+        int maxDelayMs, Action<string, object?> onChange, CancellationToken cancellationToken)
    {
        if (ThrowOnAddNotification)
            throw Exception ?? new InvalidOperationException("fake AddNotification failure");
        LastMaxDelayMs = maxDelayMs;
        var reg = new FakeNotification(symbolPath, type, bitIndex, onChange, this);
        Notifications.Add(reg);
        return Task.FromResult<ITwinCATNotificationHandle>(reg);
@@ -217,12 +217,14 @@ public sealed class TwinCATCapabilityTests
    }
    [Fact]
-    public async Task ResolveHost_falls_back_to_DriverInstanceId_when_no_devices()
+    public async Task ResolveHost_falls_back_to_unresolved_sentinel_when_no_devices()
    {
        // Driver.TwinCAT-006: empty-string sentinel — DriverInstanceId is a config-DB key, not
        // a host address, so it would collide with no GetHostStatuses() row.
        var drv = new TwinCATDriver(new TwinCATDriverOptions(), "drv-1");
        await drv.InitializeAsync("{}", CancellationToken.None);
-        drv.ResolveHost("anything").ShouldBe("drv-1");
+        drv.ResolveHost("anything").ShouldBe(TwinCATDriver.UnresolvedHostSentinel);
    }
    // ---- helpers ----
@@ -0,0 +1,264 @@
 using System.Text.Json;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT;
 namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests;
 /// <summary>
 ///     Regression coverage for the remaining (Low/Medium) code-review findings:
 ///     Driver.TwinCAT-004 (IEC time-type doc-comment accuracy), -006 (ResolveHost
 ///     sentinel for no-devices fallback), -014 (NotificationMaxDelayMs config knob
 ///     and ProbeOptions.Timeout wiring), -015 (Dispose runs a true synchronous
 ///     teardown, no sync-over-async), -016 (gap-fill tests: Structure-tag rejection,
 ///     concurrent probe + read race).
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class TwinCATLowFindingsRegressionTests
 {
    private const string DeviceA = "ads://5.23.91.23.1.1:851";
    // ---- Driver.TwinCAT-004 — TIME/DATE/DT/TOD surface unchanged but comments corrected ----
    [Fact]
    public void Iec_time_types_map_to_uint32_raw_counter()
    {
        // Documents the contract called out in the corrected comments: TIME / DATE / DT / TOD
        // surface as their raw UDINT counter (32-bit unsigned), not as decoded DateTime/TimeSpan.
        // The next implementer who wants to decode them needs to see this mapping is intentional.
        TwinCATDataType.Time.ToDriverDataType().ShouldBe(DriverDataType.UInt32);
        TwinCATDataType.Date.ToDriverDataType().ShouldBe(DriverDataType.UInt32);
        TwinCATDataType.DateTime.ToDriverDataType().ShouldBe(DriverDataType.UInt32);
        TwinCATDataType.TimeOfDay.ToDriverDataType().ShouldBe(DriverDataType.UInt32);
    }
    // ---- Driver.TwinCAT-006 — ResolveHost sentinel when no devices are configured ----
    [Fact]
    public async Task ResolveHost_returns_unresolved_sentinel_when_no_devices()
    {
        // DriverInstanceId is a logical config-DB key, not a host address; consumers expect a
        // host key that correlates with GetHostStatuses(). When there are no devices and the
        // reference is unknown, ResolveHost must return the documented unresolved sentinel
        // (empty string), not the driver-instance ID.
        var drv = new TwinCATDriver(new TwinCATDriverOptions(), "drv-1");
        await drv.InitializeAsync("{}", CancellationToken.None);
        drv.ResolveHost("anything").ShouldBe(string.Empty);
    }
    [Fact]
    public async Task ResolveHost_unresolved_sentinel_matches_no_GetHostStatuses_entry()
    {
        // Documents the contract: the sentinel should never match a real connectivity-status row.
        var drv = new TwinCATDriver(new TwinCATDriverOptions(), "drv-1");
        await drv.InitializeAsync("{}", CancellationToken.None);
        var sentinel = drv.ResolveHost("anything");
        drv.GetHostStatuses().ShouldNotContain(s => s.HostName == sentinel);
    }
    // ---- Driver.TwinCAT-014 — config surface knobs are honoured ----
    [Fact]
    public async Task ProbeOptions_Timeout_is_applied_to_probe_calls()
    {
        // The previous implementation declared a Timeout field but never read it — the probe
        // path connected with _options.Timeout. The probe must use its own configured timeout.
        var observed = new List<TimeSpan>();
        var factory = new FakeTwinCATClientFactory
        {
            Customise = () => new ProbeTimeoutCapturingFake(observed),
        };
        var drv = new TwinCATDriver(new TwinCATDriverOptions
        {
            Devices = [new TwinCATDeviceOptions(DeviceA)],
            Probe = new TwinCATProbeOptions
            {
                Enabled = true,
                Interval = TimeSpan.FromMilliseconds(100),
                Timeout = TimeSpan.FromMilliseconds(750), // distinct from the connect timeout
            },
            Timeout = TimeSpan.FromMilliseconds(2_000),
        }, "drv-1", factory);
        await drv.InitializeAsync("{}", CancellationToken.None);
        await WaitForAsync(() => observed.Count >= 1, TimeSpan.FromSeconds(2));
        await drv.ShutdownAsync(CancellationToken.None);
        observed.ShouldContain(TimeSpan.FromMilliseconds(750));
    }
    [Fact]
    public void NotificationMaxDelayMs_is_exposed_on_driver_options()
    {
        // The driver-spec lists NotificationMaxDelayMs as a per-device knob; the implementation
        // previously hard-coded 0 in NotificationSettings. Expose a configurable field so
        // operators can batch low-priority notifications.
        var options = new TwinCATDriverOptions { NotificationMaxDelayMs = 200 };
        options.NotificationMaxDelayMs.ShouldBe(200);
    }
    [Fact]
    public void NotificationMaxDelayMs_parses_from_driver_config_json()
    {
        var json = JsonSerializer.Serialize(new
        {
            devices = new[] { new { hostAddress = DeviceA } },
            notificationMaxDelayMs = 150,
        });
        var parsed = TwinCATDriverFactoryExtensions.ParseOptionsForTests(json, "drv-1");
        parsed.NotificationMaxDelayMs.ShouldBe(150);
    }
    // ---- Driver.TwinCAT-015 — Dispose runs a true synchronous teardown ----
    [Fact]
    public void Dispose_does_not_block_on_async_in_default_synchronization_context()
    {
        // Sync-over-async on a single-threaded sync context (like the OPC UA stack thread) can
        // deadlock. Dispose() must complete without scheduling continuations through a captured
        // sync context. We verify by running Dispose() inside a SynchronizationContext that
        // would deadlock a sync-over-async teardown.
        var drv = new TwinCATDriver(new TwinCATDriverOptions
        {
            Devices = [new TwinCATDeviceOptions(DeviceA)],
            Probe = new TwinCATProbeOptions { Enabled = false },
        }, "drv-1");
        drv.InitializeAsync("{}", CancellationToken.None).GetAwaiter().GetResult();
        var ctx = new SingleThreadedSyncContext();
        var prev = SynchronizationContext.Current;
        Exception? captured = null;
        try
        {
            SynchronizationContext.SetSynchronizationContext(ctx);
            // If Dispose schedules its continuations through the captured context (sync-over-
            // async pattern), and the context is single-threaded with nothing pumping, this
            // will hang. We give it 5 seconds — well above any reasonable sync teardown.
            var thread = new Thread(() =>
            {
                try
                {
                    SynchronizationContext.SetSynchronizationContext(ctx);
                    drv.Dispose();
                }
                catch (Exception ex) { captured = ex; }
            }) { IsBackground = true };
            thread.Start();
            thread.Join(TimeSpan.FromSeconds(5)).ShouldBeTrue(
                "Dispose() did not complete within 5s — likely sync-over-async deadlock " +
                "(Driver.TwinCAT-015).");
        }
        finally
        {
            SynchronizationContext.SetSynchronizationContext(prev);
        }
        captured.ShouldBeNull();
    }
    /// <summary>
    ///     Single-threaded sync context that posts continuations to an internal queue but never
    ///     pumps them. Any sync-over-async code that captures this context and waits for a
    ///     continuation will deadlock — exactly the OPC UA stack thread scenario.
    /// </summary>
    private sealed class SingleThreadedSyncContext : SynchronizationContext
    {
        private readonly System.Collections.Concurrent.ConcurrentQueue<(SendOrPostCallback cb, object? state)> _queue = new();
        public override void Post(SendOrPostCallback d, object? state) => _queue.Enqueue((d, state));
        public override void Send(SendOrPostCallback d, object? state) => d(state);
    }
    // ---- Driver.TwinCAT-016 — gap-fill tests for previously closed findings ----
    [Fact]
    public void Structure_typed_pre_declared_tag_is_rejected_at_config_parse()
    {
        // Driver.TwinCAT-003 — config-time rejection. A Structure tag must fail loudly with a
        // clear error rather than reading as a garbage int blob or failing late on a write.
        var json = JsonSerializer.Serialize(new
        {
            devices = new[] { new { hostAddress = DeviceA } },
            tags = new[]
            {
                new
                {
                    name = "Udt1",
                    deviceHostAddress = DeviceA,
                    symbolPath = "MAIN.fbInstance",
                    dataType = "Structure",
                },
            },
        });
        var ex = Should.Throw<InvalidOperationException>(() =>
            TwinCATDriverFactoryExtensions.ParseOptionsForTests(json, "drv-1"));
        ex.Message.ShouldContain("Structure");
        ex.Message.ShouldContain("Udt1");
    }
    [Fact]
    public async Task Probe_loop_and_read_share_one_client_per_device()
    {
        // Driver.TwinCAT-007 / -009 — gap-fill: race the probe loop against concurrent reads on
        // the same device. The per-device gate must serialize connect; the probe-task await on
        // ShutdownAsync must let the loop exit cleanly. Without these, the test trips a leaked
        // client or a disposal race.
        var factory = new FakeTwinCATClientFactory
        {
            Customise = () => new FakeTwinCATClient { Values = { ["GVL.X"] = 1 }, ProbeResult = true },
        };
        var drv = new TwinCATDriver(new TwinCATDriverOptions
        {
            Devices = [new TwinCATDeviceOptions(DeviceA)],
            Tags = [new TwinCATTagDefinition("X", DeviceA, "GVL.X", TwinCATDataType.DInt)],
            Probe = new TwinCATProbeOptions
            {
                Enabled = true,
                Interval = TimeSpan.FromMilliseconds(20),
                Timeout = TimeSpan.FromMilliseconds(50),
            },
        }, "drv-1", factory);
        await drv.InitializeAsync("{}", CancellationToken.None);
        // Race 64 readers against the probe loop for ~500ms.
        var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(500));
        var work = Enumerable.Range(0, 64).Select(_ => Task.Run(async () =>
        {
            while (!cts.IsCancellationRequested)
            {
                try { await drv.ReadAsync(["X"], CancellationToken.None); }
                catch { /* shutdown-races on the very last call are ok */ }
            }
        })).ToArray();
        await Task.WhenAll(work);
        await drv.ShutdownAsync(CancellationToken.None);
        // One client total. If the gate is broken, concurrent connects leak additional clients.
        factory.Clients.Count.ShouldBe(1);
        factory.Clients[0].ConnectCount.ShouldBe(1);
    }
    private static async Task WaitForAsync(Func<bool> condition, TimeSpan timeout)
    {
        var deadline = DateTime.UtcNow + timeout;
        while (!condition() && DateTime.UtcNow < deadline)
            await Task.Delay(20);
    }
    /// <summary>Captures the timeout argument from ProbeAsync invocations (via the connect path).</summary>
    private sealed class ProbeTimeoutCapturingFake : FakeTwinCATClient
    {
        private readonly List<TimeSpan> _observed;
        public ProbeTimeoutCapturingFake(List<TimeSpan> observed) { _observed = observed; }
        public override Task ConnectAsync(TwinCATAmsAddress address, TimeSpan timeout, CancellationToken ct)
        {
            // The driver calls EnsureConnectedAsync with the probe timeout for probe-initiated
            // connects. The probe timeout is distinct from the driver-level Timeout; we record
            // it on the first connect.
            lock (_observed) _observed.Add(timeout);
            return base.ConnectAsync(address, timeout, ct);
        }
    }
 }
@@ -137,12 +137,12 @@ public sealed class TwinCATNativeNotificationTests
        public override Task<ITwinCATNotificationHandle> AddNotificationAsync(
            string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime,
-            Action<string, object?> onChange, CancellationToken cancellationToken)
+            int maxDelayMs, Action<string, object?> onChange, CancellationToken cancellationToken)
        {
            AddCallCount++;
            if (AddCallCount > _succeedBefore)
                throw new InvalidOperationException($"fake fail on call #{AddCallCount}");
-            return base.AddNotificationAsync(symbolPath, type, bitIndex, cycleTime, onChange, cancellationToken);
+            return base.AddNotificationAsync(symbolPath, type, bitIndex, cycleTime, maxDelayMs, onChange, cancellationToken);
        }
    }
Author	SHA1	Message	Date
Joseph Doherty	61c0311938	docs(code-reviews): regenerate index — 24 Low findings resolved Batch 4 cleared Open findings in Driver.TwinCAT, Driver.Modbus, Driver.OpcUaClient, Driver.Historian.Wonderware, and Driver.Modbus.Addressing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:18:21 -04:00
Joseph Doherty	9263519852	fix(driver-modbus-addressing): resolve Low code-review findings (Driver.Modbus.Addressing-006,007,009) - Driver.Modbus.Addressing-006: broaden the catch in TryParseFamilyNative so a future helper throwing a non-Argument/Overflow type still satisfies the try-parse contract. - Driver.Modbus.Addressing-007: document that the address grammar does not carry ModbusStringByteOrder (the structured-tag path does); add a 'Grammar scope' bullet to docs/v2/dl205.md. - Driver.Modbus.Addressing-009: reword the ModbusModiconAddress comments so they don't imply a leading-digit invariant the parser doesn't enforce. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:18:15 -04:00
Joseph Doherty	1f29b215c8	fix(driver-historian-wonderware): resolve Low code-review findings (Driver.Historian.Wonderware-004,005,007,008,010,011,012) - Driver.Historian.Wonderware-004: ToHistorianEvent synthesises a fresh Guid when the upstream EventId is unparseable and logs the substitution instead of writing the historian with Guid.Empty. - Driver.Historian.Wonderware-005: GetHealthSnapshot derives the connection-open booleans from the active-node fields so the snapshot is self-consistent without depending on the secondary lock. - Driver.Historian.Wonderware-007: SID-mismatch branch in PipeServer now sends a HelloAck { Accepted=false, RejectReason } so the client sees a symmetric rejection. - Driver.Historian.Wonderware-008: classify StartQuery failures — connection-class codes drop the connection, query-class codes throw QueryClassStartQueryException so the IPC layer surfaces Success=false. - Driver.Historian.Wonderware-010: RequestTimeoutSeconds now enforced via BuildRequestCts linked to the caller's CancellationToken. - Driver.Historian.Wonderware-011: refreshed XML docs to describe the current sidecar / named-pipe architecture (Galaxy.Host / Proxy references reframed as historical context). - Driver.Historian.Wonderware-012: pinned the previously-uncovered HistorianDataSource behaviours with five new test files; also removed the stale empty tests/ZB.MOM.WW.OtOpcUa.Driver.Historian.Wonderware.Tests directory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:18:10 -04:00
Joseph Doherty	42aa82de29	fix(driver-opcuaclient): resolve Low code-review findings (Driver.OpcUaClient-011,014) - Driver.OpcUaClient-011: rewrote the ValueRank comment with the OPC UA Part 3 constants and an explicit scalar/array boundary at valueRank >= 0. - Driver.OpcUaClient-014: track every MonitoredItem.Notification handler in a MonitoredItemNotificationHandle record; UnsubscribeAsync / UnsubscribeAlarmsAsync / ShutdownAsync detach the handler before Subscription.DeleteAsync so the SDK's invocation list no longer keeps the driver alive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:17:55 -04:00
Joseph Doherty	d5322b0f9a	fix(driver-modbus): resolve Low code-review findings (Driver.Modbus-003,007,008,009,010,011,012) - Driver.Modbus-003: route every _health access through ReadHealth / WriteHealth helpers backed by Volatile.Read / Volatile.Write so a burst of concurrent ReadAsync callers always sees a complete snapshot. - Driver.Modbus-007: promoted the Int64 / UInt64 → Int32 surfacing caveat to a full <remarks> block; rewrote DisableFC23's doc to flag it as reserved / no-op. - Driver.Modbus-008: deleted stale duplicate doc, rewrote the prohibition-block summaries to credit the shipped re-probe loop, and removed the unused 'status' local in the ModbusException catch arm. - Driver.Modbus-009: bind-time validation rejects StringLength < 1 for String tags; ModbusTcpTransport clamps keep-alive intervals to whole seconds (>=1). - Driver.Modbus-010: documented WriteOnChangeOnly's cache-invalidation policy (reads-only) and the write-only-tag caveat. - Driver.Modbus-011: collected the scattered instance fields into a single contiguous block at the top of ModbusDriver. - Driver.Modbus-012: covered the previously-uncovered Reinitialize state-hygiene, malformed/truncated/empty-bitmap response, and DisposeAsync teardown paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:17:51 -04:00
Joseph Doherty	3c75db7eb6	fix(driver-twincat): resolve Low code-review findings (Driver.TwinCAT-004,006,014,015,016) - Driver.TwinCAT-004: corrected the IEC time-type inline comments; documented that the driver currently surfaces them as raw UInt32 counters. - Driver.TwinCAT-006: ResolveHost returns a documented UnresolvedHost sentinel when no devices are configured instead of returning the logical DriverInstanceId (which never matches GetHostStatuses). - Driver.TwinCAT-014: wired Probe.Timeout into the probe-loop call and added a NotificationMaxDelayMs config knob threaded through AddNotificationAsync. - Driver.TwinCAT-015: Dispose() runs a genuinely synchronous teardown with bounded waits (no sync-over-async deadlock pattern). - Driver.TwinCAT-016: pinned the Structure-tag rejection and the probe-loop vs read disposal race with regression tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:17:42 -04:00