Inspect polish: leading-whitespace pre-filter under-reports; FOCAS/MQTT advisories can swallow the rejection backstop #556

New Issue

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

dohertj2 commented

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

Two deploy-gate wording/coverage defects found during #554, both under-report-only (never over-report):

Leading whitespace defeats every Inspect's pre-filter. Seven factories (Modbus/S7/AbCip/AbLegacy/TwinCAT/FOCAS/Mqtt) test reference[0] != '{' with no TrimStart(), while their mappers have no such gate — so " {…}" is parsed (and possibly rejected) by the runtime but draws zero deploy warnings. Fix: trim before the framing check, uniformly.
FOCAS and MQTT Inspect can swallow their own backstop. Both append advisories unconditionally and then guard the catch-all rejection message on warnings.Count == 0 — so a rejection neither DescribeRejection can name is silently dropped whenever an advisory is present. The authoring gate (TagConfigParseGate, #554) is unaffected (it has its own backstop); only the deploy gate's message can go missing. Fix: gate the backstop on "no rejection-class warning" rather than "no warning at all" (requires distinguishing the two classes, which #551 already documented as distinct for FOCAS).

Per-factory structural invariant tests exist (*InspectAcceptGateTests: a rejected blob never draws zero warnings) — extend them with the whitespace shape and an advisory-plus-unnameable-rejection shape.

Two deploy-gate wording/coverage defects found during #554, both under-report-only (never over-report): 1. **Leading whitespace defeats every Inspect's pre-filter.** Seven factories (Modbus/S7/AbCip/AbLegacy/TwinCAT/FOCAS/Mqtt) test `reference[0] != '{'` with no `TrimStart()`, while their mappers have no such gate — so `" {…}"` is parsed (and possibly rejected) by the runtime but draws zero deploy warnings. Fix: trim before the framing check, uniformly. 2. **FOCAS and MQTT `Inspect` can swallow their own backstop.** Both append advisories unconditionally and then guard the catch-all rejection message on `warnings.Count == 0` — so a rejection neither `DescribeRejection` can name is silently dropped whenever an advisory is present. The authoring gate (`TagConfigParseGate`, #554) is unaffected (it has its own backstop); only the deploy gate's message can go missing. Fix: gate the backstop on "no rejection-class warning" rather than "no warning at all" (requires distinguishing the two classes, which #551 already documented as distinct for FOCAS). Per-factory structural invariant tests exist (`*InspectAcceptGateTests`: a rejected blob never draws zero warnings) — extend them with the whitespace shape and an advisory-plus-unnameable-rejection shape.

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 referenced this issue

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

MQTT runtime TagConfig mappers reject leading-whitespace JSON that every other driver tolerates #559

dohertj2 commented

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

Closed by merge be15ed16 (branch fix/556-inspect-polish), verified by the 14-suite consolidated run on the merged tree (4,583 tests, 0 failures).

Whitespace pre-filter. All seven Contracts factory Inspects now TrimStart() before the [0] != '{' framing check, so a leading-whitespace blob is judged on its content, not its first byte. Discovery during the fix: MQTT's runtime mappers uniquely also carry the framing gate, so for MQTT a leading-whitespace blob is genuinely refused at runtime — its trimmed Inspect now reports that as a real rejection (accept-gate-first symmetry holds). The other six drivers' runtimes tolerate the whitespace (JsonDocument.Parse skips it), which is a cross-driver served-behavior inconsistency out of scope here — filed as #559.

Rejection vs advisory, classified structurally. Each Inspect now builds two separate lists — rejections (parse would refuse) and advisories (parse accepts, but worth flagging) — and the "a rejected blob never draws zero warnings" backstop gates on rejections.Count == 0, not the combined count. No string matching anywhere; the classification is by which list the code appended to. This closes the advisory-swallows-backstop gap: previously an advisory on an accepted blob could satisfy the backstop vacuously.

FOCAS backstop unreachable today — its parse currently has no refusal the Inspect doesn't already name, so the backstop cannot fire; pinned as a count-relation test (documented, not deleted) so it starts failing the day FOCAS gains an unnamed refusal.

Falsification: each half neutered independently reddens exactly its own tests (trim removed ⇒ whitespace cases fail; classification collapsed to one list ⇒ backstop-vacuity controls fail); controls stay green.

Closed by merge `be15ed16` (branch `fix/556-inspect-polish`), verified by the 14-suite consolidated run on the merged tree (4,583 tests, 0 failures). **Whitespace pre-filter.** All seven Contracts factory `Inspect`s now `TrimStart()` before the `[0] != '{'` framing check, so a leading-whitespace blob is judged on its content, not its first byte. Discovery during the fix: **MQTT's runtime mappers uniquely also carry the framing gate**, so for MQTT a leading-whitespace blob is genuinely refused at runtime — its trimmed Inspect now reports that as a real rejection (accept-gate-first symmetry holds). The other six drivers' runtimes tolerate the whitespace (`JsonDocument.Parse` skips it), which is a cross-driver served-behavior inconsistency out of scope here — filed as **#559**. **Rejection vs advisory, classified structurally.** Each `Inspect` now builds two separate lists — `rejections` (parse would refuse) and `advisories` (parse accepts, but worth flagging) — and the "a rejected blob never draws zero warnings" backstop gates on `rejections.Count == 0`, not the combined count. No string matching anywhere; the classification is by which list the code appended to. This closes the advisory-swallows-backstop gap: previously an advisory on an accepted blob could satisfy the backstop vacuously. **FOCAS backstop unreachable today** — its parse currently has no refusal the Inspect doesn't already name, so the backstop cannot fire; pinned as a count-relation test (documented, not deleted) so it starts failing the day FOCAS gains an unnamed refusal. Falsification: each half neutered independently reddens exactly its own tests (trim removed ⇒ whitespace cases fail; classification collapsed to one list ⇒ backstop-vacuity controls fail); controls stay green.

dohertj2 closed this issue

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

dohertj2 referenced this issue

2026-08-10 03:41:18 -04:00

MQTT runtime TagConfig mappers reject leading-whitespace JSON that every other driver tolerates #559

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)

dohertj2 referenced this issue from a commit

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

merge: #556 + #557(Mqtt) — Inspect pre-filters trim before framing; rejection vs advisory classified structurally (backstop gates on rejections only); Mqtt union verdict kept with a mode-dependency advisory

dohertj2 referenced this issue from a commit