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

Batch 5 cleared Open findings in Driver.AbCip.Cli, Driver.AbLegacy.Cli, Driver.S7.Cli, Driver.TwinCAT.Cli, and Driver.Modbus.Cli. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(driver-modbus-cli): resolve Low code-review findings (Driver.Modbus.Cli-003,004,005,006,007,008)
2026-05-23 08:35:12 -04:00 · 2026-05-23 08:35:05 -04:00 · 2026-05-23 08:34:57 -04:00 · 2026-05-23 08:34:48 -04:00 · 2026-05-23 08:34:32 -04:00 · 2026-05-23 08:34:25 -04:00
48 changed files with 2069 additions and 189 deletions
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 6 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -96,7 +96,7 @@ into `AbCipCommandBase`.
 | Severity | Low |
 | Category | Concurrency & thread safety |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:50-56,60-61` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `OnDataChange` handler writes change lines to `console.Output`
 (a `TextWriter`) from the driver's poll-engine callback thread, while the command's
@@ -112,7 +112,12 @@ during the watch loop widens it.
 writes during the subscription with a shared lock so poll-thread and main-thread
 output cannot interleave.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — moved the "Subscribed to ... Ctrl+C to stop."
 banner write to BEFORE `driver.OnDataChange +=` (and therefore before
 `SubscribeAsync`). With the handler not yet attached when the banner runs, the
 poll thread cannot fire change events into `console.Output` concurrently with the
 main-thread banner write. After `+=` the only writer to `console.Output` is the
 poll-thread handler, so no interleaving is possible.
 ### Driver.AbCip.Cli-004
@@ -121,7 +126,7 @@ output cannot interleave.
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:28,58`; `AbCipCommandBase.cs:26-34` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `--interval-ms` (`IntervalMs`) is taken verbatim and passed as
 `TimeSpan.FromMilliseconds(IntervalMs)` to `SubscribeAsync` with no validation. A
@@ -135,7 +140,16 @@ downstream component to sanitise operator input is fragile. `--timeout-ms` on
 `ExecuteAsync` / in `AbCipCommandBase`, throwing a `CommandException` with the
 accepted range when out of bounds.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added `SubscribeCommand.ValidateInterval(int)`
 (throws `CommandException` for `IntervalMs <= 0`) called at the top of
 `SubscribeCommand.ExecuteAsync`; moved `TimeoutMs > 0` validation into the
 `AbCipCommandBase.Timeout` getter (throws `CommandException` for non-positive
 `TimeoutMs`) and added a `_ = Timeout` touch in `SubscribeCommand.ExecuteAsync` to
 fire that guard before the driver opens. Other commands trip the same guard
 naturally via `BuildOptions(...).Timeout`. Regression tests
 `AbCipCommandBaseTests.Timeout_get_throws_CommandException_when_TimeoutMs_is_non_positive`
 and `SubscribeCommandIntervalTests.ValidateInterval_rejects_non_positive` cover
 both paths.
 ### Driver.AbCip.Cli-005
@@ -144,7 +158,7 @@ accepted range when out of bounds.
 | Severity | Low |
 | Category | Performance & resource management |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `ConfigureLogging` assigns a freshly created Serilog logger to the
 process-global `Log.Logger` but never calls `Log.CloseAndFlush()`. For a short-lived
@@ -159,7 +173,12 @@ module's review.)
 `AppDomain.ProcessExit` or a `finally` in the command), or have the CLI use a
 disposable logger scoped to `ExecuteAsync`.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `Driver.Cli.Common` already exposes
 `DriverCommandBase.FlushLogging()` (a `Log.CloseAndFlush()` wrapper); the AB CIP
 CLI was not calling it. Added `FlushLogging()` in the `finally` block of all four
 commands (`ProbeCommand`, `ReadCommand`, `WriteCommand`, `SubscribeCommand`) so
 buffered Serilog output is flushed before the process exits, including the
 Ctrl+C-driven `subscribe` path. No edits to `Driver.Cli.Common` were needed.
 ### Driver.AbCip.Cli-006
@@ -168,7 +187,7 @@ disposable logger scoped to `ExecuteAsync`.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs:29-34` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `AbCipCommandBase` overrides the abstract `DriverCommandBase.Timeout`
 property with a getter derived from `TimeoutMs` and an empty `init` body
@@ -183,9 +202,13 @@ bug.
 **Recommendation:** Either drop the `init` accessor entirely (make the override a
 get-only expression-bodied property) or have the empty `init` throw
 `NotSupportedException` to make the "driven by TimeoutMs" contract explicit and
-fail-fast.
+fail-fast. (Drop is not viable because the abstract base declares `{ get; init; }`
 and an override must provide both accessors.)
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — the `init` accessor now throws
 `NotSupportedException` with a message pointing the caller at `TimeoutMs`. A new
 test `AbCipCommandBaseTests.Timeout_setter_is_inert_and_does_not_silently_swallow_assignments`
 asserts that an object-initializer assignment to `Timeout` fails fast.
 ### Driver.AbCip.Cli-007
@@ -194,7 +217,7 @@ fail-fast.
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/WriteCommandParseValueTests.cs` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The only test file covers `WriteCommand.ParseValue` and
 `ReadCommand.SynthesiseTagName` — both pure static helpers. There is no coverage for
@@ -213,7 +236,12 @@ driver CLIs.
 (`HostAddress`, `PlcFamily`, `DeviceName`), the supplied tag list, and the `Timeout`
 derived from `TimeoutMs`.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added
 `tests/.../AbCipCommandBaseTests.cs` covering `BuildOptions` (probe disabled,
 controller-browse disabled, alarm-projection disabled, single device with
 `HostAddress` / `PlcFamily` / `cli-{Family}` device name, tag list passed verbatim,
 `Timeout` derived from `TimeoutMs`) and `DriverInstanceId` (`abcip-cli-{Gateway}`),
 plus the `RejectStructure` guard (throws for `Structure`, no-op for atomic types).
 ### Driver.AbCip.Cli-008
@@ -222,7 +250,7 @@ derived from `TimeoutMs`.
 | Severity | Low |
 | Category | Documentation & comments |
 | Location | `docs/Driver.AbCip.Cli.md:8-9` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `docs/Driver.AbCip.Cli.md` opens with "Second of four driver
 test-client CLIs (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT)." The count "four"
@@ -235,4 +263,6 @@ doc's "four" and the truncated chain are both stale.
 and complete the chain (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT -> FOCAS), or
 drop the explicit count and link `docs/DriverClis.md` as the authoritative roster.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — rewrote the lead paragraph to "Second of six
 driver test-client CLIs (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT -> FOCAS)"
 and added a link to `docs/DriverClis.md` as the authoritative roster.
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 6 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -68,7 +68,7 @@ type (mirroring the existing `Bit` and unsupported-type branches). Either catch
 | Severity | Low |
 | Category | Correctness & logic bugs |
 | Location | `Commands/WriteCommand.cs:27-29`, `Program.cs:6-9` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `--value` option help text states "booleans accept
 true/false/1/0", but `ParseBool` (`WriteCommand.cs:74-80`) and the error message
@@ -82,7 +82,10 @@ with both the code and the design doc.
 matching the wording used elsewhere (e.g. "booleans accept
 true/false, 1/0, on/off, yes/no").
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — updated `WriteCommand.Value` description to
 list the full alias set: "booleans accept true/false, 1/0, on/off, yes/no".
 Regression test `CommandMetadataTests.WriteCommand_value_help_lists_full_boolean_alias_set`
 asserts the description contains every alias group.
 ### Driver.AbLegacy.Cli-003
@@ -91,7 +94,7 @@ true/false, 1/0, on/off, yes/no").
 | Severity | Low |
 | Category | Concurrency & thread safety |
 | Location | `Commands/SubscribeCommand.cs:47-53` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `OnDataChange` handler calls `console.Output.WriteLine(line)`
 (the synchronous overload) directly from the `PollGroupEngine` poll thread. The
@@ -109,7 +112,10 @@ change events through a `Channel<string>` drained by a single consumer task, or
 guard the `WriteLine` with a lock. At minimum, document that the interleaving is
 accepted because output is human-facing and line-buffered.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `SubscribeCommand` now serialises every
 console write through a shared `consoleGate` lock: the poll-thread
 `OnDataChange` callback and the command-thread "Subscribed to ..." line both take
 the lock before calling `WriteLine`. Comment in the source documents the intent.
 ### Driver.AbLegacy.Cli-004
@@ -118,7 +124,7 @@ accepted because output is human-facing and line-buffered.
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `Commands/ProbeCommand.cs:37-56`, `Commands/ReadCommand.cs:39-50`, `Commands/WriteCommand.cs:48-59`, `Commands/SubscribeCommand.cs:41-76` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Every command does `await using var driver = new AbLegacyDriver(...)`
 *and* an explicit `await driver.ShutdownAsync(...)` in the `finally`. `AbLegacyDriver`
@@ -135,7 +141,12 @@ cleanup on every exit path including exceptions.
 since the commands deliberately pass `CancellationToken.None` to shutdown so teardown
 is not cut short by a cancelled `ct`.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — replaced `await using var driver` with a
 plain `var driver` in all four commands (`ProbeCommand`, `ReadCommand`,
 `WriteCommand`, `SubscribeCommand`), keeping the explicit
 `finally { await driver.ShutdownAsync(CancellationToken.None) }` as the single
 teardown path. Comment in each command documents the intent so readers do not
 have to verify idempotency.
 ### Driver.AbLegacy.Cli-005
@@ -144,7 +155,7 @@ is not cut short by a cancelled `ct`.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `Commands/SubscribeCommand.cs:23-25`, `docs/Driver.AbLegacy.Cli.md:94-96` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The subscribe command interval option is `--interval-ms`
 (default 1000). `docs/Driver.AbLegacy.Cli.md` shows the subscribe example as
@@ -160,7 +171,13 @@ but the documented contract drifts between the two CLIs.
 `--interval-ms` description for parity with the AbCip CLI, and mention the
 `--interval-ms` long form + 1000 ms default in `docs/Driver.AbLegacy.Cli.md`.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — extended the `SubscribeCommand.IntervalMs`
 help text to match AbCip ("Publishing interval in milliseconds (default 1000).
 PollGroupEngine floors sub-250ms values.") and added a paragraph under the
 `subscribe` section in `docs/Driver.AbLegacy.Cli.md` naming the `-i` /
 `--interval-ms` long form, the 1000 ms default, and the 250 ms floor. Regression
 test `CommandMetadataTests.SubscribeCommand_interval_ms_help_notes_PollGroupEngine_floor`
 asserts the description mentions "250".
 ### Driver.AbLegacy.Cli-006
@@ -169,7 +186,7 @@ but the documented contract drifts between the two CLIs.
 | Severity | Low |
 | Category | Code organization & conventions |
 | Location | `Commands/ProbeCommand.cs:20-22` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `ProbeCommand` declares its `--type` option with no short alias,
 while `ReadCommand`, `WriteCommand`, and `SubscribeCommand` all declare `--type`
@@ -182,7 +199,13 @@ it silently rejected on `probe`.
 for consistency with the other three commands. (The AbCip CLI `ProbeCommand` has
 the same omission, so a cross-CLI sweep is worthwhile.)
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added the `'t'` short alias to
 `ProbeCommand.DataType`. Regression test
 `CommandMetadataTests.ProbeCommand_type_has_short_alias_t` (plus the parity test
 `Other_commands_keep_type_short_alias_t` for read/write/subscribe) asserts the
 short alias is present on every command. The same omission still exists in the
 AbCip CLI's `ProbeCommand` — flagged as a sibling sweep but out of scope for
 this module.
 ### Driver.AbLegacy.Cli-007
@@ -191,7 +214,7 @@ the same omission, so a cross-CLI sweep is worthwhile.)
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/WriteCommandParseValueTests.cs` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The only test file in the CLI test project covers
 `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName`. Two behaviours that
@@ -210,4 +233,11 @@ Driver.AbLegacy.Cli-001. `BuildOptions` is reachable via `InternalsVisibleTo`
 tag passthrough) and an overflow-input test for `ParseValue` so the fix for
 Driver.AbLegacy.Cli-001 is locked in by a regression test.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added `BuildOptionsTests` (five tests:
 probe disabled, device shape from Gateway+PlcType, tag passthrough, timeout
 propagation, empty tag list) covering `AbLegacyCommandBase.BuildOptions` via a
 nested `TestCommand` subclass annotated with `[Command]` to satisfy the CliFx
 analyzer. The overflow path for `ParseValue` is already covered by
 `WriteCommandParseValueTests.ParseValue_out_of_range_throws_CommandException`
 (theory with `short.Parse` + `AnalogInt` overflow inputs), added when finding
 Driver.AbLegacy.Cli-001 was resolved.
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 6 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -87,7 +87,7 @@ message explaining coils carry a single bit.
 | Severity | Low |
 | Category | Correctness & logic bugs |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/ModbusCommandBase.cs:14-24` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `Port` (`int`) and `TimeoutMs` (`int`) accept any 32-bit value,
 including negatives and ports above 65535. `UnitId` is a `byte`, so it accepts
@@ -103,7 +103,13 @@ error. None of these are validated at parse time.
 message — consistent with how `WriteCommand` already rejects bad regions and
 boolean strings.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added `ModbusCommandBase.ValidateEndpoint()`
 that throws `CliFx.Exceptions.CommandException` for `--port` outside 1..65535,
 non-positive `--timeout-ms`, and `--unit-id` outside 1..247 (Modbus spec unicast
 range — rejects the broadcast 0 and reserved 248-255). Each of `ProbeCommand`,
 `ReadCommand`, `WriteCommand`, and `SubscribeCommand` now calls it at the top of
 `ExecuteAsync` after `ConfigureLogging()`. Regression tests live in
 `ModbusCommandBaseTests` (range + boundary cases for all three knobs).
 ### Driver.Modbus.Cli-004
@@ -113,7 +119,7 @@ boolean strings.
 | Severity | Low |
 | Category | Concurrency & thread safety |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/SubscribeCommand.cs:61-67` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `OnDataChange` handler is invoked from the driver's
 `PollGroupEngine` background thread and calls `console.Output.WriteLine`
@@ -127,7 +133,11 @@ any synchronization, so overlapping poll ticks could interleave partial lines.
 write failures so a transient console-write error cannot tear down the poll loop.
 A single `lock` around the write also removes the interleave risk.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — wrapped the `OnDataChange` handler in
 `try/catch (Exception)` that logs to Serilog at Warning and swallows the failure
 so a transient console-write error cannot fault the `PollGroupEngine` background
 loop. The console write is also serialized through a local `lock` object, removing
 the partial-line interleave risk when multiple poll ticks overlap.
 ### Driver.Modbus.Cli-005
@@ -136,7 +146,7 @@ A single `lock` around the write also removes the interleave risk.
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `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` |
-| Status | Open |
+| Status | Resolved |
 **Description:** All three commands call `ConfigureLogging()` then
 `console.RegisterCancellationHandler()`, but if the operator presses Ctrl+C
@@ -152,7 +162,14 @@ commands do not catch it around their driver calls.
 the noisy trace on Ctrl+C-during-connect is acceptable. Consistency with
 `SubscribeCommand`'s handling is the cleaner choice.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added a
 `catch (OperationCanceledException) when (ct.IsCancellationRequested)` clause
 around the driver-call bodies in `ProbeCommand`, `ReadCommand`, and `WriteCommand`
 that prints `Cancelled.` and falls through to the existing `finally`-block
 shutdown. Matches `SubscribeCommand`'s existing handling so all four commands now
 exit quietly on Ctrl+C. Regression tests in `CommandCancellationTests` pre-cancel
 the CliFx `FakeInMemoryConsole` before calling `ExecuteAsync` and assert no
 exception escapes.
 ### Driver.Modbus.Cli-006
@@ -161,7 +178,7 @@ the noisy trace on Ctrl+C-during-connect is acceptable. Consistency with
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ProbeCommand.cs:35-53` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `probe` reports `Health: {health.State}` from `GetHealth()`.
 After a successful `InitializeAsync` the driver sets state to `Healthy`
@@ -179,7 +196,14 @@ snapshot's `StatusCode` (Good vs Bad) rather than — or in addition to — the
 `State`, or print a single combined verdict line so the two cannot contradict each
 other.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — `ProbeCommand` now prints a single combined
 `Verdict:` line above the bare driver-state line; the headline is computed by a
 new `ProbeCommand.ComputeVerdict(DriverState, statusCode)` helper that combines
 the driver state with the probe snapshot's OPC UA quality class (top 2 bits of
 the StatusCode). Verdict is `FAIL` whenever the driver is not Healthy OR the
 snapshot is Bad, `DEGRADED` when the driver is Healthy but the snapshot is
 Uncertain, and `OK` only when both are Good. Regression tests in
 `ProbeCommandTests` pin the verdict-grid behaviour.
 ### Driver.Modbus.Cli-007
@@ -188,7 +212,7 @@ other.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `docs/Driver.Modbus.Cli.md:124-156`; `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `docs/Driver.Modbus.Cli.md` devotes a whole "v2 addressing
 grammar" section to the industry-standard tag-address strings (`40001:F:CDAB`,
@@ -205,7 +229,14 @@ driver's address-string parser (and `--family` for the DL205/MELSEC native
 syntax), or scope the "v2 addressing grammar" section of the doc to note it
 applies to `DriverConfig` JSON and is not a CLI flag.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — chose the doc-scoping fix (an
 `--address-string` flag is a bigger feature that warrants its own design
 discussion). Added a "CLI scope" callout to the top of the
 `docs/Driver.Modbus.Cli.md` "v2 addressing grammar" section stating the CLI
 accepts only the structured `--region` + `--address` + `--type` triple and that
 the address-string grammar is a `DriverConfig` JSON feature. The pre-existing
 closing paragraph already said the same thing; the new callout makes it visible
 before the grammar examples instead of after them. Code surface left unchanged.
 ### Driver.Modbus.Cli-008
@@ -214,7 +245,7 @@ applies to `DriverConfig` JSON and is not a CLI flag.
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests/` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The test project covers only the two pure-function seams:
 `ReadCommand.SynthesiseTagName` and `WriteCommand.ParseValue`. There is no coverage
