Skip to content

Commit 94b8c2a

Browse files
wshallwshallwshallwshall
andauthored
fix(dryrun): apply the listener's ingress guards in the preview (BACKLOG #1689, #1690) (#1169)
`dry_run` skipped the decode and post-decode guards the live listener runs before routing, so `messagefoundry check` and the Test Bench previewed RECEIVED for bodies the engine would NAK, and ERROR or mojibake for bodies it accepts. The divergence ran both ways. Extract the sequence into `pipeline/ingress_guards.py` -- decode with the connection's declared `encoding` at errors="strict", reject an embedded NUL, bound the size -- and point `dry_run` and `_dry_run_raw` at it. Resolve the peek ceiling from the connection's `max_message_bytes` instead of the bare 16 MiB default, and carry a binary content type via ADR 0028 rather than text-decoding it. `split_messages` / `read_messages` / `read_message_sets` now hand on BYTES. They decoded UTF-8 tolerantly before any inbound was in scope, so the declared charset never got a chance to apply. The boundary search moves to a lossless latin-1 byte view, which is charset-agnostic; one fixture is cross-producted against many inbounds with different encodings, so bytes is the only contract that can express the operation. An unmapped HL7 fixture no longer cross-products against a binary inbound. That question -- would this HL7 file route as a DICOM object -- has one answer and carries no information; a PINNED fixture still runs against its binary feed, exactly as one pinned to a not-deployed feed does. #1690: `disposition_for` reports NOT_DEPLOYED for a routed run whose every Send addressed a present-but-not-deployed destination, `DryRunResult` carries `declined`, the `.expect` vocabulary accepts NOT_DEPLOYED, and the dryrun CLI emits the declined names. Two live guards are deliberately not mirrored, recorded in the module docstring and the PR body: the strict-validation timeout (mirroring it forces `dry_run` async) and `_detach_documents` (async and side-effecting). The listener still carries its own inline copy of the sequence -- pointing `_handle_inbound` at the shared function is part B -- so a parity test pins the two together. Measured on this repository's own fixtures: 2/260 failing runs before, the same 2/220 after. Every surviving pair keeps its before-state disposition; the 40 dropped runs were all UNROUTED on the two binary feeds. Co-authored-by: wshallwshall <mefordev@messagefoundry.org>
1 parent c0ae68c commit 94b8c2a

7 files changed

Lines changed: 689 additions & 39 deletions

File tree

