Repository navigation
Batch 265 wave A: X12 trailer and the frame deadline, admission epoch, disabled-cap warnings, negative min_age refused, 0x20 frame byte refused (BACKLOG #2938 #2939 #2749 #3151 #2900) - #2353
Conversation
…r_only (BACKLOG #2938) A newline after an X12 interchange's IEA terminator that arrived in a read of its own decoded nothing, so the shared frame clock started on it and a quiet, healthy peer would be cut as frame_deadline at max_frame_seconds. X12FrameReader now carries an eagerly computed trailer_only, set when feed() is called: true only for a read holding nothing but CR/LF bytes, at most two in all, directly after a completed interchange's full terminator. Any other byte ends the allowance, so a peer cannot trickle line ends to hold the deadline off. The rule mirrors FrameDecoder's, with local constants so the parsing layer imports nothing from framing.py. The flag only observes: the bytes still reach the buffer, so the 2-byte noise tail, the #1595 resume points, in_frame and the reuse path's leftover reading are unchanged. The X12 listener passes it to FrameClock.after_read. Tests: x12 rows (CR LF alone, LF after a CR sent with the IEA, and the controls Z and three line ends, which still cut) in the shared line-end test; the two non-control rows fail on the unfixed wiring. A reader unit test in tests/test_x12_parsing.py.
A /simplify pass: both callers of the counting helper only asked whether every byte was a line end, so the helper is now that yes/no check. The length test runs first, so a large leftover buffer is never copied. The reset of the tail allowance in _clear() is dropped: no path reaches it with an allowance left, because a non-zero allowance means no interchange is open to overflow the cap. Behaviour is unchanged; the tests from the previous commit pin it.
…gler cannot undercount a restarted listener's per-host cap (BACKLOG #2939) ListenerAdmission.admit() now returns an epoch, and release(peer_host, epoch) requires it, with no default, so mypy strict flags any call site that leaves it out. reset() advances the epoch only after its "not stopping" guard, so a late stop() that skips its reset leaves earlier releases counting, as they must: the tables still hold their slots. A release from an older epoch still gives back `active`, which reset() never clears (the socket was open and counted until then), and leaves per_host and host_capacity_warned alone. That answers the row's closing step 3 on `active`. All four listeners (MLLP, raw TCP, X12, HTTP) keep the token from admit and pass it to release; in http_listener.py only the admit and release lines in _on_client changed. The MLLPSource.stop() comment that called this "a separate change" now says what the epoch does. Tests: restart, refill the host to its cap, release the pre-restart token: the count is unchanged and check() still refuses (fails without the epoch check); and a skipped reset() followed by an old-token release still lowers the count (fails if the epoch advances before the guard). BACKLOG #2938 and BACKLOG #2939 ride this branch. Proposed PR title: X12 trailer newline no longer starts the frame deadline; a post-restart straggler no longer undercounts the per-host cap (BACKLOG #2938 #2939) Proposed banner, #2938: SHIPPED -- X12FrameReader exposes trailer_only, set eagerly in feed(): true only for a read of at most two CR/LF bytes directly after a completed interchange's full terminator; any other byte ends the allowance. The X12 listener passes it to FrameClock.after_read. x12 rows, with controls (a non-line-end byte, three line ends), added to the shared line-end test in tests/test_listener_admission.py; a reader unit test in tests/test_x12_parsing.py. framing.py untouched. Proposed banner, #2939: SHIPPED -- ListenerAdmission.admit returns an epoch, release requires it, reset advances it past its stopping guard. A release from an earlier epoch lowers `active` only (closing step 3: `active` counts open sockets across runs, so it still drops). All four listeners pass the token. Tests pin the restart-refill-stale-release case and the skipped-reset case.
A reply is neutralised by replacing each frame byte with a space, so a space frame byte would be replaced by itself and the reply could not go as one frame. FrameCodec.__post_init__ now refuses 0x20 for start and end; a 0x20 trailer stays allowed. ADR 0205's reply rule is unchanged. - framing.py: the refusal, the reason once in the class docstring, the frame_neutralised caveat made unconditional, b" " as one constant. - tests: refused directly, via _codec_from_settings and at TcpSource/TcpDestination build, with 0x21 as the control; a neutralised reply decodes as one frame. - CONNECTIONS.md TCP start/end/trailer row, changelog fragment. Proposed PR title: FrameCodec refuses 0x20 as a start or end frame byte (BACKLOG #2900) Proposed banner: CLOSED -- option A, the Manager's decision (batch 265): 0x20 is refused as a start or end byte at codec construction; a 0x20 trailer stays allowed; ADR 0205 unamended.
…onds (BACKLOG #2749 #3151) BACKLOG #2749: HttpSource max_body_bytes, FileSource max_file_bytes and RemoteFileSource max_file_bytes set to 0/None now log a WARNING when the connector is built, naming the connection and the setting, never the value. Same shape as the PR 2292 max_decompressed_bytes warning (not rebuilt). Warn, no refusal, no security_loosenings entry, per the owner's 2026-09-27 ruling on #2006 ("Once the engine at least warns, R5 applies"). Each warning test has a control that the default cap still refuses an over-size input. CONNECTIONS.md cells updated. BACKLOG #3151: FileSource refuses a negative min_age_seconds (value withheld); zero stays the default. The processed_files SQL comment and ADR 0129 section 2 (dated correction) now describe the remote key with the listed mtime (#2758). Timer interval_seconds and the File NaN/inf half were already done by PR 2277. Proposed PR title: Disabled size caps warn at build, File refuses a negative min_age_seconds (BACKLOG #2749 #3151) Banner #2749: CLOSED -- the HTTP max_body_bytes and File/RemoteFile max_file_bytes caps warn at build when turned off; max_decompressed_bytes was #1129's. The 5.2.1 and 2.3.2 re-read is vault work after merge. Banner #3151: CLOSED -- a negative File min_age_seconds is refused at build; the store comment and ADR 0129 describe the remote mtime key. Timer interval_seconds was done by PR 2277.
… read, an end-to-end straggler test - docs/CONNECTIONS.md: the max_frame_seconds bullet that called the X12 newline-after-IEA a known gap now states the two-byte allowance (#2938). - X12FrameReader.feed(b"") leaves the line-end allowance alone, as FrameDecoder does; the reader test pins it. The comment on the local tail constants now gives the real reason they are copies: parsing may not import messagefoundry.framing (tests/test_dependency_boundaries.py). feed() says the previous call's generator must be drained first. - ListenerAdmission.release states, once, the limit the epoch leaves: while a straggler is still open its host may hold one socket over max_connections_per_host, since reset() already dropped its slot. The MLLP stop() comment links to it rather than restating it. - New test, mllp/tcp/x12: a client held in its handler outlives a stop() cut short, the same instance restarts and re-admits the host, then the straggler releases; the restarted count still holds its connection. Each listener fails it when its release passes the current epoch instead of its own. Proposed PR title: X12 trailer newline no longer starts the frame deadline; a post-restart straggler no longer undercounts the per-host cap (BACKLOG #2938 #2939) Proposed banner, BACKLOG #2938: SHIPPED -- X12FrameReader exposes trailer_only, set eagerly in feed(): true only for a read of at most two CR/LF bytes directly after a completed interchange's full terminator; any other byte ends the allowance. The X12 listener passes it to FrameClock.after_read. x12 rows with controls (a non-line-end byte, three line ends) in the shared line-end test; a reader unit test; docs/CONNECTIONS.md updated. framing.py untouched. Left open: the outbound reuse path's leftover flag still reads a trailing CR/LF as a desync (out of scope by Manager decision). Proposed banner, BACKLOG #2939: SHIPPED -- ListenerAdmission.admit returns an epoch, release requires it, reset advances it past its stopping guard. A release from an earlier epoch lowers `active` only (closing step 3: `active` counts open sockets across runs, so it still drops). All four listeners pass the token; MLLP, raw TCP and X12 are pinned end to end, HTTP by mypy only. Left open: a live straggler is not counted per host after the reset, so its host may hold one socket over the cap until it closes.
|
QA (lane ADM, BACKLOG #2938 #2939, head 389c4a6): code-review skill at level xhigh (BUILDER.md step 11), args "xhigh Review this branch's diff against origin/main; read docs/REVIEW-STANDARDS.md and apply its rules." Two rounds (the cap). Tag: none printed; the skill returned only a JSON findings array. Round 1 fixed: CONNECTIONS.md gap, over-read MLLP comment, missing listener-level test, interchange comment rationale, drain contract. Round 2 fixed: empty-feed allowance, layering claim corrected to the real allowlist rule, limit stated once. Shipped open: findings 1-4 in the PR body. |
|
QA (lane W, BACKLOG #2749 #3151, head 155b860): code-review skill at level xhigh (BUILDER.md step 11), args "xhigh Review this branch's diff against origin/main; read docs/REVIEW-STANDARDS.md and apply its rules." Two rounds. Tag: none printed; each run reported "Skill code-review completed (forked execution)". Round 1: no blocking bug; fixed warning placement, doc wording, ADR correction trim, File bullet, HTTP control on the real listener. Round 2: none that crashes; fixed comment claim, "0" spelling in tests, changelog fragments, doc wording, test id. Shipped open: findings 1-5 in the PR body. |
|
QA (lane S, BACKLOG #2900, head 53c4da3): code-review skill at level xhigh (BUILDER.md step 11), args "xhigh Review this branch's diff against origin/main; read docs/REVIEW-STANDARDS.md and apply its rules." Two rounds. Tag: none printed; it returned a findings list each time. Round 1 fixed: decode round-trip test, build-path tests, changelog fragment, refusal text for both directions, reason moved to class docstring, over-broad claim. Round 2: prose-only fixes. Shipped open: findings 1-5 in the PR body. |
|
Lander: the #2900 option A decision stands, under the owner's standing driver rule. An adversarial review upheld it: option A only adds a build-time refusal of 0x20 as a start or end byte, so no working HL7 configuration is lost and no control weakens. S finding 1 exists on main already (trailer == start mis-splits frames on both main and d8b82b2), so this merge does not ship it; it and finding 2 become ledger rows. To land, this PR needs: all its open findings filed; a QA line naming the merge head; and green required checks. |
|
QA (merge head, PR 2353): on head d8b82b2 (batch/b265-a, merge base eebd8d2), the code-review skill ran at level xhigh (BUILDER.md step 11) over Two real defects are already on main eebd8d2 and pass through unchanged, in code the diff does not touch:
The other eight: Rows worth filing: 1, 2, 3, 5 and 10. None needs an owner ruling. |
|
Lander: enqueuing at pinned head d8b82b2. Review covers the head: the three lane heads (389c4a6, 155b860, 53c4da3) carry QA, the head is three clean merges onto main (merge-tree reproduces each tree exactly), and a merge-head QA comment exists. Option A was decided under the owner's driver rule after an adversarial review. Every open finding is filed (vault #3742, #3743, #3744; PR 2499 merged). Required checks are green; only 'ASVS anchors (advisory)' is red. |
1 participant
Batch 265 (transports), wave A: five items from three Builder branches. Every item is reversible by a hand revert; none acts outside the commit.
BACKLOG #2938 -- an X12 trailing newline no longer starts the frame deadline
Merge danger
Reversible: yes -- hand revert of the #2938 commits (4da73c8, 84159c4, part of 389c4a6).
Blast radius: the X12 inbound listener's frame clock (
parsing/x12/interchange.pyX12FrameReader.trailer_only, oneafter_readcall intransports/x12.py), one paragraph of docs/CONNECTIONS.md.Evidence
test_line_end_bytes_ending_a_frame_do_not_start_the_frame_deadlinefail with the wiring reverted totrailer_only=False.Z,\r\n\r) are still cut.BACKLOG #2939 -- a post-restart straggler no longer undercounts the per-host cap
Merge danger
Reversible: yes -- hand revert of 3a9e470 and its part of 389c4a6.
Blast radius:
ListenerAdmission(admit returns an epoch, release requires it) and the admit/release call sites in all four listeners (MLLP, raw TCP, X12, HTTP).Evidence
check()admits one over the cap. The MLLP/TCP/X12 end-to-end straggler test fails when release passes the current epoch.check()still refuses; a skippedreset()followed by an old-token release still lowers the count (fails if the epoch advances before the guard).BACKLOG #2749 -- a disabled size cap warns at build
Merge danger
Reversible: yes -- hand revert of 155b860's #2749 part.
Blast radius: construction-time WARNING logs in
FileSource,RemoteFileSource,HttpSource.__init__; three docs/CONNECTIONS.md cells. No refusal, no clamp, no change to base.py or settings.py. Decision: WARN, under the owner ruling of 2026-09-27 quoted in #2006.Evidence
BACKLOG #3151 -- File refuses a negative min_age_seconds; remote key wording corrected
Merge danger
Reversible: yes -- hand revert of 155b860's #3151 part.
Blast radius: File source build validation for
min_age_seconds(negative only; 0 and -0 still build), a store.py SQL comment (not DDL), a dated correction note in ADR 0129 section 2.Evidence
min_age_secondsbuilds.BACKLOG #2900 -- FrameCodec refuses 0x20 as a start or end byte
Merge danger
Reversible: yes -- hand revert of 53c4da3.
Blast radius:
FrameCodec.__post_init__in messagefoundry/framing.py (TCP framing config validation), the TCP start/end/trailer row of docs/CONNECTIONS.md. A 0x20 trailer stays allowed. ADR 0205 untouched. Decision: option A, the Manager's (the row reserved only options B/C for an owner ruling).Evidence
_codec_from_settings, and at TcpSource/TcpDestination build; 0x21 control builds; a neutralised reply decodes as exactly one payload.Combined tree (batch/b265-a at d8b82b2, merged onto origin/main eebd8d2)
ruff check .,ruff format --check .: clean.scripts/hooks/ledger_check.py --ci: rc 0.Builder reports
Lane ADM -- BACKLOG #2938, #2939
Source branch
b265-2938-2939, head389c4a65da0ead8185e511ea825c0b3a7a145565(4 commits on origin/main 8ba57ee).X12FrameReader.trailer_onlyset eagerly infeed(): true only for a read of at most 2 CR/LF bytes directly after a completed interchange's full terminator; any other byte resets the allowance; an empty read is not trailer-only. Local constants;parsingmay not importmessagefoundry.framingunder the dependency-boundaries allowlist. Bytes still go to_buf, so the IS noise tail,_seg_start/_scan,in_frameand the reuse leftover flag are unchanged.admit()returnsself._epoch;release(peer_host, epoch)requires it;reset()advances it only after its stopping guard. A stale release lowersactiveonly. All four listeners pass the token; every_admission.releasesite checked by grep and mypy strict.tests/test_sandbox.pytiming tests failed intermittently under load (a different set each run; the file imports nothing changed). Full suite not run._read_reply_reusein x12.py still reports leftover=True when a reply ends~\r\n; a persistent X12 destination would reconnect on every send for such a partner.reset(), a still-live straggler is not counted per host, so its host may hold one socket overmax_connections_per_hostuntil it closes. Stated in thereleasedocstring; docs/CONNECTIONS.md (about line 3600) lacks the caveat.epochin http_on_clientis unbound on refusal paths (safe today, a reorder would raise UnboundLocalError).activeonly. All four listeners pass the token; MLLP, raw TCP and X12 are pinned end to end, HTTP by mypy only. Left open: a live straggler is not counted per host after the reset, so its host may hold one socket over the cap until it closes.Lane W -- BACKLOG #2749, #3151
Source branch
b265-2749-3151, head155b8602fcb81ba5ec491af0fe54a03565e1c14b(1 commit on origin/main 8ba57ee).__init__so a source a later check refuses has not logged it. The fix(verify): the self smoke passed on a message the config would have dropped (BACKLOG #1707) #1129 max_decompressed_bytes warning (PR 2292) was not rebuilt. CONNECTIONS.md cells and the File "Maximum size" bullet updated. The vault record re-read waits on this merge.max_frame_bytesand X12max_interchange_bytesstill turn off silently (candidate row).min_age_secondsis accepted and would ingest nothing.min_age_secondsfrom env() is refused while caps read "" as unset (predates this change).is_file_processeddocstring in store/base.py still describes only the local key.Lane S -- BACKLOG #2900
Source branch
b265-2900, head53c4da33a3e1d5bf38d36e6d654eb416cca98010(1 commit on origin/main 8ba57ee).FrameCodec.__post_init__refuses 0x20 for start and end after the existing checks; the message names the setting. Reason stated once in the class docstring;frame_neutralisedcaveat unconditional;_SPACEmodule constant. ADR 0205, AC-9 and tests/test_harness_frame_bytes.py untouched. Added changelog.d/2900.changed.md (review round 1 flagged its absence).tests/test_sandbox.pytiming failures and 1 load error that passed alone. Full suite not run.FrameCodec(start=0x02, end=0x03, trailer=0x02)decodes three framed messages as[b'AAA', b'\x02BBB', b'\x02CCC']and leavesin_frameTrue. Unfiled; needs a row and a decision.Tcp()docs in messagefoundry/config/wiring.py (about line 2303), which feed the VS Code connection editor help, do not mention the refusal.neutraliseinserts raw 0x20, not a space under UTF-16 or EBCDIC (predates this change).Hosted legs to read
The full pytest matrix (ubuntu and both Windows legs), since no local full run finished;
test_sandbox.pyon a quiet runner;windows-service-smokeas usual. Nothing NSSM-specific changed.