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

Closed
opened 2026-08-10 01:24:31 -04:00 by dohertj2 · 1 comment
Owner

Found during #553. The driver *TagConfigModel.Validate() methods judge enum keys (via the runtime classifier, #549) and a few required fields, but non-enum value rules are unchecked at authoring: a CSV/API blob with Modbus "address":"notanumber" (previously silently laundered to 0; since #553 honestly preserved) is persisted by RawTreeService's gate and only refused at the deploy gate — where the accept-gate-first Inspect (#551) names it — or dies at the driver.

Not a silent-death class (the deploy gate covers it since #551), just feedback latency: the operator learns at deploy time instead of at save. Closing it means the models consulting the same runtime parse the Inspects wrap — the established one-authority shape — for the non-enum rejection surface (absent/non-numeric/out-of-range addresses, S7 stringLength bounds, etc.). Fleet-wide survey required first (the #549/#551 lesson: the issue names N-1 of N).

Found during #553. The driver `*TagConfigModel.Validate()` methods judge enum keys (via the runtime classifier, #549) and a few required fields, but non-enum value rules are unchecked at authoring: a CSV/API blob with Modbus `"address":"notanumber"` (previously silently laundered to `0`; since #553 honestly preserved) is persisted by `RawTreeService`'s gate and only refused at the deploy gate — where the accept-gate-first `Inspect` (#551) names it — or dies at the driver. Not a silent-death class (the deploy gate covers it since #551), just feedback latency: the operator learns at deploy time instead of at save. Closing it means the models consulting the same runtime parse the Inspects wrap — the established one-authority shape — for the non-enum rejection surface (absent/non-numeric/out-of-range addresses, S7 stringLength bounds, etc.). Fleet-wide survey required first (the #549/#551 lesson: the issue names N-1 of N).
Author
Owner

FIXED — merged to master as 7cfe7b4a (branch commit 5ca4f804).

Every mapped *TagConfigModel.Validate() now consults its driver's own runtime parse on the as-loaded blob via the new TagConfigParseGate: verdict from the parse, wording from the driver's Inspect — one authority, no Contracts factory modified, no rule restated. Two pinned design points: the gate is the PARSE verdict, never "Inspect said something" (FOCAS/MQTT/Calculation advisories on accepted blobs must not become authoring errors — the #520/#535 stricter-than-runtime trap); and the gate judges the SOURCE blob, not the leniently-loaded typed fields (which is what hid these defects).

Fleet survey (11 drivers): gaps closed in Modbus (absent region; absent/non-numeric/fractional/out-of-range address; stringLength<1; bitIndex>15; and {} itself), S7 (stringLength outside 0–254), TwinCAT (dataType:"Structure" — a valid enum name the reader refuses; invisible to any spelling check), Sql (the recognition rule: no model key and no driver marker), Mqtt ("qos":null). AbCip/AbLegacy/FOCAS/MTConnect/Calculation/OpcUaClient were already at parity — each got the parse-consult backstop anyway. Deliberately-stricter editor rules kept and pinned by name (Mqtt wildcards, Calculation triggers, FOCAS unparseable-address, AbLegacy's arrayLength>256 — kept because the runtime clamps while the materialiser advertises the raw number in ArrayDimensions, so the authored value is silently not honoured; refusing at author-time is the honest treatment).

Behaviour change worth knowing: an untouched {} Modbus config is now refused at save — it was the only mapped editor with no required field, and a {} tag reads BadNodeIdUnknown forever with the deployment green.

Falsified: neutering only the Modbus consult → exactly 14 red (its rejection theory + parity cases + the coverage guard), every #549/#550/#551 enum test green, no other driver moved. Anti-vacuity guards included (the corpus must exercise both verdicts per driver; a deliberately-stricter exemption that stops being one FAILS). Verified on the merged tree: 14 suites / 5,251 tests green.

Residuals found en route, filed: #556 (Inspect wording polish), #557 (reader asymmetries).

**FIXED** — merged to master as `7cfe7b4a` (branch commit `5ca4f804`). Every mapped `*TagConfigModel.Validate()` now consults its driver's own runtime parse on the as-loaded blob via the new `TagConfigParseGate`: **verdict from the parse, wording from the driver's `Inspect`** — one authority, no Contracts factory modified, no rule restated. Two pinned design points: the gate is the PARSE verdict, never "Inspect said something" (FOCAS/MQTT/Calculation advisories on *accepted* blobs must not become authoring errors — the #520/#535 stricter-than-runtime trap); and the gate judges the SOURCE blob, not the leniently-loaded typed fields (which is what hid these defects). Fleet survey (11 drivers): gaps closed in **Modbus** (absent `region`; absent/non-numeric/fractional/out-of-range `address`; `stringLength<1`; `bitIndex>15`; and `{}` itself), **S7** (`stringLength` outside 0–254), **TwinCAT** (`dataType:"Structure"` — a valid enum name the reader refuses; invisible to any spelling check), **Sql** (the recognition rule: no `model` key and no driver marker), **Mqtt** (`"qos":null`). AbCip/AbLegacy/FOCAS/MTConnect/Calculation/OpcUaClient were already at parity — each got the parse-consult backstop anyway. Deliberately-stricter editor rules kept and pinned by name (Mqtt wildcards, Calculation triggers, FOCAS unparseable-address, AbLegacy's `arrayLength>256` — kept because the runtime *clamps* while the materialiser advertises the raw number in `ArrayDimensions`, so the authored value is silently not honoured; refusing at author-time is the honest treatment). Behaviour change worth knowing: **an untouched `{}` Modbus config is now refused at save** — it was the only mapped editor with no required field, and a `{}` tag reads `BadNodeIdUnknown` forever with the deployment green. Falsified: neutering only the Modbus consult → exactly 14 red (its rejection theory + parity cases + the coverage guard), every #549/#550/#551 enum test green, no other driver moved. Anti-vacuity guards included (the corpus must exercise both verdicts per driver; a deliberately-stricter exemption that stops being one FAILS). Verified on the merged tree: 14 suites / 5,251 tests green. Residuals found en route, filed: #556 (Inspect wording polish), #557 (reader asymmetries).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#554