@@ -231,4 +262,19 @@ likely to regress and is currently untested. The validation gaps in findings
 setters and assert the produced `ModbusDriverOptions`). Once findings 002/003 are
 fixed, add tests for the new validation paths.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added four new test classes covering the
 previously untested branch logic:
 - `ModbusCommandBaseTests` — six tests over `BuildOptions` (probe disabled,
  `TimeoutMs` → `Timeout` mapping, `AutoReconnect` tracking
  `--disable-reconnect` in both directions, host/port/unit flow-through, tag
  pass-through) plus the new `ValidateEndpoint` range-check tests for finding
  003 (port 1..65535, timeout-ms > 0, unit-id 1..247).
 - `WriteCommandRegionValidationTests` — read-only region rejection
  (DiscreteInputs / InputRegisters) and the Coils-non-Bool guard for finding
  002.
 - `ProbeCommandTests` — the new `ComputeVerdict` helper for finding 006
  (OK / DEGRADED / FAIL grid).
 - `CommandCancellationTests` — Ctrl+C-during-initialize for `ProbeCommand` /
  `ReadCommand` / `WriteCommand` for finding 005.
 Total test count grew from 18 to 64; all pass.
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 4 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -120,7 +120,7 @@ unreachable device, not crash on it.
 | Severity | Low |
 | Category | Performance & resource management |
 | Location | `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` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Every command declares the driver with `await using var driver = new
 S7Driver(...)` and *also* calls `await driver.ShutdownAsync(...)` in a `finally` block.
@@ -136,7 +136,7 @@ not actually disposing.
 `subscribe` command `UnsubscribeAsync`. Alternatively drop `await using`
 and keep the explicit `finally`. Pick one disposal mechanism per command.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — dropped the explicit `await driver.ShutdownAsync(CancellationToken.None)` calls from the `finally` blocks of `ProbeCommand`, `ReadCommand`, `WriteCommand`, and `SubscribeCommand`; `await using` is now the sole driver-disposal mechanism per command (DisposeAsync itself runs ShutdownAsync), and the subscribe command keeps `UnsubscribeAsync` in its finally because that is a subscription-lifecycle concern, not driver disposal. Added `CommandDisposalConventionsTests` to guard the source-level convention against regression.
 ### Driver.S7.Cli-005
@@ -145,7 +145,7 @@ and keep the explicit `finally`. Pick one disposal mechanism per command.
 | Severity | Low |
 | Category | Code organization & conventions |
 | Location | `tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` |
-| Status | Open |
+| Status | Resolved |
 **Description:** 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
@@ -158,7 +158,7 @@ grepping the tree for the S7 CLI test project.
 directory (including its `obj/`). This is outside the module `src/` tree but is the
 S7 CLI own orphaned test folder, so it belongs to this module cleanup.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — deleted the stale `tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` directory (only contained a leftover `obj/` from before the move into `tests/Drivers/Cli/`; no tracked files). The real test project at `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` is untouched.
 ### Driver.S7.Cli-006
@@ -167,7 +167,7 @@ S7 CLI own orphaned test folder, so it belongs to this module cleanup.
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/WriteCommandParseValueTests.cs` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The only test file covers `WriteCommand.ParseValue` and
 `ReadCommand.SynthesiseTagName`. `S7CommandBase.BuildOptions` — which maps the
@@ -184,7 +184,7 @@ not be caught. `ParseValue` is also missing an explicit overflow-edge test (e.g.
 overflow case to the `ParseValue` numeric tests once Driver.S7.Cli-001 is resolved so
 the test asserts the wrapped `CommandException`.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — added `S7CommandBaseBuildOptionsTests` covering Probe.Enabled=false (one-shot CLI guarantee), TimeoutMs→Timeout TimeSpan mapping, host/port/CPU/rack/slot flowing through, and tag-list passthrough. The overflow-edge `ParseValue` test was already added under Driver.S7.Cli-001 (`ParseValue_overflow_for_numeric_types_throws_CommandException`).
 ### Driver.S7.Cli-007
@@ -193,7 +193,7 @@ the test asserts the wrapped `CommandException`.
 | Severity | Low |
 | Category | Documentation & comments |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/SubscribeCommand.cs:45-51` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The Modbus CLI `SubscribeCommand` carries an explanatory comment on
 the `OnDataChange` handler ("Route every data-change event to the CliFx console (not
@@ -206,4 +206,4 @@ Minor, but the rationale is worth keeping consistent across the CLI family.
 **Recommendation:** Re-add the one-line comment from the Modbus `SubscribeCommand` so
 the S7 copy explains why the event handler writes via `console.Output` synchronously.
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-23 — re-added the explanatory comment above the `OnDataChange` handler in the S7 `SubscribeCommand`, mirroring the Modbus copy: it explains the use of the CliFx `IConsole.Output` abstraction (rather than `System.Console`) and notes that the handler runs synchronously because it's raised from a driver background thread. Added `SubscribeCommandConsoleHandlerCommentTests` to guard the rationale against future copy-paste regressions.
@@ -7,7 +7,7 @@
 | Review date | 2026-05-22 |
 | Commit reviewed | `76d35d1` |
 | Status | Reviewed |
-| Open findings | 7 |
+| Open findings | 0 |
 ## Checklist coverage
@@ -40,7 +40,7 @@ a category produced nothing rather than leaving it blank.
 | Severity | Low |
 | Category | Correctness & logic bugs |
 | Location | `TwinCATCommandBase.cs:23-24`, `Commands/SubscribeCommand.cs:23-24`, `Commands/BrowseCommand.cs:21-24` |
-| Status | Open |
+| Status | Resolved |
 **Description:** Numeric command options are accepted without range validation. `--timeout-ms`
 feeds `Timeout => TimeSpan.FromMilliseconds(TimeoutMs)`; passing `--timeout-ms 0` or a negative
@@ -56,7 +56,16 @@ failure mode should be a readable up-front rejection.
 shared helper on `TwinCATCommandBase`) and throw `CliFx.Exceptions.CommandException` with a
 clear message when `TimeoutMs <= 0`, `IntervalMs <= 0`, or `AmsPort` falls outside `1..65535`.
-**Resolution:** _(open)_
+**Resolution:** Added a virtual `Validate()` helper on `TwinCATCommandBase` that rejects
 `TimeoutMs <= 0` and `AmsPort` outside `1..65535` with a clean
 `CliFx.Exceptions.CommandException` carrying the offending value. `SubscribeCommand` overrides
 `Validate()` to add the `IntervalMs > 0` check. Each `ExecuteAsync` calls `Validate()` first
 so the CLI surfaces "bad argument" up-front instead of letting the driver fail with an opaque
 transport error. Covered by `TwinCATCommandBaseTests.Validate_rejects_zero_timeout`,
 `Validate_rejects_negative_timeout`, `Validate_rejects_out_of_range_ams_port` (theory, 4
 cases), `Validate_accepts_in_range_ams_port` (theory, 4 cases),
 `SubscribeCommand_validate_rejects_zero_interval`,
 `SubscribeCommand_validate_rejects_negative_interval`.
 ### Driver.TwinCAT.Cli-002
@@ -65,7 +74,7 @@ clear message when `TimeoutMs <= 0`, `IntervalMs <= 0`, or `AmsPort` falls outsi
 | Severity | Low |
 | Category | Concurrency & thread safety |
 | Location | `Commands/SubscribeCommand.cs:46-58` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `OnDataChange` handler calls `console.Output.WriteLine(line)` synchronously.
 In native ADS-notification mode the event is raised from the `Beckhoff.TwinCAT.Ads`
@@ -83,7 +92,12 @@ serialised on one poll loop; the TwinCAT native path has no such serialisation.
 `TextWriter.Synchronized`. At minimum, gate it so the banner is written before the
 subscription is registered (it already is) and lock the per-event writes against each other.
-**Resolution:** _(open)_
+**Resolution:** Introduced a per-execution `writeLock` object inside
 `SubscribeCommand.ExecuteAsync`. Both the `OnDataChange` handler's `WriteLine` and the
 post-subscription "Subscribed to ..." banner take the lock, so notification-callback writes
 cannot interleave with the main-thread banner or with each other. Lock is local to the
 command so parallel process instances do not contend with one another (each owns its own
 console).
 ### Driver.TwinCAT.Cli-003
@@ -92,7 +106,7 @@ subscription is registered (it already is) and lock the per-event writes against
 | Severity | Low |
 | Category | Error handling & resilience |
 | Location | `Commands/SubscribeCommand.cs:56-58` |
-| Status | Open |
+| Status | Resolved |
 **Description:** 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`)
@@ -108,7 +122,15 @@ returned `ISubscriptionHandle.DiagnosticId`, which is `twincat-native-sub-*` for
 path vs the `PollGroupEngine` handle for poll mode) or soften the wording to "(requested:
 ADS notification)" so it does not over-claim.
-**Resolution:** _(open)_
+**Resolution:** Added internal static
 `SubscribeCommand.DescribeMechanism(ISubscriptionHandle)` that maps
 `DiagnosticId.StartsWith("twincat-native-sub-", Ordinal)` to `"ADS notification"` and
 anything else to `"polling"`. The banner now reads from the handle the driver actually
 returned, so the line cannot disagree with what the driver did even if a future fallback
 lands the subscription somewhere unexpected. Covered by
 `SubscribeCommandMechanismTests.DescribeMechanism_returns_ADS_notification_for_native_handle`
 (theory, 3 cases) and `DescribeMechanism_returns_polling_for_anything_else` (theory, 4 cases
 including an ordinal case-sensitivity guard).
 ### Driver.TwinCAT.Cli-004
@@ -117,7 +139,7 @@ ADS notification)" so it does not over-claim.
 | Severity | Low |
 | Category | Design-document adherence |
 | Location | `TwinCATCommandBase.cs:26-29`, `Commands/BrowseCommand.cs` |
-| Status | Open |
+| Status | Resolved |
 **Description:** `--poll-only` is declared on `TwinCATCommandBase`, so it is inherited by
 `browse`. `BrowseCommand` only ever calls `DiscoverAsync` — it never subscribes — so
@@ -131,7 +153,15 @@ disagree.
 flag) onto an intermediate base shared by only `probe`/`read`/`subscribe`, or override/hide it
 for `browse`. Alternatively document explicitly that the flag is a no-op for `browse`.
-**Resolution:** _(open)_
+**Resolution:** Introduced an intermediate `TwinCATTagCommandBase : TwinCATCommandBase` that
 hosts the `--poll-only` flag and the `BuildOptions(...)` helper. `ProbeCommand`,
 `ReadCommand`, `WriteCommand`, and `SubscribeCommand` inherit from this intermediate (they all
 build a tag-list `TwinCATDriverOptions`). `BrowseCommand` keeps inheriting from
 `TwinCATCommandBase` directly, so `--poll-only` no longer surfaces in `browse --help`. Browse
 sets `UseNativeNotifications = true` on its inline options (irrelevant either way for the
 discover-only path, but matches production wiring). Covered by
 `TwinCATCommandBaseTests.BrowseCommand_does_not_expose_poll_only_flag` and
 `ProbeCommand_still_exposes_poll_only_flag` (both reflect over the public property surface).
 ### Driver.TwinCAT.Cli-005
@@ -140,7 +170,7 @@ for `browse`. Alternatively document explicitly that the flag is a no-op for `br
 | Severity | Low |
 | Category | Code organization & conventions |
 | Location | `Commands/ProbeCommand.cs:23`, `Commands/ReadCommand.cs:20`, `Commands/WriteCommand.cs:20`, `Commands/SubscribeCommand.cs:18` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `--type` option is declared with the short alias `-t` on `read`, `write`,
 and `subscribe`, but `ProbeCommand` declares `[CommandOption("type", ...)]` with no short
@@ -151,7 +181,10 @@ take the same `TwinCATDataType` option.
 **Recommendation:** Add the `'t'` short alias to `ProbeCommand`'s `--type` option to match the
 other three commands.
-**Resolution:** _(open)_
+**Resolution:** Added the `'t'` short alias on `ProbeCommand.DataType`'s `[CommandOption]`,
 matching read/write/subscribe so muscle memory carries between the four verbs. Covered by
 `TwinCATCommandBaseTests.ProbeCommand_type_option_carries_short_alias_t` which asserts the
 `CommandOptionAttribute.ShortName` is `'t'`.
 ### Driver.TwinCAT.Cli-006