messagefoundry/__main__.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3629,6 +3629,11 @@ def _dryrun(args: argparse.Namespace) -> int:
36293629
{"to": d.to, "payload": d.payload if show_phi else _redact_body(d.payload)}
36303630
for d in result.deliveries
36313631
],
3632+
# Destinations a Handler addressed that are present but NOT deployed (#233,
3633+
# BACKLOG #1690). Printed beside `deliveries` because that list is empty for
3634+
# exactly these, and without the names a NOT_DEPLOYED row says what happened but
3635+
# not to which connection. Connection names carry no PHI, so no --show-phi gate.
3636+
"declined": result.declined,
36323637
# Declared state writes (ADR 0005). The value can be PHI (e.g. an MRN→anon
36333638
# mapping), so gate it behind --show-phi exactly like a delivery payload.
36343639
"state_ops": [

messagefoundry/checks.py

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,9 +1101,11 @@ def _check_validate(config_dir: str | Path) -> CheckResult:
11011101
# Executable acceptance criteria for dry-run fixtures (Secure Development Standards §5): a fixture may
11021102
# declare its expected dry-run disposition in a sibling ``<fixture>.expect`` file. ``dry_run`` reports
11031103
# ``RECEIVED`` (would route + deliver), ``UNROUTED`` (no handler matched), ``FILTERED`` (a handler ran
1104-
# but delivered nothing), or ``ERROR`` (parse/validate/router-handler failure). ``PROCESSED``/``ROUTED``
1105-
# are live-only post-delivery states, so they alias to ``RECEIVED`` for authoring ergonomics.
1106-
_DRYRUN_DISPOSITIONS = frozenset({"RECEIVED", "UNROUTED", "FILTERED", "ERROR"})
1104+
# but delivered nothing), ``NOT_DEPLOYED`` (a handler ran and every Send it produced addressed a
1105+
# present-but-not-deployed destination — #233, BACKLOG #1690), or ``ERROR`` (parse/validate/
1106+
# router-handler failure). ``PROCESSED``/``ROUTED`` are live-only post-delivery states, so they alias
1107+
# to ``RECEIVED`` for authoring ergonomics.
1108+
_DRYRUN_DISPOSITIONS = frozenset({"RECEIVED", "UNROUTED", "FILTERED", "NOT_DEPLOYED", "ERROR"})
11071109
_DISPOSITION_ALIASES = {
11081110
"PROCESSED": "RECEIVED",
11091111
"ROUTED": "RECEIVED",
@@ -1115,7 +1117,7 @@ def _check_validate(config_dir: str | Path) -> CheckResult:
11151117
def _expected_disposition(fixture_path: str | Path) -> str | None:
11161118
"""Read an optional ``<fixture>.expect`` sidecar declaring the expected dry-run disposition.
11171119
1118-
Returns the normalized disposition name (``RECEIVED``/``UNROUTED``/``FILTERED``/``ERROR``), or
1120+
Returns the normalized disposition name (one of :data:`_DRYRUN_DISPOSITIONS`), or
11191121
``None`` when no sidecar exists — then the fixture keeps the default "must not ERROR" semantics.
11201122
Raises ``ValueError`` for an unreadable or unrecognized declaration (a fixture-authoring mistake).
11211123
"""
@@ -1216,11 +1218,24 @@ def _check_dryrun(
12161218
# not-deployed feed must still resolve to that feed, or it would silently become "unmapped" and be
12171219
# cross-producted against every OTHER feed — worse than the problem. It is the cross-product target
12181220
# list that drops the not-deployed feeds: an unmapped fixture must not be run against a feed nobody
1219-
# deployed (its Sends are declined, so it would report FILTERED and fail a .expect). An explicitly
1221+
# deployed (its Sends are declined, so it would report NOT_DEPLOYED — truthfully since BACKLOG
1222+
# #1690, and still not what a fixture written for the OTHER feeds declared). An explicitly
12201223
# PINNED fixture still runs against its not-deployed feed — carrying the record is the point of the
12211224
# state, and dry-run resolves no env(), so previewing its router/handler logic stays free.
1225+
#
1226+
# A **binary** feed (BINARY, DICOM) leaves the cross-product for the same reason and on the same
1227+
# terms (BACKLOG #1689). `read_message_sets` reads `*.hl7` files, and a binary inbound base64-
1228+
# carries its bytes rather than decoding them (ADR 0028), so running an unmapped HL7 fixture
1229+
# against one asks "would this HL7 file route as a DICOM object" — a question whose answer is
1230+
# always no and which tells an author nothing about either feed. It became visible only when the
1231+
# preview started carrying bytes the way the listener does: the text-decoded body used to miss the
1232+
# feed's own `is_binary` guard and report a placid UNROUTED, where the engine would have carried
1233+
# it, failed the codec, and dead-lettered it. A PINNED fixture still runs against its binary feed,
1234+
# exactly as one pinned to a not-deployed feed does.
12221235
inbound_names = list(reg.inbound)
1223-
deployed_inbounds = [n for n, ic in reg.inbound.items() if ic.deployed]
1236+
crossproduct_inbounds = [
1237+
n for n, ic in reg.inbound.items() if ic.deployed and not ic.content_type.is_binary
1238+
]
12241239
message_sets = read_message_sets(mpath, inbound_names)
12251240
# #230 P4 (ADR 0104): preview under the engine's copy-on-Send posture (best-effort; fallback = the
12261241
# Settings-model default, ON) so the gate exercises the fixtures exactly as the engine would run them.
@@ -1239,7 +1254,7 @@ def _check_dryrun(
12391254
except ValueError as exc:
12401255
errors.append(f"{label}: {exc}")
12411256
continue
1242-
targets = [target] if target is not None else deployed_inbounds
1257+
targets = [target] if target is not None else crossproduct_inbounds
12431258
if target is not None:
12441259
pinned += 1
12451260
for ic_name in targets:
@@ -1263,16 +1278,17 @@ def _check_dryrun(
12631278
# check claiming a pass over a verification it never performed. Every sibling marks "I
12641279
# established nothing" with `skipped=True`, which `CheckResult.blocking` excludes; this was
12651280
# the one path reaching a non-skipped success on zero work. Keep it a postcondition on
1266-
# `total`: an equivalent precondition on `deployed_inbounds` would have to be kept in
1281+
# `total`: an equivalent precondition on `crossproduct_inbounds` would have to be kept in
12671282
# lockstep with the loop's branching, and it would miss any other path to zero.
12681283
#
12691284
# `read_message_sets` only ever pins a fixture to a name drawn from `reg.inbound`, so a
12701285
# pinned fixture always contributes a run — reaching here means every fixture is unmapped
1271-
# AND nothing is deployed. The counts below are read, not inferred, so the detail stays
1272-
# true even if some later path arrives here for a different reason.
1286+
# AND no inbound is eligible for the cross-product. The counts below are read, not inferred,
1287+
# so the detail stays true even if some later path arrives here for a different reason.
12731288
detail = (
12741289
f"{len(message_sets)} fixture(s) read but 0 dry-run(s) executed — only "
1275-
f"{len(deployed_inbounds)} of {len(inbound_names)} inbound(s) are deployed and no "
1290+
f"{len(crossproduct_inbounds)} of {len(inbound_names)} inbound(s) take an unmapped "
1291+
f"fixture (the rest are not deployed, or carry a binary content type) and no "
12761292
f"fixture is feed-pinned, so every target list was empty"
12771293
)
12781294
return CheckResult("dryrun", ok=False, required=True, detail=detail)

0 commit comments

Comments
 (0)