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

SnapshotFormatter.FormatStatus mapped four OPC UA status names to
incorrect numeric codes, mislabelling operator-facing CLI output. The
codes were corrected to their canonical OPC Foundation
Opc.Ua.StatusCodes values:

  BadTimeout                0x80060000 -> 0x800A0000
  BadNoCommunication        0x80070000 -> 0x80310000
  BadWaitingForInitialData  0x80080000 -> 0x80320000
  BadNodeIdInvalid          0x80350000 -> 0x80330000

The Cli.Common project does not reference the Opc.Ua package (only
Core.Abstractions / CliFx / Serilog), so the hex literals were
corrected in place with a sync note rather than adding a heavy new
dependency.

SnapshotFormatterTests was updated: the [Theory] expectations now use
the correct spec codes and assert the full rendered form, plus a new
regression [Theory] confirms the pre-fix wrong names no longer apply.
All 24 tests pass.

findings.md: Driver.Cli.Common-001 set to Resolved; open count 6 -> 5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 06:16:25 -04:00
parent e221371a0c
commit 6300a9e4a8
3 changed files with 34 additions and 9 deletions

View File

@@ -7,7 +7,7 @@
| Review date | 2026-05-22 | | Review date | 2026-05-22 |
| Commit reviewed | `76d35d1` | | Commit reviewed | `76d35d1` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 6 | | Open findings | 5 |
## Checklist coverage ## Checklist coverage
@@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank.
| Severity | High | | Severity | High |
| Category | Correctness & logic bugs | | Category | Correctness & logic bugs |
| Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:106-119` | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:106-119` |
| Status | Open | | Status | Resolved |
**Description:** The `FormatStatus` shortlist maps four OPC UA status names to incorrect **Description:** The `FormatStatus` shortlist maps four OPC UA status names to incorrect
numeric codes. The correct OPC UA spec values (verified against the OPC Foundation numeric codes. The correct OPC UA spec values (verified against the OPC Foundation
@@ -67,7 +67,12 @@ depends on transitively) rather than hand-maintaining a hex shortlist, so the ta
cannot drift from the spec again. If a hand-list is kept, add a test that cross-checks cannot drift from the spec again. If a hand-list is kept, add a test that cross-checks
each entry against `Opc.Ua.StatusCodes` reflection. each entry against `Opc.Ua.StatusCodes` reflection.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-22 — corrected the four mismapped `FormatStatus` codes
to their canonical `Opc.Ua.StatusCodes` values (`BadTimeout` 0x800A0000, `BadNoCommunication`
0x80310000, `BadWaitingForInitialData` 0x80320000, `BadNodeIdInvalid` 0x80330000); the CLI
project does not reference the `Opc.Ua` package so the hex literals were corrected in place
with a sync note, and `SnapshotFormatterTests` was updated with corrected expectations plus
a regression `[Theory]` asserting the pre-fix wrong names no longer apply.
### Driver.Cli.Common-002 ### Driver.Cli.Common-002

View File

@@ -103,16 +103,18 @@ public static class SnapshotFormatter
// Match the OPC UA shorthand for the statuses most-likely to land in a CLI run. // Match the OPC UA shorthand for the statuses most-likely to land in a CLI run.
// Anything outside this short-list surfaces as hex — operators can cross-reference // Anything outside this short-list surfaces as hex — operators can cross-reference
// against OPC UA Part 6 § 7.34 (StatusCode tables) or Core.Abstractions status mappers. // against OPC UA Part 6 § 7.34 (StatusCode tables) or Core.Abstractions status mappers.
// 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 name = statusCode switch
{ {
0x00000000u => "Good", 0x00000000u => "Good",
0x80000000u => "Bad", 0x80000000u => "Bad",
0x80050000u => "BadCommunicationError", 0x80050000u => "BadCommunicationError",
0x80060000u => "BadTimeout", 0x800A0000u => "BadTimeout",
0x80070000u => "BadNoCommunication", 0x80310000u => "BadNoCommunication",
0x80080000u => "BadWaitingForInitialData", 0x80320000u => "BadWaitingForInitialData",
0x80340000u => "BadNodeIdUnknown", 0x80340000u => "BadNodeIdUnknown",
0x80350000u => "BadNodeIdInvalid", 0x80330000u => "BadNodeIdInvalid",
0x80740000u => "BadTypeMismatch", 0x80740000u => "BadTypeMismatch",
0x40000000u => "Uncertain", 0x40000000u => "Uncertain",
_ => null, _ => null,

View File

@@ -25,15 +25,33 @@ public sealed class SnapshotFormatterTests
} }
[Theory] [Theory]
// Numeric codes are the canonical OPC Foundation Opc.Ua.StatusCodes values.
[InlineData(0x00000000u, "Good")] [InlineData(0x00000000u, "Good")]
[InlineData(0x80000000u, "Bad")] [InlineData(0x80000000u, "Bad")]
[InlineData(0x80050000u, "BadCommunicationError")] [InlineData(0x80050000u, "BadCommunicationError")]
[InlineData(0x80060000u, "BadTimeout")] [InlineData(0x800A0000u, "BadTimeout")]
[InlineData(0x80310000u, "BadNoCommunication")]
[InlineData(0x80320000u, "BadWaitingForInitialData")]
[InlineData(0x80340000u, "BadNodeIdUnknown")] [InlineData(0x80340000u, "BadNodeIdUnknown")]
[InlineData(0x80330000u, "BadNodeIdInvalid")]
[InlineData(0x80740000u, "BadTypeMismatch")]
[InlineData(0x40000000u, "Uncertain")] [InlineData(0x40000000u, "Uncertain")]
public void FormatStatus_names_well_known_status_codes(uint status, string expectedName) public void FormatStatus_names_well_known_status_codes(uint status, string expectedName)
{ {
SnapshotFormatter.FormatStatus(status).ShouldContain(expectedName); SnapshotFormatter.FormatStatus(status).ShouldBe($"0x{status:X8} ({expectedName})");
}
[Theory]
// Regression for Driver.Cli.Common-001: these codes were previously mapped to the
// wrong names. The hex values below are what the buggy shortlist used; they must
// now either resolve to their *correct* spec name or fall through to bare hex.
[InlineData(0x80060000u, "BadTimeout")] // was mislabelled BadTimeout
[InlineData(0x80070000u, "BadNoCommunication")] // was mislabelled BadNoCommunication
[InlineData(0x80080000u, "BadWaitingForInitialData")] // was mislabelled BadWaitingForInitialData
[InlineData(0x80350000u, "BadNodeIdInvalid")] // was mislabelled BadNodeIdInvalid
public void FormatStatus_does_not_apply_pre_fix_wrong_names(uint status, string wrongName)
{
SnapshotFormatter.FormatStatus(status).ShouldNotContain(wrongName);
} }
[Fact] [Fact]