@@ -160,7 +193,7 @@ other three commands.
 | Severity | Low |
 | Category | Testing coverage |
 | Location | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests/WriteCommandParseValueTests.cs` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The only test file covers `WriteCommand.ParseValue` and
 `ReadCommand.SynthesiseTagName`. Other deterministic, router-independent logic is untested:
@@ -177,7 +210,20 @@ module's scope but worth flagging to whoever owns the test tree.
 `BuildOptions` field wiring, and for the `CollectingAddressSpaceBuilder` prefix/max filtering
 and access-classification logic.
-**Resolution:** _(open)_
+**Resolution:** Added two new test classes. `TwinCATCommandBaseTests` covers the Gateway
 canonical form, round-trip through `TwinCATAmsAddress.TryParse`, DriverInstanceId
 composition, Timeout projection, BuildOptions field wiring (devices, tags, timeout, probe
 disabled, controller-browse disabled, UseNativeNotifications default true), and the PollOnly
 toggle flipping UseNativeNotifications. `BrowseCommandFilterTests` covers
 `CollectingAddressSpaceBuilder` (records variables in call order, treats `Folder` as a
 same-builder pass-through), `BrowseCommand.FilterByPrefix` (empty/null prefix passes
 everything, case-sensitive ordinal match), `BrowseCommand.PrintLimit` (max <= 0 = unbounded,
 caps when matched > max, no padding when matched < max), and `BrowseCommand.AccessTag`
 (ViewOnly -> RO, every other classification -> RW, theory over all 6 non-ViewOnly values).
 `BrowseCommand.CollectingAddressSpaceBuilder` made `internal` (was `private`) so the test
 project can construct it directly via the existing `InternalsVisibleTo` hook. Total tests
 for this assembly went from 27 to 69. The stale empty sibling test directory mention is
 left out of scope as noted.
 ### Driver.TwinCAT.Cli-007
@@ -186,7 +232,7 @@ and access-classification logic.
 | Severity | Low |
 | Category | Documentation & comments |
 | Location | `TwinCATCommandBase.cs:31-36` |
-| Status | Open |
+| Status | Resolved |
 **Description:** The `Timeout` override has an empty `init` accessor with the comment
 `/* driven by TimeoutMs */`. Because the base `DriverCommandBase.Timeout` is declared
@@ -199,4 +245,9 @@ gives no hint of the deliberate no-op. This is a maintainability/clarity nit, no
 computed projection of `--timeout-ms` and the `init` accessor is intentionally a no-op, so the
 design intent survives refactoring.
-**Resolution:** _(open)_
+**Resolution:** Replaced the `<inheritdoc/>` on `TwinCATCommandBase.Timeout` with an explicit
 `<summary>` documenting that `Timeout` is projected from `TimeoutMs`, that the `init`
 accessor required by the abstract base property is intentionally a no-op, and that adding a
 backing field would cause the two to drift on every refactor. The inner-block comment was
 tightened to point at the XML summary so the design intent survives whichever doc surface a
 future maintainer reads first.
@@ -23,9 +23,9 @@ Each module's `findings.md` is the source of truth; this file is generated from
 | [Core.Scripting](Core.Scripting/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 11 |
 | [Core.VirtualTags](Core.VirtualTags/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 13 |
 | [Driver.AbCip](Driver.AbCip/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 15 |
-| [Driver.AbCip.Cli](Driver.AbCip.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 6 | 8 |
+| [Driver.AbCip.Cli](Driver.AbCip.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 8 |
 | [Driver.AbLegacy](Driver.AbLegacy/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 13 |
-| [Driver.AbLegacy.Cli](Driver.AbLegacy.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 6 | 7 |
+| [Driver.AbLegacy.Cli](Driver.AbLegacy.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 7 |
 | [Driver.Cli.Common](Driver.Cli.Common/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 2 | 6 |
 | [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 |
@@ -34,12 +34,12 @@ Each module's `findings.md` is the source of truth; this file is generated from
 | [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 | 0 | 12 |
 | [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.Modbus.Cli](Driver.Modbus.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 8 |
 | [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.S7.Cli](Driver.S7.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 7 |
 | [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 |
+| [Driver.TwinCAT.Cli](Driver.TwinCAT.Cli/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 7 |
 | [Server](Server/findings.md) | Claude Code | 2026-05-22 | `76d35d1` | Reviewed | 0 | 15 |
 ## Pending findings
@@ -67,18 +67,6 @@ Findings with status `Open` or `In Progress`, ordered by severity.
 | Client.UI-009 | Low | Design-document adherence | `ViewModels/HistoryViewModel.cs:44-54` | `HistoryViewModel.AggregateTypes` exposes eight entries: `null` (Raw) plus Average, Minimum, Maximum, Count, Start, End, and `StandardDeviation`. `docs/Client.UI.md` ("Query Options" table) lists only "Raw (default), Average, Minimum, Maxi… |
 | Client.UI-010 | Low | Code organization & conventions | `Controls/DateTimeRangePicker.axaml.cs:33-37`, `Controls/DateTimeRangePicker.axaml.cs:70-80` | `DateTimeRangePicker` declares `MinDateTimeProperty` / `MaxDateTimeProperty` styled properties with public CLR accessors, but neither is read anywhere in the control. `TryParseDateTime`, `OnStartLostFocus`, and `OnEndLostFocus` never clamp… |
 | Client.UI-011 | Low | Documentation & comments | `Views/MainWindow.axaml:81`, `Services/JsonSettingsService.cs:11-15` | The certificate-store-path `TextBox` watermark reads `(default: AppData/LmxOpcUaClient/pki)`, referencing the legacy pre-task-#208 folder name. Per `CLAUDE.md` / `docs/Client.UI.md` the canonical path is now `{LocalAppData}/OtOpcUaClient/`… |
 | Driver.AbCip.Cli-003 | Low | Concurrency & thread safety | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:50-56,60-61` | The `OnDataChange` handler writes change lines to `console.Output` (a `TextWriter`) from the driver's poll-engine callback thread, while the command's main flow concurrently writes the "Subscribed to ... Ctrl+C to stop." line on the CLI th… |
 | Driver.AbCip.Cli-004 | Low | Error handling & resilience | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:28,58`; `AbCipCommandBase.cs:26-34` | `--interval-ms` (`IntervalMs`) is taken verbatim and passed as `TimeSpan.FromMilliseconds(IntervalMs)` to `SubscribeAsync` with no validation. A zero or negative value produces a non-positive `TimeSpan`; the option description claims "Poll… |
 | Driver.AbCip.Cli-005 | Low | Performance & resource management | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59` | `ConfigureLogging` assigns a freshly created Serilog logger to the process-global `Log.Logger` but never calls `Log.CloseAndFlush()`. For a short-lived one-shot command (`probe`, `read`, `write`) the process exit flushes the console sink,… |
 | Driver.AbCip.Cli-006 | Low | Design-document adherence | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs:29-34` | `AbCipCommandBase` overrides the abstract `DriverCommandBase.Timeout` property with a getter derived from `TimeoutMs` and an empty `init` body (`init { /* driven by TimeoutMs */ }`). Because the override has no `[CommandOption]` attribute,… |
 | Driver.AbCip.Cli-007 | Low | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/WriteCommandParseValueTests.cs` | The only test file covers `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName` — both pure static helpers. There is no coverage for `AbCipCommandBase.BuildOptions` (the flag-to-`AbCipDriverOptions` mapping that all four commands d… |
 | Driver.AbCip.Cli-008 | Low | Documentation & comments | `docs/Driver.AbCip.Cli.md:8-9` | `docs/Driver.AbCip.Cli.md` opens with "Second of four driver test-client CLIs (Modbus -> AB CIP -> AB Legacy -> S7 -> TwinCAT)." The count "four" contradicts the chain that follows it (five names) and contradicts `docs/DriverClis.md`, whic… |
 | Driver.AbLegacy.Cli-002 | Low | Correctness & logic bugs | `Commands/WriteCommand.cs:27-29`, `Program.cs:6-9` | The `--value` option help text states "booleans accept true/false/1/0", but `ParseBool` (`WriteCommand.cs:74-80`) and the error message also accept `on/off` and `yes/no`, and `DriverClis.md` documents the full `true/false/1/0/yes/no/on/off… |
 | Driver.AbLegacy.Cli-003 | Low | Concurrency & thread safety | `Commands/SubscribeCommand.cs:47-53` | The `OnDataChange` handler calls `console.Output.WriteLine(line)` (the synchronous overload) directly from the `PollGroupEngine` poll thread. The poll engine raises change events from a background timer/loop thread, so two ticks that fire… |
 | Driver.AbLegacy.Cli-004 | Low | Error handling & resilience | `Commands/ProbeCommand.cs:37-56`, `Commands/ReadCommand.cs:39-50`, `Commands/WriteCommand.cs:48-59`, `Commands/SubscribeCommand.cs:41-76` | Every command does `await using var driver = new AbLegacyDriver(...)` *and* an explicit `await driver.ShutdownAsync(...)` in the `finally`. `AbLegacyDriver` `DisposeAsync` itself calls `ShutdownAsync`, so the driver is shut down twice on t… |
 | Driver.AbLegacy.Cli-005 | Low | Design-document adherence | `Commands/SubscribeCommand.cs:23-25`, `docs/Driver.AbLegacy.Cli.md:94-96` | The subscribe command interval option is `--interval-ms` (default 1000). `docs/Driver.AbLegacy.Cli.md` shows the subscribe example as `otopcua-ablegacy-cli subscribe ... -i 500`, which works because of the short alias `'i'`, but the doc ne… |
 | Driver.AbLegacy.Cli-006 | Low | Code organization & conventions | `Commands/ProbeCommand.cs:20-22` | `ProbeCommand` declares its `--type` option with no short alias, while `ReadCommand`, `WriteCommand`, and `SubscribeCommand` all declare `--type` with the short alias `'t'`. `ProbeCommand` also gives `--address` the alias `'a'`, matching t… |
 | Driver.AbLegacy.Cli-007 | Low | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/WriteCommandParseValueTests.cs` | The only test file in the CLI test project covers `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName`. Two behaviours that are pure logic (testable without a device) are uncovered: (1) `AbLegacyCommandBase.BuildOptions` — that it… |
 | Driver.Cli.Common-004 | Low | Error handling & resilience | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:68-70` | `FormatTable` calls `rows.Max(r => r.Tag.Length)` (and the same for the value and status columns) without guarding against empty input. When `tagNames` and `snapshots` are both empty (equal length, so the mismatch check at line 56 passes),… |
 | Driver.Cli.Common-006 | Low | Documentation & comments | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:71`, `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:9` | Two minor doc inaccuracies. (1) The comment at `SnapshotFormatter.cs:71` states the "source-time column is fixed-width (ISO-8601 to ms) so no max-measurement needed" — true only when every snapshot has a non-null `SourceTimestampUtc`. `For… |
 | Driver.FOCAS.Cli-001 | Low | Error handling & resilience | `Commands/WriteCommand.cs:58-68` | `WriteCommand.ParseValue` parses the numeric `--value` types (`Byte`/`Int16`/`Int32`/`Float32`/`Float64`) with `sbyte.Parse` / `short.Parse` / etc. These throw raw `FormatException` or `OverflowException` for malformed or out-of-range inpu… |
@@ -91,23 +79,6 @@ Findings with status `Open` or `In Progress`, ordered by severity.
 | 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.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.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.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… |
 | Driver.TwinCAT.Cli-004 | Low | Design-document adherence | `TwinCATCommandBase.cs:26-29`, `Commands/BrowseCommand.cs` | `--poll-only` is declared on `TwinCATCommandBase`, so it is inherited by `browse`. `BrowseCommand` only ever calls `DiscoverAsync` — it never subscribes — so `UseNativeNotifications = !PollOnly` has no observable effect on a browse run. Th… |
 | Driver.TwinCAT.Cli-005 | Low | Code organization & conventions | `Commands/ProbeCommand.cs:23`, `Commands/ReadCommand.cs:20`, `Commands/WriteCommand.cs:20`, `Commands/SubscribeCommand.cs:18` | The `--type` option is declared with the short alias `-t` on `read`, `write`, and `subscribe`, but `ProbeCommand` declares `[CommandOption("type", ...)]` with no short alias. An operator who has internalised `-t` from the other three verbs… |
 | Driver.TwinCAT.Cli-006 | Low | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests/WriteCommandParseValueTests.cs` | The only test file covers `WriteCommand.ParseValue` and `ReadCommand.SynthesiseTagName`. Other deterministic, router-independent logic is untested: `TwinCATCommandBase.Gateway` (the `ads://{netId}:{port}` string the driver's `TwinCATAmsAdd… |
 | Driver.TwinCAT.Cli-007 | Low | Documentation & comments | `TwinCATCommandBase.cs:31-36` | The `Timeout` override has an empty `init` accessor with the comment `/* driven by TimeoutMs */`. Because the base `DriverCommandBase.Timeout` is declared `abstract { get; init; }`, the override must supply an `init`, but here it silently… |
 ## Closed findings
@@ -343,9 +314,21 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
 | Driver.AbCip-012 | Low | Resolved | Performance & resource management | `LibplctagTemplateReader.cs:15-35`, `AbCipDriver.cs:88-92` |
 | Driver.AbCip-013 | Low | Resolved | Design-document adherence | `AbCipDriverOptions.cs:70-73`, `PlcFamilies/AbCipPlcFamilyProfile.cs:13-19`, `LibplctagTagRuntime.cs:16-27` |
 | Driver.AbCip-015 | Low | Resolved | Documentation & comments | `AbCipDriver.cs:9-11`, `PlcTagHandle.cs:23-27,53-58`, `AbCipTemplateCache.cs:12-15`, `IAbCipTagEnumerator.cs:6-11`, `AbCipDriverOptions.cs:21` |
 | Driver.AbCip.Cli-003 | Low | Resolved | Concurrency & thread safety | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:50-56,60-61` |
 | Driver.AbCip.Cli-004 | Low | Resolved | Error handling & resilience | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/SubscribeCommand.cs:28,58`; `AbCipCommandBase.cs:26-34` |
 | Driver.AbCip.Cli-005 | Low | Resolved | Performance & resource management | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59` |
 | Driver.AbCip.Cli-006 | Low | Resolved | Design-document adherence | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/AbCipCommandBase.cs:29-34` |
 | Driver.AbCip.Cli-007 | Low | Resolved | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests/WriteCommandParseValueTests.cs` |
 | Driver.AbCip.Cli-008 | Low | Resolved | Documentation & comments | `docs/Driver.AbCip.Cli.md:8-9` |
 | Driver.AbLegacy-005 | Low | Resolved | OtOpcUa conventions | `AbLegacyDriver.cs` (whole file) |
 | Driver.AbLegacy-011 | Low | Resolved | Performance & resource management | `AbLegacyDriver.cs:440` |
 | Driver.AbLegacy-013 | Low | Resolved | Code organization & conventions | `AbLegacyDriver.cs:340-345`, `AbLegacyDriver.cs:238-264` |
 | Driver.AbLegacy.Cli-002 | Low | Resolved | Correctness & logic bugs | `Commands/WriteCommand.cs:27-29`, `Program.cs:6-9` |
 | Driver.AbLegacy.Cli-003 | Low | Resolved | Concurrency & thread safety | `Commands/SubscribeCommand.cs:47-53` |
 | Driver.AbLegacy.Cli-004 | Low | Resolved | Error handling & resilience | `Commands/ProbeCommand.cs:37-56`, `Commands/ReadCommand.cs:39-50`, `Commands/WriteCommand.cs:48-59`, `Commands/SubscribeCommand.cs:41-76` |
 | Driver.AbLegacy.Cli-005 | Low | Resolved | Design-document adherence | `Commands/SubscribeCommand.cs:23-25`, `docs/Driver.AbLegacy.Cli.md:94-96` |
 | Driver.AbLegacy.Cli-006 | Low | Resolved | Code organization & conventions | `Commands/ProbeCommand.cs:20-22` |
 | Driver.AbLegacy.Cli-007 | Low | Resolved | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests/WriteCommandParseValueTests.cs` |
 | Driver.FOCAS-007 | Low | Resolved | Error handling & resilience | `FocasDriver.cs:140-148`, `FocasDriver.cs:478-484`, `FocasDriver.cs:529-533`, `FocasAlarmProjection.cs:61-63` |
 | Driver.FOCAS-008 | Low | Resolved | Performance & resource management | `FocasDriver.cs:201`, `FocasDriver.cs:253` |
 | Driver.FOCAS-009 | Low | Resolved | Design-document adherence | `FocasDriverOptions.cs:110-115`, `FocasDriver.cs:468-486`, `FocasDriverFactoryExtensions.cs:75-80` |
@@ -372,6 +355,12 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
 | 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.Modbus.Cli-003 | Low | Resolved | Correctness & logic bugs | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/ModbusCommandBase.cs:14-24` |
 | Driver.Modbus.Cli-004 | Low | Resolved | Concurrency & thread safety | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/SubscribeCommand.cs:61-67` |
 | Driver.Modbus.Cli-005 | Low | Resolved | 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` |
 | Driver.Modbus.Cli-006 | Low | Resolved | Error handling & resilience | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ProbeCommand.cs:35-53` |
 | Driver.Modbus.Cli-007 | Low | Resolved | Design-document adherence | `docs/Driver.Modbus.Cli.md:124-156`; `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs` |
 | Driver.Modbus.Cli-008 | Low | Resolved | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests/` |
 | 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` |
@@ -379,11 +368,22 @@ Findings with status `Resolved`, `Won't Fix`, or `Deferred`.
 | 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.S7.Cli-004 | Low | Resolved | 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` |
 | Driver.S7.Cli-005 | Low | Resolved | Code organization & conventions | `tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/` |
 | Driver.S7.Cli-006 | Low | Resolved | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/WriteCommandParseValueTests.cs` |
 | Driver.S7.Cli-007 | Low | Resolved | Documentation & comments | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/SubscribeCommand.cs:45-51` |
 | 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/` |
 | Driver.TwinCAT.Cli-001 | Low | Resolved | Correctness & logic bugs | `TwinCATCommandBase.cs:23-24`, `Commands/SubscribeCommand.cs:23-24`, `Commands/BrowseCommand.cs:21-24` |
 | Driver.TwinCAT.Cli-002 | Low | Resolved | Concurrency & thread safety | `Commands/SubscribeCommand.cs:46-58` |
 | Driver.TwinCAT.Cli-003 | Low | Resolved | Error handling & resilience | `Commands/SubscribeCommand.cs:56-58` |
 | Driver.TwinCAT.Cli-004 | Low | Resolved | Design-document adherence | `TwinCATCommandBase.cs:26-29`, `Commands/BrowseCommand.cs` |
 | Driver.TwinCAT.Cli-005 | Low | Resolved | Code organization & conventions | `Commands/ProbeCommand.cs:23`, `Commands/ReadCommand.cs:20`, `Commands/WriteCommand.cs:20`, `Commands/SubscribeCommand.cs:18` |
 | Driver.TwinCAT.Cli-006 | Low | Resolved | Testing coverage | `tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests/WriteCommandParseValueTests.cs` |
 | Driver.TwinCAT.Cli-007 | Low | Resolved | Documentation & comments | `TwinCATCommandBase.cs:31-36` |
 | 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` |
@@ -4,8 +4,9 @@ Ad-hoc probe / read / write / subscribe tool for ControlLogix / CompactLogix /
 Micro800 / GuardLogix PLCs, talking to the **same** `AbCipDriver` the OtOpcUa
 server uses (libplctag under the hood).
-Second of four driver test-client CLIs (Modbus → AB CIP → AB Legacy → S7 →
+Second of six driver test-client CLIs (Modbus → AB CIP → AB Legacy → S7 →
-TwinCAT). Shares `Driver.Cli.Common` with the others.
+TwinCAT → FOCAS). Shares `Driver.Cli.Common` with the others; see
 [DriverClis.md](DriverClis.md) for the authoritative roster.
 ## Build + run
@@ -95,6 +95,9 @@ PLC-managed — use with caution.
 otopcua-ablegacy-cli subscribe -g ab://192.168.1.20/1,0 -a N7:10 -t Int -i 500
 ```
 `-i` / `--interval-ms` is the publishing interval in milliseconds — default
 `1000`. `PollGroupEngine` floors sub-250 ms values, so `-i 100` runs at 250 ms.
 ## Known caveat — ab_server upstream gap
 The integration-fixture `ab_server` Docker container accepts TCP but its PCCC
@@ -122,6 +122,14 @@ gives plausible values is the correct one for that device.
 ## v2 addressing grammar
 > **CLI scope:** the `read` / `write` / `subscribe` commands accept only
 > the structured `--region` + `--address` + `--type` triple. The
 > address-string grammar below is a **`DriverConfig` JSON** feature
 > consumed by the driver itself; it is not reachable from this CLI's
 > flags. To experiment with it via the CLI, use the structured flags;
 > to deploy spreadsheets as-is, hand-author a `DriverConfig` and run
 > the server.
 The driver accepts the industry-standard tag-address grammar so you can
 paste tag spreadsheets from Wonderware / Kepware / Ignition without
 per-row manual translation. Full reference + grammar rules:
@@ -27,10 +27,28 @@ public abstract class AbCipCommandBase : DriverCommandBase
    public int TimeoutMs { get; init; } = 5000;
    /// <inheritdoc />
    /// <remarks>
    ///     The getter validates <see cref="TimeoutMs"/> (Driver.AbCip.Cli-004) — a zero or
    ///     negative <c>--timeout-ms</c> would otherwise propagate as a non-positive
    ///     <see cref="TimeSpan"/> into the driver. The <c>init</c> accessor is unreachable
    ///     because CliFx binds <see cref="TimeoutMs"/> rather than <c>Timeout</c>; it throws
    ///     <see cref="NotSupportedException"/> so an object-initializer assignment
    ///     (<c>new ReadCommand { Timeout = ... }</c>) fails fast instead of being silently
    ///     discarded (Driver.AbCip.Cli-006).
    /// </remarks>
    public override TimeSpan Timeout
    {
-        get => TimeSpan.FromMilliseconds(TimeoutMs);
+        get
-        init { /* driven by TimeoutMs */ }
+        {
            if (TimeoutMs <= 0)
                throw new CliFx.Exceptions.CommandException(
                    $"--timeout-ms must be > 0 (got {TimeoutMs}). " +
                    "Pick a positive number of milliseconds for the per-operation timeout.");
            return TimeSpan.FromMilliseconds(TimeoutMs);
        }
        init => throw new NotSupportedException(
            $"{nameof(AbCipCommandBase)}.{nameof(Timeout)} is derived from {nameof(TimeoutMs)} " +
            "and cannot be assigned directly. Set TimeoutMs instead.");
    }
    /// <summary>
@@ -54,6 +54,9 @@ public sealed class ProbeCommand : AbCipCommandBase
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
            // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log
            // output emitted during driver shutdown is not lost.
            FlushLogging();
        }
    }
 }
@@ -49,6 +49,9 @@ public sealed class ReadCommand : AbCipCommandBase
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
            // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log
            // output emitted during driver shutdown is not lost.
            FlushLogging();
        }
    }
@@ -31,6 +31,10 @@ public sealed class SubscribeCommand : AbCipCommandBase
    {
        ConfigureLogging();
        RejectStructure(DataType);
        ValidateInterval(IntervalMs);
        // Touch Timeout to surface the --timeout-ms guard (Driver.AbCip.Cli-004) before
        // we open a driver — fast-fail with a clean CommandException on bad operator input.
        _ = Timeout;
        var ct = console.RegisterCancellationHandler();
        var tagName = ReadCommand.SynthesiseTagName(TagPath, DataType);
@@ -48,6 +52,13 @@ public sealed class SubscribeCommand : AbCipCommandBase
        {
            await driver.InitializeAsync("{}", ct);
            // Driver.AbCip.Cli-003 — emit the banner BEFORE wiring OnDataChange so the
            // main-thread write cannot interleave with poll-thread change-event writes.
            // TextWriter.WriteLine is not guaranteed thread-safe; once the handler is
            // attached and SubscribeAsync starts, change events run on the poll thread.
            await console.Output.WriteLineAsync(
                $"Subscribed to {TagPath} @ {IntervalMs}ms. Ctrl+C to stop.");
            driver.OnDataChange += (_, e) =>
            {
                var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
@@ -58,8 +69,6 @@ public sealed class SubscribeCommand : AbCipCommandBase
            handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct);
            await console.Output.WriteLineAsync(
                $"Subscribed to {TagPath} @ {IntervalMs}ms. Ctrl+C to stop.");
            try
            {
                await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, ct);
@@ -77,6 +86,23 @@ public sealed class SubscribeCommand : AbCipCommandBase
                catch { /* teardown best-effort */ }
            }
            await driver.ShutdownAsync(CancellationToken.None);
            // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log
            // lines emitted just before Ctrl+C are not lost on abrupt termination.
            FlushLogging();
        }
    }
    /// <summary>
    ///     Guards <c>--interval-ms</c> against zero or negative values (Driver.AbCip.Cli-004).
    ///     A non-positive interval would produce a non-positive <see cref="TimeSpan"/> into
    ///     <c>SubscribeAsync</c>; the CLI should fail fast with an actionable error rather
    ///     than relying on the downstream <c>PollGroupEngine</c> to clamp the value.
    /// </summary>
    internal static void ValidateInterval(int intervalMs)
    {
        if (intervalMs <= 0)
            throw new CliFx.Exceptions.CommandException(
                $"--interval-ms must be > 0 (got {intervalMs}). " +
                "PollGroupEngine floors sub-250ms values, but accepts any positive integer.");
    }
 }
