b2: #1249 ingest-plane rate limit cannot be enabled, #1235 citation gate can fail, #1253 one C0/DEL predicate (stranded branch) - #429
Closed
wshallwshall wants to merge 9 commits into
Closed
Conversation
…so (BACKLOG #1249)
The control table listed `max_messages_per_second` and `message_burst` with state
"off", which every reader takes to mean "set it to on", and the cell went on to
describe what happens "when set". No documented configuration can set them.
MEASURED, with a positive control that proves the check discriminates:
MLLP() parameters: 26, **kwargs present: False
max_messages_per_second NOT A PARAMETER
message_burst NOT A PARAMETER
max_connections ACCEPTED <- positive control
mllp.py:1378 mps = s.get("max_messages_per_second", DEFAULT_...) pacer reads it
mllp.py:116 DEFAULT_MAX_MESSAGES_PER_SECOND = None ships off
`connections.toml` routes through the SAME factory, so neither authoring surface can
express the keys. The pacer is real, reads its settings, and is unreachable.
A SECURITY DOCUMENT DESCRIBING A CONTROL, ITS BEHAVIOUR WHEN ENABLED, AND ITS
COVERAGE LIMITS -- FOR SOMETHING NOTHING CAN ENABLE -- is a compensating control
resting on a false premise, which SDS-3.7 forbids by name.
TWO THINGS DELIBERATELY KEPT, because the correction must not overshoot:
Not "unbounded intake". That would be FALSE and it runs against the engine.
Several bounds ship ON -- max_connections 256, receive_timeout 60s,
max_frame_bytes 16 MiB, per-connection max_message_bytes, source_ip_allowlist.
They bound SIZE and CONCURRENCY, not RATE. The cell now says which is which.
The off default was RULED, not accidental -- a rate on a clinical interface is
only safe at a number from a real feed profile. That reasoning stands. What the
old text conflated is that a ruled default and an unreachable setting are
different things, and only the first was intended.
NOT FIXED HERE, and not mine to decide: adding the two keys to the MLLP() signature
would change a shipped public factory, and whether the posture should be reachable
at all is a product question -- the item notes the default is ruled off for a
reason. Handed to the dispatcher; the doc half does not wait on it.
Verified: table structure intact (10 cells in the edited row, same as its
neighbour; 360 pipe-rows unchanged); link resolution and banner hygiene 30 passed;
cp1252 scan over the ADDED line -- 7 non-ASCII seen, all safe punctuation, 0 unsafe.
Coordinated: the collision gate refused this edit while another session held the
file with an unresolved merge conflict. I held rather than overriding, and they
released it. Their commits touch :56-77, :1164 and :1834 -- nowhere near this row.
No ledger edit; banner flip withheld, disposition routes to the dispatcher.
…CODE (BACKLOG #1249) 94ad263 rewrote the SECURITY.md ingest-plane row from "ships off" to "NOT REACHABLE" and did NOT update the guard that pins that wording, so the commit as handed over failed test_ingest_plane_rate_limit_is_documented_as_existing_but_off. Found by running the full suite; entirely mine. A docs-only diff is not test-neutral -- a test reads that file from disk, so a documentation commit moves a test result. The guard now reads reachability from inspect.signature(wiring.MLLP) and requires the row to AGREE with it, instead of asserting that unreachability is the correct state. #1249 IS OPEN AND THIS COMMIT DOES NOT CLOSE IT. The owner has not chosen between exposing max_messages_per_second / message_burst and rewording the row. A guard that pinned "NOT REACHABLE" as correct would settle that product question by build -- the same shape as settling one by omission. This guard flips on its own: expose the keys and it demands the row stop saying unreachable; leave them out and it demands the row say so. A green run here is not the ruling. Verified: MLLP() (config/wiring.py:1007) accepts neither pacing key, and connections.toml desugars through that same factory. Both branches are exercised -- simulating accepted keys fires the opposite assertion against the current doc. Unrelated: seven failures elsewhere in the full suite remain unlocated. They are not named here because I have not run them individually, and two positional estimates of their location were both wrong.
…t collect (BACKLOG #1230) The LOUD-OMISSION half only, per the owner's 2026-08-12 scope ruling. The worktree venv is deliberately NOT changed here. scripts/worktree/new.ps1:232 builds a worktree venv with .[dev,harness]; CI installs .[dev,harness,fhir,dicom,x12,xml,webauthn] (.github/workflows/ci.yml:273). Every module gated on that five-extra gap removes ITSELF at collection time via a module-level pytest.importorskip, so those tests never become test items: a block of coverage collapses into a short skip tally and the run still prints as green. THE DEFECT IS THE SILENCE, NOT THE ABSENCE -- skipping an uninstalled optional extra is correct; rendering an incomplete run as a complete one is not. A report-header line and a terminal-summary block now name the absent extras and the exact install command. NO PINNED TEST COUNT: a hard-coded figure is right the day it is written and silently wrong after, and re-running reproduces it, so it reads as verified. Proved in BOTH directions. The silent arm (nothing missing means nothing printed) is the one this venv cannot show on its own, so it is staged with monkeypatch rather than assumed. THE PROBE LIVES IN ITS OWN MODULE, AND THAT IS THE INTERESTING PART. It began inside conftest.py and the tests reached it with a bare `import conftest`. That passes when only tests/ is collected and FAILS IN A FULL RUN: pyproject has two testpaths, and packaging/messagefoundry-webconsole/ tests/conftest.py claims the same top-level module name, so the import bound to the webconsole's conftest and every attribute lookup failed. Seven tests passed alone and failed together -- the worst way for a test to be wrong, because isolation says it is fine. Fixed by moving the probe to tests/_extras_probe.py (no module-level side effects) and importing it package-qualified, the same idiom as tests._workflow_contexts. Importing conftest BY PATH was rejected: its module body claims a per-process test slot and registers an atexit unlink, so a second execution burns a slot for nothing. Verified against the failure mode, not just in isolation: with both testpaths collected together the suite is green, and restoring the bare import reproduces exactly those seven failures. The negative control is what makes the green meaningful. Known limit, stated rather than implied: these hooks load for any run that collects tests/, which includes every full-suite run -- the only kind that can earn "the full suite is green". A run scoped entirely to the webconsole testpath prints no banner. Measured both ways. Left rather than fixed with a repo-root conftest.py, which would change collection globally and is outside this scope.
…ACKLOG #1235) The RULE half. A citation to a number that names nothing resolves to nothing -- honest, and it advertises its own brokenness. If that number is later issued, the citation silently starts naming unrelated work, which the ledger's own erratum calls the worse outcome. THE DISTINCTION THAT DECIDES WHETHER A CITATION CAN EVER ARM, and it is not the obvious one. alloc.ps1 starts its search at $observed + 1 (config at :392, or the public-floor clamp at :389) and scans UPWARD, never filling a hole: "numbers are never reclaimed ... holes are free, collisions are not". So a number at or below the high-water mark is unreachable FOREVER; only a number above it can be issued. A check that asked only "does this resolve in the ledger" rates the two identically and raises 26 false alarms on this repository alone. Classification is by the FLOOR, deliberately not by the allocation registry. An earlier draft read the records under the git common dir. It gave the same answers and rested on the wrong thing: those records are machine-local, uncommittable and losable, so garbage-collecting a directory would appear to change a conclusion that never depended on it. The unreachability is structural. The floor is built from the ledgers only, so it can only UNDERSTATE the allocator's true floor (which also spans refs and allocations). Understating means over-warning, never a missed trap. TWO DEFECTS IN THE TOOL ITSELF, both found by RUNNING it over docs/ and both now pinned as tests. It crashed mid-scan on a character a cp1252 console cannot encode -- a scanner that dies partway prints a partial list that reads as a complete one. And it matched CSS/Mermaid hex colours: #1565c0 read as a citation of #1565, 8 of 40 hits spurious. An inflated count in a tool whose whole output is a bound is the one failure that makes it useless. Output is an UPPER BOUND ON CITATIONS, never a defect count, and says so in those words. Every hit prints with its file, line and source line so a human judges it; PR/issue/foreign-repo shapes are ANNOTATED, never trimmed, because a trimmed scan silently understates. Measured over docs/: 32 tokens, 26 below the floor, 6 above -- and all 6 are foreign references (pyodbc#1459, a Mirth forum #4849, code-server #6256). ZERO genuine live traps in this repository. NOT backlog_citation_check.py, which sits beside it and answers a different question (#1095: does the cited FILE contain the item, live ledger vs archive). Neither subsumes the other. The near- identical names are why that distinction is the first paragraph of the new module. The item's own premise needs amending -- its two named live traps are inert by construction -- but a builder may not author ledger content, so that is filed with the dispatcher, not written here.
…KLOG #1235) Docstring only; no behaviour change. The dispatcher and I converged on the allocator's mechanics through four positions between us, and the durable part is not in the code yet. Two additions: WHY THE LEDGER-ONLY FLOOR CAN ONLY UNDERSTATE, as a structural argument rather than an assertion: the allocator's observed set is a SUPERSET of the ledger headings -- it also reads refs, the working tree, claim files and a persisted high-water mark (alloc.ps1:97). Independently reproduced: ledger floor 1250 on origin/main against the allocator's own 1254. WHY AN ABOVE-FLOOR REPORT ON A FRESHLY-ALLOCATED NUMBER IS CORRECT AND NOT A FALSE POSITIVE. The ratchet persists the max of the OBSERVED set, not the number just issued (:205, :214, :215), so after issuing N the watermark holds N-1 and the only durable record of N is an untracked, never-pushed registry file. Lose it and N is re-issued. The flag then clears itself once the heading lands and the floor rises past it -- which is precisely when the number becomes permanently reserved. Recorded because both are things a future reader would otherwise "fix": by treating a live report on a just-allocated number as noise, or by adding a near-the-floor heuristic for an edge this floor already covers. #1253, #1254 and #1255 are in exactly that state today.
…abort (BACKLOG #1030)
Enforcement was per-file and hand-placed: test_cli.py asserts one STRING is encodable,
test_announce_hook.py asserts one FILE is ASCII, test_session_mail.py names five mail scripts in a
literal list. None generalises, so a glyph reaching print() from any other script was caught only by
a human reading the diff -- and the class recurred at least three times, the third TONIGHT, when a
scanner I had just written died mid-scan on U+2194 and printed a partial list that read as complete.
THE GATE IS ON REACHING AN UNGUARDED STREAM, NOT ON BARE ENCODABILITY, and that is the whole design
decision the item flagged as its real difficulty. sys.stdout carries errors='surrogateescape', which
round-trips only lone surrogates in DC80-DCFF; every other unencodable codepoint still raises.
sys.stderr carries backslashreplace and never raises. That asymmetry, not a strict/non-strict split,
is why the same text survives on stderr and aborts on stdout.
So a file may carry non-cp1252 characters IF IT HARDENS ITS OWN STDOUT. That is not an exemption
list: it is a property of the file, checked mechanically, and it is the REMEDY rather than a promise
about one. scripts/docs/backlog_status_check.py is why the distinction is load-bearing -- its
argparse description quotes the machine-parsed banner alphabet CLAUDE.md section 11 protects, and
remediation text that cannot show an author the character it wants added is not actionable. A gate
that could not express that would fire on correct code and be switched off.
That script is therefore FIXED rather than scrubbed: a stdout reconfigure at the CLI entry point,
scoped so importers are untouched. Measured before: `--help` raised UnicodeEncodeError on U+2705.
After: exit 0, the gate still reports 517 items, and parse_items still imports clean.
THE THREE PROPERTIES THE ITEM NAMES, each kept and each tested:
* PRINTS AND PINS WHAT IT SCANNED -- a walk that collapses to zero files would otherwise report
clean forever. Asserted, not merely emitted.
* READS THE WHOLE FILE, never line by line. splitlines() CONSUMES U+2028/U+2029, so a
line-oriented scan is structurally blind to them; a test demonstrates that mechanism rather than
asserting it.
* NEVER SILENTLY DROPS A FILE -- an undecodable file is a FAILURE, not a skip.
MEASURED, NOT INHERITED: the item counted 43 characters across four scripts/ files on 2026-08-05;
today it is ONE file, and it is the one that must keep them. Three were cleaned in between. The
control set is built from characters with recorded failures behind them, via chr() so this test
stays cp1252-clean itself -- verified: 0 non-cp1252 characters in it.
Scope stated in the file: scripts/**/*.py only. The engine already hardens both streams at
__main__.py; .ps1 has no equivalent reconfigure so generalising there is a different decision; docs/
is out because BACKLOG.md is a sanctioned holdout for that same alphabet.
…ed count (BACKLOG #1030)
Docstring only; no behaviour change. 14 tests unchanged and passing.
The gate's design -- harden the stream, never scrub the file -- was argued from principle. This
records the concrete case that lives in this repository, because the next reader to see a lone
variation selector in a source file will think it is lint.
backlog_status_check.py carries one non-cp1252 character beyond the banner alphabet: a lone U+FE0F
inside the banner regex as [<class>]\uFE0F?\s. That is an OPTIONAL VS-16, letting a banner be written
with or without the selector -- the handling CLAUDE.md section 11 mandates for any regex touching
that alphabet. Invisible at the point of use.
Delete it and the `?` binds to the CHARACTER CLASS. The pattern STILL COMPILES, so nothing at author
time objects. It matches an indented continuation line, group("emoji") returns None, and the dispatch
evaluates `None in _CLOSED` where _CLOSED is a str -- TypeError on any run touching the real ledger.
Verified by rebuilding the module with the character stripped and running parse_items against
docs/BACKLOG.md.
Two failure modes, and the second is the dangerous one: LOUDLY today, since every gate calling
parse_items dies; SILENTLY later, when the first banner authored WITH a selector stops matching --
and no banner carries one today, so nothing would catch that regression on the day it arrives.
NO COUNT IS PINNED, DELIBERATELY, and the omission is the point. An earlier draft of this note said
the scrub "silently inverts the parser while every test stays green". That was FALSE -- it crashes --
and it was falsifiable in one command, which would have discredited a correct surrounding argument.
The qualifying-line count is also ref-relative and grows with every filed item: measured 74 on
origin/main and 78 on an unpushed branch in the same hour. A figure would be stale the moment it was
written, and re-reading it would reproduce it, which reads as verification. Same hazard the
conftest banner refuses for the same reason.
… #1253) `ord(ch) < 0x20 or ord(ch) == 0x7F` was written out seven times across six files. Every copy agreed, so nothing was mis-screened; the cost was future-tense and is the one #1239 named -- a later hardening applied to one copy silently does not apply to the rest, and nothing reports the omission. THIS SHARES THE PREDICATE, NOT THE ACTION, and that is the design rather than an implementation detail. #1239 explicitly ruled out "collapsing the call sites into one helper with a flag", because the differing wrappers are appropriate: a raise suits a path context, a bool suits a filter, and the exception differs by layer (WiringError in config, a PHI-safe NegativeAckError in FHIR). So every call site keeps its own refusal and its own message; only the TEST moved. A flag parameter would have re-created the coupling this item exists to remove, one indirection further away. TWO ACTIONS ARE PRESERVED, and one must never be "simplified" into the other. Six sites REJECT. transports/rest.py STRIPS, on a message-derived header VALUE, and that is defensible rather than a second instance of the mutation pattern the owner ruled against in #1238: that ruling turns on basename() converting a path into a valid-but-DIFFERENT target, handing an attacker a real file. A header value has no such property -- removing CR/LF cannot redirect a request anywhere -- and rest.py already REJECTS a header NAME failing its RFC 7230 token check. Name-rejected, value-stripped. parsing/sniff.py is deliberately NOT folded in: it tests the same code points but is byte-wise rather than character-wise and subtracts an allowlist, because a text sniffer must tolerate tab, CR and LF. Folding it in would change its behaviour. TESTS. #1239 asked for proof that "the two predicates agree across a shared character corpus". With one predicate that obligation becomes a CHARACTERISATION test: the caught set is pinned over U+0000-U+02FF against an independently-written definition, the boundaries are pinned at each edge (0x1F in, 0x20 out, 0x7E out, 0x7F in, 0x80 out), and C1 plus the Unicode separators are pinned as deliberately NOT caught -- widening this is now a behaviour change at seven sites at once, which is the leverage and also the risk. 47 new tests; 256 existing tests across the six touched modules pass unchanged. BASE VERIFIED BEFORE BUILDING, because #1242's limb 4 was refused on exactly this ground: all six files are byte-identical between this branch and origin/main, and the three commits this branch is behind touch BACKLOG.md, scripts/asvs/apply.py and .claude/settings.json only. That is the discriminator -- apply.py differs here, these do not.
…ite runs it (BACKLOG #1235) #1235 was correctly ruled PARTIAL after PR #385: the detector shipped, but it was wired into nothing and could not fail. Two of that ruling's three measurements are addressed here. The third -- the written RULE -- is a convention change and is not mine to make. IT COULD NOT FAIL, AND THAT IS THE DEFECT THIS TOOL EXISTS TO CATCH ELSEWHERE. `--fail` was opt-in and nothing passed it, so a planted dangling citation was reported correctly AND the process exited 0. A checker that cannot fail is not a check. The default is now fail-closed with `--advisory` as the escape; flipping it cost nothing, because repo-wide the script was referenced by exactly two lines, both inside its own unit test. THE EXIT CODE KEYS ON THE LIVE SHAPE, NOT THE HIT COUNT. A number at or below the allocator's floor can never be issued, and a PR/issue/foreign-repo reference is not a backlog citation at all. Both are still REPORTED for a human to read, and neither reds the tree: failing on them would red it today for hits that are correct, and a gate that cries wolf gets switched off. THE SUITE IS NOW THE CALLER. A test walks the real docs/ tree and asserts no citation names a still-issuable number. Measured on origin/main before writing it: 312 markdown files, 6 above-floor hits, ALL foreign-repo shaped, ZERO live-shape -- so the gate passes today on merit rather than by being lenient. PROVED IT CAN FAIL, which is the whole point of the item. A live-shape citation was planted in docs/, the test ran, and it failed naming the file, line and number; the plant was removed in a finally block and the run without it passes. A gate whose failing arm has never been observed is the state this item is about. The population walk is pinned too (>200 files asserted, not merely printed): a walk that collapses to nothing would otherwise report clean forever.
Collaborator
Author
|
Closing without merging: the content has landed by other routes, and merging now would REVERT main. Measured, per file, branch content vs main content
The 14 identical ones are this PR's work, already on main via other merges. The 3 that differ are all cases where MAIN IS AHEAD
The decisive one: Merging would delete main's
Closing rather than rebasing: a rebase would resurrect exactly these reverts. The branch is retained, so nothing is lost and this is reversible. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opened by a different session to make stranded work visible. Not reviewed by me, not armed for auto-merge.
Existed only in a local checkout — never on
origin, no PR — while claims #1026 and #1249 pointed at a worktree directory that no longer exists. Backed the whole time asrefs/tags/rescue/branch/b2/1249-ingest-rate-docon the private remote, so nothing was at risk of loss; it simply could not land.What is here
9 commits, last dated 2026-08-14, 17 files, +1145/−19. All novel against
mainby patch-id.#1249say plainly that the ingest-plane rate limit cannot be enabled, and a guard that tests the DOC against the CODE#1235make the dangling-citation detector able to fail, run it in the suite, detect a backlog citation naming no item, and record why the floor's conservatism is the design#1253one C0/DEL predicate instead of seven copies#1030gate the wholescripts/tree against a cp1252 console abort, and record what a scrubbing gate would destroy without pinning a count#1230an incomplete local run now STATES what it could not collectThis branch does touch
BACKLOG.md, so unlike the sibling PRs it may satisfy the backlog-hygiene gate.Conflicts with #428, and the two must be sequenced
3 files overlap with
claude/modest-ellis-a46139(#428), and the collision is substantive, not incidental: both branches independently collapse the C0/DEL predicate —#1253here ("seven copies"),#1273there ("stated twice") — and both changedangling_citation_check.pyunder#1235. Whichever lands second needs a real rebase, not a mechanical one. Deciding which formulation of the predicate survives is a review call.🤖 Generated with Claude Code
Correction (author, later the same day)
The claim above that this branch "touches
BACKLOG.md" is wrong. It does not.I produced that by grepping the changed-file list for the word
backlog, case-insensitively, and reading a hit as the register. The hits werescripts/docs/backlog_status_check.py— and, on the sibling branch,tests/test_backlog_status_check.py. Those are a script named backlog and its test.docs/BACKLOG.mdis untouched, and so isdocs/archive/backlog/.The conclusion drawn from it — "so unlike the sibling PRs it may satisfy the backlog-hygiene gate" — is therefore unfounded in both directions. Reading the gate itself rather than guessing: it fires only when an explicit
BACKLOG #<digits>token appears in the pull request title or body (commit messages do not count, and a bare#123deliberately does not either), and the PR changes engine or IDE code. No such token appears in this PR's title or body, so the gate exits 0 without enforcing anything — the same as on its siblings, not differently from them.The substantive description of the work above is unaffected.