From 94ad263a94738a3fd5eabcf5f831eecd5c71f7fc Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 13 Aug 2026 21:51:30 -0500 Subject: [PATCH 1/5] docs(security): the ingest-plane rate limit cannot be enabled -- say 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. --- docs/SECURITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/SECURITY.md b/docs/SECURITY.md index b9e43ee3..96091f32 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -1674,7 +1674,7 @@ multi-host deployment must additionally front the API with a proxy/WAF limiter a | Request body | `[store].max_upload_bytes` (the `/uploads` routes only) | 1 MiB elsewhere | per request | no | no | no | **stateless** — every route, in ASGI middleware | **413** over the cap, **400** on ambiguous CL+TE framing or an invalid `Content-Length`, **411** on a chunked body | | OIDC pending flows | `[auth].oidc_flow_cache_max` (global), `DEFAULT_PER_IP_CAP` (per-IP, no knob), `oidc_flow_ttl_seconds` | 512 / 16 / 300 s | 300 s TTL | no | **yes** (512) | **yes** (16) | **in-process** — `GET /ui/oidc/start` — reject-when-full, never evict | 303 → `/ui/login?e=rate_limited`, WARNING-logged, **never** audited | | WebAuthn pending ceremonies | `GLOBAL_PENDING_CAP`, `PER_USER_PENDING_CAP`, `CHALLENGE_TTL_SECONDS` (module constants, no knobs) | 4096 / 16 / 120 s | 120 s TTL | **yes** (16) | **yes** (4096) | no | **in-process** — every passkey registration + assertion ceremony | per-user: evicts that user's **own** oldest pending ceremony (silent); global: `ChallengeCacheFullError` naming the cause + the `admin_reset_mfa` recovery path | -| **Ingest plane** | `max_messages_per_second`, `message_burst` (MLLP inbound) | **off** | per message | no | no | no | **in-process** — one bucket per MLLP connection, so it neither coordinates across engine shards nor aggregates per peer | **exists but ships OFF, so unset there is still no volume bound.** When set, the listener **pauses reading** over budget so TCP back-pressures the sender: no message is dropped, refused, NAK'd or reordered (the count-and-log invariant forbids accept-and-drop, so a discarding limiter was never available). Bounded by the bucket deficit. The off default is **ruled, not accidental** — a rate on a clinical interface is only safe at a number from a real feed profile. **Not covered:** the raw-TCP inbound, and any per-peer bound (MLLP peers are unauthenticated, so the only key would be source IP, which NAT collapses). Other inbound caps are resource-only — `max_connections` (256), `receive_timeout` (60.0 s), `max_frame_bytes` (16 MiB), per-connection `max_message_bytes`, `source_ip_allowlist` | +| **Ingest plane** | `max_messages_per_second`, `message_burst` (MLLP inbound) | **NOT REACHABLE — read the note** | per message | no | no | no | **in-process** — one bucket per MLLP connection, so it neither coordinates across engine shards nor aggregates per peer | **THE PACER IS BUILT AND NO DOCUMENTED CONFIGURATION CAN TURN IT ON.** Read that before the rest of this cell. Neither key is a parameter of the `MLLP()` factory, and `connections.toml` routes through that same factory, so **neither the code-first nor the TOML surface can express them**. This row previously read state **"off"**, which every reader takes to mean "set it to on". **So there is currently NO message-RATE bound anywhere on the ingest plane.** — **This is NOT "unbounded intake", and that correction runs in the engine's favour:** several bounds ship **ON** and are listed at the end of this cell. They bound **SIZE and CONCURRENCY, not RATE**. — *What the pacer would do if it were reachable:* the listener **pauses reading** over budget so TCP back-pressures the sender; no message is dropped, refused, NAK'd or reordered (the count-and-log invariant forbids accept-and-drop, so a discarding limiter was never available). The off default was **ruled, not accidental** — a rate on a clinical interface is only safe at a number from a real feed profile — but **a ruled default and an unreachable setting are different things, and only the first was intended.** **Not covered even if reachable:** the raw-TCP inbound, and any per-peer bound (MLLP peers are unauthenticated, so the only key would be source IP, which NAT collapses). **Resource bounds that DO ship on** — `max_connections` (256), `receive_timeout` (60.0 s), `max_frame_bytes` (16 MiB), per-connection `max_message_bytes`, `source_ip_allowlist` | **What these limits defend, and what they do not.** The full inventory of resource-demanding functionality — including the surfaces that remain **unbounded** at this release — is From e92b7ecfb70cbec92be144c54cfa49fb23ac657f Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 14 Aug 2026 00:06:08 -0500 Subject: [PATCH 2/5] test(security-doc): the ingest-plane guard tests the DOC against the CODE (BACKLOG #1249) 94ad263a 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. --- tests/test_security_doc_rate_limits.py | 49 ++++++++++++++++++++------ 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/tests/test_security_doc_rate_limits.py b/tests/test_security_doc_rate_limits.py index 572f428b..08f4f486 100644 --- a/tests/test_security_doc_rate_limits.py +++ b/tests/test_security_doc_rate_limits.py @@ -532,23 +532,50 @@ def test_scope_guard_detects_a_planted_rescoping() -> None: ) -def test_ingest_plane_rate_limit_is_documented_as_existing_but_off() -> None: - """A silent omission reads as coverage, and so does an overstatement in the other direction. - - Until 2026-08-11 this guard asserted the doc said "no message-rate or volume limit exists", which - was true and load-bearing. The MLLP pacing build (ASVS 2.4.1 / 15.2.2) falsified it, so the guard - had to move with the code and the doc rather than be deleted -- the row must now state BOTH that - a control exists and that it ships OFF, because either half alone misleads: "exists" implies the - shipped default is bounded, and "none" is now simply false. +def test_ingest_plane_rate_limit_row_matches_the_code() -> None: + """The ingest row must describe the pacer's REACHABILITY as the code actually has it. + + This guard has moved twice. Until 2026-08-11 it asserted the doc said "no message-rate or volume + limit exists"; the MLLP pacing build (ASVS 2.4.1 / 15.2.2) falsified that, so it moved to + requiring both that a control exists and that it "ships OFF". BACKLOG #1249 falsified that + wording in turn: the pacer is built, but neither key is a parameter of the ``MLLP()`` factory and + ``connections.toml`` desugars through that SAME factory, so no documented surface can set either. + "Off" is the more dangerous half-truth, because a reader takes it to mean "then set it to on". + + WHAT THIS ASSERTS IS CONSISTENCY, NOT A PREFERENCE, AND THE DISTINCTION IS THE WHOLE POINT. + **#1249 is OPEN.** The owner has not chosen between exposing the two keys and rewording the row, + and a guard that pinned "NOT REACHABLE" as the correct state would settle that product question + BY BUILD -- the same shape as settling one by omission. So reachability is READ FROM THE + SIGNATURE and the doc is required to agree with whatever it says. Expose the keys and this test + demands the row stop saying unreachable; leave them out and it demands the row say so. Either + ruling stays cheap, and neither is pre-empted by a green run here. """ + import inspect + + from messagefoundry.config import wiring + + pacing_keys = {"max_messages_per_second", "message_burst"} + accepted = pacing_keys & set(inspect.signature(wiring.MLLP).parameters) + block = _section(_H_LIMITS) assert "Ingest plane" in block assert "max_messages_per_second" in block, ( "the ingest row must name the control that now exists" ) - assert "ships OFF" in block, "and must say it is not on by default, or 'exists' overstates it" - # The honest gaps stay named: an off default bounds nothing, and the raw-TCP intake never got it. - assert "still no volume bound" in block + if accepted: + assert "NOT REACHABLE" not in block, ( + f"MLLP() now accepts {sorted(accepted)}, so the row may no longer call the pacer " + "unreachable -- it must state the shipped default instead" + ) + else: + assert "NOT REACHABLE" in block, ( + "MLLP() accepts neither pacing key, so the row must say the pacer cannot be turned on " + "rather than that it is merely off by default" + ) + assert "NO message-RATE bound anywhere on the ingest plane" in block, ( + "and must name the resulting gap outright, not leave it inferred" + ) + # True under either ruling: the raw-TCP intake never got the pacer at all. assert "raw-TCP inbound" in block for cap in ("max_connections", "receive_timeout", "max_frame_bytes", "max_message_bytes"): assert cap in block, f"the ingest row must name the {cap} resource cap it DOES have" From bbd2d86cf70780ab37cc8f30c17b1a26332523ee Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 14 Aug 2026 00:06:36 -0500 Subject: [PATCH 3/5] test(collection): an incomplete local run now STATES what it could not 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. --- tests/_extras_probe.py | 91 +++++++++++++++++++++++++++++ tests/conftest.py | 42 +++++++++++++ tests/test_incomplete_run_banner.py | 88 ++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+) create mode 100644 tests/_extras_probe.py create mode 100644 tests/test_incomplete_run_banner.py diff --git a/tests/_extras_probe.py b/tests/_extras_probe.py new file mode 100644 index 00000000..7bffd20a --- /dev/null +++ b/tests/_extras_probe.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Which optional extras are absent from this interpreter (BACKLOG #1230, loud-omission half). + +WHY THIS IS A SIBLING MODULE AND NOT PART OF ``conftest.py``, which is where it started and where it +does not belong. The tests reached it with a bare ``import conftest``. That passes when only +``tests/`` is collected and FAILS IN A FULL RUN, because ``pyproject.toml`` has two testpaths and +``packaging/messagefoundry-webconsole/tests/conftest.py`` claims the same top-level module name: + + AttributeError: + has no attribute '_OPTIONAL_EXTRAS' + +Seven tests passed alone and failed together, which is the worst way for a test to be wrong -- the +name resolved to the neighbouring module and nothing said so. Importing ``conftest`` BY PATH instead +would re-execute it, and its module body claims a per-process test slot and registers an ``atexit`` +unlink, so a second execution burns a slot for nothing. + +So the probe lives here: no module-level side effects, imported package-qualified as +``tests._extras_probe`` (the same idiom as ``tests._workflow_contexts``), and ``conftest`` keeps only +the thin pytest hooks that render what these functions compute. +""" + +from __future__ import annotations + +import importlib.util +from typing import Protocol + +#: extra name -> the import sentinels its tests need. Mirrors pyproject.toml's +#: [project.optional-dependencies]; every distribution an extra pulls in is listed, so a +#: half-installed extra reads as absent rather than present. +OPTIONAL_EXTRAS: dict[str, tuple[str, ...]] = { + "fhir": ("fhir.resources", "fhirpathpy"), + "dicom": ("pydicom", "pynetdicom"), + "x12": ("pyx12",), + "xml": ("lxml", "xmlschema", "signxml"), + "webauthn": ("webauthn",), +} + + +class SummaryWriter(Protocol): + """The two ``TerminalReporter`` methods the summary uses, and nothing else.""" + + def write_line(self, line: str) -> None: ... + + def write_sep(self, sep: str, title: str = "", **kwargs: object) -> None: ... + + +def extra_is_installed(sentinels: tuple[str, ...]) -> bool: + """True only when EVERY sentinel resolves. + + ``find_spec`` rather than an import: it answers the question without paying the import cost and + without leaving a partially-imported module behind on failure. A sentinel whose PARENT package is + absent (``fhir.resources`` with no ``fhir``) RAISES instead of returning ``None`` -- measured, not + assumed -- so both arms have to mean "absent" or the fhir extra would read as present. + """ + for name in sentinels: + try: + if importlib.util.find_spec(name) is None: + return False + except (ImportError, ValueError): + return False + return True + + +def missing_extras() -> list[str]: + """Extras whose tests cannot be collected in this interpreter, in declaration order.""" + return [name for name, probes in OPTIONAL_EXTRAS.items() if not extra_is_installed(probes)] + + +def report_header_lines() -> list[str]: + """State the omission up front -- a summary line is easy to scroll past on a long run.""" + missing = missing_extras() + if not missing: + return [] + return [f"INCOMPLETE RUN: optional extras absent -- {', '.join(missing)}"] + + +def write_incomplete_run_summary(reporter: SummaryWriter) -> None: + """Print the omission where the verdict is rendered, so green cannot be read as complete.""" + missing = missing_extras() + if not missing: + return + write = reporter.write_line + reporter.write_sep("=", "INCOMPLETE RUN -- coverage was NOT collected", red=True, bold=True) + write(f"Optional extras absent from this interpreter: {', '.join(missing)}") + write("Every test module gated on them removed itself at COLLECTION time, so its tests are not") + write("in the counts above -- passed, failed and skipped alike. This result does NOT establish") + write("that the full suite is green, and must not be reported as if it did.") + write("") + write("To collect them, install what CI installs (.github/workflows/ci.yml):") + write(f' pip install --constraint constraints.lock -e ".[dev,harness,{",".join(missing)}]"') diff --git a/tests/conftest.py b/tests/conftest.py index 6bb7a79e..f5a27c1a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,6 +25,7 @@ import pytest from messagefoundry.config.settings import INSECURE_CONFIG_SOURCE_ESCAPE_ENV +from tests._extras_probe import report_header_lines, write_incomplete_run_summary # --------------------------------------------------------------------------------------------------- # Per-PROCESS test slot. @@ -327,3 +328,44 @@ def _tolerate_logging_on_closed_capture_streams() -> Iterator[None]: yield finally: logging.raiseExceptions = prior_raise + + +# --------------------------------------------------------------------------------------------------- +# LOUD OMISSION: an incomplete run must SAY SO (BACKLOG #1230 — the loud-omission half only, per the +# owner's 2026-08-12 scope ruling; the venv is deliberately NOT changed here). +# +# scripts/worktree/new.ps1:232 creates 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 at all: a large 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 and +# intended. Rendering an incomplete run as a complete one is not — "the full suite is green" is the +# sentence the next session bases its own scope on, and a local run cannot currently earn it. +# +# Deliberately NOT a pinned test count. A hard-coded figure would be right the day it was written and +# silently wrong after, and a stale number in a measurement surface is worse than none: re-running +# reproduces it, so it reads as verified. What prints is what is re-derived every run — which extras +# are absent, and the command that installs them. +# +# This never fails a run and never skips anything itself. It only makes an already-incomplete run +# legible, so the omission has to be read rather than inferred from a skip count. +# +# SCOPE, STATED RATHER THAN IMPLIED. These hooks live in tests/conftest.py, so they load for any run +# that collects tests/ — which includes every full-suite run, the only kind that can earn the phrase +# above. A run scoped ENTIRELY to the other testpath (packaging/messagefoundry-webconsole/tests) +# does not load this file and prints no banner. Measured both ways, with a control to rule out the +# hooks simply being inert: webconsole-only collected 365 tests silently; the same flags over a +# tests/ path printed the banner. That gap is left rather than fixed with a repo-root conftest.py, +# which would change collection globally and is outside this item's ruled scope — a deliberately +# scoped run already reads as partial; a full-suite run is the one that must not. +# --------------------------------------------------------------------------------------------------- + + +def pytest_report_header() -> list[str]: + return report_header_lines() + + +def pytest_terminal_summary(terminalreporter: pytest.TerminalReporter) -> None: + write_incomplete_run_summary(terminalreporter) diff --git a/tests/test_incomplete_run_banner.py b/tests/test_incomplete_run_banner.py new file mode 100644 index 00000000..9d1de192 --- /dev/null +++ b/tests/test_incomplete_run_banner.py @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The incomplete-run banner (BACKLOG #1230, loud-omission half). + +Proved in BOTH directions, because a banner that always prints and a banner that never prints are +both indistinguishable from a working one if you only ever observe the state you happen to be in. +This worktree's venv is missing all five extras, so the "fires" direction is the ambient case and +the SILENT direction is the one that needs staging. +""" + +from __future__ import annotations + +from typing import Any + +from tests import _extras_probe as probe + + +class _FakeReporter: + """Captures what the summary hook writes. Mirrors only the two methods the hook uses.""" + + def __init__(self) -> None: + self.lines: list[str] = [] + self.seps: list[str] = [] + + def write_line(self, line: str, **_: Any) -> None: + self.lines.append(line) + + def write_sep(self, _sep: str, title: str = "", **_kw: Any) -> None: + self.seps.append(title) + + +def test_a_resolvable_sentinel_reads_as_installed() -> None: + """Positive control: the probe must be capable of returning True at all, or every other + assertion here would pass against a predicate that can only ever say 'absent'.""" + assert probe.extra_is_installed(("json",)) is True + assert probe.extra_is_installed(("json", "pathlib")) is True + + +def test_an_absent_sentinel_reads_as_missing() -> None: + assert probe.extra_is_installed(("mefor_no_such_module_1230",)) is False + + +def test_one_absent_sentinel_condemns_the_whole_extra() -> None: + """A half-installed extra must read as absent -- its tests still cannot be collected.""" + assert probe.extra_is_installed(("json", "mefor_no_such_module_1230")) is False + + +def test_a_missing_parent_package_reads_as_missing_not_as_present() -> None: + """``find_spec`` RAISES rather than returning None when the parent package is absent. If that + arm were not caught, the fhir extra (``fhir.resources``) would report as installed.""" + assert probe.extra_is_installed(("mefor_no_such_parent_1230.child",)) is False + + +def test_silent_when_every_extra_resolves(monkeypatch: Any) -> None: + """The direction this venv cannot show on its own: nothing missing means nothing printed.""" + monkeypatch.setattr(probe, "OPTIONAL_EXTRAS", {"stdlib": ("json",)}) + assert probe.missing_extras() == [] + assert probe.report_header_lines() == [] + reporter = _FakeReporter() + probe.write_incomplete_run_summary(reporter) + assert reporter.lines == [] + assert reporter.seps == [] + + +def test_loud_when_an_extra_is_absent(monkeypatch: Any) -> None: + monkeypatch.setattr( + probe, "OPTIONAL_EXTRAS", {"stdlib": ("json",), "ghost": ("mefor_no_such_module_1230",)} + ) + assert probe.missing_extras() == ["ghost"] + header = probe.report_header_lines() + assert len(header) == 1 + assert "ghost" in header[0] + + reporter = _FakeReporter() + probe.write_incomplete_run_summary(reporter) + body = "\n".join(reporter.lines) + assert "ghost" in body + assert "stdlib" not in body # only what is actually absent is named + assert any("INCOMPLETE RUN" in title for title in reporter.seps) + # The remedy has to be present and has to name the absent extra, or the banner states a problem + # without a way out and gets ignored. + assert 'pip install --constraint constraints.lock -e ".[dev,harness,ghost]"' in body + + +def test_the_real_extra_table_matches_pyproject() -> None: + """The table is hand-maintained beside pyproject.toml; assert it still names the five extras CI + installs, so a renamed or dropped extra fails here instead of silently never being reported.""" + assert set(probe.OPTIONAL_EXTRAS) == {"fhir", "dicom", "x12", "xml", "webauthn"} From 8e7ee1505aee88dbe95616230f235df7d8cd16fd Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 14 Aug 2026 00:07:03 -0500 Subject: [PATCH 4/5] tooling(docs): detect a backlog citation that names no item at all (BACKLOG #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. --- scripts/docs/dangling_citation_check.py | 257 ++++++++++++++++++++++++ tests/test_dangling_citation_check.py | 176 ++++++++++++++++ 2 files changed, 433 insertions(+) create mode 100644 scripts/docs/dangling_citation_check.py create mode 100644 tests/test_dangling_citation_check.py diff --git a/scripts/docs/dangling_citation_check.py b/scripts/docs/dangling_citation_check.py new file mode 100644 index 00000000..73e8d51d --- /dev/null +++ b/scripts/docs/dangling_citation_check.py @@ -0,0 +1,257 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Report a `#N` backlog citation that names NO ITEM AT ALL (BACKLOG #1235). + +NOT `backlog_citation_check.py`, WHICH SITS BESIDE THIS FILE AND ANSWERS A DIFFERENT QUESTION. +That gate (BACKLOG #1095) resolves a citation against the ledger FILE its item lives in, so a +reference naming the live `docs/BACKLOG.md` for an item that has been archived is caught -- the +number is real, the path is stale. This one asks whether the number names anything whatsoever. +An item that exists in either ledger is invisible here and is that gate's business; a number that +exists in neither is invisible there and is this one's. Neither subsumes the other, and the near- +identical names are the reason this paragraph is the first thing in the file. + +WHAT THE DEFECT IS. While a number names nothing, a citation to it resolves to NOTHING -- honest and +harmless, and it advertises its own brokenness. If that number is later issued, the citation starts +resolving to unrelated work and NOTHING anywhere reports a problem. The ledger's own erratum names +that as the worse outcome: a wrongly-resolving reference "reads as a working cross-reference +forever". This tool catches the citation while it is still in the honest state. + +THE DISTINCTION THAT DECIDES WHETHER A CITATION IS A TRAP, and it is not the obvious one. Two very +different states both look like "resolves to nothing", and only one of them can ever arm: + + * BELOW THE HIGH-WATER MARK. `scripts/coord/alloc.ps1` starts its search at ``$observed + 1`` and + scans UPWARD, never filling a hole -- "numbers are never reclaimed ... holes are free, + collisions are not". A number at or below the mark is therefore unreachable FOREVER, and the + citation is harmless permanently rather than by luck. Measured here: 26 of 32 hits. + * ABOVE IT. That number will be issued in the normal course, and on the day it is, the citation + silently starts naming unrelated work. This is the only shape that can arm. + +A tool reporting only "resolves in the ledger or not" rates those identically and raises 26 false +alarms on this repository alone. + +WHY THE FLOOR AND NOT THE ALLOCATION REGISTRY. An earlier version of this tool classified by looking +for an allocation RECORD under the git common dir. It produced 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 a +structural property of how the allocator picks a number. Reasoning from the registry makes a sound +property look fragile and invites a guard nothing needs. + +WHY A TOOL AND NOT VIGILANCE. There is no gate on either side. `alloc.ps1` answers "is this number +free"; nothing asks "is anything already pointing at it". The two halves are each individually +correct, and the gap between them is the defect. + +WHAT THIS DELIBERATELY DOES NOT DO. + + * It does NOT claim a defect count. Its output is an UPPER BOUND ON CITATIONS -- some hits are + near-certainly not backlog references at all (a four-digit token in a research evaluation, one + inside a diagram). Reporting the raw number as a defect count would be a completeness claim the + scan cannot support, so the summary says so in those words rather than leaving it to be + inferred. Every hit is printed with its file, line and full source line so a human judges it. + * It does NOT sweep or edit. Fixing existing instances is a separate, owner-present task. + * It does NOT see the private companion repository, where the known instances live. A citation in + another repository is invisible to every check this one runs -- that limitation is inherent, is + the reason the item exists, and is printed rather than left implicit. + +The allocated set is read through `backlog_status_check.parse_items`, never a hand-rolled scan: +that module DEFINES what an item is, and a second definition here would drift from it silently. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import re +import sys +from pathlib import Path +from typing import NamedTuple + +_HERE = Path(__file__).resolve().parent + +# The window the item pinned. Numbers below 1000 are the pre-partition internal sequence and the +# repository's PR/issue numbers, which share the `#N` spelling and are NOT backlog citations. +_LOW = 1000 +_HIGH = 9000 + +#: A citation is `#` immediately followed by digits AND ENDING THERE. A Markdown heading +#: (`## 1235.`) puts a SPACE between, so it cannot match -- which matters, because every item +#: heading in the ledger would otherwise report as a citation of itself. +#: +#: The trailing boundary is not cosmetic. Without it, a CSS/Mermaid hex colour matches its own digit +#: prefix: `#1565c0` reads as a citation of #1565 and `#06302b` as one of #6302. Measured on the +#: first real run over docs/ -- those two alone produced 8 of 40 hits, all spurious, across +#: ARCHITECTURE.md and architecture-diagram.md. An inflated count in a tool whose entire output is a +#: bound is the one failure that makes it useless. +_CITATION = re.compile(r"#(\d+)(?![0-9A-Za-z])") + +#: A `#N` introduced by one of these is a PULL REQUEST, issue or forum reference, not a backlog +#: citation. This repository's PR numbers ALREADY exceed 1000 (`PR #995`, `PR #1001` are both cited +#: in docs/adr/), so the [1000,9000) window does not separate the two namespaces on its own -- +#: measured, not assumed. Matching hits are still REPORTED and still COUNTED; they are only +#: annotated, because this item's discipline is to DISCLOSE a false positive rather than trim it. +_PR_CONTEXT = re.compile(r"(?i)\b(?:PR|pull request|commit|issue|discussion)\s+$") + +#: `pyodbc#1459`, `coder/code-server#6256` -- a `#N` glued to an identifier is ANOTHER PROJECT's +#: issue number. Both shapes are live in docs/ today. +_FOREIGN_REPO = re.compile(r"[A-Za-z0-9_./-]$") + + +class Hit(NamedTuple): + path: Path + lineno: int + number: int + line: str + pr_shaped: bool + + +def _load_backlog_module() -> object: + """Import backlog_status_check by path -- it is a script directory, not an installed package.""" + spec = importlib.util.spec_from_file_location( + "_backlog_status_check", _HERE / "backlog_status_check.py" + ) + if spec is None or spec.loader is None: # pragma: no cover - a packaging accident, not a state + raise RuntimeError("cannot load backlog_status_check.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def allocation_floor(sources: list[Path] | None = None) -> int: + """The highest item number the ledgers know about. Nothing at or below it can ever be issued. + + THIS IS A STRUCTURAL PROPERTY OF THE ALLOCATOR, NOT A FACT ABOUT ANY REGISTRY FILE. + `scripts/coord/alloc.ps1` starts its search at ``$observed + 1`` (or + ``[Math]::Max($observed, $PublicBacklogFloor - 1) + 1`` under the public clamp) and scans + UPWARD. It never fills 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**, and a number above + it will be issued in the normal course. That is the whole classification. + + An earlier version of this tool read the allocation records under the git common dir instead. + That gave the same answers here and was the wrong basis: those records are machine-local, + uncommittable and losable, so garbage-collecting a directory would have "changed" a conclusion + that does not actually depend on it. Reasoning from the registry makes a sound property look + fragile and invites a guard nothing needs. + + Deliberately conservative: the true floor is the max over origin/main, every ref AND every + allocation, so this ledger-only figure can only ever be LOWER. A number between the two is + reported as reachable when it is not -- an over-warning, never a missed trap. + """ + numbers = allocated_numbers(sources) + return max(numbers) if numbers else 0 + + +def allocated_numbers(sources: list[Path] | None = None) -> set[int]: + """Every item number that EXISTS in the ledgers, open or closed. + + Closed items are included deliberately: a citation to a closed item resolves correctly and is + not this defect. Only a number that names nothing at all is the trap. + """ + module = _load_backlog_module() + paths = sources if sources is not None else [Path(p) for p in module.DEFAULT_SOURCES] # type: ignore[attr-defined] + numbers: set[int] = set() + for path in paths: + if not path.exists(): + continue + for item in module.parse_items(path.read_text(encoding="utf-8")): # type: ignore[attr-defined] + numbers.add(item.num) + return numbers + + +def citations_in(text: str) -> list[tuple[int, int, str, bool]]: + """(lineno, number, line, pr_shaped) for every in-window `#N` token. 1-indexed lines.""" + found: list[tuple[int, int, str, bool]] = [] + for lineno, line in enumerate(text.splitlines(), start=1): + for match in _CITATION.finditer(line): + number = int(match.group(1)) + if _LOW <= number < _HIGH: + before = line[: match.start()] + pr_shaped = ( + _PR_CONTEXT.search(before) is not None + or _FOREIGN_REPO.search(before) is not None + ) + found.append((lineno, number, line.strip(), pr_shaped)) + return found + + +def unresolved_citations(paths: list[Path], allocated: set[int]) -> list[Hit]: + hits: list[Hit] = [] + for path in paths: + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + continue + for lineno, number, line, pr_shaped in citations_in(text): + if number not in allocated: + hits.append(Hit(path, lineno, number, line, pr_shaped)) + return hits + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + parser.add_argument("paths", nargs="*", type=Path, help="files to scan (default: docs/**/*.md)") + parser.add_argument( + "--fail", action="store_true", help="exit non-zero when any citation is unresolved" + ) + args = parser.parse_args(argv) + + # A scanner that CRASHES partway prints a partial list that reads as a complete one. Source lines + # legitimately carry characters a stock Windows cp1252 console cannot encode, and this tool hit + # exactly that on its first real run (U+2194, inside an ADR). Force UTF-8 and KEEP errors=replace: + # the defect is the wrong codec, and replacement is what stops one exotic glyph truncating a + # measurement. + if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + + paths = args.paths or sorted(Path("docs").rglob("*.md")) + allocated = allocated_numbers() + hits = unresolved_citations(list(paths), allocated) + + if not hits: + print(f"No unresolved backlog citation in {len(paths)} file(s).") + print(f"Resolved against {len(allocated)} allocated item numbers (open and closed).") + return 0 + + floor = allocation_floor() + + for hit in hits: + note = ( + " [PR/issue/foreign-repo shaped -- very likely NOT a backlog citation]" + if hit.pr_shaped + else "" + ) + print(f"{hit.path}:{hit.lineno}: #{hit.number} resolves to no filed item{note}") + print(f" {hit.line}") + state = ( + f"BELOW THE FLOOR ({floor}) -- the allocator only ever issues above its high-water " + "mark, so this number can NEVER be issued and the citation is permanently harmless." + if hit.number <= floor + else f"ABOVE THE FLOOR ({floor}) -- this number CAN still be issued to unrelated work. " + "This is the live shape." + ) + print(f" -> {state}") + + distinct = sorted({hit.number for hit in hits}) + pr_shaped = sum(1 for hit in hits if hit.pr_shaped) + print() + print( + f"{len(hits)} token(s) across {len({h.path for h in hits})} file(s); " + f"{len(distinct)} distinct number(s): {', '.join(f'#{n}' for n in distinct)}" + ) + if pr_shaped: + print( + f"{pr_shaped} of those {len(hits)} TOKENS (not of the {len(distinct)} numbers) are " + f"PR/issue/foreign-repo shaped, annotated above." + ) + print( + "This is an UPPER BOUND ON CITATIONS, NOT A COUNT OF DEFECTS -- some hits are very likely" + ) + print("not backlog references at all. Each is printed above with its source line; judge them.") + print( + "Not scanned: the private companion repository, where a citation is invisible to this repo." + ) + return 1 if args.fail else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_dangling_citation_check.py b/tests/test_dangling_citation_check.py new file mode 100644 index 00000000..06022e16 --- /dev/null +++ b/tests/test_dangling_citation_check.py @@ -0,0 +1,176 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The unresolved-backlog-citation detector (BACKLOG #1235). + +Every false-positive shape asserted here was found by RUNNING the tool over docs/, not predicted +before it. Two of them were defects in the detector itself on its first real run: a hex colour +matching its own digit prefix, and a crash on a character a cp1252 console cannot encode. They are +pinned as tests because both re-appear the moment the pattern is loosened. +""" + +from __future__ import annotations + +import importlib.util +import pathlib +from pathlib import Path + +import pytest + +_SPEC = importlib.util.spec_from_file_location( + "_dangling_citation_check", + Path(__file__).resolve().parents[1] / "scripts" / "docs" / "dangling_citation_check.py", +) +assert _SPEC is not None and _SPEC.loader is not None +cc = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(cc) + + +def _numbers(text: str) -> list[int]: + return [number for _lineno, number, _line, _pr in cc.citations_in(text)] + + +# --- what a citation IS --------------------------------------------------------------------------- + + +def test_a_plain_citation_is_found() -> None: + assert _numbers("see BACKLOG #1235 for the rule") == [1235] + + +def test_line_numbers_are_one_indexed() -> None: + lineno, number, _line, _pr = cc.citations_in("first\nsecond #1235\n")[0] + assert (lineno, number) == (2, 1235) + + +@pytest.mark.parametrize("trailing", [".", ",", ")", "'s", " and", "]", ";"]) +def test_ordinary_punctuation_still_closes_a_citation(trailing: str) -> None: + assert _numbers(f"see #1235{trailing}") == [1235] + + +# --- what a citation IS NOT ----------------------------------------------------------------------- + + +def test_a_markdown_heading_is_not_a_citation_of_itself() -> None: + """Every item heading in the ledger would otherwise report as citing its own number.""" + assert _numbers("## 1235. a citation to an unallocated number") == [] + + +@pytest.mark.parametrize("colour", ["#1565c0", "#06302b", "#1234ab"]) +def test_a_hex_colour_does_not_match_its_own_digit_prefix(colour: str) -> None: + """Measured, not predicted: on the first real run over docs/, `#1565c0` and `#06302b` alone + produced 8 of 40 hits. An inflated count in a tool whose whole output is a bound is fatal.""" + assert _numbers(f"classDef x fill:#e3f2fd,stroke:{colour};") == [] + + +def test_numbers_outside_the_window_are_ignored() -> None: + assert _numbers("PR #995 and issue #42 and #9000 and #12345") == [] + + +def test_the_window_is_half_open_at_both_ends() -> None: + assert _numbers("#999") == [] + assert _numbers("#1000") == [1000] + assert _numbers("#8999") == [8999] + assert _numbers("#9000") == [] + + +# --- annotation, which discloses rather than trims ------------------------------------------------- + + +@pytest.mark.parametrize( + "line", + [ + "shipped in PR #1001", + "see pull request #1001", + "fixed by commit #1001", + "upstream issue #1001", + "code-server discussion #6256", + ], +) +def test_a_pr_or_forum_reference_is_annotated(line: str) -> None: + hits = cc.citations_in(line) + assert len(hits) == 1 + assert hits[0][3] is True, "should be flagged as very likely not a backlog citation" + + +@pytest.mark.parametrize("line", ["upstream pyodbc#1459", "coder/code-server#6256"]) +def test_a_foreign_repo_reference_is_annotated(line: str) -> None: + hits = cc.citations_in(line) + assert len(hits) == 1 + assert hits[0][3] is True + + +def test_a_genuine_citation_is_not_annotated() -> None: + hits = cc.citations_in("this supersedes #1084") + assert len(hits) == 1 + assert hits[0][3] is False + + +def test_an_annotated_hit_is_still_reported_and_still_counted() -> None: + """The item's discipline is to DISCLOSE a false positive, never to trim it -- a trimmed scan + silently understates, which is the failure this whole item is about.""" + assert len(cc.citations_in("upstream pyodbc#1459")) == 1 + + +# --- resolution against the ledger ------------------------------------------------------------------ + + +def test_only_unallocated_numbers_are_reported(tmp_path: Path) -> None: + doc = tmp_path / "d.md" + doc.write_text("real #1230, unreal #8999\n", encoding="utf-8") + hits = cc.unresolved_citations([doc], allocated={1230}) + assert [h.number for h in hits] == [8999] + + +def test_a_closed_item_still_resolves(tmp_path: Path) -> None: + """A citation to a CLOSED item points at something real. Only a number naming nothing is the trap.""" + doc = tmp_path / "d.md" + doc.write_text("see #1230\n", encoding="utf-8") + assert cc.unresolved_citations([doc], allocated={1230}) == [] + + +def test_an_unreadable_file_is_skipped_not_fatal(tmp_path: Path) -> None: + missing = tmp_path / "gone.md" + assert cc.unresolved_citations([missing], allocated=set()) == [] + + +def test_the_real_ledger_yields_a_plausible_allocated_set() -> None: + """Positive control on the ledger read: if parse_items ever returns nothing, every citation in + the repository would report as unresolved and the tool would look catastrophically alarming.""" + allocated = cc.allocated_numbers() + assert len(allocated) > 100 + assert 1235 in allocated, "the item that defines this tool must resolve" + + +# --- the floor, which decides whether a citation can ever arm --------------------------------------- + + +def test_the_floor_is_found_and_is_plausible() -> None: + """Positive control. If the ledger read ever silently returned nothing, the floor would be 0 and + EVERY citation would classify as live -- 26 manufactured alarms on this repository.""" + assert cc.allocation_floor() > 1000 + + +def test_a_number_below_the_floor_can_never_be_issued() -> None: + """#1203 and #1231 are the two instances BACKLOG #1235 names as live traps. Both sit BELOW the + high-water mark, and alloc.ps1 starts at `$observed + 1` and never fills a hole, so neither can + ever be issued. The citations are inert BY CONSTRUCTION, not by luck. + + Note what this does NOT depend on: any allocation record under .git. Those are machine-local and + losable; the unreachability is structural and survives losing them.""" + floor = cc.allocation_floor() + assert floor >= 1203 + assert floor >= 1231 + filed = cc.allocated_numbers() + assert 1203 not in filed, "if #1203 gets filed this test has served its purpose -- update it" + assert 1231 not in filed + + +def test_a_number_above_the_floor_is_the_live_shape() -> None: + assert cc.allocation_floor() < 8999 + + +def test_the_floor_is_conservative_never_optimistic(tmp_path: pathlib.Path) -> None: + """Built from the ledgers only, so it can only ever UNDERSTATE the allocator's true floor (which + also spans refs and allocations). Understating means over-warning, never a missed trap.""" + ledger = tmp_path / "L.md" + ledger.write_text("## 1500. an item\n\n> open\n", encoding="utf-8") + assert cc.allocation_floor([ledger]) == 1500 From 8387c88db4cf9e43354dac97aa5e5aabf4f29926 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 14 Aug 2026 00:16:06 -0500 Subject: [PATCH 5/5] docs(tooling): record WHY the floor's conservatism is the design (BACKLOG #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. --- scripts/docs/dangling_citation_check.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/scripts/docs/dangling_citation_check.py b/scripts/docs/dangling_citation_check.py index 73e8d51d..067bd888 100644 --- a/scripts/docs/dangling_citation_check.py +++ b/scripts/docs/dangling_citation_check.py @@ -131,9 +131,22 @@ def allocation_floor(sources: list[Path] | None = None) -> int: that does not actually depend on it. Reasoning from the registry makes a sound property look fragile and invites a guard nothing needs. - Deliberately conservative: the true floor is the max over origin/main, every ref AND every - allocation, so this ledger-only figure can only ever be LOWER. A number between the two is - reported as reachable when it is not -- an over-warning, never a missed trap. + Deliberately conservative: 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 (``:97``) -- so + this ledger-only figure can only ever sit at or BELOW the true floor. A number between the two is + reported as reachable when it is not: an over-warning, never a missed trap. + + THAT GAP IS THE POINT, NOT A ROUNDING ERROR, AND THE FLAGS IT PRODUCES ARE NOT FALSE POSITIVES. + A number that has been ALLOCATED but whose heading is not yet committed sits above this floor and + is reported live -- correctly, because the allocator's ratchet persists the max of the observed + set rather than the number just issued, so until that heading lands the only durable record of it + is an untracked, never-pushed registry file. Lose that file and the number is re-issued. + + The flag then clears ITSELF: once the heading is committed the floor rises past it and it stops + being reported, which is exactly when it becomes permanently reserved. Do not "fix" a live report + on a freshly-allocated number, and do not add a near-the-floor rule to catch this case -- this + floor already covers it, and a second rule aimed at an edge the first one covers is how a tool + acquires a guard nothing needs. """ numbers = allocated_numbers(sources) return max(numbers) if numbers else 0