@@ -60,6 +60,9 @@ public sealed class WriteCommand : AbCipCommandBase
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
            // Driver.AbCip.Cli-005 — flush Serilog before process exit so buffered log
            // output emitted during driver shutdown is not lost.
            FlushLogging();
        }
    }
@@ -17,7 +17,7 @@ public sealed class ProbeCommand : AbLegacyCommandBase
        "the pre-populated register every SLC / MicroLogix / PLC-5 ships with.")]
    public string Address { get; init; } = "N7:0";
-    [CommandOption("type", Description =
+    [CommandOption("type", 't', Description =
        "PCCC data type of the probe address (default Int — matches N files).")]
    public AbLegacyDataType DataType { get; init; } = AbLegacyDataType.Int;
@@ -34,7 +34,10 @@ public sealed class ProbeCommand : AbLegacyCommandBase
            Writable: false);
        var options = BuildOptions([probeTag]);
-        await using var driver = new AbLegacyDriver(options, DriverInstanceId);
+        // Plain `var driver`: explicit ShutdownAsync(CancellationToken.None) in the
        // finally is the deliberate teardown path; combining it with `await using`
        // (which itself calls ShutdownAsync) would tear the driver down twice.
        var driver = new AbLegacyDriver(options, DriverInstanceId);
        try
        {
            await driver.InitializeAsync("{}", ct);
@@ -36,7 +36,10 @@ public sealed class ReadCommand : AbLegacyCommandBase
            Writable: false);
        var options = BuildOptions([tag]);
-        await using var driver = new AbLegacyDriver(options, DriverInstanceId);
+        // Plain `var driver`: explicit ShutdownAsync(CancellationToken.None) in the
        // finally is the deliberate teardown path; combining it with `await using`
        // (which itself calls ShutdownAsync) would tear the driver down twice.
        var driver = new AbLegacyDriver(options, DriverInstanceId);
        try
        {
            await driver.InitializeAsync("{}", ct);
@@ -21,7 +21,8 @@ public sealed class SubscribeCommand : AbLegacyCommandBase
    public AbLegacyDataType DataType { get; init; } = AbLegacyDataType.Int;
    [CommandOption("interval-ms", 'i', Description =
-        "Publishing interval in milliseconds (default 1000).")]
+        "Publishing interval in milliseconds (default 1000). PollGroupEngine floors " +
        "sub-250ms values.")]
    public int IntervalMs { get; init; } = 1000;
    public override async ValueTask ExecuteAsync(IConsole console)
@@ -38,8 +39,17 @@ public sealed class SubscribeCommand : AbLegacyCommandBase
            Writable: false);
        var options = BuildOptions([tag]);
-        await using var driver = new AbLegacyDriver(options, DriverInstanceId);
+        // Plain `var driver` (no `await using`): driver.DisposeAsync internally calls
        // ShutdownAsync, so combining `await using` with an explicit finally-shutdown
        // would tear the driver down twice. The explicit teardown is preferred because
        // it deliberately passes CancellationToken.None — `await using` would otherwise
        // happen on a cancelled `ct` path which can cut teardown short.
        var driver = new AbLegacyDriver(options, DriverInstanceId);
        ISubscriptionHandle? handle = null;
        // Serialise console writes from the poll-thread OnDataChange callback against
        // the command-thread "Subscribed to ..." line and against each other; the
        // PollGroupEngine raises change events on a background timer/loop thread.
        var consoleGate = new object();
        try
        {
            await driver.InitializeAsync("{}", ct);
@@ -49,13 +59,19 @@ public sealed class SubscribeCommand : AbLegacyCommandBase
                var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
                    $"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " +
                    $"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})";
-                console.Output.WriteLine(line);
+                lock (consoleGate)
                {
                    console.Output.WriteLine(line);
                }
            };
            handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct);
-            await console.Output.WriteLineAsync(
+            lock (consoleGate)
-                $"Subscribed to {Address} @ {IntervalMs}ms. Ctrl+C to stop.");
+            {
                console.Output.WriteLine(
                    $"Subscribed to {Address} @ {IntervalMs}ms. Ctrl+C to stop.");
            }
            try
            {
                await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, ct);
@@ -25,7 +25,7 @@ public sealed class WriteCommand : AbLegacyCommandBase
    public AbLegacyDataType DataType { get; init; } = AbLegacyDataType.Int;
    [CommandOption("value", 'v', Description =
-        "Value to write. Parsed per --type (booleans accept true/false/1/0).",
+        "Value to write. Parsed per --type (booleans accept true/false, 1/0, on/off, yes/no).",
        IsRequired = true)]
    public string Value { get; init; } = default!;
@@ -45,7 +45,10 @@ public sealed class WriteCommand : AbLegacyCommandBase
        var parsed = ParseValue(Value, DataType);
-        await using var driver = new AbLegacyDriver(options, DriverInstanceId);
+        // Plain `var driver`: explicit ShutdownAsync(CancellationToken.None) in the
        // finally is the deliberate teardown path; combining it with `await using`
        // (which itself calls ShutdownAsync) would tear the driver down twice.
        var driver = new AbLegacyDriver(options, DriverInstanceId);
        try
        {
            await driver.InitializeAsync("{}", ct);
@@ -1,5 +1,6 @@
 using CliFx.Attributes;
 using CliFx.Infrastructure;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.Cli.Common;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Commands;
@@ -21,6 +22,7 @@ public sealed class ProbeCommand : ModbusCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        ConfigureLogging();
        ValidateEndpoint();
        var ct = console.RegisterCancellationHandler();
        // Build with one probe tag + Probe.Enabled=false so InitializeAsync connects the
@@ -39,17 +41,59 @@ public sealed class ProbeCommand : ModbusCommandBase
            var snapshot = await driver.ReadAsync(["__probe"], ct);
            var health = driver.GetHealth();
            // Driver.Modbus.Cli-006: derive the headline verdict from BOTH the driver state
            // AND the probe-read StatusCode so the operator never sees the previous
            // contradictory pair (`Health: Healthy` over a Bad snapshot line). The bare
            // driver state is still printed below for diagnostics, but the verdict is what
            // the operator scans first.
            var verdict = ComputeVerdict(health.State, snapshot[0].StatusCode);
            await console.Output.WriteLineAsync($"Host:         {Host}:{Port} (unit {UnitId})");
-            await console.Output.WriteLineAsync($"Health:       {health.State}");
+            await console.Output.WriteLineAsync($"Verdict:      {verdict}");
            await console.Output.WriteLineAsync($"Driver state: {health.State}");
            if (health.LastError is { } err)
                await console.Output.WriteLineAsync($"Last error:   {err}");
            await console.Output.WriteLineAsync();
            await console.Output.WriteLineAsync(
                SnapshotFormatter.Format($"HR[{ProbeAddress}]", snapshot[0]));
        }
        catch (OperationCanceledException) when (ct.IsCancellationRequested)
        {
            // Driver.Modbus.Cli-005: Ctrl+C during InitializeAsync — exit quietly so CliFx
            // does not render a full stack trace for a user-initiated cancellation.
            await console.Output.WriteLineAsync("Cancelled.");
        }
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
        }
    }
    /// <summary>
    ///     Driver.Modbus.Cli-006: combine the driver-side <see cref="DriverState"/> with the
    ///     probe snapshot's OPC UA <c>StatusCode</c> into a single headline verdict. Order
    ///     of precedence:
    ///     <list type="number">
    ///       <item><c>FAIL</c> — driver did not reach <see cref="DriverState.Healthy"/>
    ///         (Faulted / Reconnecting / Unknown) OR the snapshot reports Bad quality.</item>
    ///       <item><c>DEGRADED</c> — driver Healthy but the snapshot quality is Uncertain.</item>
    ///       <item><c>OK</c> — driver Healthy and snapshot Good.</item>
    ///     </list>
    /// </summary>
    public static string ComputeVerdict(DriverState state, uint statusCode)
    {
        // OPC UA StatusCode top 2 bits encode the quality class:
        //   0x00xxxxxx → Good, 0x40xxxxxx → Uncertain, 0x80xxxxxx / 0xC0xxxxxx → Bad.
        var qualityClass = statusCode & 0xC0000000u;
        var snapshotGood = qualityClass == 0x00000000u;
        var snapshotUncertain = qualityClass == 0x40000000u;
        if (state != DriverState.Healthy || !snapshotGood && !snapshotUncertain)
            return $"FAIL (driver={state}, probe={SnapshotFormatter.FormatStatus(statusCode)})";
        if (snapshotUncertain)
            return $"DEGRADED (driver={state}, probe={SnapshotFormatter.FormatStatus(statusCode)})";
        return $"OK (driver={state}, probe={SnapshotFormatter.FormatStatus(statusCode)})";
    }
 }
@@ -46,6 +46,7 @@ public sealed class ReadCommand : ModbusCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        ConfigureLogging();
        ValidateEndpoint();
        var ct = console.RegisterCancellationHandler();
        var tagName = SynthesiseTagName(Region, Address, DataType);
@@ -68,6 +69,12 @@ public sealed class ReadCommand : ModbusCommandBase
            var snapshot = await driver.ReadAsync([tagName], ct);
            await console.Output.WriteLineAsync(SnapshotFormatter.Format(tagName, snapshot[0]));
        }
        catch (OperationCanceledException) when (ct.IsCancellationRequested)
        {
            // Driver.Modbus.Cli-005: Ctrl+C during driver connect/read — exit quietly so
            // CliFx does not render a full stack trace for a user-initiated cancellation.
            await console.Output.WriteLineAsync("Cancelled.");
        }
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
@@ -53,6 +53,7 @@ public sealed class SubscribeCommand : ModbusCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        ConfigureLogging();
        ValidateEndpoint();
        var ct = console.RegisterCancellationHandler();
        var tagName = ReadCommand.SynthesiseTagName(Region, Address, DataType);
@@ -69,6 +70,9 @@ public sealed class SubscribeCommand : ModbusCommandBase
        var options = BuildOptions([tag]);
        await using var driver = new ModbusDriver(options, DriverInstanceId);
        // Driver.Modbus.Cli-004: serialize console writes from the PollGroupEngine background
        // thread so overlapping poll ticks can't interleave partial lines.
        var writeLock = new object();
        ISubscriptionHandle? handle = null;
        try
        {
@@ -78,10 +82,26 @@ public sealed class SubscribeCommand : ModbusCommandBase
            // analyzer flags it + IConsole is the testable abstraction).
            driver.OnDataChange += (_, e) =>
            {
-                var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
+                // Driver.Modbus.Cli-004: swallow + log write failures so a transient stdout
-                    $"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " +
+                // error (closed pipe, IO exception on a redirected stream) cannot tear down
-                    $"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})";
+                // the poll-engine background loop. Without this guard the unhandled
-                console.Output.WriteLine(line);
+                // exception would fault the long-running subscribe.
                try
                {
                    var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
                        $"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " +
                        $"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})";
                    lock (writeLock)
                    {
                        console.Output.WriteLine(line);
                    }
                }
                catch (Exception ex)
                {
                    Serilog.Log.Logger.Warning(ex,
                        "SubscribeCommand: console write failed for {Tag}; continuing poll loop.",
                        e.FullReference);
                }
            };
            handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct);
@@ -54,6 +54,7 @@ public sealed class WriteCommand : ModbusCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        ConfigureLogging();
        ValidateEndpoint();
        var ct = console.RegisterCancellationHandler();
        if (Region is not (ModbusRegion.Coils or ModbusRegion.HoldingRegisters))
@@ -92,6 +93,12 @@ public sealed class WriteCommand : ModbusCommandBase
            var results = await driver.WriteAsync([new WriteRequest(tagName, parsed)], ct);
            await console.Output.WriteLineAsync(SnapshotFormatter.FormatWrite(tagName, results[0]));
        }
        catch (OperationCanceledException) when (ct.IsCancellationRequested)
        {
            // Driver.Modbus.Cli-005: Ctrl+C during driver connect/write — exit quietly so
            // CliFx does not render a full stack trace for a user-initiated cancellation.
            await console.Output.WriteLineAsync("Cancelled.");
        }
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
@@ -1,4 +1,5 @@
 using CliFx.Attributes;
 using CliFx.Exceptions;
 using ZB.MOM.WW.OtOpcUa.Driver.Cli.Common;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli;
@@ -57,4 +58,33 @@ public abstract class ModbusCommandBase : DriverCommandBase
    ///     multiple endpoints in parallel can distinguish the logs.
    /// </summary>
    protected string DriverInstanceId => $"modbus-cli-{Host}:{Port}";
    /// <summary>
    ///     Driver.Modbus.Cli-003: validate the endpoint flags at parse time so the operator
    ///     gets a clear CliFx error instead of an opaque socket / argument exception thrown
    ///     deep inside the driver. Ranges:
    ///     <list type="bullet">
    ///       <item><c>--port</c>: 1..65535 (IANA TCP port space, excludes the
    ///         "any" sentinel 0 and rejects negative / overflowed values).</item>
    ///       <item><c>--timeout-ms</c>: strictly positive — a non-positive
    ///         <see cref="TimeSpan"/> would propagate as an immediate-timeout into the
    ///         driver and surface as a confusing TimeoutException.</item>
    ///       <item><c>--unit-id</c>: 1..247 — the Modbus spec unicast unit-id range.
    ///         0 is the broadcast address and not valid for read/write requests; 248-255
    ///         are reserved. (Documented in <c>docs/Driver.Modbus.Cli.md</c>.)</item>
    ///     </list>
    /// </summary>
    protected void ValidateEndpoint()
    {
        if (Port < 1 || Port > 65535)
            throw new CommandException(
                $"--port must be in 1..65535 (got {Port}).");
        if (TimeoutMs <= 0)
            throw new CommandException(
                $"--timeout-ms must be strictly positive (got {TimeoutMs}).");
        if (UnitId < 1 || UnitId > 247)
            throw new CommandException(
                $"--unit-id must be in 1..247 per the Modbus spec (got {UnitId}); " +
                $"0 is the broadcast address, 248-255 are reserved.");
    }
 }
@@ -33,6 +33,9 @@ public sealed class ProbeCommand : S7CommandBase
            Writable: false);
        var options = BuildOptions([probeTag]);
        // Driver.S7.Cli-004: `await using` is the sole disposal mechanism — S7Driver.DisposeAsync
        // already invokes ShutdownAsync, so the previous explicit ShutdownAsync(CancellationToken.None)
        // call in a finally block ran shutdown twice. The await-using on the next line is enough.
        await using var driver = new S7Driver(options, DriverInstanceId);
        // Driver.S7.Cli-003: wrap the entire probe sequence so that a refused/unreachable TCP
        // connect still prints the structured Host/CPU/Health lines instead of crashing with a
@@ -66,9 +69,5 @@ public sealed class ProbeCommand : S7CommandBase
            if (health.LastError is { } err)
                await console.Output.WriteLineAsync($"Last error:   {err}");
        }
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
        }
    }
 }
@@ -45,17 +45,13 @@ public sealed class ReadCommand : S7CommandBase
            StringLength: StringLength);
        var options = BuildOptions([tag]);
        // Driver.S7.Cli-004: `await using` is the sole disposal mechanism — S7Driver.DisposeAsync
        // already invokes ShutdownAsync, so a redundant explicit ShutdownAsync(CancellationToken.None)
        // in a finally block ran shutdown twice. The await-using on the next line is enough.
        await using var driver = new S7Driver(options, DriverInstanceId);
-        try
+        await driver.InitializeAsync("{}", ct);
-        {
+        var snapshot = await driver.ReadAsync([tagName], ct);
-            await driver.InitializeAsync("{}", ct);
+        await console.Output.WriteLineAsync(SnapshotFormatter.Format(Address, snapshot[0]));
            var snapshot = await driver.ReadAsync([tagName], ct);
            await console.Output.WriteLineAsync(SnapshotFormatter.Format(Address, snapshot[0]));
        }
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
        }
    }
    /// <summary>Tag-name key used internally. Address + type is already unique.</summary>
@@ -37,12 +37,20 @@ public sealed class SubscribeCommand : S7CommandBase
            Writable: false);
        var options = BuildOptions([tag]);
        // Driver.S7.Cli-004: `await using` is the sole driver-disposal mechanism — S7Driver.DisposeAsync
        // already invokes ShutdownAsync, so a redundant ShutdownAsync(CancellationToken.None) in finally
        // ran shutdown twice. Only UnsubscribeAsync stays in the finally block — that's a subscription
        // lifecycle concern that is not part of driver disposal.
        await using var driver = new S7Driver(options, DriverInstanceId);
        ISubscriptionHandle? handle = null;
        try
        {
            await driver.InitializeAsync("{}", ct);
            // Driver.S7.Cli-007: route every data-change event to the CliFx console (not
            // System.Console — the analyzer flags it + IConsole is the testable abstraction).
            // The handler is synchronous because OnDataChange is raised from a driver
            // background thread; the IConsole.Output writer is thread-safe for line writes.
            driver.OnDataChange += (_, e) =>
            {
                var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
@@ -71,7 +79,6 @@ public sealed class SubscribeCommand : S7CommandBase
                try { await driver.UnsubscribeAsync(handle, CancellationToken.None); }
                catch { /* teardown best-effort */ }
            }
            await driver.ShutdownAsync(CancellationToken.None);
        }
    }
 }
