Reader asymmetries: OpcUaClient legacy PascalCase NodeId is readable but unsaveable; Mqtt Sparkplug parse accepts a payloadFormat the Plain parse rejects #557

New Issue

2026-08-10T02:01:35-04:00

dohertj2 commented

2026-08-10 02:01:35 -04:00

Two runtime-reader asymmetries found during #554, both pre-existing and now explicitly exempted-by-name in the parity tests:

OpcUaClient: TryReadNodeId still honours a legacy PascalCase NodeId key when camelCase nodeId is wholly absent, but OpcUaClientTagConfigModel reads only nodeId — so a legacy blob is served by the driver yet cannot be saved untouched through the editor (authoring stricter than reader). Options: model reads both spellings at the reader's precedence and normalises on save (the #550 pattern), or the legacy spelling is formally retired from the reader with a migration note.
Mqtt: the Sparkplug-mode parse accepts an invalid payloadFormat that the Plain-mode parse rejects, and the union verdict (used by both Inspect and the #554 authoring gate — both gates agree) therefore admits it. payloadFormat is meaningless in Sparkplug mode, so accepting it is arguably correct — but then the PLAIN parse rejecting it while the union admits it means the served behavior depends on the driver's mode config, which no tag-level gate can see. Decide the intended contract and pin it.

Also worth folding in if touched: OpcUaClientDriverBrowser.DriverType and GalaxyDriverBrowser.DriverType return string literals rather than DriverTypeNames constants (the drift class the constants exist to prevent; harmless today, unguarded) — see #558 for the guard side.

Two runtime-reader asymmetries found during #554, both pre-existing and now explicitly exempted-by-name in the parity tests: 1. **OpcUaClient**: `TryReadNodeId` still honours a legacy PascalCase `NodeId` key when camelCase `nodeId` is wholly absent, but `OpcUaClientTagConfigModel` reads only `nodeId` — so a legacy blob is served by the driver yet cannot be saved untouched through the editor (authoring stricter than reader). Options: model reads both spellings at the reader's precedence and normalises on save (the #550 pattern), or the legacy spelling is formally retired from the reader with a migration note. 2. **Mqtt**: the Sparkplug-mode parse accepts an invalid `payloadFormat` that the Plain-mode parse rejects, and the union verdict (used by both `Inspect` and the #554 authoring gate — both gates agree) therefore admits it. `payloadFormat` is meaningless in Sparkplug mode, so accepting it is arguably correct — but then the PLAIN parse rejecting it while the union admits it means the served behavior depends on the driver's mode config, which no tag-level gate can see. Decide the intended contract and pin it. Also worth folding in if touched: `OpcUaClientDriverBrowser.DriverType` and `GalaxyDriverBrowser.DriverType` return string literals rather than `DriverTypeNames` constants (the drift class the constants exist to prevent; harmless today, unguarded) — see #558 for the guard side.

dohertj2 referenced this issue

2026-08-10 02:01:35 -04:00

Authoring models validate only enum keys — non-enum TagConfig keys (e.g. Modbus address) meet no gate until deploy #554

dohertj2 commented

2026-08-10 03:15:24 -04:00

Closed by merges 93a05828 (branch fix/557-opcuaclient-legacy-nodeid — OpcUaClient half) and be15ed16 (branch fix/556-inspect-polish — Mqtt half), verified by the 14-suite consolidated run (4,583 tests, 0 failures).

OpcUaClient legacy NodeId readable-but-unsaveable — FIXED. OpcUaClientTagConfigModel gained a NodeIdKeys list + ReadNodeId implementing the runtime's presence-shadows semantics — TryGetProperty("nodeId", out n) || TryGetProperty("NodeId", out n) then one ValueKind==String + non-blank check. This is deliberately NOT MTConnect's FirstNonBlank: a present-but-blank or non-string nodeId short-circuits the || and blocks fallback to the legacy spelling, so blank-nodeId-beside-valid-NodeId is a REJECTED blob at runtime and the model now agrees. ToJson drops non-canonical aliases keyed off the SAME NodeIdKeys array (the #550 pattern — read and drop cannot drift). The OpcUaClient exemption in TagConfigNonEnumParityTests is removed (the anti-vacuity control Every_deliberately_stricter_entry_is_still_exactly_that would have failed had it gone stale), and a precedence theory pins the presence-shadows cases including the blank-shadows-valid one.

Mqtt union-parse asymmetry — contract DECIDED, not changed. The union verdict stands: a blob accepted by either runtime mapper is accepted, because FromSparkplugTagConfig hardcodes PayloadFormat.Json and never reads the payloadFormat key — so a typo'd payloadFormat on a sparkplug-shaped blob is genuinely servable, and refusing it at authoring/Inspect would be stricter than runtime (the exact drift class this arc exists to kill). Mitigation: a mode-dependency advisory rides the accepted blob, stating the key only takes effect in plain-MQTT mode. Advisories ride accepted blobs by design (#554), so this never trips the rejection backstop.

Closed by merges `93a05828` (branch `fix/557-opcuaclient-legacy-nodeid` — OpcUaClient half) and `be15ed16` (branch `fix/556-inspect-polish` — Mqtt half), verified by the 14-suite consolidated run (4,583 tests, 0 failures). **OpcUaClient legacy `NodeId` readable-but-unsaveable — FIXED.** `OpcUaClientTagConfigModel` gained a `NodeIdKeys` list + `ReadNodeId` implementing the runtime's **presence-shadows** semantics — `TryGetProperty("nodeId", out n) || TryGetProperty("NodeId", out n)` then one ValueKind==String + non-blank check. This is deliberately NOT MTConnect's `FirstNonBlank`: a *present-but-blank or non-string* `nodeId` short-circuits the `||` and blocks fallback to the legacy spelling, so blank-`nodeId`-beside-valid-`NodeId` is a REJECTED blob at runtime and the model now agrees. `ToJson` drops non-canonical aliases keyed off the SAME `NodeIdKeys` array (the #550 pattern — read and drop cannot drift). The OpcUaClient exemption in `TagConfigNonEnumParityTests` is removed (the anti-vacuity control `Every_deliberately_stricter_entry_is_still_exactly_that` would have failed had it gone stale), and a precedence theory pins the presence-shadows cases including the blank-shadows-valid one. **Mqtt union-parse asymmetry — contract DECIDED, not changed.** The union verdict stands: a blob accepted by *either* runtime mapper is accepted, because `FromSparkplugTagConfig` hardcodes `PayloadFormat.Json` and **never reads the `payloadFormat` key** — so a typo'd `payloadFormat` on a sparkplug-shaped blob is genuinely servable, and refusing it at authoring/Inspect would be stricter than runtime (the exact drift class this arc exists to kill). Mitigation: a **mode-dependency advisory** rides the accepted blob, stating the key only takes effect in plain-MQTT mode. Advisories ride accepted blobs by design (#554), so this never trips the rejection backstop.

dohertj2 closed this issue

2026-08-10 03:15:24 -04:00

dohertj2 referenced this issue from a commit

2026-08-10 04:37:34 -04:00

fix(adminui): OpcUaClient editor reads the legacy PascalCase NodeId at the reader's precedence (#557 item 1)

dohertj2 referenced this issue from a commit

2026-08-10 04:37:34 -04:00

merge: #557 (OpcUaClient half) — editor reads the legacy PascalCase NodeId at the reader's presence-shadows precedence and drops it on save; #554 stale exemption removed with the anti-vacuity guard as control

dohertj2 referenced this issue from a commit

2026-08-10 04:37:34 -04:00

fix(inspect): trim before the framing check; stop advisories swallowing the backstop (#556); pin the MQTT payloadFormat mode contract (#557 item 2)

Sign in to join this conversation.