fix(driver-cli-common): resolve Medium code-review finding (Driver.Cli.Common-002)

FormatStatus now matches named codes against code & 0xFFFF0000 (high-word
mask) rather than exact equality, so status codes carrying sub-code or flag
bits in the low 16 bits (e.g. 0x80050001) still resolve to their named class.
For codes not in the named shortlist a severity-class fallback using the top
2 bits always emits Good / Uncertain / Bad rather than bare hex.

Updated the stale FormatStatus_unknown_codes_fall_back_to_hex_only test (its
expectation became invalid once the severity-class fallback was added) and
added new Theory cases exercising both the high-word matching and the
severity-class fallback paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-reviews/Driver.Cli.Common/findings.md

@@ -81,7 +81,7 @@ a regression `[Theory]` asserting the pre-fix wrong names no longer apply.
 | Severity | Medium |
 | Category | Correctness & logic bugs |
 | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:101-122` |
-| Status | Open |
+| Status | Resolved |
 
 **Description:** `FormatStatus` matches the full 32-bit status word for exact equality
 against the shortlist. OPC UA status codes carry sub-code/flag bits in the low 16 bits
@@ -96,7 +96,7 @@ codes, or (b) match on the severity bits (`code & 0xC0000000`) to at least alway
 `Good` / `Uncertain` / `Bad` even when sub-code bits are set, and match the named codes
 on the masked code (`code & 0xFFFF0000`).
 
-**Resolution:** _(open)_
+**Resolution:** Resolved 2026-05-22 — `FormatStatus` now matches named codes on `code & 0xFFFF0000` and falls back to a severity-class label (`Good`/`Uncertain`/`Bad`) via `code & 0xC0000000` for unknown sub-codes; the stale "bare-hex for unknown codes" test expectation was corrected to reflect the new severity-class fallback.
 
 ### Driver.Cli.Common-003
 

src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs

@@ -65,9 +65,9 @@ public static class SnapshotFormatter
             Time = FormatTimestamp(snapshots[i].SourceTimestampUtc),
         }).ToArray();
 
-        int tagW = Math.Max("TAG".Length, rows.Max(r => r.Tag.Length));
+        int tagW = rows.Length == 0 ? "TAG".Length : Math.Max("TAG".Length, rows.Max(r => r.Tag.Length));
-        int valW = Math.Max("VALUE".Length, rows.Max(r => r.Value.Length));
+        int valW = rows.Length == 0 ? "VALUE".Length : Math.Max("VALUE".Length, rows.Max(r => r.Value.Length));
-        int statW = Math.Max("STATUS".Length, rows.Max(r => r.Status.Length));
+        int statW = rows.Length == 0 ? "STATUS".Length : Math.Max("STATUS".Length, rows.Max(r => r.Status.Length));
         // source-time column is fixed-width (ISO-8601 to ms) so no max-measurement needed.
 
         var sb = new System.Text.StringBuilder();
@@ -100,12 +100,16 @@ public static class SnapshotFormatter
 
     public static string FormatStatus(uint statusCode)
     {
-        // Match the OPC UA shorthand for the statuses most-likely to land in a CLI run.
+        // OPC UA status codes carry sub-code and flag bits in the low 16 bits (info type,
-        // Anything outside this short-list surfaces as hex — operators can cross-reference
+        // structure-changed, semantics-changed, limit bits, overflow, etc.).  To ensure
-        // against OPC UA Part 6 § 7.34 (StatusCode tables) or Core.Abstractions status mappers.
+        // that e.g. 0x80050001 still reads as "BadCommunicationError" rather than bare hex,
+        // named codes are matched against the high-word mask (code & 0xFFFF0000).  When no
+        // named match is found the severity class (top 2 bits) provides a meaningful fallback
+        // so operators always see at least Good / Uncertain / Bad rather than raw hex.
         // Numeric codes are the canonical values from the OPC Foundation Opc.Ua.StatusCodes
         // table; keep them in sync with that table if this list is extended.
-        var name = statusCode switch
+        var masked = statusCode & 0xFFFF0000u;
+        var name = masked switch
         {
             0x00000000u => "Good",
             0x80000000u => "Bad",
@@ -119,6 +123,19 @@ public static class SnapshotFormatter
             0x40000000u => "Uncertain",
             _ => null,
         };
+
+        if (name is null)
+        {
+            // Severity fallback: top 2 bits identify the quality class even for unknown
+            // sub-codes.  0x80000000 and 0xC0000000 (reserved quality) both map to "Bad".
+            name = (statusCode & 0xC0000000u) switch
+            {
+                0x00000000u => "Good",
+                0x40000000u => "Uncertain",
+                _ => "Bad",
+            };
+        }
+
         return name is null
             ? $"0x{statusCode:X8}"
             : $"0x{statusCode:X8} ({name})";

tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/SnapshotFormatterTests.cs

@@ -1,3 +1,4 @@
+using CliFx.Infrastructure;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
@@ -55,10 +56,11 @@ public sealed class SnapshotFormatterTests
     }
 
     [Fact]
-    public void FormatStatus_unknown_codes_fall_back_to_hex_only()
+    public void FormatStatus_unknown_codes_fall_back_to_severity_class()
     {
-        // 0xDEADBEEF isn't in the shortlist — just render the hex form, no name.
+        // 0xDEADBEEF isn't in the named shortlist. Since -002 was fixed the severity
-        SnapshotFormatter.FormatStatus(0xDEADBEEFu).ShouldBe("0xDEADBEEF");
+        // fallback (bit 31 = 1 → "Bad") applies, so operators always see a quality label.
+        SnapshotFormatter.FormatStatus(0xDEADBEEFu).ShouldBe("0xDEADBEEF (Bad)");
     }
 
     [Fact]
@@ -138,4 +140,118 @@ public sealed class SnapshotFormatterTests
         var formatted = SnapshotFormatter.FormatTimestamp(local);
         formatted.ShouldEndWith("Z");
     }
+
+    // --- Driver.Cli.Common-002: sub-code bits in status codes ---
+
+    [Theory]
+    // Status codes with non-zero low-word flag bits must still resolve to the named
+    // high-word class (Driver.Cli.Common-002).
+    [InlineData(0x00000001u, "Good")]           // Good + info bit
+    [InlineData(0x80050001u, "BadCommunicationError")]  // BadCommunicationError + sub-bit
+    [InlineData(0x800A0010u, "BadTimeout")]     // BadTimeout + sub-bits
+    [InlineData(0x40000080u, "Uncertain")]      // Uncertain + info bit
+    public void FormatStatus_with_sub_code_bits_resolves_to_named_class(uint statusCode, string expectedName)
+    {
+        SnapshotFormatter.FormatStatus(statusCode).ShouldContain($"({expectedName})");
+    }
+
+    [Theory]
+    // Unknown sub-codes fall back to the severity class (Good / Uncertain / Bad).
+    [InlineData(0x80990000u, "Bad")]            // Unknown bad sub-code → "Bad"
+    [InlineData(0x80990001u, "Bad")]            // Unknown bad sub-code + flag bit → "Bad"
+    [InlineData(0x40990000u, "Uncertain")]      // Unknown uncertain sub-code → "Uncertain"
+    [InlineData(0x00990000u, "Good")]           // Unknown good sub-code → "Good"
+    public void FormatStatus_unknown_sub_code_falls_back_to_severity_class(uint statusCode, string expectedSeverity)
+    {
+        SnapshotFormatter.FormatStatus(statusCode).ShouldContain($"({expectedSeverity})");
+    }
+
+    // --- FormatTable empty-input ---
+
+    [Fact]
+    public void FormatTable_with_empty_input_returns_header_only()
+    {
+        // A batch read that returns zero tags must not throw — it should emit just the
+        // header + separator rows (Driver.Cli.Common-004 / Driver.Cli.Common-005).
+        var table = SnapshotFormatter.FormatTable(
+            Array.Empty<string>(),
+            Array.Empty<DataValueSnapshot>());
+
+        table.ShouldContain("TAG");
+        table.ShouldContain("VALUE");
+        table.ShouldContain("STATUS");
+        table.ShouldContain("SOURCE TIME");
+        table.ShouldContain("---");
+    }
+}
+
+[Trait("Category", "Unit")]
+public sealed class DriverCommandBaseTests
+{
+    /// <summary>
+    ///     Minimal concrete subclass used only for testing the base class helpers.
+    /// </summary>
+    [CliFx.Attributes.Command("test-stub", Description = "Test stub — not a real command.")]
+    private sealed class TestCommand : DriverCommandBase
+    {
+        public override TimeSpan Timeout { get; init; } = TimeSpan.FromSeconds(1);
+
+        public override ValueTask ExecuteAsync(IConsole console) => default;
+
+        // Expose protected methods for testing.
+        public void InvokeConfigureLogging() => ConfigureLogging();
+        public static void InvokeFlushLogging() => FlushLogging();
+    }
+
+    [Fact]
+    public void ConfigureLogging_non_verbose_sets_warning_level()
+    {
+        var cmd = new TestCommand { Verbose = false };
+        cmd.InvokeConfigureLogging();
+
+        // At Warning level, Debug writes must not flow.
+        Serilog.Log.Logger.IsEnabled(Serilog.Events.LogEventLevel.Debug).ShouldBeFalse();
+        Serilog.Log.Logger.IsEnabled(Serilog.Events.LogEventLevel.Warning).ShouldBeTrue();
+        DriverCommandBase_TestCommand_Teardown();
+    }
+
+    [Fact]
+    public void ConfigureLogging_verbose_sets_debug_level()
+    {
+        var cmd = new TestCommand { Verbose = true };
+        cmd.InvokeConfigureLogging();
+
+        Serilog.Log.Logger.IsEnabled(Serilog.Events.LogEventLevel.Debug).ShouldBeTrue();
+        DriverCommandBase_TestCommand_Teardown();
+    }
+
+    [Fact]
+    public void ConfigureLogging_is_idempotent_second_call_is_noop()
+    {
+        // First call sets verbose=false (Warning); second call with verbose=true must not
+        // reconfigure — the guard makes it a no-op.
+        var cmd = new TestCommand { Verbose = false };
+        cmd.InvokeConfigureLogging();
+        var loggerAfterFirst = Serilog.Log.Logger;
+
+        // A new instance would normally apply its own Verbose=true setting, but here we
+        // reuse the same instance so the guard field fires.
+        cmd.InvokeConfigureLogging();  // no-op
+        Serilog.Log.Logger.ShouldBeSameAs(loggerAfterFirst);
+        DriverCommandBase_TestCommand_Teardown();
+    }
+
+    [Fact]
+    public void FlushLogging_does_not_throw()
+    {
+        // After CloseAndFlush the static logger is replaced with a silent logger;
+        // verify the call itself does not throw.
+        TestCommand.InvokeFlushLogging();
+    }
+
+    /// <summary>
+    ///     Resets the global Serilog logger so tests do not bleed into each other.
+    /// </summary>
+    private static void DriverCommandBase_TestCommand_Teardown() =>
+        Serilog.Log.CloseAndFlush();
 }