@@ -52,17 +52,13 @@ public sealed class WriteCommand : S7CommandBase
        var parsed = ParseValue(Value, DataType);
        // Driver.S7.Cli-004: `await using` is the sole disposal mechanism — S7Driver.DisposeAsync
        // already invokes ShutdownAsync, so a redundant explicit ShutdownAsync(CancellationToken.None)
        // in a finally block ran shutdown twice. The await-using on the next line is enough.
        await using var driver = new S7Driver(options, DriverInstanceId);
-        try
+        await driver.InitializeAsync("{}", ct);
-        {
+        var results = await driver.WriteAsync([new WriteRequest(tagName, parsed)], ct);
-            await driver.InitializeAsync("{}", ct);
+        await console.Output.WriteLineAsync(SnapshotFormatter.FormatWrite(Address, results[0]));
            var results = await driver.WriteAsync([new WriteRequest(tagName, parsed)], ct);
            await console.Output.WriteLineAsync(SnapshotFormatter.FormatWrite(Address, results[0]));
        }
        finally
        {
            await driver.ShutdownAsync(CancellationToken.None);
        }
    }
    /// <summary>Parse <c>--value</c> per <see cref="S7DataType"/>, invariant culture throughout.</summary>
@@ -10,6 +10,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 ///     when <c>EnableControllerBrowse = true</c> — structured UDTs / function-block instances
 ///     won't appear because the driver filters to the supported primitive surface.
 /// </summary>
 /// <remarks>
 ///     Inherits from <see cref="TwinCATCommandBase"/> rather than
 ///     <see cref="TwinCATTagCommandBase"/> so the <c>--poll-only</c> flag does NOT surface in
 ///     <c>browse --help</c>: browse never subscribes, the flag would be a no-op, and the help
 ///     text would mislead users (Driver.TwinCAT.Cli-004).
 /// </remarks>
 [Command("browse", Description = "Enumerate controller symbols via the driver's DiscoverAsync walk.")]
 public sealed class BrowseCommand : TwinCATCommandBase
 {
@@ -25,18 +31,21 @@ public sealed class BrowseCommand : TwinCATCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        Validate();
        ConfigureLogging();
        var ct = console.RegisterCancellationHandler();
        // Browse-only — no declared tags. EnableControllerBrowse=true flips DiscoverAsync's
-        // symbol-walk on so every recognized primitive surfaces through the builder.
+        // symbol-walk on so every recognized primitive surfaces through the builder. Native
        // ADS notifications are irrelevant here (DiscoverAsync never subscribes); leave the
        // default on so the options record matches the production wiring.
        var options = new TwinCATDriverOptions
        {
            Devices = [new TwinCATDeviceOptions(Gateway, $"cli-{AmsNetId}:{AmsPort}")],
            Tags = [],
            Timeout = Timeout,
            Probe = new TwinCATProbeOptions { Enabled = false },
-            UseNativeNotifications = !PollOnly,
+            UseNativeNotifications = true,
            EnableControllerBrowse = true,
        };
@@ -52,10 +61,8 @@ public sealed class BrowseCommand : TwinCATCommandBase
            await driver.ShutdownAsync(CancellationToken.None);
        }
-        var matched = builder.Variables
+        var matched = FilterByPrefix(builder.Variables, Prefix);
-            .Where(v => string.IsNullOrEmpty(Prefix) || v.BrowseName.StartsWith(Prefix, StringComparison.Ordinal))
+        var printLimit = PrintLimit(matched.Count, Max);
            .ToList();
        var printLimit = Max <= 0 ? matched.Count : Math.Min(Max, matched.Count);
        await console.Output.WriteLineAsync($"AMS:          {AmsNetId}:{AmsPort}");
        await console.Output.WriteLineAsync(
@@ -64,8 +71,7 @@ public sealed class BrowseCommand : TwinCATCommandBase
        foreach (var v in matched.Take(printLimit))
        {
-            var access = v.Info.SecurityClass == SecurityClassification.ViewOnly ? "RO" : "RW";
+            await console.Output.WriteLineAsync($"  [{AccessTag(v.Info)}] {v.Info.DriverDataType,-8} {v.BrowseName}");
            await console.Output.WriteLineAsync($"  [{access}] {v.Info.DriverDataType,-8} {v.BrowseName}");
        }
        if (matched.Count > printLimit)
@@ -73,7 +79,35 @@ public sealed class BrowseCommand : TwinCATCommandBase
                $"  … {matched.Count - printLimit} more — raise --max or tighten --prefix");
    }
-    private sealed class CollectingAddressSpaceBuilder : IAddressSpaceBuilder
+    /// <summary>
    ///     Case-sensitive prefix filter. A null/empty prefix keeps everything; otherwise we
    ///     keep symbols whose browse name starts with <paramref name="prefix"/> under
    ///     <see cref="StringComparison.Ordinal"/> — TwinCAT identifiers are case-sensitive on
    ///     the wire, so a relaxed match would be misleading.
    /// </summary>
    internal static List<(string BrowseName, DriverAttributeInfo Info)> FilterByPrefix(
        IReadOnlyList<(string BrowseName, DriverAttributeInfo Info)> source, string? prefix)
        => source
            .Where(v => string.IsNullOrEmpty(prefix) || v.BrowseName.StartsWith(prefix, StringComparison.Ordinal))
            .ToList();
    /// <summary>
    ///     Cap-to-max projection. <paramref name="max"/> &lt;= 0 means unbounded, otherwise the
    ///     min of <paramref name="matchedCount"/> and <paramref name="max"/>.
    /// </summary>
    internal static int PrintLimit(int matchedCount, int max)
        => max <= 0 ? matchedCount : Math.Min(max, matchedCount);
    /// <summary>
    ///     Coarse RO/RW label used in the browse output. <see cref="SecurityClassification.ViewOnly"/>
    ///     is the only classification that is unconditionally read-only; everything else can be
    ///     written from at least one ACL tier, so the CLI labels it RW. The real per-tier
    ///     authorization is enforced server-side.
    /// </summary>
    internal static string AccessTag(DriverAttributeInfo info)
        => info.SecurityClass == SecurityClassification.ViewOnly ? "RO" : "RW";
    internal sealed class CollectingAddressSpaceBuilder : IAddressSpaceBuilder
    {
        public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = [];
@@ -11,7 +11,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 ///     server near the endpoint.
 /// </summary>
 [Command("probe", Description = "Verify the TwinCAT runtime is reachable and a sample symbol reads.")]
-public sealed class ProbeCommand : TwinCATCommandBase
+public sealed class ProbeCommand : TwinCATTagCommandBase
 {
    [CommandOption("symbol", 's', Description =
        "Symbol path to probe. System-global examples: " +
@@ -20,11 +20,14 @@ public sealed class ProbeCommand : TwinCATCommandBase
        IsRequired = true)]
    public string SymbolPath { get; init; } = default!;
-    [CommandOption("type", Description = "Data type (default DInt — TwinCAT DINT maps to int32).")]
+    [CommandOption("type", 't', Description =
        "Bool / SInt / USInt / Int / UInt / DInt / UDInt / LInt / ULInt / Real / LReal / " +
        "String / WString / Time / Date / DateTime / TimeOfDay (default DInt).")]
    public TwinCATDataType DataType { get; init; } = TwinCATDataType.DInt;
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        Validate();
        ConfigureLogging();
        var ct = console.RegisterCancellationHandler();
@@ -9,7 +9,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 ///     member list into individual reads if you need them.
 /// </summary>
 [Command("read", Description = "Read a single TwinCAT symbol.")]
-public sealed class ReadCommand : TwinCATCommandBase
+public sealed class ReadCommand : TwinCATTagCommandBase
 {
    [CommandOption("symbol", 's', Description =
        "Symbol path. Program scope: 'MAIN.bStart'. Global: 'GVL.Counter'. " +
@@ -24,6 +24,7 @@ public sealed class ReadCommand : TwinCATCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        Validate();
        ConfigureLogging();
        var ct = console.RegisterCancellationHandler();
@@ -1,4 +1,5 @@
 using CliFx.Attributes;
 using CliFx.Exceptions;
 using CliFx.Infrastructure;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.Cli.Common;
@@ -10,7 +11,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 ///     pushes on its own cycle); pass <c>--poll-only</c> to fall through to PollGroupEngine.
 /// </summary>
 [Command("subscribe", Description = "Watch a TwinCAT symbol via ADS notification or poll, until Ctrl+C.")]
-public sealed class SubscribeCommand : TwinCATCommandBase
+public sealed class SubscribeCommand : TwinCATTagCommandBase
 {
    [CommandOption("symbol", 's', Description = "Symbol path — same format as `read`.", IsRequired = true)]
    public string SymbolPath { get; init; } = default!;
@@ -23,8 +24,17 @@ public sealed class SubscribeCommand : TwinCATCommandBase
    [CommandOption("interval-ms", 'i', Description = "Publishing interval ms (default 1000).")]
    public int IntervalMs { get; init; } = 1000;
    protected override void Validate()
    {
        base.Validate();
        if (IntervalMs <= 0)
            throw new CommandException(
                $"--interval-ms must be greater than 0 (got {IntervalMs}).");
    }
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        Validate();
        ConfigureLogging();
        var ct = console.RegisterCancellationHandler();
@@ -43,19 +53,39 @@ public sealed class SubscribeCommand : TwinCATCommandBase
        {
            await driver.InitializeAsync("{}", ct);
            // Native ADS notifications fire OnDataChange from the Beckhoff.TwinCAT.Ads
            // notification callback thread — unlike the poll-mode path (which serialises on a
            // single PollGroupEngine loop), the native callback can interleave with the banner
            // write below and with subsequent change events if the PLC pushes faster than a
            // single console write completes. A TextWriter is not guaranteed thread-safe, so
            // we serialise every write through a lock to keep output clean for
            // screen-recorded bug-report timelines (Driver.TwinCAT.Cli-002).
            var writeLock = new object();
            driver.OnDataChange += (_, e) =>
            {
                var line = $"[{DateTime.UtcNow:HH:mm:ss.fff}] " +
                    $"{e.FullReference} = {SnapshotFormatter.FormatValue(e.Snapshot.Value)} " +
                    $"({SnapshotFormatter.FormatStatus(e.Snapshot.StatusCode)})";
-                console.Output.WriteLine(line);
+                lock (writeLock)
                {
                    console.Output.WriteLine(line);
                }
            };
            handle = await driver.SubscribeAsync([tagName], TimeSpan.FromMilliseconds(IntervalMs), ct);
-            var mode = PollOnly ? "polling" : "ADS notification";
+            // Driver.TwinCAT.Cli-003: derive the banner mechanism from the actual subscription
-            await console.Output.WriteLineAsync(
+            // handle the driver returned, not from --poll-only. The native ADS path tags its
-                $"Subscribed to {SymbolPath} @ {IntervalMs}ms ({mode}). Ctrl+C to stop.");
+            // handle with a "twincat-native-sub-*" DiagnosticId; anything else means we landed
            // on the shared PollGroupEngine. That way the line cannot disagree with what the
            // driver actually did (e.g. a future fallback inside SubscribeAsync).
            var mode = DescribeMechanism(handle);
            lock (writeLock)
            {
                console.Output.WriteLine(
                    $"Subscribed to {SymbolPath} @ {IntervalMs}ms ({mode}). Ctrl+C to stop.");
            }
            try
            {
                await Task.Delay(System.Threading.Timeout.InfiniteTimeSpan, ct);
@@ -75,4 +105,16 @@ public sealed class SubscribeCommand : TwinCATCommandBase
            await driver.ShutdownAsync(CancellationToken.None);
        }
    }
    /// <summary>
    ///     Maps the returned subscription handle's <see cref="ISubscriptionHandle.DiagnosticId"/>
    ///     to the banner label. The TwinCAT driver tags native ADS subscriptions with
    ///     <c>twincat-native-sub-*</c> and the shared <c>PollGroupEngine</c> handle uses a
    ///     different format — anything else means we landed on the poll loop. Internal so the
    ///     test assembly can cover the mapping without spinning a real driver.
    /// </summary>
    internal static string DescribeMechanism(ISubscriptionHandle handle) =>
        handle.DiagnosticId.StartsWith("twincat-native-sub-", StringComparison.Ordinal)
            ? "ADS notification"
            : "polling";
 }
@@ -11,7 +11,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 ///     JSON for those.
 /// </summary>
 [Command("write", Description = "Write a single TwinCAT symbol.")]
-public sealed class WriteCommand : TwinCATCommandBase
+public sealed class WriteCommand : TwinCATTagCommandBase
 {
    [CommandOption("symbol", 's', Description =
        "Symbol path — same format as `read`.", IsRequired = true)]
@@ -29,6 +29,7 @@ public sealed class WriteCommand : TwinCATCommandBase
    public override async ValueTask ExecuteAsync(IConsole console)
    {
        Validate();
        ConfigureLogging();
        var ct = console.RegisterCancellationHandler();
@@ -1,13 +1,15 @@
 using CliFx.Attributes;
 using CliFx.Exceptions;
 using ZB.MOM.WW.OtOpcUa.Driver.Cli.Common;
 namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli;
 /// <summary>
 ///     Base for every TwinCAT CLI command. Carries the AMS target options
-///     (<c>--ams-net-id</c> + <c>--ams-port</c>) + the notification-mode toggle that the
+///     (<c>--ams-net-id</c> + <c>--ams-port</c>) + the per-call timeout. Commands that build
-///     driver itself takes. Exposes <see cref="BuildOptions"/> so each command can build a
+///     a single-device / single-tag <see cref="TwinCATDriverOptions"/> from flag input inherit
-///     single-device / single-tag <see cref="TwinCATDriverOptions"/> from flag input.
+///     from <see cref="TwinCATTagCommandBase"/> instead — that intermediate adds the
 ///     <c>--poll-only</c> flag and the <c>BuildOptions</c> helper.
 /// </summary>
 public abstract class TwinCATCommandBase : DriverCommandBase
 {
@@ -23,16 +25,19 @@ public abstract class TwinCATCommandBase : DriverCommandBase
    [CommandOption("timeout-ms", Description = "Per-operation timeout in ms (default 5000).")]
    public int TimeoutMs { get; init; } = 5000;
-    [CommandOption("poll-only", Description =
+    /// <summary>
-        "Disable native ADS notifications and fall through to the shared PollGroupEngine " +
+    ///     The per-operation timeout, projected from <see cref="TimeoutMs"/>. The CliFx
-        "(same as setting UseNativeNotifications=false in a real driver config).")]
+    ///     <c>init</c> accessor required by the abstract base property is intentionally a
-    public bool PollOnly { get; init; }
+    ///     no-op: <see cref="TimeoutMs"/> is the only source of truth, so any value an
-
+    ///     `init` initialiser supplies to <see cref="Timeout"/> directly is silently
-    /// <inheritdoc />
+    ///     dropped. Do NOT add a backing field "fixing" the empty body — it would diverge
    ///     from <see cref="TimeoutMs"/> and the two would drift on every refactor
    ///     (Driver.TwinCAT.Cli-007).
    /// </summary>
    public override TimeSpan Timeout
    {
        get => TimeSpan.FromMilliseconds(TimeoutMs);
-        init { /* driven by TimeoutMs */ }
+        init { /* see XML summary — driven by TimeoutMs */ }
    }
    /// <summary>
@@ -41,22 +46,29 @@ public abstract class TwinCATCommandBase : DriverCommandBase
    /// </summary>
    protected string Gateway => $"ads://{AmsNetId}:{AmsPort}";
    /// <summary>
    ///     Build a <see cref="TwinCATDriverOptions"/> with the AMS target this base collected +
    ///     the tag list a subclass supplies. Probe disabled, controller-browse disabled,
    ///     native notifications toggled by <see cref="PollOnly"/>.
    /// </summary>
    protected TwinCATDriverOptions BuildOptions(IReadOnlyList<TwinCATTagDefinition> tags) => new()
    {
        Devices = [new TwinCATDeviceOptions(
            HostAddress: Gateway,
            DeviceName: $"cli-{AmsNetId}:{AmsPort}")],
        Tags = tags,
        Timeout = Timeout,
        Probe = new TwinCATProbeOptions { Enabled = false },
        UseNativeNotifications = !PollOnly,
        EnableControllerBrowse = false,
    };
    protected string DriverInstanceId => $"twincat-cli-{AmsNetId}:{AmsPort}";
    /// <summary>
    ///     Validates the numeric options every TwinCAT CLI command shares (timeout + AMS port).
    ///     Subclasses override and call <c>base.Validate()</c> first to add their own range
    ///     checks. Throwing here surfaces a clean CliFx one-line error before the driver gets
    ///     a chance to fail with an opaque transport error (Driver.TwinCAT.Cli-001).
    /// </summary>
    protected virtual void Validate()
    {
        if (TimeoutMs <= 0)
            throw new CommandException(
                $"--timeout-ms must be greater than 0 (got {TimeoutMs}).");
        if (AmsPort is <= 0 or > 65535)
            throw new CommandException(
                $"--ams-port must be in the range 1..65535 (got {AmsPort}).");
    }
    // ---- Test hooks ----
    // Protected members are exposed to the test assembly through these internal accessors so the
    // test project can cover Gateway / DriverInstanceId composition + range validation without
    // needing reflection on every assertion (Driver.TwinCAT.Cli-006).
    internal string GatewayForTest => Gateway;
    internal string DriverInstanceIdForTest => DriverInstanceId;
    internal void ValidateForTest() => Validate();
 }
@@ -0,0 +1,40 @@
 using CliFx.Attributes;
 namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli;
 /// <summary>
 ///     Intermediate base for the four TwinCAT CLI commands that build a single-device /
 ///     single-tag <see cref="TwinCATDriverOptions"/> — <c>probe</c>, <c>read</c>, <c>write</c>,
 ///     <c>subscribe</c>. Adds the <c>--poll-only</c> flag (relevant only when the driver is
 ///     about to register native ADS notifications, which is why it does NOT live on the
 ///     <c>browse</c> command — Driver.TwinCAT.Cli-004) and the <c>BuildOptions</c> helper that
 ///     assembles the driver-side options record.
 /// </summary>
 public abstract class TwinCATTagCommandBase : TwinCATCommandBase
 {
    [CommandOption("poll-only", Description =
        "Disable native ADS notifications and fall through to the shared PollGroupEngine " +
        "(same as setting UseNativeNotifications=false in a real driver config).")]
    public bool PollOnly { get; init; }
    /// <summary>
    ///     Build a <see cref="TwinCATDriverOptions"/> with the AMS target this base collected +
    ///     the tag list a subclass supplies. Probe disabled, controller-browse disabled,
    ///     native notifications toggled by <see cref="PollOnly"/>.
    /// </summary>
    protected TwinCATDriverOptions BuildOptions(IReadOnlyList<TwinCATTagDefinition> tags) => new()
    {
        Devices = [new TwinCATDeviceOptions(
            HostAddress: Gateway,
            DeviceName: $"cli-{AmsNetId}:{AmsPort}")],
        Tags = tags,
        Timeout = Timeout,
        Probe = new TwinCATProbeOptions { Enabled = false },
        UseNativeNotifications = !PollOnly,
        EnableControllerBrowse = false,
    };
    // ---- Test hook ----
    internal TwinCATDriverOptions BuildOptionsForTest(IReadOnlyList<TwinCATTagDefinition> tags)
        => BuildOptions(tags);
 }
@@ -0,0 +1,210 @@
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests;
 /// <summary>
 ///     Covers <see cref="AbCipCommandBase"/>: the shared <c>BuildOptions</c> projection
 ///     (driver-options mapping the four commands depend on), the <c>RejectStructure</c>
 ///     guard, the <c>Timeout</c> override behaviour, and <c>TimeoutMs</c> validation.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class AbCipCommandBaseTests
 {
    /// <summary>
    ///     Local subclass that surfaces the protected helpers + properties under test.
    /// </summary>
    [CliFx.Attributes.Command("test")]
    private sealed class TestableCommand : AbCipCommandBase
    {
        public AbCipDriverOptions InvokeBuildOptions(IReadOnlyList<AbCipTagDefinition> tags)
            => BuildOptions(tags);
        public string InvokeDriverInstanceId => DriverInstanceId;
        public override ValueTask ExecuteAsync(CliFx.Infrastructure.IConsole console)
            => ValueTask.CompletedTask;
    }
    private static AbCipTagDefinition SampleTag(string name = "Motor01") => new(
        Name: name,
        DeviceHostAddress: "ab://10.0.0.5/1,0",
        TagPath: "Motor01",
        DataType: AbCipDataType.DInt,
        Writable: false);
    [Fact]
    public void BuildOptions_disables_probe_so_cli_does_not_race_operator_reads()
    {
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.ControlLogix,
            TimeoutMs = 5000,
        };
        var options = cmd.InvokeBuildOptions([SampleTag()]);
        options.Probe.Enabled.ShouldBeFalse();
    }
    [Fact]
    public void BuildOptions_disables_controller_browse()
    {
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.ControlLogix,
            TimeoutMs = 5000,
        };
        var options = cmd.InvokeBuildOptions([SampleTag()]);
        options.EnableControllerBrowse.ShouldBeFalse();
    }
    [Fact]
    public void BuildOptions_disables_alarm_projection()
    {
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.ControlLogix,
            TimeoutMs = 5000,
        };
        var options = cmd.InvokeBuildOptions([SampleTag()]);
        options.EnableAlarmProjection.ShouldBeFalse();
    }
    [Fact]
    public void BuildOptions_produces_one_device_with_gateway_family_and_derived_name()
    {
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.CompactLogix,
            TimeoutMs = 5000,
        };
        var options = cmd.InvokeBuildOptions([SampleTag()]);
        options.Devices.Count.ShouldBe(1);
        var device = options.Devices[0];
        device.HostAddress.ShouldBe("ab://10.0.0.5/1,0");
        device.PlcFamily.ShouldBe(AbCipPlcFamily.CompactLogix);
        device.DeviceName.ShouldBe("cli-CompactLogix");
    }
    [Fact]
    public void BuildOptions_passes_supplied_tag_list_verbatim()
    {
        var tags = new[] { SampleTag("t1"), SampleTag("t2") };
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.ControlLogix,
            TimeoutMs = 5000,
        };
        var options = cmd.InvokeBuildOptions(tags);
        options.Tags.Count.ShouldBe(2);
        options.Tags[0].Name.ShouldBe("t1");
        options.Tags[1].Name.ShouldBe("t2");
    }
    [Fact]
    public void BuildOptions_carries_TimeoutMs_through_to_Timeout()
    {
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.ControlLogix,
            TimeoutMs = 7500,
        };
        var options = cmd.InvokeBuildOptions([SampleTag()]);
        options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7500));
    }
    [Fact]
    public void DriverInstanceId_embeds_gateway_for_log_disambiguation()
    {
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Family = AbCipPlcFamily.ControlLogix,
        };
        cmd.InvokeDriverInstanceId.ShouldBe("abcip-cli-ab://10.0.0.5/1,0");
    }
    [Fact]
    public void Timeout_setter_is_inert_and_does_not_silently_swallow_assignments()
    {
        // Driver.AbCip.Cli-006 — the empty init body would silently discard an
        // object-initializer assignment, hiding a "driven by TimeoutMs" misuse. The fix
        // makes it fail-fast with NotSupportedException so the contract is explicit.
        Should.Throw<NotSupportedException>(() => new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            Timeout = TimeSpan.FromSeconds(99),
        });
    }
    [Theory]
    [InlineData(0)]
    [InlineData(-1)]
    public void Timeout_get_throws_CommandException_when_TimeoutMs_is_non_positive(int badMs)
    {
        // Driver.AbCip.Cli-004 — TimeoutMs must be > 0. Validation is exposed via the
        // Timeout getter so any command path that touches Timeout sees the same guard.
        var cmd = new TestableCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            TimeoutMs = badMs,
        };
        var ex = Should.Throw<CliFx.Exceptions.CommandException>(() => _ = cmd.Timeout);
        ex.Message.ShouldContain("--timeout-ms");
    }
    [Fact]
    public void RejectStructure_throws_for_Structure_DataType()
    {
        var ex = Should.Throw<CliFx.Exceptions.CommandException>(
            () => CallRejectStructure(AbCipDataType.Structure));
        ex.Message.ShouldContain("Structure");
    }
    [Theory]
    [InlineData(AbCipDataType.DInt)]
    [InlineData(AbCipDataType.Bool)]
    [InlineData(AbCipDataType.Real)]
    public void RejectStructure_passes_for_atomic_types(AbCipDataType type)
    {
        // No throw — atomic types are allowed.
        Should.NotThrow(() => CallRejectStructure(type));
    }
    // The static helper is protected; reflect to it once so the test stays at AbCipCommandBase.
    private static void CallRejectStructure(AbCipDataType type)
    {
        var method = typeof(AbCipCommandBase).GetMethod(
            "RejectStructure",
            System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static)
            ?? throw new InvalidOperationException("RejectStructure not found");
        try
        {
            method.Invoke(null, [type]);
        }
        catch (System.Reflection.TargetInvocationException tie) when (tie.InnerException is not null)
        {
            throw tie.InnerException;
        }
    }
 }
@@ -0,0 +1,34 @@
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli.Tests;
 /// <summary>
 ///     Covers <see cref="SubscribeCommand.ValidateInterval(int)"/> — the guard that
 ///     stops a zero / negative <c>--interval-ms</c> from reaching <c>SubscribeAsync</c>
 ///     as a non-positive <see cref="TimeSpan"/>.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class SubscribeCommandIntervalTests
 {
    [Theory]
    [InlineData(0)]
    [InlineData(-1)]
    [InlineData(-500)]
    public void ValidateInterval_rejects_non_positive(int badMs)
    {
        var ex = Should.Throw<CliFx.Exceptions.CommandException>(
            () => SubscribeCommand.ValidateInterval(badMs));
        ex.Message.ShouldContain("--interval-ms");
    }
    [Theory]
    [InlineData(1)]
    [InlineData(250)]
    [InlineData(60_000)]
    public void ValidateInterval_accepts_positive(int goodMs)
    {
        Should.NotThrow(() => SubscribeCommand.ValidateInterval(goodMs));
    }
 }
@@ -0,0 +1,134 @@
 using System;
 using System.Collections.Generic;
 using CliFx.Attributes;
 using CliFx.Infrastructure;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy;
 using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies;
 namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests;
 /// <summary>
 ///     Locks in <see cref="AbLegacyCommandBase.BuildOptions"/>: probe disabled,
 ///     device shape populated from <c>--gateway</c> + <c>--plc-type</c>, tag list
 ///     forwarded verbatim, and timeout propagated from <c>--timeout-ms</c>. A
 ///     regression here silently changes every AbLegacy CLI command's behaviour, so
 ///     covering it explicitly closes the test gap called out by finding
 ///     Driver.AbLegacy.Cli-007.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class BuildOptionsTests
 {
    // Concrete subclass needed because AbLegacyCommandBase is abstract. Exposes the
    // protected BuildOptions via a public surface for the test.
    // [Command] satisfies CliFx's analyzer (ICommand subtypes must be annotated);
    // we never run it through CliFx, only invoke Build() directly.
    [Command("test-build-options")]
    private sealed class TestCommand : AbLegacyCommandBase
    {
        public AbLegacyDriverOptions Build(IReadOnlyList<AbLegacyTagDefinition> tags)
            => BuildOptions(tags);
        public override System.Threading.Tasks.ValueTask ExecuteAsync(IConsole console)
            => throw new NotSupportedException("TestCommand is for BuildOptions inspection only.");
    }
    private static readonly IReadOnlyList<AbLegacyTagDefinition> SampleTags =
    [
        new AbLegacyTagDefinition(
            Name: "N7:0:Int",
            DeviceHostAddress: "ab://192.168.1.20/1,0",
            Address: "N7:0",
            DataType: AbLegacyDataType.Int,
            Writable: false),
        new AbLegacyTagDefinition(
            Name: "F8:0:Float",
            DeviceHostAddress: "ab://192.168.1.20/1,0",
            Address: "F8:0",
            DataType: AbLegacyDataType.Float,
            Writable: true),
    ];
    [Fact]
    public void BuildOptions_disables_probe_for_cli_oneshot_runs()
    {
        var cmd = new TestCommand
        {
            Gateway = "ab://192.168.1.20/1,0",
            PlcType = AbLegacyPlcFamily.Slc500,
            TimeoutMs = 5000,
        };
        var options = cmd.Build(SampleTags);
        options.Probe.ShouldNotBeNull();
        options.Probe.Enabled.ShouldBeFalse(
            "CLI commands are one-shot; the background probe loop is unwanted overhead.");
    }
    [Fact]
    public void BuildOptions_populates_single_device_from_gateway_and_plc_type()
    {
        var cmd = new TestCommand
        {
            Gateway = "ab://10.0.0.5/1,0",
            PlcType = AbLegacyPlcFamily.MicroLogix,
            TimeoutMs = 5000,
        };
        var options = cmd.Build(SampleTags);
        options.Devices.Count.ShouldBe(1);
        options.Devices[0].HostAddress.ShouldBe("ab://10.0.0.5/1,0");
        options.Devices[0].PlcFamily.ShouldBe(AbLegacyPlcFamily.MicroLogix);
        options.Devices[0].DeviceName.ShouldBe("cli-MicroLogix");
    }
    [Fact]
    public void BuildOptions_forwards_tag_list_verbatim()
    {
        var cmd = new TestCommand
        {
            Gateway = "ab://192.168.1.20/1,0",
            PlcType = AbLegacyPlcFamily.Slc500,
            TimeoutMs = 5000,
        };
        var options = cmd.Build(SampleTags);
        options.Tags.ShouldBe(SampleTags);
    }
    [Fact]
    public void BuildOptions_propagates_timeout_ms()
    {
        var cmd = new TestCommand
        {
            Gateway = "ab://192.168.1.20/1,0",
            PlcType = AbLegacyPlcFamily.Slc500,
            TimeoutMs = 7500,
        };
        var options = cmd.Build(SampleTags);
        options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7500));
    }
    [Fact]
    public void BuildOptions_with_empty_tag_list_yields_empty_tags_collection()
    {
        var cmd = new TestCommand
        {
            Gateway = "ab://192.168.1.20/1,0",
            PlcType = AbLegacyPlcFamily.Plc5,
            TimeoutMs = 5000,
        };
        var options = cmd.Build([]);
        options.Tags.ShouldBeEmpty();
        options.Devices.Count.ShouldBe(1);
        options.Devices[0].PlcFamily.ShouldBe(AbLegacyPlcFamily.Plc5);
    }
 }
@@ -0,0 +1,80 @@
 using System.Linq;
 using System.Reflection;
 using CliFx.Attributes;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Cli.Tests;
 /// <summary>
 ///     Locks in the CLI command-option contract surface area — short aliases and
 ///     help-text wording — that the AbLegacy CLI is expected to keep in parity with
 ///     its sibling AbCip CLI and with <c>docs/Driver.AbLegacy.Cli.md</c>.
 ///     Regression coverage for findings Driver.AbLegacy.Cli-002, -005, -006.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class CommandMetadataTests
 {
    private static CommandOptionAttribute GetOption<TCommand>(string propertyName)
    {
        var prop = typeof(TCommand).GetProperty(
            propertyName,
            BindingFlags.Public | BindingFlags.Instance);
        prop.ShouldNotBeNull($"property {propertyName} is missing from {typeof(TCommand).Name}");
        var attr = prop!.GetCustomAttribute<CommandOptionAttribute>();
        attr.ShouldNotBeNull(
            $"property {propertyName} on {typeof(TCommand).Name} lacks [CommandOption]");
        return attr!;
    }
    // ---------- Driver.AbLegacy.Cli-006 — ProbeCommand --type needs short alias 't' ----------
    [Fact]
    public void ProbeCommand_type_has_short_alias_t()
    {
        // Parity with read / write / subscribe: --type / -t works everywhere.
        var attr = GetOption<ProbeCommand>(nameof(ProbeCommand.DataType));
        attr.ShortName.ShouldBe('t');
    }
    [Theory]
    [InlineData(typeof(ReadCommand), nameof(ReadCommand.DataType))]
    [InlineData(typeof(WriteCommand), nameof(WriteCommand.DataType))]
    [InlineData(typeof(SubscribeCommand), nameof(SubscribeCommand.DataType))]
    public void Other_commands_keep_type_short_alias_t(System.Type commandType, string propName)
    {
        var prop = commandType.GetProperty(propName, BindingFlags.Public | BindingFlags.Instance);
        prop.ShouldNotBeNull();
        var attr = prop!.GetCustomAttribute<CommandOptionAttribute>();
        attr.ShouldNotBeNull();
        attr!.ShortName.ShouldBe('t');
    }
    // ---------- Driver.AbLegacy.Cli-002 — WriteCommand --value help lists full bool alias set ----------
    [Fact]
    public void WriteCommand_value_help_lists_full_boolean_alias_set()
    {
        // ParseBool accepts true/false, 1/0, on/off, yes/no — the help text must say so
        // (DriverClis.md documents the full alias set as the shared CLI contract).
        var attr = GetOption<WriteCommand>(nameof(WriteCommand.Value));
        attr.Description.ShouldNotBeNull();
        attr.Description!.ShouldContain("true/false", Case.Insensitive);
        attr.Description!.ShouldContain("1/0");
        attr.Description!.ShouldContain("on/off", Case.Insensitive);
        attr.Description!.ShouldContain("yes/no", Case.Insensitive);
    }
    // ---------- Driver.AbLegacy.Cli-005 — SubscribeCommand --interval-ms help notes 250ms floor ----------
    [Fact]
    public void SubscribeCommand_interval_ms_help_notes_PollGroupEngine_floor()
    {
        // Parity with AbCip CLI: operators passing -i 100 deserve a heads-up that
        // PollGroupEngine floors sub-250ms values.
        var attr = GetOption<SubscribeCommand>(nameof(SubscribeCommand.IntervalMs));
        attr.Description.ShouldNotBeNull();
        attr.Description!.ShouldContain("250", Case.Insensitive);
    }
 }
@@ -0,0 +1,66 @@
 using CliFx.Infrastructure;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests;
 /// <summary>
 ///     Covers Driver.Modbus.Cli-005: <c>probe</c> / <c>read</c> / <c>write</c> must swallow
 ///     <see cref="OperationCanceledException"/> so a Ctrl+C during InitializeAsync exits
 ///     cleanly instead of dumping a full stack trace through CliFx. <c>SubscribeCommand</c>
 ///     already handles this around its <c>Task.Delay</c>; these tests pin the same behaviour
 ///     to the connect/read/write commands.
 ///     The test pre-cancels the CliFx <see cref="FakeInMemoryConsole"/>; the driver's
 ///     <c>ConnectAsync</c> observes the token via <c>Dns.GetHostAddressesAsync</c> and throws
 ///     OCE before any socket I/O happens, so the test is hermetic — no real PLC needed.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class CommandCancellationTests
 {
    [Fact]
    public async Task ProbeCommand_swallows_cancellation_during_initialize()
    {
        using var console = new FakeInMemoryConsole();
        console.RequestCancellation(); // simulate Ctrl+C before ExecuteAsync runs
        var sut = new ProbeCommand { Host = "127.0.0.1" };
        await Should.NotThrowAsync(async () => await sut.ExecuteAsync(console));
    }
    [Fact]
    public async Task ReadCommand_swallows_cancellation_during_initialize()
    {
        using var console = new FakeInMemoryConsole();
        console.RequestCancellation();
        var sut = new ReadCommand
        {
            Host = "127.0.0.1",
            Region = ModbusRegion.HoldingRegisters,
            Address = 0,
            DataType = ModbusDataType.UInt16,
        };
        await Should.NotThrowAsync(async () => await sut.ExecuteAsync(console));
    }
    [Fact]
    public async Task WriteCommand_swallows_cancellation_during_initialize()
    {
        using var console = new FakeInMemoryConsole();
        console.RequestCancellation();
        var sut = new WriteCommand
        {
            Host = "127.0.0.1",
            Region = ModbusRegion.HoldingRegisters,
            Address = 0,
            DataType = ModbusDataType.UInt16,
            Value = "42",
        };
        await Should.NotThrowAsync(async () => await sut.ExecuteAsync(console));
    }
 }
@@ -0,0 +1,164 @@
 using CliFx.Attributes;
 using CliFx.Infrastructure;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Cli.Common;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests;
 /// <summary>
 ///     Covers <see cref="ModbusCommandBase.BuildOptions"/> — the pure, deterministic mapping
 ///     from the base's host / port / unit-id / timeout / disable-reconnect flags onto a
 ///     <c>ModbusDriverOptions</c>. The CLI is one-shot so the background connectivity probe
 ///     must be disabled; <c>AutoReconnect</c> is the inverse of <c>--disable-reconnect</c>.
 ///     Also covers the input-range validation introduced for Driver.Modbus.Cli-003.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class ModbusCommandBaseTests
 {
    // Test-only ModbusCommandBase concrete subclass that exposes the protected BuildOptions
    // helper + ValidateEndpoint. The [Command] attribute is required by the CliFx analyzer
    // (CliFx_CommandMustBeAnnotated) — this command is never registered with the CLI app
    // but the analyzer rule fires for every ICommand implementor in the compilation.
    [Command("noop-test", Description = "Test-only probe of ModbusCommandBase.BuildOptions.")]
    private sealed class ProbeOnly : ModbusCommandBase
    {
        public override ValueTask ExecuteAsync(IConsole console) => default;
        public ModbusDriverOptions Invoke(IReadOnlyList<ModbusTagDefinition> tags) => BuildOptions(tags);
        public void InvokeValidate() => ValidateEndpoint();
    }
    [Fact]
    public void BuildOptions_disables_probe_for_one_shot_cli_runs()
    {
        var sut = new ProbeOnly { Host = "10.0.0.5" };
        var options = sut.Invoke([]);
        options.Probe.ShouldNotBeNull();
        options.Probe.Enabled.ShouldBeFalse();
    }
    [Fact]
    public void BuildOptions_maps_TimeoutMs_to_Timeout_TimeSpan()
    {
        var sut = new ProbeOnly { Host = "h", TimeoutMs = 7500 };
        var options = sut.Invoke([]);
        options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7500));
    }
    [Fact]
    public void BuildOptions_AutoReconnect_defaults_to_true_when_flag_unset()
    {
        var sut = new ProbeOnly { Host = "h" };
        var options = sut.Invoke([]);
        options.AutoReconnect.ShouldBeTrue();
    }
    [Fact]
    public void BuildOptions_AutoReconnect_becomes_false_when_disable_reconnect_flag_set()
    {
        var sut = new ProbeOnly { Host = "h", DisableAutoReconnect = true };
        var options = sut.Invoke([]);
        options.AutoReconnect.ShouldBeFalse();
    }
    [Fact]
    public void BuildOptions_flows_host_port_unit_through()
    {
        var sut = new ProbeOnly { Host = "plc.shop.local", Port = 5020, UnitId = 17, TimeoutMs = 3000 };
        var options = sut.Invoke([]);
        options.Host.ShouldBe("plc.shop.local");
        options.Port.ShouldBe(5020);
        options.UnitId.ShouldBe((byte)17);
    }
    [Fact]
    public void BuildOptions_forwards_tag_list_verbatim()
    {
        var sut = new ProbeOnly { Host = "h" };
        var tag = new ModbusTagDefinition(
            Name: "T", Region: ModbusRegion.HoldingRegisters, Address: 0, DataType: ModbusDataType.UInt16);
        var options = sut.Invoke([tag]);
        options.Tags.Count.ShouldBe(1);
        options.Tags[0].ShouldBeSameAs(tag);
    }
    // --- Driver.Modbus.Cli-003: parse-time endpoint validation -------------------------------
    [Theory]
    [InlineData(0)]
    [InlineData(-1)]
    [InlineData(65536)]
    [InlineData(int.MinValue)]
    [InlineData(int.MaxValue)]
    public void ValidateEndpoint_rejects_port_outside_1_to_65535(int port)
    {
        var sut = new ProbeOnly { Host = "h", Port = port };
        Should.Throw<CliFx.Exceptions.CommandException>(() => sut.InvokeValidate());
    }
    [Theory]
    [InlineData(1)]
    [InlineData(502)]
    [InlineData(65535)]
    public void ValidateEndpoint_accepts_port_in_range(int port)
    {
        var sut = new ProbeOnly { Host = "h", Port = port };
        Should.NotThrow(() => sut.InvokeValidate());
    }
    [Theory]
    [InlineData(0)]
    [InlineData(-1)]
    [InlineData(-2000)]
    public void ValidateEndpoint_rejects_non_positive_timeout(int timeoutMs)
    {
        var sut = new ProbeOnly { Host = "h", TimeoutMs = timeoutMs };
        Should.Throw<CliFx.Exceptions.CommandException>(() => sut.InvokeValidate());
    }
    [Theory]
    [InlineData(0)]   // broadcast — disallowed for unicast read/write requests
    [InlineData(248)]
    [InlineData(255)]
    public void ValidateEndpoint_rejects_unit_id_outside_1_to_247(byte unitId)
    {
        var sut = new ProbeOnly { Host = "h", UnitId = unitId };
        Should.Throw<CliFx.Exceptions.CommandException>(() => sut.InvokeValidate());
    }
    [Theory]
    [InlineData(1)]
    [InlineData(247)]
    [InlineData(50)]
    public void ValidateEndpoint_accepts_unit_id_in_range(byte unitId)
    {
        var sut = new ProbeOnly { Host = "h", UnitId = unitId };
        Should.NotThrow(() => sut.InvokeValidate());
    }
    [Fact]
    public void ValidateEndpoint_accepts_default_options()
    {
        // Defaults: Port=502, UnitId=1, TimeoutMs=2000. All inside the valid ranges.
        var sut = new ProbeOnly { Host = "h" };
        Should.NotThrow(() => sut.InvokeValidate());
    }
 }
@@ -0,0 +1,60 @@
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests;
 /// <summary>
 ///     Covers <see cref="ProbeCommand.ComputeVerdict"/> — the headline-string helper that
 ///     combines the driver-side <see cref="DriverState"/> with the probe snapshot's OPC UA
 ///     <see cref="DataValueSnapshot.StatusCode"/> so the operator never sees the previous
 ///     contradictory pair (`Health: Healthy` above a `Status: Bad...` snapshot line — see
 ///     Driver.Modbus.Cli-006).
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class ProbeCommandTests
 {
    [Fact]
    public void ComputeVerdict_returns_OK_when_state_is_Healthy_and_status_is_Good()
    {
        var verdict = ProbeCommand.ComputeVerdict(DriverState.Healthy, statusCode: 0x00000000u);
        verdict.ShouldContain("OK");
    }
    [Theory]
    [InlineData(DriverState.Faulted)]
    [InlineData(DriverState.Reconnecting)]
    [InlineData(DriverState.Unknown)]
    public void ComputeVerdict_returns_FAIL_when_driver_state_is_not_Healthy(DriverState state)
    {
        var verdict = ProbeCommand.ComputeVerdict(state, statusCode: 0x00000000u);
        verdict.ShouldContain("FAIL");
    }
    [Theory]
    [InlineData(0x80050000u)] // BadCommunicationError
    [InlineData(0x800A0000u)] // BadTimeout
    [InlineData(0x80740000u)] // BadTypeMismatch
    [InlineData(0x80000000u)] // generic Bad
    public void ComputeVerdict_returns_FAIL_when_snapshot_status_is_Bad_even_if_driver_state_Healthy(uint statusCode)
    {
        // Driver.Modbus.Cli-006: a successful InitializeAsync sets DriverState.Healthy, but
        // the FC03 probe read may still fail (snapshot.StatusCode != Good). Previously the
        // headline reported Healthy while the snapshot line below showed Bad. The verdict
        // must reflect the actual probe-read outcome.
        var verdict = ProbeCommand.ComputeVerdict(DriverState.Healthy, statusCode);
        verdict.ShouldContain("FAIL");
    }
    [Fact]
    public void ComputeVerdict_returns_DEGRADED_for_uncertain_status_with_healthy_driver()
    {
        var verdict = ProbeCommand.ComputeVerdict(DriverState.Healthy, statusCode: 0x40000000u);
        verdict.ShouldContain("DEGRADED");
    }
 }
@@ -0,0 +1,63 @@
 using CliFx.Infrastructure;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli.Tests;
 /// <summary>
 ///     Covers the branch validation inside <see cref="WriteCommand.ExecuteAsync"/>:
 ///     1. Driver.Modbus.Cli-002 — write to <see cref="ModbusRegion.Coils"/> must use
 ///        <c>--type Bool</c>.
 ///     2. Read-only regions (DiscreteInputs / InputRegisters) reject any write.
 ///     The actual driver call is never reached for these guard cases — they throw a
 ///     <see cref="CliFx.Exceptions.CommandException"/> before the driver is constructed,
 ///     so we can exercise <c>ExecuteAsync</c> against an unreachable host.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class WriteCommandRegionValidationTests
 {
    [Theory]
    [InlineData(ModbusRegion.DiscreteInputs, ModbusDataType.Bool, "0")]
    [InlineData(ModbusRegion.InputRegisters, ModbusDataType.UInt16, "1")]
    public async Task ExecuteAsync_rejects_read_only_regions(
        ModbusRegion region, ModbusDataType type, string value)
    {
        var sut = new WriteCommand
        {
            // Host is required, but the guard fires before any socket use.
            Host = "127.0.0.1",
            Region = region,
            Address = 0,
            DataType = type,
            Value = value,
        };
        using var console = new FakeInMemoryConsole();
        await Should.ThrowAsync<CliFx.Exceptions.CommandException>(
            async () => await sut.ExecuteAsync(console));
    }
    [Theory]
    [InlineData(ModbusDataType.UInt16)]
    [InlineData(ModbusDataType.Int16)]
    [InlineData(ModbusDataType.Float32)]
    [InlineData(ModbusDataType.Int32)]
    public async Task ExecuteAsync_rejects_non_Bool_type_for_Coils_region(ModbusDataType type)
    {
        var sut = new WriteCommand
        {
            Host = "127.0.0.1",
            Region = ModbusRegion.Coils,
            Address = 5,
            DataType = type,
            Value = "42",
        };
        using var console = new FakeInMemoryConsole();
        var ex = await Should.ThrowAsync<CliFx.Exceptions.CommandException>(
            async () => await sut.ExecuteAsync(console));
        ex.Message.ShouldContain("Coils");
        ex.Message.ShouldContain("Bool");
    }
 }
@@ -0,0 +1,61 @@
 using Shouldly;
 using Xunit;
 namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests;
 /// <summary>
 ///     Driver.S7.Cli-004: every S7 CLI command must own one disposal mechanism for the
 ///     <c>S7Driver</c>, not two. The chosen mechanism is <c>await using var driver = ...</c>
 ///     — <c>S7Driver.DisposeAsync</c> already calls <c>ShutdownAsync</c>, so an additional
 ///     explicit <c>driver.ShutdownAsync(...)</c> in a <c>finally</c> block runs shutdown
 ///     twice (three times on subscribe). These tests guard against that regression by
 ///     scanning the command source files.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class CommandDisposalConventionsTests
 {
    private static readonly string CommandsDir = LocateCommandsDir();
    [Theory]
    [InlineData("ProbeCommand.cs")]
    [InlineData("ReadCommand.cs")]
    [InlineData("WriteCommand.cs")]
    [InlineData("SubscribeCommand.cs")]
    public void Command_does_not_call_ShutdownAsync_explicitly(string commandFile)
    {
        var path = Path.Combine(CommandsDir, commandFile);
        File.Exists(path).ShouldBeTrue($"Expected {path} to exist.");
        var source = File.ReadAllText(path);
        // The await-using statement is the single disposal mechanism. An explicit
        // driver.ShutdownAsync(...) call (typically inside a finally block) re-invokes
        // a shutdown path that DisposeAsync already runs and is the smell -004 flags.
        source.ShouldNotContain("driver.ShutdownAsync(");
    }
    [Theory]
    [InlineData("ProbeCommand.cs")]
    [InlineData("ReadCommand.cs")]
    [InlineData("WriteCommand.cs")]
    [InlineData("SubscribeCommand.cs")]
    public void Command_uses_await_using_for_S7Driver(string commandFile)
    {
        var path = Path.Combine(CommandsDir, commandFile);
        var source = File.ReadAllText(path);
        source.ShouldContain("await using var driver = new S7Driver(");
    }
    private static string LocateCommandsDir()
    {
        // Walk up from the test assembly bin/ folder to the repo root, then into the
        // source project's Commands/ directory. The test-host puts CWD somewhere under
        // bin/Debug/net10.0 so we resolve relative to AppContext.BaseDirectory.
        var dir = new DirectoryInfo(AppContext.BaseDirectory);
        while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx")))
            dir = dir.Parent;
        dir.ShouldNotBeNull("Could not find solution root (ZB.MOM.WW.OtOpcUa.slnx).");
        return Path.Combine(
            dir!.FullName, "src", "Drivers", "Cli", "ZB.MOM.WW.OtOpcUa.Driver.S7.Cli", "Commands");
    }
 }
@@ -0,0 +1,92 @@
 using CliFx.Attributes;
 using CliFx.Infrastructure;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.S7.Cli;
 using S7NetCpuType = global::S7.Net.CpuType;
 namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests;
 /// <summary>
 ///     Covers <see cref="S7CommandBase.BuildOptions"/> — the pure, deterministic mapping
 ///     from the base's host/port/CPU/rack/slot/timeout flags onto an
 ///     <c>S7DriverOptions</c>. The CLI is one-shot so the background connectivity probe
 ///     must be disabled.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class S7CommandBaseBuildOptionsTests
 {
    // Test-only S7CommandBase concrete subclass that exposes the protected BuildOptions
    // helper. The [Command] attribute is required by the CliFx analyzer
    // (CliFx_CommandMustBeAnnotated) — this command is never registered with the CLI app
    // but the analyzer rule fires for every ICommand implementor in the compilation.
    [Command("noop-test", Description = "Test-only probe of S7CommandBase.BuildOptions.")]
    private sealed class ProbeOnly : S7CommandBase
    {
        public override ValueTask ExecuteAsync(IConsole console) => default;
        public S7DriverOptions Invoke(IReadOnlyList<S7TagDefinition> tags) => BuildOptions(tags);
    }
    [Fact]
    public void BuildOptions_disables_probe_for_one_shot_cli_runs()
    {
        var sut = new ProbeOnly
        {
            Host = "10.0.0.5",
            Port = 102,
            CpuType = S7NetCpuType.S71500,
            Rack = 0,
            Slot = 0,
            TimeoutMs = 5000,
        };
        var options = sut.Invoke([]);
        options.Probe.ShouldNotBeNull();
        options.Probe.Enabled.ShouldBeFalse();
    }
    [Fact]
    public void BuildOptions_maps_TimeoutMs_to_Timeout_TimeSpan()
    {
        var sut = new ProbeOnly { Host = "h", TimeoutMs = 7500 };
        var options = sut.Invoke([]);
        options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7500));
    }
    [Fact]
    public void BuildOptions_flows_host_port_cpu_rack_slot_through()
    {
        var sut = new ProbeOnly
        {
            Host = "plc.shop.local",
            Port = 4102,
            CpuType = S7NetCpuType.S7300,
            Rack = 1,
            Slot = 2,
            TimeoutMs = 3000,
        };
        var options = sut.Invoke([]);
        options.Host.ShouldBe("plc.shop.local");
        options.Port.ShouldBe(4102);
        options.CpuType.ShouldBe(S7NetCpuType.S7300);
        options.Rack.ShouldBe((short)1);
        options.Slot.ShouldBe((short)2);
    }
    [Fact]
    public void BuildOptions_forwards_tag_list_verbatim()
    {
        var sut = new ProbeOnly { Host = "h" };
        var tag = new S7TagDefinition("t", "MW0", S7DataType.Int16, Writable: false);
        var options = sut.Invoke([tag]);
        options.Tags.Count.ShouldBe(1);
        options.Tags[0].ShouldBeSameAs(tag);
    }
 }
@@ -0,0 +1,32 @@
 using Shouldly;
 using Xunit;
 namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests;
 /// <summary>
 ///     Driver.S7.Cli-007: the S7 subscribe command — a near-verbatim copy of the Modbus
 ///     subscribe command — must keep the comment that explains why <c>OnDataChange</c>
 ///     uses <c>console.Output.WriteLine</c> (synchronous, on a driver background thread)
 ///     instead of <c>System.Console</c> or the async <c>WriteLineAsync</c>. The rationale
 ///     is non-obvious to a reader and the Modbus copy carries it; the S7 copy must too.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class SubscribeCommandConsoleHandlerCommentTests
 {
    [Fact]
    public void SubscribeCommand_explains_why_OnDataChange_uses_console_Output_synchronously()
    {
        var dir = new DirectoryInfo(AppContext.BaseDirectory);
        while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx")))
            dir = dir.Parent;
        dir.ShouldNotBeNull();
        var source = File.ReadAllText(Path.Combine(
            dir!.FullName, "src", "Drivers", "Cli", "ZB.MOM.WW.OtOpcUa.Driver.S7.Cli",
            "Commands", "SubscribeCommand.cs"));
        // The comment must reference the CliFx console abstraction so future copy-pastes
        // do not lose the rationale.
        source.ShouldContain("CliFx console");
        source.ShouldContain("IConsole");
    }
 }
@@ -0,0 +1,123 @@
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests;
 /// <summary>
 ///     Driver.TwinCAT.Cli-006: covers the prefix / max filtering and the RO/RW classification
 ///     logic inside <see cref="BrowseCommand"/>. The collecting builder + the filter pipeline
 ///     are pure — no ADS contact required — so they unit-test cleanly.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class BrowseCommandFilterTests
 {
    private static DriverAttributeInfo Info(SecurityClassification cls, DriverDataType dt = DriverDataType.Int32)
        => new(
            FullName: "ignored",
            DriverDataType: dt,
            IsArray: false,
            ArrayDim: null,
            SecurityClass: cls,
            IsHistorized: false);
    [Fact]
    public void Collector_records_each_variable_in_call_order()
    {
        var builder = new BrowseCommand.CollectingAddressSpaceBuilder();
        builder.Variable("GVL.A", "GVL.A", Info(SecurityClassification.Operate));
        builder.Variable("GVL.B", "GVL.B", Info(SecurityClassification.ViewOnly));
        builder.Variables.Count.ShouldBe(2);
        builder.Variables[0].BrowseName.ShouldBe("GVL.A");
        builder.Variables[1].BrowseName.ShouldBe("GVL.B");
    }
    [Fact]
    public void Folder_returns_same_builder_so_nested_variables_land_in_one_flat_list()
    {
        // BrowseCommand expects a flat list — TwinCAT's flat-mode symbol walk doesn't nest
        // into sub-folders, but DiscoverAsync may still call Folder() before Variable().
        var builder = new BrowseCommand.CollectingAddressSpaceBuilder();
        var nested = builder.Folder("Discovered", "Discovered");
        nested.Variable("GVL.X", "GVL.X", Info(SecurityClassification.Operate));
        builder.Variables.Count.ShouldBe(1);
        builder.Variables[0].BrowseName.ShouldBe("GVL.X");
    }
    [Fact]
    public void FilterAndLimit_empty_prefix_returns_everything_up_to_max()
    {
        var symbols = new List<(string BrowseName, DriverAttributeInfo Info)>
        {
            ("GVL_A.x", Info(SecurityClassification.Operate)),
            ("GVL_B.y", Info(SecurityClassification.ViewOnly)),
            ("MAIN.z",  Info(SecurityClassification.Operate)),
        };
        var matched = BrowseCommand.FilterByPrefix(symbols, prefix: null);
        matched.Count.ShouldBe(3);
    }
    [Fact]
    public void FilterAndLimit_prefix_is_case_sensitive()
    {
        var symbols = new List<(string BrowseName, DriverAttributeInfo Info)>
        {
            ("GVL_Fixture.x", Info(SecurityClassification.Operate)),
            ("gvl_fixture.y", Info(SecurityClassification.Operate)),
            ("MAIN.z",        Info(SecurityClassification.Operate)),
        };
        var matched = BrowseCommand.FilterByPrefix(symbols, prefix: "GVL_Fixture");
        matched.Count.ShouldBe(1);
        matched[0].BrowseName.ShouldBe("GVL_Fixture.x");
    }
    [Fact]
    public void FilterAndLimit_zero_max_means_unbounded()
    {
        var symbols = Enumerable.Range(0, 10)
            .Select(i => ($"GVL.S{i}", Info(SecurityClassification.Operate)))
            .ToList();
        var limit = BrowseCommand.PrintLimit(symbols.Count, max: 0);
        limit.ShouldBe(10);
    }
    [Fact]
    public void FilterAndLimit_caps_to_max_when_more_matched()
    {
        BrowseCommand.PrintLimit(matchedCount: 1000, max: 50).ShouldBe(50);
    }
    [Fact]
    public void FilterAndLimit_does_not_pad_to_max_when_fewer_matched()
    {
        BrowseCommand.PrintLimit(matchedCount: 3, max: 50).ShouldBe(3);
    }
    [Fact]
    public void AccessTag_returns_RO_for_ViewOnly_attribute()
    {
        BrowseCommand.AccessTag(Info(SecurityClassification.ViewOnly)).ShouldBe("RO");
    }
    [Theory]
    [InlineData(SecurityClassification.FreeAccess)]
    [InlineData(SecurityClassification.Operate)]
    [InlineData(SecurityClassification.SecuredWrite)]
    [InlineData(SecurityClassification.VerifiedWrite)]
    [InlineData(SecurityClassification.Tune)]
    [InlineData(SecurityClassification.Configure)]
    public void AccessTag_returns_RW_for_anything_except_ViewOnly(SecurityClassification cls)
    {
        // BrowseCommand's display logic flips ViewOnly = RO, everything else = RW. The real
        // ACL is enforced server-side from the SecurityClassification — the CLI label is just
        // a coarse "is this writable from any tier" indicator.
        BrowseCommand.AccessTag(Info(cls)).ShouldBe("RW");
    }
 }
@@ -0,0 +1,37 @@
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests;
 /// <summary>
 ///     Driver.TwinCAT.Cli-003: the subscribe banner mechanism label is derived from the
 ///     <see cref="ISubscriptionHandle.DiagnosticId"/> the driver actually returned, not from
 ///     the <c>--poll-only</c> flag. That way the banner cannot disagree with what the driver
 ///     did even if a future fallback path lands the subscription somewhere unexpected.
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class SubscribeCommandMechanismTests
 {
    private sealed record StubHandle(string DiagnosticId) : ISubscriptionHandle;
    [Theory]
    [InlineData("twincat-native-sub-1")]
    [InlineData("twincat-native-sub-42")]
    [InlineData("twincat-native-sub-9223372036854775807")]
    public void DescribeMechanism_returns_ADS_notification_for_native_handle(string diagId)
    {
        SubscribeCommand.DescribeMechanism(new StubHandle(diagId)).ShouldBe("ADS notification");
    }
    [Theory]
    [InlineData("pollgroup-1")]
    [InlineData("modbus-poll-7")]
    [InlineData("")]
    [InlineData("TWINCAT-NATIVE-SUB-1")] // ordinal comparison — uppercase prefix does NOT match.
    public void DescribeMechanism_returns_polling_for_anything_else(string diagId)
    {
        SubscribeCommand.DescribeMechanism(new StubHandle(diagId)).ShouldBe("polling");
    }
 }
@@ -0,0 +1,238 @@
 using System.Reflection;
 using CliFx.Attributes;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Commands;
 namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Cli.Tests;
 /// <summary>
 ///     Covers <see cref="TwinCATCommandBase"/> / <see cref="TwinCATTagCommandBase"/> wiring:
 ///     the canonical gateway string, the driver instance id, the BuildOptions field projection
 ///     (Driver.TwinCAT.Cli-006), and the up-front range validation guards
 ///     (Driver.TwinCAT.Cli-001).
 /// </summary>
 [Trait("Category", "Unit")]
 public sealed class TwinCATCommandBaseTests
 {
    [Fact]
    public void Gateway_uses_canonical_ads_scheme_with_port()
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "192.168.1.40.1.1",
            AmsPort = 851,
            SymbolPath = "MAIN.bRunning",
        };
        cmd.GatewayForTest.ShouldBe("ads://192.168.1.40.1.1:851");
    }
    [Fact]
    public void Gateway_round_trips_through_TwinCATAmsAddress_TryParse()
    {
        // Driver.TwinCAT.Cli-006: a regression in the Gateway string breaks every command
        // because the driver's TwinCATAmsAddress.TryParse refuses anything not shaped
        // ads://{netId}:{port}.
        var cmd = new ProbeCommand
        {
            AmsNetId = "5.23.91.23.1.1",
            AmsPort = 852,
            SymbolPath = "MAIN.x",
        };
        var parsed = TwinCAT.TwinCATAmsAddress.TryParse(cmd.GatewayForTest);
        parsed.ShouldNotBeNull();
        parsed!.NetId.ShouldBe("5.23.91.23.1.1");
        parsed.Port.ShouldBe(852);
    }
    [Fact]
    public void DriverInstanceId_includes_ams_target()
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            AmsPort = 851,
            SymbolPath = "MAIN.x",
        };
        cmd.DriverInstanceIdForTest.ShouldBe("twincat-cli-127.0.0.1.1.1:851");
    }
    [Fact]
    public void Timeout_is_projection_of_TimeoutMs_and_init_is_noop()
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            TimeoutMs = 7777,
            SymbolPath = "MAIN.x",
        };
        cmd.Timeout.ShouldBe(TimeSpan.FromMilliseconds(7777));
    }
    [Fact]
    public void BuildOptions_wires_device_tags_timeout_and_disables_probe()
    {
        // Driver.TwinCAT.Cli-006: cover the property-by-property wiring that the four runtime
        // commands depend on. Probe must be disabled (CLI is one-shot — the probe loop would
        // race the operator's own reads) and controller-browse must stay off.
        var cmd = new ProbeCommand
        {
            AmsNetId = "10.0.0.1.1.1",
            AmsPort = 851,
            TimeoutMs = 4321,
            SymbolPath = "MAIN.x",
        };
        var tag = new TwinCAT.TwinCATTagDefinition(
            Name: "n1",
            DeviceHostAddress: cmd.GatewayForTest,
            SymbolPath: "MAIN.x",
            DataType: TwinCAT.TwinCATDataType.DInt,
            Writable: false);
        var options = cmd.BuildOptionsForTest([tag]);
        options.Devices.Count.ShouldBe(1);
        options.Devices[0].HostAddress.ShouldBe("ads://10.0.0.1.1.1:851");
        options.Devices[0].DeviceName.ShouldBe("cli-10.0.0.1.1.1:851");
        options.Tags.ShouldBe([tag]);
        options.Timeout.ShouldBe(TimeSpan.FromMilliseconds(4321));
        options.Probe.Enabled.ShouldBeFalse();
        options.EnableControllerBrowse.ShouldBeFalse();
        // Default UseNativeNotifications = true (no --poll-only).
        options.UseNativeNotifications.ShouldBeTrue();
    }
    [Fact]
    public void BuildOptions_PollOnly_flips_UseNativeNotifications_off()
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "10.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            PollOnly = true,
        };
        cmd.BuildOptionsForTest([]).UseNativeNotifications.ShouldBeFalse();
    }
    // ---- Driver.TwinCAT.Cli-001 (range validation) ----
    [Fact]
    public void Validate_rejects_zero_timeout()
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            TimeoutMs = 0,
        };
        var ex = Should.Throw<CliFx.Exceptions.CommandException>(() => cmd.ValidateForTest());
        ex.Message.ShouldContain("--timeout-ms");
    }
    [Fact]
    public void Validate_rejects_negative_timeout()
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            TimeoutMs = -1,
        };
        Should.Throw<CliFx.Exceptions.CommandException>(() => cmd.ValidateForTest());
    }
    [Theory]
    [InlineData(0)]
    [InlineData(-1)]
    [InlineData(65536)]
    [InlineData(100000)]
    public void Validate_rejects_out_of_range_ams_port(int port)
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            AmsPort = port,
        };
        var ex = Should.Throw<CliFx.Exceptions.CommandException>(() => cmd.ValidateForTest());
        ex.Message.ShouldContain("--ams-port");
    }
    [Theory]
    [InlineData(1)]
    [InlineData(801)]
    [InlineData(851)]
    [InlineData(65535)]
    public void Validate_accepts_in_range_ams_port(int port)
    {
        var cmd = new ProbeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            AmsPort = port,
        };
        Should.NotThrow(() => cmd.ValidateForTest());
    }
    [Fact]
    public void SubscribeCommand_validate_rejects_zero_interval()
    {
        var cmd = new SubscribeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            IntervalMs = 0,
        };
        var ex = Should.Throw<CliFx.Exceptions.CommandException>(() => cmd.ValidateForTest());
        ex.Message.ShouldContain("--interval-ms");
    }
    [Fact]
    public void SubscribeCommand_validate_rejects_negative_interval()
    {
        var cmd = new SubscribeCommand
        {
            AmsNetId = "127.0.0.1.1.1",
            SymbolPath = "MAIN.x",
            IntervalMs = -100,
        };
        Should.Throw<CliFx.Exceptions.CommandException>(() => cmd.ValidateForTest());
    }
    // ---- Driver.TwinCAT.Cli-004 (PollOnly off BrowseCommand surface) ----
    [Fact]
    public void BrowseCommand_does_not_expose_poll_only_flag()
    {
        // Driver.TwinCAT.Cli-004: the flag has no observable effect on browse — surfacing it
        // misleads users. After the refactor, PollOnly lives on an intermediate base shared
        // only by the commands that actually consume native ADS notifications.
        var props = typeof(BrowseCommand)
            .GetProperties(BindingFlags.Public | BindingFlags.Instance);
        props.ShouldNotContain(p => p.Name == "PollOnly");
    }
    [Fact]
    public void ProbeCommand_still_exposes_poll_only_flag()
    {
        // Probe / Read / Write / Subscribe all build TwinCATDriverOptions and so still take
        // the --poll-only toggle.
        var props = typeof(ProbeCommand)
            .GetProperties(BindingFlags.Public | BindingFlags.Instance);
        props.ShouldContain(p => p.Name == "PollOnly");
    }
    // ---- Driver.TwinCAT.Cli-005 (probe --type short alias) ----
    [Fact]
    public void ProbeCommand_type_option_carries_short_alias_t()
    {
        // Driver.TwinCAT.Cli-005: --type on read/write/subscribe takes the -t short alias;
        // probe must match so muscle memory works the same way across all four verbs.
        var dataTypeProp = typeof(ProbeCommand).GetProperty("DataType");
        dataTypeProp.ShouldNotBeNull();
        var attr = dataTypeProp!.GetCustomAttribute<CommandOptionAttribute>();
        attr.ShouldNotBeNull();
        attr!.ShortName.ShouldBe('t');
    }
 }
Author	SHA1	Message	Date
Joseph Doherty	2a941b255f	docs(code-reviews): regenerate index — 29 Low findings resolved Batch 5 cleared Open findings in Driver.AbCip.Cli, Driver.AbLegacy.Cli, Driver.S7.Cli, Driver.TwinCAT.Cli, and Driver.Modbus.Cli. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:35:12 -04:00
Joseph Doherty	80ef8806e0	fix(driver-modbus-cli): resolve Low code-review findings (Driver.Modbus.Cli-003,004,005,006,007,008) - Driver.Modbus.Cli-003: ModbusCommandBase.ValidateEndpoint rejects --port outside 1..65535, non-positive --timeout-ms, and --unit-id outside 1..247. - Driver.Modbus.Cli-004: wrapped SubscribeCommand's OnDataChange handler body in a try/catch (warn-and-swallow) and serialised the console write through a lock. - Driver.Modbus.Cli-005: Probe / Read / Write now catch the cancellation-during-init OperationCanceledException and print 'Cancelled.' instead of dumping a stack trace. - Driver.Modbus.Cli-006: ProbeCommand.ComputeVerdict derives the headline from BOTH the driver state and the probe snapshot's OPC UA quality class so the headline can't disagree with the wire result. - Driver.Modbus.Cli-007: docs/Driver.Modbus.Cli.md carries an explicit 'CLI scope' callout — the address-string grammar is a DriverConfig JSON feature; the CLI takes the structured triple only. - Driver.Modbus.Cli-008: pinned BuildOptions, ValidateEndpoint, the region-validation guards, ComputeVerdict, and the cancellation-during- initialize paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:35:05 -04:00
Joseph Doherty	f2ee027145	fix(driver-twincat-cli): resolve Low code-review findings (Driver.TwinCAT.Cli-001,002,003,004,005,006,007) - Driver.TwinCAT.Cli-001: TwinCATCommandBase.Validate rejects non-positive TimeoutMs / IntervalMs and AmsPort outside 1..65535; ExecuteAsync calls it first. - Driver.TwinCAT.Cli-002: SubscribeCommand serialises every WriteLine through a writeLock to remove the notification-callback vs banner interleave risk. - Driver.TwinCAT.Cli-003: SubscribeCommand.DescribeMechanism derives the banner label from the returned ISubscriptionHandle.DiagnosticId so it can't disagree with what the driver actually did. - Driver.TwinCAT.Cli-004: introduced TwinCATTagCommandBase carrying --poll-only + BuildOptions; BrowseCommand stays on the slimmer TwinCATCommandBase so --poll-only no longer surfaces in browse --help. - Driver.TwinCAT.Cli-005: ProbeCommand --type now carries the 't' short alias to match the other commands. - Driver.TwinCAT.Cli-006: 35 new tests covering Gateway / AmsAddress parse / BuildOptions / PollOnly / browse-helpers / probe-alias / mechanism derivation. - Driver.TwinCAT.Cli-007: replaced the empty-init <inheritdoc/> with an explicit summary warning future maintainers about the no-op init. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:34:57 -04:00
Joseph Doherty	67ef6c4ebc	fix(driver-s7-cli): resolve Low code-review findings (Driver.S7.Cli-004,005,006,007) - Driver.S7.Cli-004: 'await using var driver' is the sole driver disposal path; dropped the redundant explicit await ShutdownAsync from each command's finally. - Driver.S7.Cli-005: deleted the stale empty tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli.Tests/ directory (the real test project lives under tests/Drivers/Cli/). - Driver.S7.Cli-006: S7CommandBaseBuildOptionsTests cover the probe toggle, timeout mapping, host/port/CPU/rack/slot wiring, and tag list passthrough. - Driver.S7.Cli-007: re-added the SubscribeCommand handler comment explaining the CliFx IConsole.Output usage and that the poll-thread raises events. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:34:48 -04:00
Joseph Doherty	f46e126208	fix(driver-ablegacy-cli): resolve Low code-review findings (Driver.AbLegacy.Cli-002,003,004,005,006,007) - Driver.AbLegacy.Cli-002: WriteCommand.Value description lists the full true/false, 1/0, on/off, yes/no alias set. - Driver.AbLegacy.Cli-003: SubscribeCommand serialises every WriteLine via a per-execution consoleGate lock so the poll-thread OnDataChange handler can't interleave with the banner. - Driver.AbLegacy.Cli-004: dropped 'await using var driver' in favour of a plain 'var driver' + explicit await ShutdownAsync in finally; the driver is no longer shut down twice. - Driver.AbLegacy.Cli-005: SubscribeCommand.IntervalMs description carries the PollGroupEngine 250ms-floor caveat; docs/Driver.AbLegacy.Cli.md spells out the same. - Driver.AbLegacy.Cli-006: ProbeCommand --type now carries the short alias 't' to match the other commands. - Driver.AbLegacy.Cli-007: BuildOptionsTests cover the probe-disabled, device-shape, tag-passthrough, timeout-propagation, and empty-tag-list paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:34:32 -04:00
Joseph Doherty	759af8c1bb	fix(driver-abcip-cli): resolve Low code-review findings (Driver.AbCip.Cli-003,004,005,006,007,008) - Driver.AbCip.Cli-003: SubscribeCommand prints the 'Subscribed' banner BEFORE wiring OnDataChange so the main thread can't interleave its write with the poll-thread handler. - Driver.AbCip.Cli-004: AbCipCommandBase.Timeout and SubscribeCommand validate TimeoutMs / IntervalMs and throw CommandException on non-positive values. - Driver.AbCip.Cli-005: every command now calls FlushLogging() in its finally block. - Driver.AbCip.Cli-006: Timeout init throws NotSupportedException with a pointer at TimeoutMs instead of silently swallowing assignments. - Driver.AbCip.Cli-007: added AbCipCommandBaseTests covering BuildOptions shape, probe / controller-browse / alarm toggles, host address, family selection, tag list passthrough. - Driver.AbCip.Cli-008: rewrote the opening paragraph in docs/Driver.AbCip.Cli.md to credit the six-CLI roster with a pointer at docs/DriverClis.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-23 08:34:25 -04:00