From 311b87ea45cd0fead809c9faba7c94ff6f6c914e Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 07:51:44 -0500 Subject: [PATCH 01/11] test(conftest): keep the two top-level conftest modules unreachable by bare name (BACKLOG #1255) `testpaths` names two roots and both ship a `conftest.py`. Neither root is a package and no `importmode` is set, so pytest's default `prepend` gives both files the same importable name, `conftest`, and a run collecting both trees lets only one win `sys.modules`. The trap is silent rather than loud, which is what earns a guard instead of a comment: the two conftests duplicate the logging-quiesce machinery, so a mis-bound `import conftest` need not raise -- it can SUCCEED and hand back the wrong tree's implementation. Measured by AST over both files, they share 10 top-level names counting module-level constants, 8 counting only defs and classes. The item records 8; the two figures agree on the same population and differ only over whether `_ABOVE_CRITICAL` and `_QUIESCE_TARGETS` count, so the narrower rule is the item's own. THE ITEM'S RECOMMENDED FIX WAS MEASURED AND DOES NOT WORK, which is why this lands as a guard. `__init__.py` in BOTH roots is what #1255 proposes: both directories are named `tests`, so both conftests become `tests.conftest`, the collision moves up one level and turns fatal -- `_pytest.pathlib.ImportPathMismatchError`, and the whole suite fails to collect. A marker in the root tree only leaves the mis-bind permanently pointed at the web tree. `importmode = "importlib"` DOES remove the collision, and is deliberately NOT taken here rather than rejected. #1255 states it would break the 44 files under these roots that import the `tests` package; that did not reproduce on pytest 9.1.1 in a sandbox. One helper in a sandbox is not 44 real files, so the honest status is that the item's stated risk needs re-measuring on this tree before anyone adopts or dismisses it. Switching the import semantics of 691 files on an unreproduced premise is the change this guard exists to avoid needing. No present-tense defect is claimed: today's behaviour is correct because nothing does the bare import. This makes the house idiom -- shared helpers in named modules, imported package-qualified -- enforceable instead of customary. The guard ships its own controls, because an empty findings list is what both a clean tree and a dead walker return: a positive control on a planted import, a negative control pinning the three shapes that must NOT be findings, a scope control asserting every testpath root was reached and a floor on import statements seen, and a premise pin that reds if the two conftests ever stop claiming one name, telling the reader to re-price rather than leaving decoration behind. Roots are read from `pyproject.toml`, so a third testpath is covered without widening a literal. Verified: guard reds on a planted bare import naming the exact offender, reverts byte-identical and greens again; 13 passed across the guard and the partition test; ruff 0.15.22 clean; mypy strict clean; zero cp1252-unsafe characters. --- tests/test_conftest_name_collision_guard.py | 220 ++++++++++++++++++++ tests/test_tooling_partition.py | 11 + 2 files changed, 231 insertions(+) create mode 100644 tests/test_conftest_name_collision_guard.py diff --git a/tests/test_conftest_name_collision_guard.py b/tests/test_conftest_name_collision_guard.py new file mode 100644 index 00000000..1bbdebfc --- /dev/null +++ b/tests/test_conftest_name_collision_guard.py @@ -0,0 +1,220 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Keep the two top-level ``conftest`` modules unreachable by bare name (BACKLOG #1255). + +``testpaths`` names two roots and BOTH ship a ``conftest.py``; neither root is a package and +``pyproject.toml`` sets no ``importmode``, so pytest runs its default ``prepend`` and both files +claim the same importable name, ``conftest``. In a run that collects both trees only one wins +``sys.modules``. + +**THE TRAP IS SILENT, NOT LOUD, WHICH IS WHY IT EARNS A GUARD RATHER THAN A COMMENT.** The two +conftests duplicate the logging-quiesce machinery, so they share top-level names and a mis-bound +``import conftest`` does not necessarily raise -- it can SUCCEED and hand back the wrong tree's +implementation. Measured by AST over both files: **10 shared top-level names counting module-level +constants, 8 counting only defs and classes.** (BACKLOG #1255 records 8; the two figures agree on +the same population and differ only in whether ``_ABOVE_CRITICAL`` and ``_QUIESCE_TARGETS`` count, +so the narrower rule is the item's, not a stale reading.) + +Demonstrated on these real trees, not only in the abstract: with a bare ``import conftest`` planted +in ``tests/`` and both testpaths collected together, collection succeeded with no error and +``sys.modules["conftest"]`` resolved to the WEB tree's file. + +**WHY A GUARD RATHER THAN A STRUCTURAL FIX.** The two package-marker options were measured against +this tree and both are worse than the status quo; the import-mode option is unresolved rather than +rejected: + +* ``__init__.py`` in BOTH roots is the option BACKLOG #1255 recommends, and it does not work. Both + directories are named ``tests``, so both conftests become ``tests.conftest`` -- the collision does + not go away, it moves up one level and turns fatal. Measured in a scratch sandbox with this + topology: ``_pytest.pathlib.ImportPathMismatchError``, and **the whole suite fails to collect.** +* ``__init__.py`` in the root tree ONLY leaves the mis-bind in place: the root tree's bare import + then resolves to the web tree's module every time, rather than by collection order. +* ``importmode = "importlib"`` DOES remove the collision -- a bare ``import conftest`` becomes a + clean ``ModuleNotFoundError``. It is not taken here because its cost is **unmeasured against this + tree**, not because it was shown to break: 44 files under these two roots import the ``tests`` + package, INCLUDING ``tests/conftest.py`` itself. #1255 states that importlib mode does not put + rootdir on ``sys.path`` and would therefore break them; **that did not reproduce on pytest 9.1.1** + (a ``from tests.X import ...`` resolved under importlib in the sandbox, with and without + ``pythonpath``, under the bare ``pytest`` entry point as well as ``python -m pytest``). One + trivial helper in a sandbox is not 44 real files, so the honest status is that the item's stated + risk needs re-measuring on this tree before anyone adopts or dismisses the option. Switching the + import semantics of 691 files on an unreproduced premise is the change this guard exists to avoid + needing. + +So the cheap, correct move is to keep the module name unreachable. That is already the house idiom +-- shared helpers live in named modules imported package-qualified (``tests/_workflow_contexts.py``, +imported as ``from tests._workflow_contexts import ...``) -- and this file makes the idiom +enforceable instead of customary. + +**DO NOT "FIX" A FUTURE VIOLATION BY IMPORTING ``conftest`` BY PATH.** ``tests/conftest.py`` claims +a per-process test slot and registers an ``atexit`` unlink, so importing it a second time under +another name has side effects. Move the helper into a named module instead. + +**AST, NOT ``grep``.** An earlier attempt to census these names with ``grep -oP`` died on this box's +locale and printed nothing, which is indistinguishable from a clean result. The scan below parses, +and pairs its null with a positive control, for the same reason. +""" + +from __future__ import annotations + +import ast +import tomllib +import warnings +from dataclasses import dataclass +from functools import cache +from pathlib import Path + +from tests._workflow_contexts import ROOT + +#: The scan is worthless if it silently walks an empty tree, so it asserts it saw at least this many +#: import statements overall. Far below the real figure (7121 at the time of writing) -- this is a +#: liveness floor for the walker, not a pinned count that rots on every added import. +_MIN_IMPORT_STATEMENTS = 500 + + +@dataclass(frozen=True) +class _Scan: + """What the walk found, plus enough about HOW it walked to tell a null from a dead instrument.""" + + findings: tuple[str, ...] + files_by_root: tuple[tuple[str, int], ...] + import_statements: int + + +def _testpath_roots() -> tuple[Path, ...]: + """Read the roots from ``pyproject.toml`` rather than hard-coding them. + + A third testpath added tomorrow ships a third top-level ``conftest.py`` candidate, and this guard + has to cover it without anyone remembering to widen a literal here. + """ + cfg = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) + testpaths: list[str] = cfg["tool"]["pytest"]["ini_options"]["testpaths"] + return tuple(ROOT / p for p in testpaths) + + +def bare_conftest_imports(tree: ast.Module) -> list[tuple[int, str]]: + """Return ``(lineno, rendered)`` for every import binding the top-level name ``conftest``. + + Relative imports (``from . import conftest``) are unambiguous -- they resolve against the + importing module's own package -- so ``node.level > 0`` is deliberately not a finding. Nor is + ``from tests.conftest import ...``: package-qualified is exactly the shape this guard steers to. + """ + hits: list[tuple[int, str]] = [] + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + if alias.name == "conftest" or alias.name.startswith("conftest."): + hits.append((node.lineno, f"import {alias.name}")) + elif ( + isinstance(node, ast.ImportFrom) + and node.level == 0 + and node.module is not None + and (node.module == "conftest" or node.module.startswith("conftest.")) + ): + hits.append((node.lineno, f"from {node.module} import ...")) + return hits + + +def _parse(path: Path) -> ast.Module: + # A scanned file's own SyntaxWarning (invalid escape sequences live in at least one test module) + # is that file's business, not this guard's -- it must not colour this run. + with warnings.catch_warnings(): + warnings.simplefilter("ignore", SyntaxWarning) + return ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + + +@cache +def _scan() -> _Scan: + findings: list[str] = [] + files_by_root: list[tuple[str, int]] = [] + import_statements = 0 + for root in _testpath_roots(): + count = 0 + for py in sorted(root.rglob("*.py")): + count += 1 + tree = _parse(py) + import_statements += sum( + 1 for n in ast.walk(tree) if isinstance(n, (ast.Import, ast.ImportFrom)) + ) + for lineno, what in bare_conftest_imports(tree): + findings.append(f"{py.relative_to(ROOT).as_posix()}:{lineno}: {what}") + files_by_root.append((root.relative_to(ROOT).as_posix(), count)) + return _Scan(tuple(findings), tuple(files_by_root), import_statements) + + +def _importable_name(conftest: Path) -> str: + """The top-level module name pytest's ``prepend`` mode would give this file. + + Prepend walks up from the file while each directory is a package, then inserts the first + non-package ancestor on ``sys.path``. So a ``conftest.py`` in a non-package directory is simply + ``conftest``, and two of those collide. + """ + parts = [conftest.stem] + parent = conftest.parent + while (parent / "__init__.py").exists(): + parts.append(parent.name) + parent = parent.parent + return ".".join(reversed(parts)) + + +def test_no_module_under_a_testpath_imports_conftest_by_bare_name() -> None: + """THE GUARD. Reintroducing the bare import arms the collision, so it reds here.""" + scan = _scan() + assert not scan.findings, ( + "A bare `import conftest` binds to whichever testpath root pytest loaded first, and the two " + "conftests share top-level names, so this can succeed and return the WRONG tree's " + "implementation. Move the helper into a named module and import it package-qualified " + "(see tests/_workflow_contexts.py). Offenders:\n " + "\n ".join(scan.findings) + ) + + +def test_the_detector_trips_on_a_planted_bare_import() -> None: + """POSITIVE CONTROL. Without it, a dead detector reads exactly like a clean tree.""" + planted = ast.parse( + "import conftest\nfrom conftest import _Baseline\nfrom conftest.sub import x\n" + ) + assert len(bare_conftest_imports(planted)) == 3 + + # NEGATIVE CONTROL: the shapes that must NOT be findings, or the guard would forbid the very + # idiom it is steering people towards. + allowed = ast.parse( + "from tests.conftest import x\nimport conftesting\nfrom . import conftest\n" + ) + assert bare_conftest_imports(allowed) == [] + + +def test_the_scan_reached_every_testpath_root() -> None: + """SCOPE CONTROL. A walk that visited nothing returns the same empty findings as a clean tree.""" + scan = _scan() + roots = dict(scan.files_by_root) + assert set(roots) == {r.relative_to(ROOT).as_posix() for r in _testpath_roots()} + for name, count in roots.items(): + assert count > 0, f"scanned zero files under {name}; the guard proved nothing" + assert scan.import_statements >= _MIN_IMPORT_STATEMENTS, ( + f"only {scan.import_statements} import statements seen across {roots}; the walker is not " + "reading these files, so its empty findings mean nothing" + ) + + +def test_the_collision_that_makes_this_guard_necessary_is_still_present() -> None: + """PREMISE PIN. When this reds, the guard has become re-priceable -- read the module docstring. + + It fails in exactly one direction that matters: someone lands a structural change and the two + conftests stop claiming one name. That is good news, not a defect, and the guard can then be + retired rather than quietly kept on as decoration. + """ + names = [ + _importable_name(root / "conftest.py") + for root in _testpath_roots() + if (root / "conftest.py").exists() + ] + assert len(names) >= 2, ( + f"only {len(names)} testpath root(s) ship a conftest.py ({names}), so two of them can no " + "longer claim one module name. Nothing is wrong here -- the premise behind this guard " + "changed, so re-price it against BACKLOG #1255 rather than leaving it in place unexplained." + ) + assert len(set(names)) < len(names), ( + f"the testpath conftests now resolve to distinct module names {names}, so the bare-name " + "collision this guard exists for is gone. Re-price the guard against BACKLOG #1255 rather " + "than leaving it in place unexplained." + ) diff --git a/tests/test_tooling_partition.py b/tests/test_tooling_partition.py index bcda4a96..cc5bb01b 100644 --- a/tests/test_tooling_partition.py +++ b/tests/test_tooling_partition.py @@ -61,6 +61,17 @@ "test_asvs_apply.py", "test_asvs_residual_lint.py", "test_c901_delta.py", + # NOT engine source, so this entry WIDENS the list's stated rule and the claim is spelled out + # for review. Its subject is the TEST TREES: it scans both `testpaths` roots for a bare + # `import conftest`, which binds to whichever root pytest loaded first (BACKLOG #1255). The + # gating argument is the one that keeps test_control_char_check.py here, applied to a + # different scanned population: what it guards is `tests/**` and + # `packaging/messagefoundry-webconsole/tests/**`, and a change to either sets `code=true` (a + # .py path) but NOT `tooling=true` -- that gate names only three tests/ files (conftest.py, + # tooling_manifest.txt, test_tooling_partition.py). Listed as tooling it would be deselected + # by `-m 'not tooling'` on the engine legs AND unreached by the tooling job's path gate, so + # the PR that adds the offending import would face nothing. + "test_conftest_name_collision_guard.py", "test_cp1252_console_safety.py", # Arrived with #421 while this branch was in flight. Same shape as cp1252_console_safety and # licence_header_gate above: a repo-wide scanner over TRACKED TEXT, which includes From 924095e174d96d3349e952de2c5193240ad35a27 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 07:56:55 -0500 Subject: [PATCH 02/11] fix(test): give the subtree walk a bounded extension, and split could-not-measure from broken (BACKLOG #1290) `test_subtree_re_resolution_picks_up_a_late_spawned_child` red-ed a REQUIRED context on `main` (`test (windows-2022, py3.14)`, and the windows-2025 leg since) on the `walked_ok` assertion: the process-table walk returned None on every one of 6 attempts in 30 s, the Windows path allowing 5 s per walk. It is nondeterministic -- the identical leg passed on another branch in the same hour, and re-running the failed job alone on the same head returned success. The failure fires on the FIRST of the test's two assertions, so the re-resolution regression assertion below it never runs. While this is red the test is not merely failing, it is SILENT on the defect it was written to detect -- which is the worse half and the reason this is not just noise. WHY MORE ATTEMPTS WERE NOT THE FIX. The old loop retried at the same 5 s bound, so six attempts bought six identical cut-offs and no measurement. The extension grants a LONGER budget (3 x `_PROBE_TIMEOUT_S`), which is what actually attacks a starved runner. It is entered ONLY when no walk succeeded at all, so it cannot grind a genuine missing-child result into a pass; it is bounded at two walks; it is trimmed to fit the per-test watchdog READ from `--timeout` rather than hardcoded (ci.yml passes 60 s on ubuntu and 120 s on the Windows legs, and a copy here would drift in the bad direction -- an extension outgrowing the watchdog turns a clean skip into a timeout kill with no verdict); and it is undone at teardown. THE SKIP IS NARROW ON PURPOSE, BECAUSE A SKIP-ON-LOAD IS HOW A PROBE REGRESSION HIDES FOREVER. A walk that spends its whole budget measures the RUNNER, and now skips. A walk that returns None WITHOUT spending its budget is a broken enumerator -- it errored or returned zero rows -- and still FAILS. Those get opposite verdicts off `_BUDGET_CONSUMED_FRACTION`, stated once. `subprocess.run` cannot raise `TimeoutExpired` before its timeout, so a genuine timeout always lands above that line and an immediate error far below it. ALSO FIXES THE TEST'S OWN INSTRUMENT, found while doing the above. The loop scored a walk by `sampler._pids is not None`, but `_pids` RETAINS the last good resolution across a subsequent failed walk -- that is the cache's purpose -- so once any walk succeeded, every later walk scored a success and reported the stale cache as that walk's result. `_resolve_errored` is set per walk and answers the question actually being asked. No production code changed: `harness/load/connscale/probe.py` is untouched. VERIFIED, both directions of the discriminator, by planting into the probe and reverting it byte-identical each time: broken enumerator, returns None instantly -> FAILED, "121 of them returned None WITHOUT spending the timeout they were given", not downgraded starved runner, each walk spends its budget -> SKIPPED, "6 walk(s) at a 5s budget, spending 5.0-5.0s", extension granted 0 walks against the 60 s watchdog, which is the correct answer there healthy path -> 21 passed Attribution of the 25 pre-existing mypy errors in this file done by controlled revert plus a second instrument: the error MESSAGE SETS at HEAD and with this change are identical (not merely equal in count), and zero of them mention any symbol this change adds. The 16 cp1252-unsafe characters in this file are likewise unchanged by it -- census identical at HEAD and here. Neither is introduced by this commit and neither is fixed by it. ruff 0.15.22 clean. --- tests/test_connscale_cpu_probe.py | 183 ++++++++++++++++++++++++++---- 1 file changed, 161 insertions(+), 22 deletions(-) diff --git a/tests/test_connscale_cpu_probe.py b/tests/test_connscale_cpu_probe.py index 55d113e0..e830086b 100644 --- a/tests/test_connscale_cpu_probe.py +++ b/tests/test_connscale_cpu_probe.py @@ -33,6 +33,7 @@ import pytest +from harness.load.connscale import probe from harness.load.connscale.probe import ( _CREATION_SKEW_TOLERANCE_S, _PROBE_TIMEOUT_S, @@ -53,6 +54,41 @@ #: the transient-failure tolerance `_resolve_pids` is written to provide. _RESOLUTION_DEADLINE_S = max(30.0, 6 * _PROBE_TIMEOUT_S) +#: Per-walk timeout granted during the bounded extension below, in place of the probe's production +#: ``_PROBE_TIMEOUT_S``, for the duration of THIS test only (BACKLOG #1290). +#: +#: The measured cause of the #1290 red is ONE walk failing to complete within 5 s on a starved hosted +#: runner. More 5 s attempts do not attack that: each is cut off at the same point, so six of them buy +#: six identical timeouts and no measurement. A LONGER walk does attack it. The 5 s bound exists to stop +#: a hung shell-out wedging the RUNNER's poll tick; this test is not on that cadence, and no test in this +#: file asserts the bound's value, so raising it here removes no coverage. +_STALLED_WALK_TIMEOUT_S = 3.0 * _PROBE_TIMEOUT_S + +#: Hard ceiling on how many long walks the extension may make. BOUNDED on purpose: it is entered only +#: on a runner that has already spent ``_RESOLUTION_DEADLINE_S`` without one usable enumeration, and an +#: unbounded retry there would trade a red required context for a hung job. The healthy path is +#: untouched by any of this — it resolves in one or two walks and costs well under a second. +_STALL_EXTENSION_WALKS = 2 + +#: Share of this test's own pytest-timeout watchdog that the whole poll may spend before it must reach +#: a verdict. The extension is trimmed to fit, so the walks it is granted DERIVE from the watchdog. +#: +#: This bound is not optional. Being killed by the watchdog fails the test with a thread dump and NO +#: verdict at all — strictly worse than the failure #1290 is fixing, and it would arrive on exactly the +#: stalled runner the extension exists to serve. Reading the watchdog rather than hardcoding it matters +#: because the value is per leg: ci.yml's matrix passes 60 s on ubuntu and 120 s on the Windows legs +#: (`pytest_timeout`), and the Windows legs are the ones where the walk stalls. On a 60 s watchdog the +#: extension trims to ZERO walks and the test degrades to a 30 s poll then a skip, which is the correct +#: answer there; on the 120 s Windows legs both extension walks fit inside 60 s of an 84 s share. +_WATCHDOG_SHARE = 0.7 + +#: A failed walk counts as BUDGET-EXHAUSTED (the runner was too slow to enumerate) rather than a FAST +#: error (the enumeration itself is broken) when it spent at least this fraction of the timeout it was +#: given. The two get OPPOSITE verdicts -- skip and fail -- so the discriminator is stated once, here. +#: The slack absorbs clock granularity only: ``subprocess.run`` cannot raise ``TimeoutExpired`` before +#: its timeout, so a genuine timeout always lands above this line and an immediate error far below it. +_BUDGET_CONSUMED_FRACTION = 0.9 + def _sample(elapsed: float) -> EngineSample: return EngineSample( @@ -260,15 +296,89 @@ def test_sampler_measures_a_descendant_that_actually_burns_cpu() -> None: assert after.cpu_seconds - first.cpu_seconds > 0.5 +def _granted_extension_walks(config: pytest.Config) -> int: + """How many long walks the bounded extension may make here, trimmed to fit the per-test watchdog. + + The watchdog is READ (``--timeout``, which ci.yml passes per leg) rather than hardcoded: a copy of + the number in this file would drift from the matrix, and the direction it drifts in is the bad one — + an extension that outgrows the watchdog converts a clean skip into a timeout kill with no verdict. + Returns 0 when there is no room, which is a correct outcome, not a degraded one: the poll still runs + for ``_RESOLUTION_DEADLINE_S`` and still reports which of the two failures occurred.""" + watchdog = config.getoption("timeout", default=None) + if not isinstance(watchdog, int | float) or watchdog <= 0: + return _STALL_EXTENSION_WALKS # no watchdog to fit inside; the hard ceiling still applies + spare_s = watchdog * _WATCHDOG_SHARE - _RESOLUTION_DEADLINE_S + return max(0, min(_STALL_EXTENSION_WALKS, int(spare_s // _STALLED_WALK_TIMEOUT_S))) + + +def _spend_summary(failed: list[tuple[float, float]]) -> str: + """Group failed walks by the per-walk budget they were given: how many got it, and the range they + actually spent. + + SUMMARISED rather than enumerated. A stalled runner makes on the order of a hundred attempts inside + ``_RESOLUTION_DEADLINE_S``, and a per-walk list that long is unreadable in a CI log — which is the + same "message nobody can act on" failure #1290 is fixing at the verdict level, so it must not be + reintroduced in the text of the verdict.""" + by_budget: dict[float, list[float]] = {} + for spent, budget in failed: + by_budget.setdefault(budget, []).append(spent) + return "; ".join( + f"{len(spent)} walk(s) at a {budget:.0f}s budget, spending {min(spent):.1f}-{max(spent):.1f}s" + for budget, spent in sorted(by_budget.items()) + ) + + +def _walk_succeeded(sampler: FdSampler) -> bool: + """Run ONE subtree re-resolution and report whether THAT walk enumerated successfully. + + Reads ``_resolve_errored`` rather than ``sampler._pids is not None``, which is what the caller used + to read. ``_pids`` retains the last GOOD resolution across a subsequent FAILED walk (that is the + point of the cache), so ``_pids is not None`` scores every later walk a success once any earlier one + has succeeded, and then reports the stale cache as that walk's result. ``_resolve_errored`` is set + per walk, so it answers the question actually being asked. With ``resolve_every=1`` the cached-serve + branch of ``_resolve_pids`` (which clears the flag without walking) is never taken, so the flag here + is exactly this walk's outcome.""" + sampler.sample_proc() + return not sampler._resolve_errored + + @pytest.mark.skipif(sys.platform not in ("win32", "linux"), reason="OS FD probe path") -def test_subtree_re_resolution_picks_up_a_late_spawned_child() -> None: +def test_subtree_re_resolution_picks_up_a_late_spawned_child( + monkeypatch: pytest.MonkeyPatch, + pytestconfig: pytest.Config, +) -> None: # A3: the subtree used to be resolved exactly ONCE. A sharded engine's `serve --shard` workers appear # AFTER the supervisor, so a one-shot walk pins the sampler to an idle parent for the whole run. sampler = FdSampler(os.getpid(), resolve_every=1) sampler.sample_proc() # walk 1 — before the child exists resolved_before = list(sampler._pids or []) + walks = 0 + walked_ok = False # did ANY enumeration succeed? separates "cannot measure" from "wrong answer" + resolved_after: list[int] = [] + failed: list[tuple[float, float]] = [] # (seconds spent, seconds allowed) per FAILED walk + extension_walks = _granted_extension_walks(pytestconfig) + child = subprocess.Popen([sys.executable, "-c", _BURN]) # noqa: S603 - fixed argv, no shell + + def _attempt(budget_s: float) -> bool: + """One walk. True once the late-spawned child appears in a FRESH successful resolution. + + A failed walk is recorded with what it SPENT against what it was ALLOWED, because that pair is + the only thing that separates "this runner is too slow to enumerate" from "this enumeration is + broken" — and those two get opposite verdicts below.""" + nonlocal walks, walked_ok, resolved_after + started = time.monotonic() + ok = _walk_succeeded(sampler) + spent = time.monotonic() - started + walks += 1 + if not ok: + failed.append((spent, budget_s)) + return False + walked_ok = True + resolved_after = list(sampler._pids or []) + return child.pid in resolved_after + try: # POLL, don't sleep-and-hope. This used to be `time.sleep(1.0)` then ONE `sample_proc()`, which # contradicted the contract under test: `_resolve_pids` treats an ERRORED enumeration as @@ -281,22 +391,26 @@ def test_subtree_re_resolution_picks_up_a_late_spawned_child() -> None: # property. Measured on windows-2025 (2026-07-30): failed twice in one job, while windows-2022 # and ubuntu passed the identical commit. deadline = time.monotonic() + _RESOLUTION_DEADLINE_S - walks = 0 - walked_ok = ( - False # did ANY enumeration succeed? separates "cannot measure" from "wrong answer" - ) - resolved_after: list[int] = [] - while True: - sampler.sample_proc() - walks += 1 - if sampler._pids is not None: - walked_ok = True - resolved_after = list(sampler._pids) - if child.pid in resolved_after: - break - if time.monotonic() >= deadline: + found = False + while not found: + found = _attempt(_PROBE_TIMEOUT_S) + if found or time.monotonic() >= deadline: break time.sleep(0.25) + + # BOUNDED EXTENSION (BACKLOG #1290). Entered only when the loop above produced NO usable + # enumeration at all — never when a walk succeeded, so it cannot be used to grind a genuine + # missing-child result into a pass. Each extension walk gets a LONGER budget, because the + # failure being extended past is a walk that ran out of budget; repeating it at the same 5 s + # bound reproduces the same cut-off. This is the only place the probe's production timeout is + # raised, it is undone at teardown, and the walk count is trimmed to fit the watchdog. + # (`found` is not tested here: it can only be True via a successful walk, so it implies + # `walked_ok` and would add a condition a reader could mistake for an independent one.) + if not walked_ok and extension_walks: + monkeypatch.setattr(probe, "_PROBE_TIMEOUT_S", _STALLED_WALK_TIMEOUT_S) + for _ in range(extension_walks): + if _attempt(_STALLED_WALK_TIMEOUT_S): + break finally: child.kill() child.wait(timeout=10) @@ -306,14 +420,39 @@ def test_subtree_re_resolution_picks_up_a_late_spawned_child() -> None: # Two distinct failures, reported distinctly. Collapsing them is what made the original message # useless: "the probe could not enumerate at all" is an environment/probe problem, while "it # enumerated and missed a live child" is the re-resolution regression this test exists to catch. - assert walked_ok, ( - f"the process-table walk never succeeded in {_RESOLUTION_DEADLINE_S:.0f}s ({walks} attempts) — " - f"every one returned None (enumeration errored or timed out; the Windows path allows " - f"{_PROBE_TIMEOUT_S:.0f}s per walk). The probe could not measure at all, so this test could not " - f"assess re-resolution. Not the same as missing the child." - ) + # #1290 keeps that separation and adds the verdict it was missing: the first is not a defect in the + # tree under test, so it must stop reddening a required context — but only when the walk actually + # ran out of budget. A walk that returns None WITHOUT spending its budget is a broken enumerator, + # and downgrading that to a skip is how a skip-on-load hides a probe regression forever. + if not walked_ok: + assert failed, "the loop made no attempt at all, so neither failure can be reported" + fast = [(s, b) for s, b in failed if s < b * _BUDGET_CONSUMED_FRACTION] + spend = _spend_summary(failed) + if fast: + pytest.fail( + f"ENUMERATION FAILURE -- failure ONE of the two this test keeps apart, NOT the " + f"re-resolution regression. The process-table walk never succeeded in {walks} " + f"attempts, and {len(fast)} of them returned None WITHOUT spending the timeout they " + f"were given [{spend}]. A walk that fails FAST did not run out of budget -- the " + f"enumeration errored or returned zero rows -- so this is a probe defect, reported as a " + f"failure and deliberately NOT downgraded to a skip. Re-resolution could not be " + f"assessed either way." + ) + pytest.skip( + f"ENUMERATION TIMEOUT -- failure ONE of the two this test keeps apart, NOT the " + f"re-resolution regression. The process-table walk never succeeded in {walks} attempts " + f"[{spend}], every one of them spending its whole timeout; the bounded extension was " + f"granted {extension_walks} walk(s) of {_STALLED_WALK_TIMEOUT_S:.0f}s against a per-test " + f"watchdog of {pytestconfig.getoption('timeout', default=None)}s. The probe could not " + f"enumerate AT ALL, so this test could not assess re-resolution: it is reporting COULD NOT " + f"MEASURE, not measured-and-wrong. Skipped rather than failed because an exhausted walk " + f"budget measures the runner, not this tree (BACKLOG #1290); a walk that fails WITHOUT " + f"spending its budget still FAILS, so a broken enumerator cannot hide behind this skip." + ) + assert child.pid in resolved_after, ( - f"the walk SUCCEEDED but did not include the late-spawned child {child.pid} after {walks} " + f"RE-RESOLUTION REGRESSION -- failure TWO of the two this test keeps apart. The walk SUCCEEDED " + f"but did not include the late-spawned child {child.pid} after {walks} " f"attempts over {_RESOLUTION_DEADLINE_S:.0f}s; last resolution was {resolved_after}. This is " f"the A3 regression: a subtree resolved once pins the sampler to an idle parent, so a sharded " f"engine's `serve --shard` workers are never counted." From 991bfa6f8866963705a46a6d1c428ee5234da233 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 09:27:23 -0500 Subject: [PATCH 03/11] fix(test): the bounded extension could only ever produce a FALSE regression verdict (BACKLOG #1290) Fixes a BLOCKING defect in b3bc8026, found by an adversarial review of that commit and reproduced here before changing anything. b3bc8026 is unpushed and in no PR, so this never reached anyone. THE DEFECT. `_BURN` is a BOUNDED loop -- measured on this box at 12.3 s, 12.2 s and 11.6 s, exiting on its own with rc=0. The bounded extension is entered only after the poll has spent `_RESOLUTION_DEADLINE_S` (30 s) with no successful walk, which is already about 2.5x the child's lifetime. So every extension walk necessarily runs after the child is dead, and when one SUCCEEDS -- which is precisely what it was built to do -- `walked_ok` flips, the skip arm is bypassed, and control falls through to the final assertion, which reports RE-RESOLUTION REGRESSION. That is a confident false accusation of the A3 product defect, on a run that never tested for it. It is strictly WORSE than the behaviour it replaced: before b3bc8026 the same conditions said "the process-table walk never succeeded", which was true. WHY MY OWN RED-FIRST MISSED IT, and this is the transferable part. I planted two violations and reported the fix proven "in both directions". Both were the same direction. A broken enumerator and a starved runner BOTH make every walk fail, so `walked_ok` never flips and only the skip and fail arms are reachable. The extension's only path is FAIL-THEN-SUCCEED, and neither plant produced it. Two plants that agree are not two directions. THE FIX is a liveness latch, not a wider tolerance. A successful walk records whether the target was still running at that moment; if no successful walk happened during the child's lifetime and the child is absent, the verdict is COULD NOT MEASURE (failure one) rather than a regression (failure two). A LATCH, NOT A SAMPLE, AND THAT DISTINCTION IS THE SECOND DEFECT. The first version of this fix sampled liveness at the LAST successful walk, and it suppressed the genuine regression: when the child is never found the poll runs the full 30 s, so the last walk is post-exit even in runs where EARLY walks succeeded while the child was alive and legitimately showed it missing. Latching on ANY in-lifetime success keeps the real finding and discards only the unmeasurable one. A NARROW FORM WAS REACHABLE BEFORE b3bc8026 TOO -- a walk failing past about 14 s then succeeding inside the 30 s deadline hits it identically. The extension widened the window and made post-exit success the DESIGNED path. This guard closes both, so the file ends up better than it started rather than merely repaired. VERIFIED ACROSS ALL FIVE PATHS, each plant reverted byte-identical afterwards: healthy -> 21 passed broken enumerator, fails fast without spending budget -> FAILED, enumeration failure starved runner, every walk spends its whole budget -> SKIPPED, enumeration timeout walk needs 8 s: fails at the 5 s bound, succeeds at 15 s -> SKIPPED, could not measure walk succeeds instantly, child ALIVE, child omitted -> FAILED, RE-RESOLUTION REGRESSION The last one is the control that matters: it proves the guard did not make the regression assertion unreachable. The first version of the fix skipped it, which is how the latch defect was caught. ruff 0.15.22 clean; no cp1252-unsafe character introduced; the 25 pre-existing mypy errors in this file are unchanged and none mentions a symbol this change adds. --- tests/test_connscale_cpu_probe.py | 45 +++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/tests/test_connscale_cpu_probe.py b/tests/test_connscale_cpu_probe.py index e830086b..5d7acb32 100644 --- a/tests/test_connscale_cpu_probe.py +++ b/tests/test_connscale_cpu_probe.py @@ -355,6 +355,7 @@ def test_subtree_re_resolution_picks_up_a_late_spawned_child( walks = 0 walked_ok = False # did ANY enumeration succeed? separates "cannot measure" from "wrong answer" + child_alive_at_success = False # was the target still running when a walk finally succeeded? resolved_after: list[int] = [] failed: list[tuple[float, float]] = [] # (seconds spent, seconds allowed) per FAILED walk extension_walks = _granted_extension_walks(pytestconfig) @@ -367,7 +368,7 @@ def _attempt(budget_s: float) -> bool: A failed walk is recorded with what it SPENT against what it was ALLOWED, because that pair is the only thing that separates "this runner is too slow to enumerate" from "this enumeration is broken" — and those two get opposite verdicts below.""" - nonlocal walks, walked_ok, resolved_after + nonlocal walks, walked_ok, resolved_after, child_alive_at_success started = time.monotonic() ok = _walk_succeeded(sampler) spent = time.monotonic() - started @@ -376,6 +377,17 @@ def _attempt(budget_s: float) -> bool: failed.append((spent, budget_s)) return False walked_ok = True + # A LATCH, NOT A SAMPLE, AND THE DISTINCTION IS LOAD-BEARING. `_BURN` is a BOUNDED loop -- + # roughly 12 s, then it exits on its own -- so a walk succeeding late enough is enumerating a + # table the child has already left, and its absence there proves nothing about re-resolution. + # But the poll runs to `_RESOLUTION_DEADLINE_S` (30 s) whenever the child is never found, so + # the LAST successful walk is post-exit even in runs where EARLY walks succeeded while the + # child was alive and legitimately showed it missing. Sampling at the end therefore suppresses + # the real A3 regression; latching on ANY in-lifetime success preserves it. Measured: with a + # probe that enumerates instantly but never reports descendants -- the genuine defect -- the + # sampled form skipped and this form fails, which is the whole point of the assertion. + if child.poll() is None: + child_alive_at_success = True resolved_after = list(sampler._pids or []) return child.pid in resolved_after @@ -450,12 +462,35 @@ def _attempt(budget_s: float) -> bool: f"spending its budget still FAILS, so a broken enumerator cannot hide behind this skip." ) + # THE TARGET MUST HAVE BEEN ALIVE WHEN THE WALK SUCCEEDED, OR ITS ABSENCE PROVES NOTHING. This + # guard is failure ONE's third form, and it is the one a bounded burner makes reachable: `_BURN` + # completes in roughly 12 s on its own, while a walk can succeed later than that -- always, once + # the bounded extension has spent `_RESOLUTION_DEADLINE_S` first. A child absent because it EXITED + # is not a child the re-resolution missed, so reporting it as the A3 regression would be a + # confident false accusation of a product defect that is not there. Skipped rather than failed for + # the same reason as the enumeration timeout above: the test could not MEASURE, and a green that + # rests on an unmeasurable run is what this whole item is about. + # + # A NARROW FORM OF THIS WAS REACHABLE BEFORE #1290 TOO -- a walk failing past roughly 14 s and then + # succeeding inside the 30 s deadline hits it identically. The bounded extension widened the window + # and made post-exit success the DESIGNED path, which is what turned a latent edge into the normal + # one. This guard closes both. + if walked_ok and child.pid not in resolved_after and not child_alive_at_success: + pytest.skip( + f"COULD NOT MEASURE -- the walk succeeded only AFTER the target exited, so the child's " + f"absence from the process table proves nothing about re-resolution. `_BURN` is a bounded " + f"loop (about 12 s); the successful walk landed after it had already completed, following " + f"{walks} attempt(s) over {_RESOLUTION_DEADLINE_S:.0f}s. This is failure ONE, not the A3 " + f"regression: reporting it as a regression would accuse the product of a defect the run " + f"never tested for." + ) + assert child.pid in resolved_after, ( f"RE-RESOLUTION REGRESSION -- failure TWO of the two this test keeps apart. The walk SUCCEEDED " - f"but did not include the late-spawned child {child.pid} after {walks} " - f"attempts over {_RESOLUTION_DEADLINE_S:.0f}s; last resolution was {resolved_after}. This is " - f"the A3 regression: a subtree resolved once pins the sampler to an idle parent, so a sharded " - f"engine's `serve --shard` workers are never counted." + f"WHILE THE CHILD WAS STILL RUNNING and still did not include the late-spawned child " + f"{child.pid}, after {walks} attempts over {_RESOLUTION_DEADLINE_S:.0f}s; last resolution was " + f"{resolved_after}. This is the A3 regression: a subtree resolved once pins the sampler to an " + f"idle parent, so a sharded engine's `serve --shard` workers are never counted." ) From 27ff16c92ae1a2a2f53fe80105b36e6f87c64fab Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 13:43:37 -0500 Subject: [PATCH 04/11] fix(test): a zero-read engine now reports the engine's OWN reason instead of a bare assert 0 > 0 tests/test_multishard_smoke.py has been red on main with `assert e.reads > 0` firing as `assert 0 > 0`, and the failure could not be attributed. This does not change what the test ASSERTS. It changes what the failure SAYS, which is the part that was unusable. I HANDED THIS INVESTIGATION A HYPOTHESIS AND IT WAS WRONG. I reasoned that because the two assertions above it PASS -- `inbound_rows == _COUNT_PER_ENGINE` and `foreign_rows == 0` -- the rows had ARRIVED and only the counter was wrong, so this was a counter defect. That is refuted. Both of those counters are CONFIG-derived, not traffic-derived: the `/connections` builder appends a source row for EVERY registry inbound unconditionally and sets `read` to an int rather than None, so both pass unchanged on an engine that received NOTHING -- including one whose listeners never bound. The test's own docstring already conceded it: the isolation proof "is config-derived so it holds regardless of the write lock". `reads` is the ONLY traffic-derived counter of the three, and `reads == 0` means the engine genuinely received nothing. The pair I called discriminating carries zero traffic information. THE ENGINE ALREADY KNOWS WHY, AND THE HARNESS WAS THROWING IT AWAY. A lane that failed to start is reported as not-listening with a reason (ADR 0031, surfaced as `/connections`.error). The harness fetched that response, read `name` and `read` off each row, and discarded `error`. So the one artifact that could attribute the failure was fetched and dropped on every run. `EngineAttribution` now carries `failed_lanes`, the engine's verbatim reasons; the assertion prints them and distinguishes the two cases -- lanes reported as not listening (the engine never received traffic) versus no failed lanes at all (they bound and the traffic did not arrive or did not commit, a different cause this assertion cannot narrow further and now says so rather than implying it can). The JSON artifact carries the field too, so a CI reader with only the uploaded file can attribute it without re-running anything, which is the whole point. Collected for EVERY inbound row rather than only this engine's own: a lane failing under a peer's tag is equally diagnostic, and filtering by tag here would drop the cross-engine case the isolation assertion above exists to catch. READ DIRECTLY AS `row.error`, NOT `getattr(row, "error", None)`. `EngineClient.connections()` is typed `list[ConnectionRow]` and that model declares the field, so a default could only ever mask a RENAME -- after which this would report "no failed lanes" forever, silently, on precisely the runs it exists to explain. A diagnostic field that fails closed to "nothing to report" is worse than no field. VERIFIED with both controls, because a green run has no failed lanes and therefore proves nothing about the new path: failed-lane case -> inbound_rows=2, foreign_rows=0, reads=0 (the exact CI triple) AND both reasons collected into failed_lanes clean case -> reads=8, failed_lanes=() -- silent on success, so the field is not always-on noise tests/test_multishard_smoke.py: 2 passed. ruff 0.15.22 clean. No cp1252-unsafe character introduced. WHAT THIS DOES NOT DO, explicitly: it does not fix the CI red and does not claim to. Which of the conditions fired on CI is NOT ESTABLISHED -- the engine's own stdout is written to a temp file that `EngineNode` discards on stop unless `MEFOR_BENCH_KEEP_NODE_LOGS` names a directory, and the CI leg leaves it unset, so the bind-failure warning is thrown away on every run. Capturing that is the next step and is a ci.yml change I have not made. This commit makes the NEXT occurrence self-attributing, which is what four sessions lacked when a neighbouring red was mis-attributed three times today. --- harness/load/multishard.py | 34 +++++++++++++++++++++++++++++++++- tests/test_multishard_smoke.py | 28 +++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/harness/load/multishard.py b/harness/load/multishard.py index 71ae0fed..59d60035 100644 --- a/harness/load/multishard.py +++ b/harness/load/multishard.py @@ -91,6 +91,22 @@ class EngineAttribution: inbound_rows: int # number of inbound (source) connection rows this engine reports foreign_rows: int # inbound rows whose name does NOT carry this engine's tag (a steal ⇒ > 0) reads: int # Σ inbound read across this engine's own rows + #: The engine's OWN reason for each lane it reports as not-listening, verbatim from `/connections` + #: (`error`, which the API sets from `connection_failed()` per ADR 0031). Empty on a clean run. + #: + #: CARRIED BECAUSE `reads == 0` CANNOT DIAGNOSE ITSELF WITHOUT IT, and the engine already knows. + #: `inbound_rows` and `foreign_rows` are CONFIG-derived, not traffic-derived -- the API appends a + #: source row for every registry inbound unconditionally and `read` is always an int, never None + #: (`api/app.py`, the `/connections` builder), so both pass unchanged on an engine that received + #: NOTHING, including one whose listeners never bound. The test's own docstring says as much: the + #: isolation proof "is config-derived so it holds regardless of the write lock". So when `reads` + #: reads 0 the other two counters say nothing about why, and the failure message was left naming a + #: number with no cause attached. + #: + #: Measured: occupying one engine's inbound ports with a listen-never-accept squatter reproduces + #: the exact CI triple -- inbound_rows PASS, foreign_rows PASS, reads 0 FAIL -- and the engine + #: reported `status: "failed"` on both lanes throughout. The information was present and discarded. + failed_lanes: tuple[str, ...] = () @dataclass(frozen=True) @@ -195,6 +211,9 @@ def to_json_dict(self) -> dict[str, object]: "inbound_rows": e.inbound_rows, "foreign_rows": e.foreign_rows, "reads": e.reads, + # In the artifact as well as the assertion: a CI reader who has only the uploaded + # JSON must be able to attribute a zero-read engine without re-running anything. + "failed_lanes": list(e.failed_lanes), } for e in self.per_engine ], @@ -693,6 +712,7 @@ def _attribute_engines_sync( # empty attribution — the smoke asserts positive rows, so it won't silently pass. out.append(EngineAttribution(node.node_id, tag, 0, 0, 0)) continue + failed: list[str] = [] for row in rows: if row.read is None: # inbound (source) rows carry a read counter; skip outbound rows continue @@ -701,7 +721,19 @@ def _attribute_engines_sync( reads += row.read else: foreign_rows += 1 - out.append(EngineAttribution(node.node_id, tag, inbound_rows, foreign_rows, reads)) + # Collected for EVERY inbound row, not only this engine's own: a lane that failed to bind + # under a peer's tag is exactly as diagnostic, and filtering by tag here would drop the + # cross-engine case the isolation assertion above exists to catch. + # DIRECT ATTRIBUTE ACCESS, NOT `getattr(row, "error", None)`. `EngineClient.connections()` + # is typed `list[ConnectionRow]` and that model declares `error`, so the field is + # guaranteed and a default would only ever mask a RENAME -- after which this would report + # "no failed lanes" forever, silently, on exactly the runs it exists to explain. A + # diagnostic field that fails closed to "nothing to report" is worse than no field. + if row.error: + failed.append(f"{row.name}: {row.error}") + out.append( + EngineAttribution(node.node_id, tag, inbound_rows, foreign_rows, reads, tuple(failed)) + ) return out diff --git a/tests/test_multishard_smoke.py b/tests/test_multishard_smoke.py index ad81b082..3ba44893 100644 --- a/tests/test_multishard_smoke.py +++ b/tests/test_multishard_smoke.py @@ -141,7 +141,33 @@ async def test_multishard_two_engines_shared_sqlite() -> None: for e in rec.per_engine: assert e.inbound_rows == _COUNT_PER_ENGINE, e # exactly its own C lanes, no more assert e.foreign_rows == 0, e # none of a peer's lanes bled in - assert e.reads > 0, e # this engine independently received traffic on its own lanes + # This engine independently received traffic on its own lanes. + # + # THE MESSAGE CARRIES THE ENGINE'S OWN REASON, because the two assertions above CANNOT supply + # one. `inbound_rows` and `foreign_rows` are CONFIG-derived, not traffic-derived -- the API + # emits a source row for every registry inbound and `read` is always an int -- so both pass + # unchanged on an engine that received NOTHING, including one whose listeners never bound. + # The docstring above already concedes this ("config-derived so it holds regardless of the + # write lock"); the consequence for THIS line is that a bare `assert 0 > 0` names a number and + # no cause, which is what made the CI red undiagnosable. + # + # The engine knows: it reports the lane as not-listening with a reason (ADR 0031, surfaced as + # `/connections`.error), and the harness was fetching that response and discarding the field. + # Reproduced deterministically with a control -- a listen-never-accept squatter on one engine's + # inbound ports gives exactly inbound_rows PASS / foreign_rows PASS / reads 0 FAIL, with the + # engine reporting failed lanes throughout. + assert e.reads > 0, ( + f"engine {e.name_tag} read 0 messages on its own lanes. " + + ( + f"IT REPORTS THESE LANES AS NOT LISTENING: {e.failed_lanes}. That is the cause -- the " + f"engine never received traffic, rather than receiving it and miscounting." + if e.failed_lanes + else "It reports NO failed lanes, so the listeners bound and the traffic did not " + "arrive or did not commit -- a different cause from a bind failure, and one this " + "assertion cannot narrow further on its own." + ) + + f" Full attribution: {e}" + ) # (c) Zero-loss end-to-end is NOT required on shared SQLite (the single writer can strand a row on a # SQLITE_LOCKED delivery commit — the server-DB bench is the real zero-loss gate). A clean drain is a From e528424c9ce2045ca03dd01d200398352661439f Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 15:49:01 -0500 Subject: [PATCH 05/11] wip(1292): in-flight intake-audit discriminator, committed under USAGE URGENT STOP NOT A FINISHED CHANGE AND NOT REVIEWED. Committed mid-flight because the usage ladder reached URGENT STOP, where the instruction is to commit whatever exists rather than risk losing a tree. A partial commit beats a lost one; an unreviewed commit on an unpushed branch costs nothing and can be amended. WHAT THIS IS: the BACKLOG #1292 store-side discriminator, being built to separate a harness reconciliation race from a real count-and-log intake loss. Its design stage returned confidence NOT-ESTABLISHED, which was the honest answer and the one I told the agent was acceptable -- the deliverable is an instrument that ATTRIBUTES the next occurrence, not a verdict on which branch is real. STATE, stated plainly so nobody mistakes this for finished work: - the implement stage was STILL RUNNING when this was committed - the ADVERSARIAL REVIEW STAGE HAD NOT RUN AT ALL - I have NOT verified any of it: no red-first, no ruff, no mypy, no test run by me - the two new modules (intake_audit.py, test_connscale_intake_audit.py) may be incomplete DO NOT LAND THIS. It needs the review stage, then my own verification, then almost certainly a correction commit -- the reviews of my last two commits each found a real defect, one of which let a genuine FD collapse pass. Assume the same here until shown otherwise. Also carried, unrelated and equally unverified: whatever the agent touched in driver.py, profile.py, report.py, runner.py, sender.py, docs/LOAD-TESTING.md and harness/load/__init__.py. The ledger is deliberately untouched, as it has been in every commit on this branch. --- docs/LOAD-TESTING.md | 26 + harness/load/__init__.py | 21 +- harness/load/connscale/driver.py | 7 + harness/load/connscale/intake_audit.py | 537 ++++++++++++++++++++ harness/load/connscale/profile.py | 8 + harness/load/connscale/report.py | 40 ++ harness/load/connscale/runner.py | 231 +++++++-- harness/load/sender.py | 24 +- tests/test_connscale_intake_audit.py | 659 +++++++++++++++++++++++++ tests/test_connscale_smoke.py | 57 +++ 10 files changed, 1577 insertions(+), 33 deletions(-) create mode 100644 harness/load/connscale/intake_audit.py create mode 100644 tests/test_connscale_intake_audit.py diff --git a/docs/LOAD-TESTING.md b/docs/LOAD-TESTING.md index 4a81e48d..dc495d2d 100644 --- a/docs/LOAD-TESTING.md +++ b/docs/LOAD-TESTING.md @@ -297,6 +297,32 @@ headroom denominator (in + out events, not messages). Exit codes match `--load`. backlog stayed low) so engine numbers are never silently the harness's own ceiling. - The `zero_loss` gate is **exact** by default (no message may be lost). At-least-once re-deliveries (`sink_received > engine_written`) are reported as a count and are *not* treated as loss. +- **`intake_audit` (connection-scale runs only) is the per-MESSAGE companion to `zero_loss`, and it + answers a question `zero_loss` cannot ask.** `zero_loss` compares COUNTS, so its + `engine_read N < confirmed sent M (lost K on intake)` reads identically whether the engine lost an + acknowledged message or the harness's own `engine_read` gauge was short — and `engine_read` is + itself a `COUNT(*)` sampled through two HTTP layers, so a second count could not separate them. + The audit records each send's control id as its response frame comes back and then asks the step's + own store, per message, whether that row is there. Its verdict is on the console, in the JSON + artifact under `records[].intake_audit`, and appended to a failing `no_loss` detail: + - `INTAKE_COMPLETE` — every confirmed send has a row. + - `SAMPLING_LAG` — a shortfall was reported and every confirmed send has a row anyway, so the + shortfall is in the gauge (sample attribution or per-inbound sum coverage). A harness defect. + - `INVARIANT_SUSPECT` — a send the engine accept-ACKed has no row in its own stopped, committed + store. The engine branch: on a deployment an acknowledged message would be lost at intake. The + verdict names the sequence numbers, so it is reproducible rather than statistical. + - `CORRELATION_SUSPECT` — only *rejected* sends are unmatched. Not an engine finding: several NAK + paths record their row with a NULL control id, so a rejected message is expected to be + unmatchable by control id. + - `PROBE_UNUSABLE` — the audit could not answer (its own read came back empty, was truncated, or + the send ledger was incomplete). Deliberately **not** rendered as "everything is missing", and + deliberately **not** a pass either. + + It runs at two moments: LIVE (engine still up, only on a shortfall) and POST-MORTEM (engine + stopped, always). The post-mortem one is authoritative — a live read can be explained away as + early sampling; a read of a stopped engine's store cannot. Set `intake_audit = false` in the + `[connscale]` profile to skip it on a heavy operator sweep; it is on by default, because a check + an operator has to remember to enable is absent on exactly the run that needed it. ## Known limitations diff --git a/harness/load/__init__.py b/harness/load/__init__.py index 609db8ef..a0838c78 100644 --- a/harness/load/__init__.py +++ b/harness/load/__init__.py @@ -8,10 +8,23 @@ fan-out and times each message end-to-end; an engine poller samples the HTTP API for throughput, backlog, and drain. See ``docs/LOAD-TESTING.md``. -Like :mod:`harness.scenarios`, this package imports no PySide6 and never imports the engine's -``pipeline``/``store``/``config`` internals — only the **pure** surfaces the harness is allowed to -use: the MLLP framing primitives (:mod:`messagefoundry.transports.mllp`), the parsing library, the -generators, and the HTTP :class:`~messagefoundry.apiclient.EngineClient`. +Like :mod:`harness.scenarios`, this package imports no PySide6, and drives the engine through the +**pure** surfaces a client is allowed to use: the MLLP framing primitives +(:mod:`messagefoundry.transports.mllp`), the parsing library, the generators, and the HTTP +:class:`~messagefoundry.apiclient.EngineClient`. + +**The store carve-out, and it is not the client rule being bent.** The rigs that OWN the engine +subprocess they measure — they spawn it, hand it a store, and stop it — are test rigs rather than +clients, and some of their jobs are only doable against the store directly. In +:mod:`harness.load.connscale` that is at least emptying a shared server store between sweep steps +(``runner._reset_server_store``) and the BACKLOG #1292 intake audit's per-message read +(``runner._store_reader``); :mod:`harness.load.shardcert` provisions its own store the same way. +Each goes through the ``Store`` protocol via ``open_store``, lazily imported inside the function so +the import graph of everything else is unchanged, and each is a read/reset path on a store the rig +itself provisioned — never a shortcut around the API for something the API could answer. (Separately +and harmlessly, several modules import the ``AckMode`` enum from ``config``; that is a value type, +not engine state.) The Qt-free client rule itself is unchanged: nothing here imports PySide6, and +the monitoring path is still the HTTP API. """ from __future__ import annotations diff --git a/harness/load/connscale/driver.py b/harness/load/connscale/driver.py index 0926dc42..9c86eed7 100644 --- a/harness/load/connscale/driver.py +++ b/harness/load/connscale/driver.py @@ -22,6 +22,7 @@ import asyncio +from harness.load.connscale.intake_audit import IntakeLedger from harness.load.corpus import Corpus, Outgoing from harness.load.correlator import Correlator from harness.load.metrics import LiveMetrics @@ -50,6 +51,7 @@ def __init__( correlator: Correlator, metrics: LiveMetrics, queue_max: int = 256, + ledger: IntakeLedger | None = None, ) -> None: if count < 1: raise ValueError("connection count must be >= 1") @@ -57,6 +59,10 @@ def __init__( self._base_port = base_port self._count = count self._m = metrics + # BACKLOG #1292: ONE ledger SHARED across the N connections, exactly as the correlator and + # LiveMetrics are. The reconcile it discriminates aggregates across all N, so a per-connection + # ledger would answer a narrower question than the assertion it has to explain. + self._ledger = ledger # One persistent, pipelined connection per inbound port; expect_ack so each send→ACK is timed. self._conns = [ PersistentConnection( @@ -66,6 +72,7 @@ def __init__( metrics, expect_ack=True, queue_max=queue_max, + ledger=ledger, ) for i in range(count) ] diff --git a/harness/load/connscale/intake_audit.py b/harness/load/connscale/intake_audit.py new file mode 100644 index 00000000..bedabb83 --- /dev/null +++ b/harness/load/connscale/intake_audit.py @@ -0,0 +1,537 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The intake audit (BACKLOG #1292) -- a PER-MESSAGE discriminator for an ``engine_read`` shortfall. + +WHAT IT IS FOR. ``harness.load.connscale.runner._reconcile`` fails a step with +``engine_read {read} < confirmed sent {sent - excused} (lost N on intake)``. That message is a +COUNT-vs-COUNT comparison and it cannot be attributed after the fact: the shortfall reads identically +whether the engine lost an acknowledged message (the count-and-log invariant, a real defect) or the +harness's own ``engine_read`` gauge was sampled early / summed short (an instrument defect). This +module asks a DIFFERENT question -- *is THIS message's row there* -- so that the two separate. + +WHY NOT ANOTHER COUNT. ``engine_read`` is ALREADY ``COUNT(*) FROM messages`` for the run's channels, +sampled through two HTTP layers: the store's ``_collect_connection_metrics`` -> the engine's +``connection_metrics_view`` -> ``GET /connections``'s ``read`` field -> ``enginepoll``'s re-sum of the +per-inbound rows. A probe that counted rows would re-derive the number already under dispute. This +one compares SETS of control ids. + +WHAT THE SENDER CONTRIBUTES. :class:`IntakeLedger` is filled by +:class:`~harness.load.sender.PersistentConnection` at the two points where a send LEAVES ``_inflight``: + +* CONFIRMED -- a response frame was read back for it (``_on_ack``), carrying its MSA-1 code and + whether that code was an accept. Measured on this rig the accounting identity + ``sent == acked + nak + timeouts`` holds exactly, so the confirmed set is precisely + ``sent - excused`` -- the same quantity ``_reconcile`` bounds. Matching the reconcile's own + arithmetic is the point; keying on ``acked`` alone would answer a neighbouring question. +* UNCONFIRMED -- still outstanding when the connection closed (``_fail_inflight``), which the + reconcile EXCUSES. Reported separately, and the subset that turns out to be in the store + (``late_unconfirmed_total``) is the honest measure of how loose that excusal is. + +WHAT IT DOES NOT ASSERT. A confirmed send whose MSA-1 was a REJECT and whose row is absent is NOT an +engine finding, and is reported as :data:`VERDICT_CORRELATION_SUSPECT` rather than as loss. Several of +the NAK limbs in ``pipeline/wiring_runner.py`` write their ``messages`` row with ``control_id=None`` +(the decode-error, NUL, parse-failure and oversize paths record the row BEFORE anything parsed an +MSH-10), so such a row EXISTS but is unfindable by control id. A rejected message is therefore +expected to be unmatchable here, and only the ACCEPTED-and-absent set carries the count-and-log +invariant. + +PHI. ``report.py`` states the rule for this artifact family: metrics and metadata only, never message +bodies and never control-id lists. So the audit reports SEQUENCE NUMBERS (dense integers minted by +the harness's own counter, meaningless outside the run) and the DISTINCT MSA-1 codes involved -- both +sufficient to act on, neither a control-id list. Full control ids stay in the harness log line. +""" + +from __future__ import annotations + +import logging +from collections.abc import Awaitable, Callable, Mapping +from dataclasses import dataclass +from typing import Any, Final + +log = logging.getLogger(__name__) + +#: When the audit was taken. Running BOTH is what removes the ambiguity: ``live`` still has the engine +#: up (so "we sampled too early" is available as an explanation), ``post_mortem`` runs against the +#: stopped, committed store (so it is not). +MOMENT_LIVE: Final = "live" +MOMENT_POST_MORTEM: Final = "post_mortem" + +#: The audit did not run (disabled by profile, or the live moment was not triggered). +VERDICT_NOT_RUN: Final = "NOT_RUN" +#: The probe itself could not answer. NEVER read as loss -- see :func:`judge` for the ordering rule. +VERDICT_PROBE_UNUSABLE: Final = "PROBE_UNUSABLE" +#: Every confirmed send has a row, and the reconcile saw no shortfall either. +VERDICT_INTAKE_COMPLETE: Final = "INTAKE_COMPLETE" +#: A shortfall was reported, yet every confirmed send HAS a row -> the ``engine_read`` gauge, not the +#: engine, is short. A harness/instrument defect (sample attribution or sum coverage). +VERDICT_SAMPLING_LAG: Final = "SAMPLING_LAG" +#: A send the engine ACCEPT-ACKed has no ``messages`` row -> the count-and-log invariant would be +#: broken. The engine branch, and the only one that justifies the P1. +VERDICT_INVARIANT_SUSPECT: Final = "INVARIANT_SUSPECT" +#: Only REJECT-ACKed sends are unmatched -> the harness's frame-to-message correspondence, or the +#: ``control_id=None`` NAK limbs above. Not an engine finding. +VERDICT_CORRELATION_SUSPECT: Final = "CORRELATION_SUSPECT" + +#: How many sends one ledger holds before it stops recording and declares itself overflowed. A ledger +#: that silently stopped recording would render as a clean audit, so overflow is a PROBE_UNUSABLE +#: input, not a shrug. +DEFAULT_LEDGER_CAPACITY: Final = 500_000 + +#: How many sequence numbers a verdict names. The COUNT is always exact and reported beside the +#: sample, so a truncated list never understates the finding. +SAMPLE_CAP: Final = 32 + +_PAGE = 1000 # rows per list_messages page during the store sweep + + +@dataclass(frozen=True) +class ConfirmedSend: + """One send for which the harness READ A RESPONSE FRAME back, with what that frame said.""" + + seq: int + code: str # MSA-1 verbatim ("" when the frame carried no parsable MSA-1) + accepted: bool # the sender's OWN accept decision, passed in rather than re-derived here + + +class IntakeLedger: + """Per-message record of what the sender observed, keyed by control id (MSH-10). + + Written only from the event loop (``PersistentConnection._on_ack`` / ``_fail_inflight``), so no + locking. Optional on the connection and ``None`` by default -- the same seam ``tracker`` uses -- + so the steady-state write path is unchanged when no audit is wanted. + """ + + __slots__ = ("_capacity", "_confirmed", "_duplicates", "_overflow", "_unconfirmed") + + def __init__(self, *, capacity: int = DEFAULT_LEDGER_CAPACITY) -> None: + if capacity < 1: + raise ValueError("ledger capacity must be >= 1") + self._capacity = capacity + self._confirmed: dict[str, ConfirmedSend] = {} + self._unconfirmed: dict[str, int] = {} + self._overflow = 0 + self._duplicates = 0 + + def record_confirmed(self, control_id: str, seq: int, code: str, *, accepted: bool) -> None: + """A response frame was read for ``control_id``. ``accepted`` is the SENDER's decision.""" + if self._reject(control_id): + return + self._confirmed[control_id] = ConfirmedSend(seq, code, accepted) + + def record_unconfirmed(self, control_id: str, seq: int) -> None: + """``control_id`` was still in flight when its connection closed (the reconcile excuses it).""" + if self._reject(control_id): + return + self._unconfirmed[control_id] = seq + + def _reject(self, control_id: str) -> bool: + """Refuse a record, counting WHY. Both counters feed PROBE_UNUSABLE rather than being + absorbed: a ledger that quietly stopped recording, or one whose keys are not unique, produces + a clean-looking set comparison that means nothing.""" + if len(self._confirmed) + len(self._unconfirmed) >= self._capacity: + self._overflow += 1 + return True + if control_id in self._confirmed or control_id in self._unconfirmed: + self._duplicates += 1 + return True + return False + + @property + def confirmed(self) -> Mapping[str, ConfirmedSend]: + return self._confirmed + + @property + def unconfirmed(self) -> Mapping[str, int]: + return self._unconfirmed + + @property + def total(self) -> int: + """Sends accounted for. Compared against ``sent`` -- a mismatch means the ledger is partial, + which invalidates a NULL result (though not a positive one).""" + return len(self._confirmed) + len(self._unconfirmed) + + @property + def overflow(self) -> int: + return self._overflow + + @property + def duplicates(self) -> int: + return self._duplicates + + +@dataclass(frozen=True) +class StoreSnapshot: + """What one read of the step's own store returned. + + ``error`` and ``truncated`` are carried BESIDE the data, never folded into it: an empty + ``control_ids`` is produced identically by a working sweep of an empty store and by a broken + query, and those warrant opposite verdicts. + """ + + control_ids: frozenset[str] + total: int # COUNT(*) of the messages table -- the sweep's positive control + truncated: bool = False + error: str | None = None + + +StoreReader = Callable[[], Awaitable[StoreSnapshot]] + + +@dataclass(frozen=True) +class IntakeAudit: + """One audit: the verdict, its inputs, and enough detail to act on without re-running.""" + + moment: str + verdict: str + read_short: int # the reconcile shortfall this audit was taken against + sent: int + confirmed_total: int + unconfirmed_total: int + store_total: int + missing_accepted_total: int + missing_rejected_total: int + late_unconfirmed_total: int + missing_accepted_seqs: tuple[int, ...] = () # bounded sample; the totals above are exact + missing_rejected_seqs: tuple[int, ...] = () + missing_codes: tuple[str, ...] = () # DISTINCT MSA-1 codes across the missing set, sorted + detail: str = "" + + @property + def conclusive(self) -> bool: + """Did this audit actually answer the question? PROBE_UNUSABLE and NOT_RUN did not, and must + never be read as a pass.""" + return self.verdict in ( + VERDICT_INTAKE_COMPLETE, + VERDICT_SAMPLING_LAG, + VERDICT_INVARIANT_SUSPECT, + VERDICT_CORRELATION_SUSPECT, + ) + + @property + def engine_suspect(self) -> bool: + """Is this the branch that implicates the ENGINE (vs the harness or the probe)?""" + return self.verdict == VERDICT_INVARIANT_SUSPECT + + def summary(self) -> str: + """One line a CI reader can act on without re-running anything.""" + return ( + f"intake audit [{self.moment}] {self.verdict}: {self.detail} " + f"(sent={self.sent} confirmed={self.confirmed_total} " + f"unconfirmed={self.unconfirmed_total} store_rows={self.store_total} " + f"missing_accepted={self.missing_accepted_total} " + f"missing_rejected={self.missing_rejected_total} " + f"late_unconfirmed={self.late_unconfirmed_total} " + f"seqs={list(self.missing_accepted_seqs)} codes={list(self.missing_codes)})" + ) + + def to_json_dict(self) -> dict[str, object]: + return { + "moment": self.moment, + "verdict": self.verdict, + "read_short": self.read_short, + "sent": self.sent, + "confirmed": self.confirmed_total, + "unconfirmed": self.unconfirmed_total, + "store_total": self.store_total, + "missing_accepted": self.missing_accepted_total, + "missing_rejected": self.missing_rejected_total, + "late_unconfirmed": self.late_unconfirmed_total, + # Sequence numbers, not control ids (PHI rule, see the module docstring). Bounded sample. + "missing_accepted_seqs": list(self.missing_accepted_seqs), + "missing_rejected_seqs": list(self.missing_rejected_seqs), + "missing_codes": list(self.missing_codes), + "detail": self.detail, + } + + +def not_run(reason: str, *, moment: str = MOMENT_POST_MORTEM) -> IntakeAudit: + """The audit was not taken. Distinct from a clean audit AND from an unusable one.""" + return IntakeAudit( + moment=moment, + verdict=VERDICT_NOT_RUN, + read_short=0, + sent=0, + confirmed_total=0, + unconfirmed_total=0, + store_total=0, + missing_accepted_total=0, + missing_rejected_total=0, + late_unconfirmed_total=0, + detail=reason, + ) + + +def judge( + ledger: IntakeLedger, + snapshot: StoreSnapshot, + *, + moment: str, + sent: int, + read_short: int, +) -> IntakeAudit: + """Turn a ledger + one store read into a verdict. Pure -- the whole decision table, unit-testable. + + THE ORDERING IS THE DESIGN, not an accident of writing. A POSITIVE finding is self-evidencing; a + NULL is printed identically by every silent instrument failure, so each way the probe can be + blind is ruled out BEFORE a null is allowed to mean anything: + + 1. the sender-side ledger is overflowed / non-unique / empty -> PROBE_UNUSABLE. + 2. the store sweep failed or was truncated -> PROBE_UNUSABLE. + 3. the store sweep read ZERO rows against a non-empty ledger -> PROBE_UNUSABLE. This is the + positive control, and it is checked HERE so a broken query renders as "unusable" and never as + "every message is missing" -- the worst possible false positive to hang a P1 on. + 4. an ACCEPT-ACKed send with no row -> INVARIANT_SUSPECT. Checked BEFORE the partial-ledger guard + below: a short ledger under-reports, so a finding inside it is still a real finding. + 5. only REJECT-ACKed sends unmatched -> CORRELATION_SUSPECT (see the module docstring). + 6. the ledger did not account for every send -> PROBE_UNUSABLE, because a null over a partial + ledger proves nothing. This is step 4's mirror image, and why the two are split rather than + both being checked up front. + 7. a shortfall with every confirmed send present -> SAMPLING_LAG: the gauge is short, not intake. + 8. otherwise INTAKE_COMPLETE. + """ + confirmed = ledger.confirmed + ledger_total = ledger.total + store_ids = snapshot.control_ids + + def _unusable(detail: str) -> IntakeAudit: + return _build( + moment=moment, + verdict=VERDICT_PROBE_UNUSABLE, + read_short=read_short, + sent=sent, + ledger=ledger, + snapshot=snapshot, + missing_accepted=(), + missing_rejected=(), + late_unconfirmed=0, + detail=detail, + ) + + if ledger.overflow: + return _unusable( + f"the send ledger overflowed after {ledger_total} entries ({ledger.overflow} send(s) " + f"unrecorded), so an absent control id cannot be told from an unrecorded one" + ) + if ledger.duplicates: + return _unusable( + f"{ledger.duplicates} duplicate control id(s) reached the ledger -- the ids are not " + f"unique this run, so set membership does not identify a message" + ) + if ledger_total == 0 and sent > 0: + return _unusable( + f"the send ledger recorded NOTHING against {sent} counted send(s) -- the sender-side " + f"instrument did not run, so a clean set comparison here would be vacuous" + ) + if snapshot.error is not None: + return _unusable(f"the store sweep failed: {snapshot.error}") + if snapshot.truncated: + return _unusable( + f"the store sweep was truncated at {len(store_ids)} of {snapshot.total} row(s) -- the " + f"unread remainder is indistinguishable from absence" + ) + if snapshot.total == 0 and ledger_total > 0: + return _unusable( + f"the store sweep read 0 row(s) against {ledger_total} accounted send(s) -- the query " + f"answered nothing rather than the store being empty; reported as unusable, NOT as " + f"{ledger_total} lost messages" + ) + + missing_accepted = tuple( + sorted( + (rec.seq, rec.code) + for cid, rec in confirmed.items() + if rec.accepted and cid not in store_ids + ) + ) + missing_rejected = tuple( + sorted( + (rec.seq, rec.code) + for cid, rec in confirmed.items() + if not rec.accepted and cid not in store_ids + ) + ) + late_unconfirmed = sum(1 for cid in ledger.unconfirmed if cid in store_ids) + + if missing_accepted: + partial = ( + "" + if ledger_total == sent + else f" (the ledger accounted {ledger_total} of {sent} send(s), so this is a LOWER BOUND)" + ) + return _build( + moment=moment, + verdict=VERDICT_INVARIANT_SUSPECT, + read_short=read_short, + sent=sent, + ledger=ledger, + snapshot=snapshot, + missing_accepted=missing_accepted, + missing_rejected=missing_rejected, + late_unconfirmed=late_unconfirmed, + detail=( + f"{len(missing_accepted)} send(s) the engine ACCEPT-ACKed have no messages row in " + f"its own store ({snapshot.total} row(s) present){partial} -- on a deployment the " + f"count-and-log invariant would not hold for those messages" + ), + ) + if missing_rejected: + return _build( + moment=moment, + verdict=VERDICT_CORRELATION_SUSPECT, + read_short=read_short, + sent=sent, + ledger=ledger, + snapshot=snapshot, + missing_accepted=(), + missing_rejected=missing_rejected, + late_unconfirmed=late_unconfirmed, + detail=( + f"{len(missing_rejected)} REJECT-ACKed send(s) are unmatched and no accepted send " + f"is -- not an engine finding: a rejected message may be recorded with a NULL " + f"control id, and the harness pops response frames strictly FIFO" + ), + ) + if ledger_total != sent: + return _unusable( + f"the send ledger accounted {ledger_total} of {sent} counted send(s) -- a clean set " + f"comparison over a partial ledger cannot exclude a loss among the " + f"{sent - ledger_total} it never saw" + ) + if read_short > 0: + return _build( + moment=moment, + verdict=VERDICT_SAMPLING_LAG, + read_short=read_short, + sent=sent, + ledger=ledger, + snapshot=snapshot, + missing_accepted=(), + missing_rejected=(), + late_unconfirmed=late_unconfirmed, + detail=( + f"engine_read is short by {read_short} yet all {len(confirmed)} confirmed send(s) " + f"HAVE a messages row ({snapshot.total} row(s) present) -- the shortfall is in the " + f"engine_read gauge (sample attribution or per-inbound sum coverage), not in intake" + ), + ) + return _build( + moment=moment, + verdict=VERDICT_INTAKE_COMPLETE, + read_short=read_short, + sent=sent, + ledger=ledger, + snapshot=snapshot, + missing_accepted=(), + missing_rejected=(), + late_unconfirmed=late_unconfirmed, + detail=( + f"all {len(confirmed)} confirmed send(s) have a messages row; {snapshot.total} row(s) " + f"present, {late_unconfirmed} excused send(s) arrived anyway" + ), + ) + + +def _build( + *, + moment: str, + verdict: str, + read_short: int, + sent: int, + ledger: IntakeLedger, + snapshot: StoreSnapshot, + missing_accepted: tuple[tuple[int, str], ...], + missing_rejected: tuple[tuple[int, str], ...], + late_unconfirmed: int, + detail: str, +) -> IntakeAudit: + # "(none)" rather than "" so an unparsable MSA-1 is a NAMED cause in the artifact instead of an + # empty string a reader would take for a serialization gap. + codes = sorted({code or "(none)" for _seq, code in (*missing_accepted, *missing_rejected)}) + return IntakeAudit( + moment=moment, + verdict=verdict, + read_short=read_short, + sent=sent, + confirmed_total=len(ledger.confirmed), + unconfirmed_total=len(ledger.unconfirmed), + store_total=snapshot.total, + missing_accepted_total=len(missing_accepted), + missing_rejected_total=len(missing_rejected), + late_unconfirmed_total=late_unconfirmed, + missing_accepted_seqs=tuple(seq for seq, _code in missing_accepted[:SAMPLE_CAP]), + missing_rejected_seqs=tuple(seq for seq, _code in missing_rejected[:SAMPLE_CAP]), + missing_codes=tuple(codes), + detail=detail, + ) + + +async def run_intake_audit( + ledger: IntakeLedger, + reader: StoreReader, + *, + moment: str, + sent: int, + read_short: int, +) -> IntakeAudit: + """Read the store once through ``reader`` and judge. + + A reader failure becomes a PROBE_UNUSABLE snapshot rather than an exception: the audit is an + instrument, and an instrument must never fail the run it was added to diagnose. + """ + try: + snapshot = await reader() + except Exception as exc: # noqa: BLE001 - any reader failure is a probe outcome, not a run failure + snapshot = StoreSnapshot(frozenset(), 0, error=f"{type(exc).__name__}: {exc}") + audit = judge(ledger, snapshot, moment=moment, sent=sent, read_short=read_short) + if audit.verdict != VERDICT_INTAKE_COMPLETE: + log.warning("%s", audit.summary()) + return audit + + +async def sweep_store(store: Any, *, row_cap: int) -> StoreSnapshot: + """Collect every ``control_id`` in ``store``'s ``messages`` table, unfiltered and paged. + + UNFILTERED BY CHANNEL, DELIBERATELY. The connscale runner gives each SQLite step its own DB file + and empties the shared server store before each step, so this store holds exactly this step's + rows. Filtering by the LIVE inbound registry -- the natural-looking choice -- would reproduce the + exact blind spot the audit exists to detect: the API emits a ``read`` figure only for a channel + still present in ``rr.registry.inbound``, so a row whose channel left the registry (the mid-hold + reload probe) is committed but uncounted. An unfiltered sweep SEES that row. + + ``row_cap`` bounds the work: a table larger than the cap is reported TRUNCATED rather than + partially swept, because an unread remainder is indistinguishable from absence. + + ``control_id`` is stored in CLEARTEXT (``_insert_message`` ciphers only raw/error/summary/ + metadata), so this reads an encrypted store unchanged. Typed against the ``Store`` PROTOCOL + surface (``count_messages``/``list_messages``) rather than a backend, so SQLite and the two + server backends go down one path. + + KNOWN AND DELIBERATE: the paging is ``ORDER BY received_at DESC`` + OFFSET, so a row committed + WHILE the sweep is walking lands at offset 0 and shifts the window, which can drop the last page's + final row. That is why :data:`MOMENT_POST_MORTEM` -- taken after the engine process has exited, so + no insert is possible -- is the authoritative moment and the one the CI assertion reads. A LIVE + sweep can therefore report a message it did not actually miss; the live/post-mortem DELTA is + diagnostic rather than a defect, and a live finding that the post-mortem does not reproduce is + itself evidence about timing. + """ + total = int(await store.count_messages()) + if total > row_cap: + return StoreSnapshot(frozenset(), total, truncated=True) + ids: set[str] = set() + offset = 0 + while offset < total: + rows = await store.list_messages(limit=_PAGE, offset=offset) + if not rows: + # Fewer rows than COUNT(*) promised. Report TRUNCATED rather than returning a short set + # that would read as absence for every row the sweep never reached. + return StoreSnapshot(frozenset(ids), total, truncated=True) + for row in rows: + # Unguarded on purpose: all three backends project `control_id` in `list_messages`. If + # one ever stopped, the KeyError becomes a PROBE_UNUSABLE snapshot in `run_intake_audit` + # -- which is the right verdict for a probe that cannot read its own key, and far better + # than a `.get()` default that would render every row as an absent control id. + cid = row["control_id"] + if cid: + ids.add(str(cid)) + offset += len(rows) + return StoreSnapshot(frozenset(ids), total) diff --git a/harness/load/connscale/profile.py b/harness/load/connscale/profile.py index 8edc56ef..d4bd0fd8 100644 --- a/harness/load/connscale/profile.py +++ b/harness/load/connscale/profile.py @@ -78,6 +78,7 @@ "base_port", "transform", "reload_probe", + "intake_audit", "store_backend", "corpus_count_per_trigger", "correlator_capacity", @@ -157,6 +158,12 @@ class ConnScaleProfile: # variance from a single run: the runner loops each (claim_mode, fuse, sweep_mode, count) cell # ``trials`` times as distinct steps, and build_fuse_comparison aggregates the repeats by key. trials: int = 1 + # BACKLOG #1292: run the PER-MESSAGE intake audit alongside the count-based no-loss reconcile. + # Default ON, because the whole point of the item is that a shortfall in CI cannot be attributed + # after the fact -- an audit an operator has to remember to enable would be absent on exactly the + # run that needed it. It costs one paged sweep of the step's own store per step, so the heavy + # operator sweeps (N=1500, long holds) can turn it off. + intake_audit: bool = True def modes(self) -> tuple[str, ...]: """The sweep modes to run (``both`` expands to both, in a stable order).""" @@ -308,6 +315,7 @@ def _profile_from_data(data: dict[str, Any], *, where: str) -> ConnScaleProfile: ), transform=transform, reload_probe=_opt_bool(cs, "reload_probe", f"{where} [connscale]", default=False), + intake_audit=_opt_bool(cs, "intake_audit", f"{where} [connscale]", default=True), store_backend=store_backend, corpus_count_per_trigger=_opt_int( cs, "corpus_count_per_trigger", f"{where} [connscale]", default=20, minimum=1 diff --git a/harness/load/connscale/report.py b/harness/load/connscale/report.py index 0c0a7c8e..193d2666 100644 --- a/harness/load/connscale/report.py +++ b/harness/load/connscale/report.py @@ -7,6 +7,11 @@ reconcile, plus an SLO verdict. **Metrics + metadata only** — never message bodies or control-id lists (PHI rule). Pure + deterministic, so it unit-tests without a live run. +That rule is why the BACKLOG #1292 intake audit reports **sequence numbers**, not the control ids it +actually matched on: a seq is a dense integer minted by the harness's own counter and meaningless +outside the run, so it identifies the message for a follow-up without putting a list of message +identifiers into a shared artifact. The control ids stay in the harness log line. + The thundering-herd measurement is reported **explicitly and separated** (critic must-change #3): the ``fixed_aggregate`` sweep (constant R across N) IS the herd measurement, so the report carries the ``empty_claims_wake_fanout``-per-second slope vs N AS the wake-fanout cost, kept DISTINCT from the @@ -22,6 +27,13 @@ from typing import TYPE_CHECKING from harness._spreadsheet import SPREADSHEET_FORMULA_TRIGGERS, spreadsheet_safe +from harness.load.connscale.intake_audit import ( + MOMENT_LIVE, + VERDICT_INTAKE_COMPLETE, + VERDICT_NOT_RUN, + IntakeAudit, + not_run, +) if TYPE_CHECKING: from harness.load.connscale.compare import ( @@ -162,6 +174,19 @@ class ConnScaleRecord: fd_probe_ticks: int = 0 fd_probe_degraded_ticks: int = 0 fd_probe_degraded: tuple[str, ...] = () + # --- BACKLOG #1292: the intake audit, the PER-MESSAGE discriminator for a no_loss shortfall --- + # `no_loss` compares COUNTS, and a shortfall in it reads identically whether the engine lost an + # acknowledged message or the `engine_read` gauge was short. These carry the per-message verdict + # that separates the two. `intake_audit` is the POST-MORTEM one (taken against the stopped, + # committed store, so sampling timing cannot explain it) and is the authoritative field; + # `intake_audit_live` is the one taken while the engine was still up, and runs only on a + # shortfall -- the DELTA between them is what says sample-lag vs sum-coverage. Both default to a + # NOT_RUN verdict so an older artifact / a record built without the audit deserializes unchanged + # and never reads as a clean pass it did not earn. + intake_audit: IntakeAudit = field(default_factory=lambda: not_run("audit not wired")) + intake_audit_live: IntakeAudit = field( + default_factory=lambda: not_run("audit not wired", moment=MOMENT_LIVE) + ) def to_json_dict(self) -> dict[str, object]: return { @@ -235,6 +260,12 @@ def to_json_dict(self) -> dict[str, object]: "degraded": list(self.fd_probe_degraded), }, }, + # BACKLOG #1292. Sequence numbers and MSA-1 codes only -- never control ids, per the + # module docstring's metadata-only rule. + "intake_audit": { + "post_mortem": self.intake_audit.to_json_dict(), + "live": self.intake_audit_live.to_json_dict(), + }, "wall5_reload": {"seconds": self.reload_seconds}, "wall6_ack_ms": { "p50": round(self.ack_p50_ms, 3), @@ -401,6 +432,15 @@ def render_console(self) -> str: f"fd probe: {r.sweep_mode}@N={r.count} -- {r.fd_probe_degraded_ticks} of " f"{r.fd_probe_ticks} tick(s) measured nothing [{causes}]" ) + # BACKLOG #1292: the per-message attribution, printed whenever it says anything beyond "clean". + # A `no_loss` shortfall renders as a bare count in the table above, which is exactly the + # unattributable failure this exists to replace -- so the verdict goes on the console beside + # it, not only in the JSON artifact. + for r in self.records: + for audit in (r.intake_audit_live, r.intake_audit): + if audit.verdict in (VERDICT_INTAKE_COMPLETE, VERDICT_NOT_RUN): + continue + lines.append(f"{r.sweep_mode}@N={r.count} -- {audit.summary()}") lines.append("") lines.append("SLOs:") if not self.slos: diff --git a/harness/load/connscale/runner.py b/harness/load/connscale/runner.py index a465a760..9a749e54 100644 --- a/harness/load/connscale/runner.py +++ b/harness/load/connscale/runner.py @@ -30,10 +30,11 @@ import tempfile import time from collections.abc import Mapping -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from typing import Any +from harness.load.connscale import intake_audit from harness.load.connscale.compare import ( ClaimModeComparison, FuseModeComparison, @@ -42,6 +43,7 @@ build_fuse_comparison, ) from harness.load.connscale.driver import ConnScaleDriver +from harness.load.connscale.intake_audit import IntakeAudit, IntakeLedger, StoreReader from harness.load.connscale.probe import FdSampler, ProcSample, time_reload from harness.load.connscale.profile import ConnScaleProfile from harness.load.connscale.report import ( @@ -333,33 +335,35 @@ async def _run_one_step( if profile.store_backend is None: db_dir = tempfile.mkdtemp(prefix="mefor-connscale-") db_path = str(Path(db_dir) / f"{tag}.db") - node = EngineNode( - tag, - api_port, - env=_node_env( - base_env, - claim_mode=claim_mode, - fuse_mode=fuse_mode, - batch_mode=batch_mode, - count=count, - base_port=profile.base_port, - transform=profile.transform, - sink_host=sink_host, - sink_port=sink_port, - sink_ports=sink_ports, - install_executor_shim=install_executor_shim, - db_path=db_path, - ), - config_dir=_CONFIG_DIR, - cwd=cwd, + # Captured rather than inlined into EngineNode: the BACKLOG #1292 intake audit opens THIS step's + # store afterwards, and it must resolve the same MEFOR_STORE_* the engine itself was given (the + # per-step SQLite file, or the shared server connection) instead of re-deriving them. + node_env = _node_env( + base_env, + claim_mode=claim_mode, + fuse_mode=fuse_mode, + batch_mode=batch_mode, + count=count, + base_port=profile.base_port, + transform=profile.transform, + sink_host=sink_host, + sink_port=sink_port, + sink_ports=sink_ports, + install_executor_shim=install_executor_shim, + db_path=db_path, ) + node = EngineNode(tag, api_port, env=node_env, config_dir=_CONFIG_DIR, cwd=cwd) poller = EnginePoller(node.url, token=None, origin=time.perf_counter()) + # BACKLOG #1292: the per-message send ledger the intake audit reads. None disables the audit + # wholesale (the sender's write path is then byte-identical to pre-#1292). + ledger = IntakeLedger() if profile.intake_audit else None driver = ConnScaleDriver( host=sink_host, base_port=profile.base_port, count=count, correlator=correlator, metrics=metrics, + ledger=ledger, ) fd_sampler: FdSampler | None = None samples: list[EngineSample] = [] @@ -462,6 +466,53 @@ async def _run_one_step( final = await poller.sample_once() if final is not None: samples.append(final) + + # --- BACKLOG #1292: the intake audit, at its TWO moments ------------------------------- + # MOMENT 1, LIVE (engine still up), and only on a shortfall: does every message the harness + # read a response frame for actually HAVE a row right now? If it does, nothing was lost and + # the shortfall is in the `engine_read` gauge. Gated on the shortfall because on a clean step + # it would only re-confirm what MOMENT 2 confirms anyway, at a page sweep per step. + # `poller.baseline`/`poller.final`, NOT the local `final`: those are the exact two samples + # `_build_record` hands `_reconcile`, and the audit has to be triggered by the shortfall the + # step will actually REPORT. The two diverge on the drain-timeout path, where the local + # `final` can be None while the poller still holds an earlier sample. + read_short = _read_shortfall( + metrics.counters, poller.baseline, poller.final, unconfirmed_budget=count + ) + audit_live = intake_audit.not_run( + "no intake shortfall to attribute at this moment", moment=intake_audit.MOMENT_LIVE + ) + if ledger is not None and read_short > 0: + audit_live = await intake_audit.run_intake_audit( + ledger, + _store_reader(node_env, metrics.counters.sent), + moment=intake_audit.MOMENT_LIVE, + sent=metrics.counters.sent, + read_short=read_short, + ) + # MOMENT 2, POST-MORTEM. Stop the engine FIRST, so the store is committed and quiesced: with + # no process running, "we sampled too early" is no longer available as an explanation, which + # is what separates outcome 1 (sample lag) from outcome 2 (sum coverage). `stop()` is + # idempotent, so the `finally` below still runs it on every other path. + audit_final = intake_audit.not_run("intake audit disabled for this profile") + if ledger is not None: + with contextlib.suppress(Exception): + await node.stop() + if node.alive: + # Refuse to CALL it a post-mortem when it would not be one. A read taken while the + # engine is still running answers the LIVE question, and labelling it post_mortem + # would destroy the one distinction the second moment exists to make. + audit_final = intake_audit.not_run( + "the engine did not stop, so a post-mortem read would not be post-mortem" + ) + else: + audit_final = await intake_audit.run_intake_audit( + ledger, + _store_reader(node_env, metrics.counters.sent), + moment=intake_audit.MOMENT_POST_MORTEM, + sent=metrics.counters.sent, + read_short=read_short, + ) return _build_record( claim_mode=claim_mode, fuse_mode=fuse_mode, @@ -475,6 +526,8 @@ async def _run_one_step( samples=samples, drain_seconds=drain_seconds, reload_seconds=reload_seconds, + audit_live=audit_live, + audit_final=audit_final, ) finally: with contextlib.suppress(Exception): @@ -563,6 +616,37 @@ def _node_env( return env +def _store_reader(node_env: Mapping[str, str], sent: int) -> StoreReader: + """A one-shot reader over THIS step's own store, for the BACKLOG #1292 intake audit. + + Goes through the ``Store`` protocol (``open_store`` -> ``count_messages``/``list_messages``) so + SQLite and the two server backends take one path, and resolves its settings from the SAME env the + engine subprocess was given. It deliberately does NOT go through ``GET /messages``: that route is + ``require_phi_read`` and charges a per-actor anti-automation budget, so a per-message sweep would + be 429'd -- and it would also read the numbers through the very API layer under suspicion. + + The engine-package import sits INSIDE the call, mirroring ``_reset_server_store`` directly above: + this rig OWNS the engine subprocess and inspects its store, which is why it is the one part of + ``harness/load`` that reaches past the HTTP API at all. + """ + # 4x the sends plus a floor: `messages` holds one row per RECEIVED message and this store is + # exclusive to the step, so a table meaningfully larger than the run means the assumption is + # wrong -- report TRUNCATED rather than sweep an unbounded table. + row_cap = max(4 * sent + 1000, 10_000) + + async def _read() -> intake_audit.StoreSnapshot: + from messagefoundry.config.settings import load_settings + from messagefoundry.store.base import open_store + + store = await open_store(load_settings(environ=node_env).store) + try: + return await intake_audit.sweep_store(store, row_cap=row_cap) + finally: + await store.close() + + return _read + + async def _reset_server_store(backend: str, env: Mapping[str, str]) -> tuple[int, int]: """Empty the pipeline tables of the SHARED server store before a step, so every (mode, count) step is apples-to-apples (the pooled arm never inherits the per_lane arm's rows). Opens a short-lived @@ -754,6 +838,8 @@ def _build_record( samples: list[EngineSample], drain_seconds: float | None, reload_seconds: float | None, + audit_live: IntakeAudit | None = None, + audit_final: IntakeAudit | None = None, ) -> ConnScaleRecord: c = metrics_counters.snapshot() base, final = poller.baseline, poller.final @@ -763,6 +849,14 @@ def _build_record( # half-the-run fraction, and its separate intake floor keeps `read >= sent // 2` required here # even when `count` exceeds half the step's sends (the short-hold smoke cells). no_loss = _reconcile(c, base, final, unconfirmed_budget=count) + live = audit_live if audit_live is not None else intake_audit.not_run("audit not wired") + post = audit_final if audit_final is not None else intake_audit.not_run("audit not wired") + # BACKLOG #1292: ATTRIBUTE the reconcile's own failure text, never soften it. `ok` is untouched -- + # a genuine invariant failure still fails the step on the count check exactly as before -- and the + # audit verdict is APPENDED so a CI reader gets the attribution in the same message that + # currently gives them only a number they cannot act on. + if not no_loss.ok and post.verdict != intake_audit.VERDICT_NOT_RUN: + no_loss = replace(no_loss, detail=f"{no_loss.detail}; {post.summary()}") in_pipeline_peak = max((s.in_pipeline for s in samples), default=0) # Wall #1: executor saturation (None when the shim isn't installed → all-None samples). @@ -841,6 +935,57 @@ def _build_record( working_set_peak_bytes=proc.working_set_peak_bytes, fuse_thread_hops=fuse_mode, batch_handoff_statements=batch_mode, + intake_audit=post, + intake_audit_live=live, + ) + + +@dataclass(frozen=True) +class _Excusal: + """How many unconfirmed sends the intake bound forgives this step, and whether that broke.""" + + unconfirmed: int + budget: int + excused: int + over_budget: bool + + +def _excusal(c: Counters, *, unconfirmed_budget: int) -> _Excusal: + """THE definition of the unconfirmed-send excusal, extracted so ``_reconcile`` and the BACKLOG + #1292 intake audit compute the SAME ``sent - excused``. + + The audit exists to explain the ``engine_read {read} < confirmed sent {sent - excused}`` message, + so it has to be triggered by that exact quantity. A second copy of this arithmetic beside it + would let the audit fire on a shortfall the reconcile does not report, or stay silent on one it + does -- either way attributing the wrong failure. Behaviour is verbatim what ``_reconcile`` + computed inline before the extraction. + """ + unconfirmed = c.timeouts + # Three quarters, not half — half was sized against a 16% worst-observed and windows-2025 has + # since produced 51% on a lossless run, failing `main` at 9b03057f by ONE message. `excused` is + # clamped rather than zeroed so an over-budget failure stops claiming intake loss it cannot show. + # `ok` still requires `not over_budget`, so the verdict is unchanged. Full rationale: report.py. + budget = max(unconfirmed_budget, 3 * c.sent // 4) + over_budget = unconfirmed > budget + return _Excusal(unconfirmed, budget, 0 if over_budget else unconfirmed, over_budget) + + +def _read_shortfall( + c: Counters, + base: EngineSample | None, + final: EngineSample | None, + *, + unconfirmed_budget: int, +) -> int: + """``confirmed sent - engine_read`` -- the shortfall ``_reconcile`` reports as intake loss, and + the trigger for the LIVE intake audit. 0 when the engine gauges are unavailable (there is then no + shortfall to attribute; ``_reconcile`` fails the step on its own for that).""" + if base is None or final is None: + return 0 + return ( + c.sent + - _excusal(c, unconfirmed_budget=unconfirmed_budget).excused + - (final.read - base.read) ) @@ -889,14 +1034,13 @@ def _reconcile( # guarantee is enforced SEPARATELY below as an intake floor the excusal cannot lower. See # harness/load/report.py's copy for the full rationale; the three copies are kept in step # deliberately. - unconfirmed = c.timeouts - # Three quarters, not half — half was sized against a 16% worst-observed and windows-2025 has - # since produced 51% on a lossless run, failing `main` at 9b03057f by ONE message. `excused` is - # clamped rather than zeroed so an over-budget failure stops claiming intake loss it cannot show. - # `ok` still requires `not over_budget`, so the verdict is unchanged. Full rationale: report.py. - budget = max(unconfirmed_budget, 3 * sent // 4) - over_budget = unconfirmed > budget - excused = 0 if over_budget else unconfirmed + ex = _excusal(c, unconfirmed_budget=unconfirmed_budget) + unconfirmed, budget, excused, over_budget = ( + ex.unconfirmed, + ex.budget, + ex.excused, + ex.over_budget, + ) read_short = sent - excused - read # The anti-vacuity guarantee, independent of the excusal: at least half the sends must be # observed at intake whatever the budget forgives (nothing clamps `excused` to `sent`, so without @@ -1140,6 +1284,37 @@ def _evaluate_slos(profile: ConnScaleProfile, records: list[ConnScaleRecord]) -> if slo.zero_loss: all_ok = all(r.no_loss.ok for r in records) out.append(SloCheck("zero_loss", True, all_ok, all_ok)) + if profile.intake_audit: + # BACKLOG #1292, and DELIBERATELY NOT folded into zero_loss: this is a per-MESSAGE check and + # zero_loss is a per-COUNT one, so they can disagree, and each disagreement is informative. + # It is strictly ADDITIONAL -- it can fail a step whose counts reconciled, because a + # confirmed-then-absent message that happened to be excused as a timeout passes the count + # check today. PROBE_UNUSABLE does NOT fail here: an unanswerable instrument is not evidence + # of a defect in either direction, and it is reported as its own observation instead. + suspect = [r for r in records if r.intake_audit.engine_suspect] + # THE SCOPE TRAVELS WITH THE VERDICT, and that is not decoration here. `_evaluate_slos` is + # SHARED with the batch-box aggregate (batchbox.py), whose records are folded from + # remote-driver reports and carry a NOT_RUN audit by construction -- those driver processes + # poll a REMOTE engine and have no store to read. A bare "clean" there would be a green + # earned by nothing at all. So the observation always names how many steps were actually + # audited, and zero-audited says so in those words instead of passing itself off as a finding + # of no defect. The per-step guarantee is asserted in tests/test_connscale_smoke.py, where + # the audit genuinely runs; this line is the operator-facing summary of it. + audited = sum(1 for r in records if r.intake_audit.conclusive) + if suspect: + observed = "; ".join( + f"{r.sweep_mode}@N={r.count} {r.intake_audit.summary()}" for r in suspect + ) + elif audited == 0: + observed = ( + f"NOT AUDITED -- 0 of {len(records)} step(s) carry an intake audit, so this says " + f"nothing about per-message intake" + ) + else: + observed = f"clean ({audited} of {len(records)} step(s) audited)" + out.append( + SloCheck("intake_audit", "no accept-ACKed message absent", observed, not suspect) + ) if slo.max_drain_seconds is not None: worst = max( (r.drain_seconds for r in records if r.drain_seconds is not None), diff --git a/harness/load/sender.py b/harness/load/sender.py index 6651d94d..2fd32e1c 100644 --- a/harness/load/sender.py +++ b/harness/load/sender.py @@ -22,6 +22,7 @@ from collections import deque from collections.abc import Callable +from harness.load.connscale.intake_audit import IntakeLedger from harness.load.corpus import Outgoing from harness.load.correlator import Correlator from harness.load.failover_track import FailoverTracker @@ -66,6 +67,7 @@ def __init__( expect_ack: bool = True, queue_max: int = 1000, tracker: FailoverTracker | None = None, + ledger: IntakeLedger | None = None, ) -> None: self._host = host self._port = port @@ -73,6 +75,16 @@ def __init__( self._m = metrics self._expect_ack = expect_ack self._tracker = tracker # failover-only: record which seqs the engine accept-ACKed + # BACKLOG #1292 intake audit: an optional PER-MESSAGE record of how each send left `_inflight` + # (a response frame was read, or the connection closed on it). Same opt-in seam as `tracker` + # above, and None by default, so the steady-state write path is unchanged when nothing wants + # it. Meaningful only with `expect_ack` -- see `_write_loop`. + self._ledger = ledger + if ledger is not None and not expect_ack: + # Refuse rather than fill a ledger that can only ever be empty: with no ACK expected no + # response frame is ever read, so every send would be unaccounted and the audit's set + # comparison would be vacuous. Loud here beats a clean-looking verdict over nothing. + raise ValueError("an intake ledger requires expect_ack (it records response frames)") self._queue: asyncio.Queue[_Job] = asyncio.Queue(maxsize=queue_max) self._inflight: deque[tuple[int, int, str, OnDone | None]] = deque() self._stop = asyncio.Event() @@ -211,7 +223,15 @@ def _on_ack(self, ack: bytes) -> None: # MLLP ACKs are in-order per connection (the engine ACKs on receipt in send order). _seq, send_ns, _cid, on_done = self._inflight.popleft() self._m.ack.record(float(ack_ns - send_ns)) - if _ack_code(ack) in _ACCEPT: + # ONE accept decision, made here and PASSED to the ledger rather than re-derived beside it: + # a second copy of `_ACCEPT` in the audit module is a two-place constant, and the two + # disagreeing would silently move a message between the engine-suspect and harness-suspect + # buckets -- the exact attribution the audit exists to get right. + code = _ack_code(ack) + accepted = code in _ACCEPT + if self._ledger is not None: + self._ledger.record_confirmed(_cid, _seq, code, accepted=accepted) + if accepted: self._m.counters.acked += 1 if self._tracker is not None: # An accept-ACK means the engine durably committed this to the ingress stage (ACK-on- @@ -228,6 +248,8 @@ def _fail_inflight(self) -> None: return for _seq, _send_ns, _cid, on_done in self._inflight: self._m.counters.timeouts += 1 + if self._ledger is not None: + self._ledger.record_unconfirmed(_cid, _seq) if on_done is not None: on_done() self._inflight.clear() diff --git a/tests/test_connscale_intake_audit.py b/tests/test_connscale_intake_audit.py new file mode 100644 index 00000000..4d2998aa --- /dev/null +++ b/tests/test_connscale_intake_audit.py @@ -0,0 +1,659 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The BACKLOG #1292 intake audit -- the per-message discriminator for an ``engine_read`` shortfall. + +THE DEFECT UNDER TEST is an ATTRIBUTION defect, not a counting one. ``connscale``'s no-loss reconcile +fails with ``engine_read {read} < confirmed sent {sent - excused} (lost N on intake)``, and that +sentence is produced identically by an engine that lost an acknowledged message and by a harness +gauge that was sampled early or summed short. So the assertions here are about which VERDICT a given +world produces, and the decisive test is that three worlds which are indistinguishable to the count +check produce three DIFFERENT verdicts here. + +The three planted worlds mirror the three ways this can go, and they are deliberately not variations +of one: + +* the rows ARE all there while a shortfall is reported -> SAMPLING_LAG (harness/instrument) +* an ACCEPT-ACKed row is genuinely absent -> INVARIANT_SUSPECT (the engine branch) +* the probe's own read comes back empty -> PROBE_UNUSABLE (the null guard) + +The third is not optional. Without it a broken query renders as "every message is missing", which is +the worst possible false positive to hang a P1 on -- a catastrophic-looking engine finding produced +entirely by the instrument. +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +from harness.load.connscale.intake_audit import ( + MOMENT_LIVE, + MOMENT_POST_MORTEM, + VERDICT_CORRELATION_SUSPECT, + VERDICT_INTAKE_COMPLETE, + VERDICT_INVARIANT_SUSPECT, + VERDICT_NOT_RUN, + VERDICT_PROBE_UNUSABLE, + VERDICT_SAMPLING_LAG, + IntakeAudit, + IntakeLedger, + StoreSnapshot, + judge, + not_run, + run_intake_audit, + sweep_store, +) +from harness.load.connscale.profile import load_connscale_profile_text +from harness.load.connscale.report import ConnScaleRecord, SloCheck +from harness.load.connscale.runner import ( + _build_record, + _evaluate_slos, + _read_shortfall, + _reconcile, +) +from harness.load.enginepoll import EnginePoller, EngineSample +from harness.load.metrics import Counters, Histogram +from messagefoundry.store.store import MessageStore + + +def _ledger(*, accepted: int = 3, rejected: int = 0, unconfirmed: int = 0) -> IntakeLedger: + """A ledger shaped like one real step: ``accepted`` AA sends, ``rejected`` AE sends, and + ``unconfirmed`` sends stranded at a connection close.""" + led = IntakeLedger() + seq = 0 + for _ in range(accepted): + led.record_confirmed(f"CID{seq:04d}", seq, "AA", accepted=True) + seq += 1 + for _ in range(rejected): + led.record_confirmed(f"CID{seq:04d}", seq, "AE", accepted=False) + seq += 1 + for _ in range(unconfirmed): + led.record_unconfirmed(f"CID{seq:04d}", seq) + seq += 1 + return led + + +def _all_ids(led: IntakeLedger) -> frozenset[str]: + return frozenset({*led.confirmed, *led.unconfirmed}) + + +# --- PLANT A: the rows are all there, yet the count check reported a shortfall ------------------- + + +def test_plant_a_full_store_with_shortfall_is_sampling_lag() -> None: + led = _ledger(accepted=5) + snap = StoreSnapshot(_all_ids(led), total=5) + + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=2) + + assert audit.verdict == VERDICT_SAMPLING_LAG + # Nothing is claimed lost -- the whole point is that the shortfall is in the gauge. + assert audit.missing_accepted_total == 0 + assert audit.read_short == 2 and audit.store_total == 5 + assert "engine_read gauge" in audit.detail + assert not audit.engine_suspect and audit.conclusive + + +# --- PLANT B: an accept-ACKed message is genuinely absent --------------------------------------- + + +def test_plant_b_absent_accepted_message_is_invariant_suspect_and_names_it() -> None: + led = _ledger(accepted=5) + present = frozenset(cid for cid in led.confirmed if cid != "CID0002") + snap = StoreSnapshot(present, total=4) + + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=1) + + assert audit.verdict == VERDICT_INVARIANT_SUSPECT + assert audit.engine_suspect + assert audit.missing_accepted_total == 1 + # NAMED, so the finding is reproducible rather than statistical -- by SEQUENCE NUMBER, because + # the artifact rule for this family forbids control-id lists. + assert audit.missing_accepted_seqs == (2,) + assert audit.missing_codes == ("AA",) + assert "count-and-log invariant" in audit.detail + + +def test_plant_b_verdict_differs_from_plant_a_on_the_same_shortfall() -> None: + """THE ITEM, in one assertion: two worlds the count check cannot tell apart. + + Both have ``sent=5`` and a reported shortfall, so both produce the SAME + ``engine_read ... < confirmed sent ...`` message today. The audit separates them, and separates + them into the two branches that have opposite owners. + """ + led = _ledger(accepted=5) + lag = judge( + led, StoreSnapshot(_all_ids(led), 5), moment=MOMENT_POST_MORTEM, sent=5, read_short=1 + ) + loss = judge( + led, + StoreSnapshot(frozenset(c for c in led.confirmed if c != "CID0000"), 4), + moment=MOMENT_POST_MORTEM, + sent=5, + read_short=1, + ) + assert lag.read_short == loss.read_short == 1 # identical to the count check + assert lag.verdict != loss.verdict + assert (lag.engine_suspect, loss.engine_suspect) == (False, True) + + +# --- PLANT C: the probe itself read nothing ------------------------------------------------------ + + +def test_plant_c_empty_store_read_is_unusable_not_total_loss() -> None: + """A NULL NEEDS A MECHANISM. An empty read is what a broken query returns, and it is also what an + empty store returns; the two warrant opposite verdicts, so the empty read is refused rather than + rendered as the catastrophic reading.""" + led = _ledger(accepted=5) + snap = StoreSnapshot(frozenset(), total=0) + + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=5) + + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert not audit.engine_suspect + # The decisive assertion: it did NOT report five lost messages. + assert audit.missing_accepted_total == 0 + assert "NOT as 5 lost messages" in audit.detail + + +def test_the_three_plants_yield_three_distinct_verdicts() -> None: + """Two plants that agree are not two directions. Enumerated, so a future edit that collapses two + of these paths into one fails here rather than quietly halving the instrument.""" + led = _ledger(accepted=4) + verdicts = { + judge( + led, StoreSnapshot(_all_ids(led), 4), moment=MOMENT_POST_MORTEM, sent=4, read_short=1 + ).verdict, + judge( + led, + StoreSnapshot(frozenset(list(led.confirmed)[1:]), 3), + moment=MOMENT_POST_MORTEM, + sent=4, + read_short=1, + ).verdict, + judge( + led, StoreSnapshot(frozenset(), 0), moment=MOMENT_POST_MORTEM, sent=4, read_short=1 + ).verdict, + } + assert verdicts == {VERDICT_SAMPLING_LAG, VERDICT_INVARIANT_SUSPECT, VERDICT_PROBE_UNUSABLE} + + +# --- the fourth outcome: a rejected send is not an engine finding -------------------------------- + + +def test_absent_rejected_message_is_correlation_suspect_not_loss() -> None: + """Several NAK limbs record their ``messages`` row with a NULL control id (they run before an + MSH-10 has been parsed), so a rejected send is EXPECTED to be unmatchable by control id. Reading + that as intake loss would manufacture a P1 out of correct engine behaviour.""" + led = _ledger(accepted=3, rejected=1) + present = frozenset(cid for cid, rec in led.confirmed.items() if rec.accepted) + snap = StoreSnapshot(present, total=3) + + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=4, read_short=1) + + assert audit.verdict == VERDICT_CORRELATION_SUSPECT + assert not audit.engine_suspect + assert audit.missing_rejected_total == 1 and audit.missing_accepted_total == 0 + assert audit.missing_codes == ("AE",) + + +# --- the positive controls ------------------------------------------------------------------------ + + +def test_clean_run_reports_late_unconfirmed_as_its_positive_control() -> None: + """``late_unconfirmed`` proves the sweep sees BEYOND the confirmed set: an excused send that + nevertheless arrived. A sweep that only ever returned the confirmed ids would score clean here + and would be blind to exactly the messages the reconcile forgives.""" + led = _ledger(accepted=3, unconfirmed=1) + snap = StoreSnapshot(_all_ids(led), total=4) + + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=4, read_short=0) + + assert audit.verdict == VERDICT_INTAKE_COMPLETE + assert audit.late_unconfirmed_total == 1 + assert audit.unconfirmed_total == 1 and audit.confirmed_total == 3 + assert audit.store_total == 4 + + +def test_empty_ledger_against_real_sends_is_unusable() -> None: + """The sender-side positive control. An audit over a ledger that recorded nothing is vacuously + clean, so it is refused.""" + audit = judge( + IntakeLedger(), StoreSnapshot(frozenset(), 0), moment=MOMENT_LIVE, sent=7, read_short=1 + ) + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert "recorded NOTHING against 7" in audit.detail + + +def test_zero_send_step_is_complete_not_unusable() -> None: + """A step that sent nothing has nothing to audit and is not an instrument failure.""" + audit = judge( + IntakeLedger(), StoreSnapshot(frozenset(), 0), moment=MOMENT_LIVE, sent=0, read_short=0 + ) + assert audit.verdict == VERDICT_INTAKE_COMPLETE + + +# --- the partial-ledger split: a positive finding survives it, a null does not -------------------- + + +def test_partial_ledger_makes_a_NULL_unusable() -> None: + led = _ledger(accepted=3) + snap = StoreSnapshot(_all_ids(led), total=3) + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=0) + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert "accounted 3 of 5" in audit.detail + + +def test_partial_ledger_does_not_suppress_a_POSITIVE_finding() -> None: + """The mirror image, and the reason the two guards sit on opposite sides of the finding checks: a + short ledger under-reports, so a message inside it that is genuinely absent is still absent.""" + led = _ledger(accepted=3) + snap = StoreSnapshot(frozenset(list(led.confirmed)[1:]), total=2) + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=3) + assert audit.verdict == VERDICT_INVARIANT_SUSPECT + assert "LOWER BOUND" in audit.detail + + +def test_ledger_overflow_and_duplicates_are_unusable() -> None: + small = IntakeLedger(capacity=2) + for i in range(4): + small.record_confirmed(f"C{i}", i, "AA", accepted=True) + assert small.overflow == 2 + assert ( + judge( + small, StoreSnapshot(frozenset(), 0), moment=MOMENT_LIVE, sent=4, read_short=0 + ).verdict + == VERDICT_PROBE_UNUSABLE + ) + + dup = IntakeLedger() + dup.record_confirmed("SAME", 0, "AA", accepted=True) + dup.record_confirmed("SAME", 1, "AA", accepted=True) + assert dup.duplicates == 1 + audit = judge( + dup, StoreSnapshot(frozenset({"SAME"}), 1), moment=MOMENT_LIVE, sent=2, read_short=0 + ) + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert "duplicate control id" in audit.detail + + +def test_not_run_is_neither_a_pass_nor_a_finding() -> None: + audit = not_run("audit disabled") + assert audit.verdict == VERDICT_NOT_RUN + assert not audit.conclusive and not audit.engine_suspect + + +# --- run_intake_audit: a broken reader is a probe outcome, never a run failure -------------------- + + +def test_reader_exception_becomes_probe_unusable() -> None: + led = _ledger(accepted=2) + + async def _boom() -> StoreSnapshot: + raise RuntimeError("no such table: messages") + + audit = asyncio.run( + run_intake_audit(led, _boom, moment=MOMENT_POST_MORTEM, sent=2, read_short=2) + ) + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert "RuntimeError" in audit.detail and "no such table" in audit.detail + assert audit.missing_accepted_total == 0 + + +# --- the REAL store sweep, against a real SQLite store ------------------------------------------- + + +def test_sweep_reads_control_ids_from_a_real_store(tmp_path: Path) -> None: + """RUN THE THING: the reader against a real ``MessageStore``, not a stub. + + Includes its own positive control -- a control id that was never inserted must NOT come back -- + because a sweep that returned everything asked of it would pass a membership test without ever + querying anything. + """ + + async def go() -> None: + store = await MessageStore.open(tmp_path / "sweep.db") + try: + for i in range(3): + await store.enqueue_ingress( + channel_id="IB_CS_00000", + raw=f"MSH|^~\\&|A|B|C|D|20260101||ADT^A01|SWEEP{i:04d}|P|2.5\r", + control_id=f"SWEEP{i:04d}", + message_type="ADT^A01", + ) + snap = await sweep_store(store, row_cap=1000) + assert snap.total == 3 and not snap.truncated and snap.error is None + assert snap.control_ids == {"SWEEP0000", "SWEEP0001", "SWEEP0002"} + assert "SWEEP9999" not in snap.control_ids # the sweep discriminates, it does not echo + + # And the cap: a table bigger than the cap is TRUNCATED, never a short set that would + # read as absence for every row the sweep did not reach. + capped = await sweep_store(store, row_cap=2) + assert capped.truncated and capped.total == 3 and capped.control_ids == frozenset() + assert ( + judge( + _ledger(accepted=3), capped, moment=MOMENT_POST_MORTEM, sent=3, read_short=1 + ).verdict + == VERDICT_PROBE_UNUSABLE + ) + finally: + await store.close() + + asyncio.run(go()) + + +def test_sweep_of_an_empty_real_store_is_refused_by_judge(tmp_path: Path) -> None: + """The end-to-end null guard: a REAL sweep of a REAL empty store returns the same empty set a + broken query would, and ``judge`` refuses it rather than reporting total loss.""" + + async def go() -> None: + store = await MessageStore.open(tmp_path / "empty.db") + try: + snap = await sweep_store(store, row_cap=1000) + assert snap.total == 0 and snap.control_ids == frozenset() and snap.error is None + audit = judge( + _ledger(accepted=2), snap, moment=MOMENT_POST_MORTEM, sent=2, read_short=2 + ) + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert audit.missing_accepted_total == 0 + finally: + await store.close() + + asyncio.run(go()) + + +# --- the SENDER seam, driven over a real socket --------------------------------------------------- + + +def test_sender_ledger_records_both_exits_over_a_real_socket() -> None: + """RUN THE THING at the other end: a real :class:`PersistentConnection` against a real MLLP + listener that ACKs two frames, NAKs one, and then closes on a fourth without answering. + + All three ledger states have to be reachable from the actual sender, not just constructible: the + audit's arithmetic assumes CONFIRMED is exactly ``sent - excused``, and that assumption is only + worth anything if ``_on_ack`` and ``_fail_inflight`` both feed it. + """ + from harness.load.corpus import Outgoing + from harness.load.correlator import Correlator + from harness.load.metrics import Counters, Histogram, LiveMetrics + from harness.load.sender import PersistentConnection + from messagefoundry.transports.mllp import MLLPDecoder, frame + + ledger = IntakeLedger() + metrics = LiveMetrics(Counters(), Histogram(), Histogram()) + + async def go() -> None: + answered = 0 + + async def handle(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + nonlocal answered + decoder = MLLPDecoder() + while True: + chunk = await reader.read(65536) + if not chunk: + break + for _msg in decoder.feed(chunk): + answered += 1 + if answered > 3: + # The fourth frame is swallowed and the socket dropped: the send is left in + # `_inflight` and must land in the ledger as UNCONFIRMED. + writer.close() + return + code = "AA" if answered <= 2 else "AE" + writer.write( + frame(f"MSH|^~\\&|E|E|H|H|20260101||ACK|A{answered}|P|2.5\rMSA|{code}|X\r") + ) + await writer.drain() + + server = await asyncio.start_server(handle, "127.0.0.1", 0) + port = server.sockets[0].getsockname()[1] + correlator = Correlator(1000, metrics) + conn = PersistentConnection( + "127.0.0.1", port, correlator, metrics, expect_ack=True, ledger=ledger + ) + conn.start() + for i in range(4): + await conn.submit( + Outgoing( + seq=i, + code="ADT", + control_id=f"LG{i:04d}", + payload=f"MSH|^~\\&|A|B|C|D|20260101||ADT^A01|LG{i:04d}|P|2.5\r", + ) + ) + # Give the exchange time to complete, then stop (which sweeps whatever is still in flight). + for _ in range(200): + if ledger.total >= 4: + break + await asyncio.sleep(0.01) + await conn.stop(0.2) + server.close() + await server.wait_closed() + + asyncio.run(go()) + + accepted = {cid for cid, rec in ledger.confirmed.items() if rec.accepted} + rejected = {cid for cid, rec in ledger.confirmed.items() if not rec.accepted} + assert accepted == {"LG0000", "LG0001"}, ledger.confirmed + assert rejected == {"LG0002"}, ledger.confirmed + assert set(ledger.unconfirmed) == {"LG0003"}, dict(ledger.unconfirmed) + # The accounting identity the audit's arithmetic rests on, measured rather than assumed. + c = metrics.counters + assert c.sent == c.acked + c.nak + c.timeouts == ledger.total == 4 + assert not ledger.overflow and not ledger.duplicates + + +def test_ledger_requires_expect_ack() -> None: + """A ledger with no response frames to record can only ever be empty, and an empty ledger scores + vacuously clean. Refused loudly at construction instead.""" + from harness.load.correlator import Correlator + from harness.load.metrics import Counters, Histogram, LiveMetrics + from harness.load.sender import PersistentConnection + + metrics = LiveMetrics(Counters(), Histogram(), Histogram()) + with pytest.raises(ValueError, match="expect_ack"): + PersistentConnection( + "127.0.0.1", + 1, + Correlator(10, metrics), + metrics, + expect_ack=False, + ledger=IntakeLedger(), + ) + + +# --- the runner wiring: one definition of the shortfall, and it reaches the artifact -------------- + + +def _sample(read: int) -> EngineSample: + return EngineSample( + elapsed_s=0.0, + pending=0, + inflight=0, + done=0, + dead=0, + read=read, + written=0, + out_dead=0, + queue_depth=0, + in_pipeline=0, + db_size_bytes=0, + journal_mode="wal", + synchronous="normal", + uptime_s=0.0, + ) + + +def test_read_shortfall_is_the_same_number_the_reconcile_prints() -> None: + """ONE DEFINITION, asserted rather than assumed. + + The audit exists to explain ``engine_read N < confirmed sent M (lost K on intake)``, so it has to + fire on exactly that ``K``. A second copy of the unconfirmed-send excusal beside it would let the + audit trigger on a shortfall the step does not report, or stay silent on one it does -- and either + way it would be attributing the wrong failure. The excusal is deliberately non-trivial here + (``timeouts`` inside the budget, so some sends ARE excused), so a version that ignored it would + not agree by accident. + """ + c = Counters(sent=40, timeouts=4) + base, final = _sample(0), _sample(30) + + short = _read_shortfall(c, base, final, unconfirmed_budget=8) + no_loss = _reconcile(c, base, final, unconfirmed_budget=8) + + assert short == 6 # 40 sent - 4 excused - 30 read + assert not no_loss.ok + # The number the failing message actually carries, read back out of the message itself. + assert f"(lost {short} on intake)" in no_loss.detail + assert "confirmed sent 36" in no_loss.detail + + +def test_read_shortfall_is_zero_without_engine_gauges() -> None: + """No samples means no shortfall to ATTRIBUTE. ``_reconcile`` fails the step on its own for that, + and the audit must not invent a finding out of a missing measurement.""" + assert _read_shortfall(Counters(sent=10), None, _sample(0), unconfirmed_budget=1) == 0 + assert _read_shortfall(Counters(sent=10), _sample(0), None, unconfirmed_budget=1) == 0 + + +def _record_with(audit: IntakeAudit, *, read: int, sent: int) -> ConnScaleRecord: + poller = EnginePoller("http://127.0.0.1:1", token=None, origin=0.0) + poller._samples = [_sample(0), _sample(read)] + return _build_record( + claim_mode="per_lane", + fuse_mode=False, + batch_mode=False, + mode="fixed_aggregate", + count=4, + aggregate_rate=10.0, + metrics_counters=Counters(sent=sent), + ack_hist=Histogram(), + poller=poller, + samples=[], + drain_seconds=1.0, + reload_seconds=None, + audit_live=not_run("not triggered", moment=MOMENT_LIVE), + audit_final=audit, + ) + + +def test_a_failing_reconcile_carries_the_audit_verdict_into_its_own_message() -> None: + """The deliverable: a CI reader gets the attribution WITHOUT re-running anything. + + ``no_loss.ok`` is untouched -- the count check still fails the step exactly as before -- but its + detail, which is what the smoke's assertion message prints, now says WHICH branch it was. + """ + led = _ledger(accepted=4) + audit = judge( + led, + StoreSnapshot(frozenset(list(led.confirmed)[1:]), 3), + moment=MOMENT_POST_MORTEM, + sent=4, + read_short=1, + ) + rec = _record_with(audit, read=3, sent=4) + + assert rec.no_loss.ok is False # unchanged: the count check still fails + assert "lost 1 on intake" in rec.no_loss.detail # the original message survives verbatim + assert "INVARIANT_SUSPECT" in rec.no_loss.detail # and now says which branch + assert "seqs=[0]" in rec.no_loss.detail + assert rec.intake_audit is audit and rec.intake_audit.engine_suspect + assert "intake_audit" in rec.to_json_dict() + + +def test_a_passing_reconcile_is_left_byte_identical() -> None: + """No verdict is appended to a detail that reports no problem: the audit rides in its own field + and on the console, and a clean step's ``no_loss`` string is unchanged from pre-#1292.""" + led = _ledger(accepted=4) + audit = judge( + led, StoreSnapshot(_all_ids(led), 4), moment=MOMENT_POST_MORTEM, sent=4, read_short=0 + ) + rec = _record_with(audit, read=4, sent=4) + assert rec.no_loss.ok + assert rec.no_loss.detail == "read>=sent, sink_received>=written, backlog drained" + + +# --- the SLO: a green must not be earned by an audit that never ran ------------------------------ + + +def _profile(intake_audit: bool = True) -> object: + flag = "true" if intake_audit else "false" + return load_connscale_profile_text( + "[connscale]\n" + 'name = "slo-it"\n' + "counts = [4]\n" + "base_port = 41000\n" + "aggregate_rate = 10.0\n" + f"intake_audit = {flag}\n" + "\n" + "[connscale.slo]\n" + "zero_loss = false\n" + ) + + +def _slo(record: ConnScaleRecord, *, enabled: bool = True) -> SloCheck | None: + checks = _evaluate_slos(_profile(enabled), [record]) # type: ignore[arg-type] + return next((c for c in checks if c.name == "intake_audit"), None) + + +def test_slo_fails_on_a_suspect_record_and_names_the_sequence_numbers() -> None: + led = _ledger(accepted=4) + audit = judge( + led, + StoreSnapshot(frozenset(list(led.confirmed)[1:]), 3), + moment=MOMENT_POST_MORTEM, + sent=4, + read_short=1, + ) + check = _slo(_record_with(audit, read=3, sent=4)) + assert check is not None and not check.ok + assert "INVARIANT_SUSPECT" in str(check.observed) and "seqs=[0]" in str(check.observed) + + +def test_slo_states_its_scope_rather_than_claiming_a_bare_clean() -> None: + """A GREEN THAT MEANS LESS, headed off. ``_evaluate_slos`` is shared with the batch-box aggregate, + whose records carry a NOT_RUN audit by construction (its driver processes poll a REMOTE engine and + have no store to read). A bare "clean" there would be a pass earned by nothing, so the observation + always names how many steps were actually audited -- and zero-audited says so in those words.""" + led = _ledger(accepted=4) + clean = judge( + led, StoreSnapshot(_all_ids(led), 4), moment=MOMENT_POST_MORTEM, sent=4, read_short=0 + ) + audited = _slo(_record_with(clean, read=4, sent=4)) + assert audited is not None and audited.ok + assert str(audited.observed) == "clean (1 of 1 step(s) audited)" + + never = _slo(_record_with(not_run("audit not wired"), read=4, sent=4)) + assert never is not None and never.ok # not a FAILURE -- but it must not read as a finding + assert "NOT AUDITED" in str(never.observed) and "0 of 1" in str(never.observed) + + +def test_slo_is_absent_when_the_profile_turns_the_audit_off() -> None: + led = _ledger(accepted=4) + clean = judge( + led, StoreSnapshot(_all_ids(led), 4), moment=MOMENT_POST_MORTEM, sent=4, read_short=0 + ) + assert _slo(_record_with(clean, read=4, sent=4), enabled=False) is None + + +# --- the sweep's short-page limb ----------------------------------------------------------------- + + +def test_sweep_reports_truncated_when_a_page_returns_fewer_rows_than_counted() -> None: + """``COUNT(*)`` promised more rows than the pages delivered. Returning the short set would read as + absence for every row the sweep never reached -- the same catastrophic false positive the + empty-read guard exists for, one page further in.""" + + class _ShortStore: + async def count_messages(self) -> int: + return 5 + + async def list_messages(self, *, limit: int, offset: int) -> list[dict[str, object]]: + return [{"control_id": "A"}] if offset == 0 else [] + + snap = asyncio.run(sweep_store(_ShortStore(), row_cap=100)) + assert snap.truncated and snap.total == 5 and snap.control_ids == frozenset({"A"}) + verdict = judge( + _ledger(accepted=5), snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=4 + ).verdict + assert verdict == VERDICT_PROBE_UNUSABLE diff --git a/tests/test_connscale_smoke.py b/tests/test_connscale_smoke.py index 18ed8a85..30355026 100644 --- a/tests/test_connscale_smoke.py +++ b/tests/test_connscale_smoke.py @@ -23,6 +23,7 @@ import pytest +from harness.load.connscale.intake_audit import MOMENT_POST_MORTEM from harness.load.connscale.probe import ProbeDegraded from harness.load.connscale.profile import load_connscale_profile_text from harness.load.connscale.report import ConnScaleRecord @@ -173,6 +174,44 @@ def _assert_fd_probe(records: Sequence[ConnScaleRecord]) -> None: ) +def _assert_intake_audit(records: Sequence[ConnScaleRecord]) -> None: + """Assert the BACKLOG #1292 discriminator on every step, in the order its verdicts matter. + + Four properties, each pinning a different way this could go quietly wrong: + + 1. NO step is ``engine_suspect``. This is the finding the item is about: the engine framed an + accept-ACK for a message and its own stopped, committed store has no row for it. + 2. EVERY step's audit is CONCLUSIVE. A PROBE_UNUSABLE result is not a pass -- it means the + instrument could not answer, and an instrument that silently stops answering leaves the + original unattributable failure in place while looking green. + 3. EVERY step's sweep read rows (``store_total > 0``) against a step that sent messages. This is + the positive control: a set comparison against an empty set is clean for the wrong reason. + 4. The authoritative audit is the POST-MORTEM one. A live read could be explained away as early + sampling; a read of a stopped engine's store cannot, and mislabelling one as the other would + destroy the only distinction the second moment exists to make. + """ + for r in records: + audit = r.intake_audit + assert not audit.engine_suspect, ( + f"COUNT-AND-LOG INVARIANT -- {r.sweep_mode}@N={r.count}: {audit.summary()}. The engine " + f"accept-ACKed those sends and its own stopped, committed store has no messages row for " + f"them, so a deploying site would be able to lose an acknowledged message at intake. " + f"The sequence numbers above name them; this is reproducible, not statistical." + ) + assert audit.conclusive, ( + f"INTAKE AUDIT COULD NOT ANSWER -- {r.sweep_mode}@N={r.count}: {audit.summary()}. The " + f"discriminator is the whole point of this step's no-loss coverage, so a probe that did " + f"not run or could not read is reported as a failure rather than tolerated: tolerating " + f"it restores exactly the unattributable red this check exists to replace." + ) + assert audit.moment == MOMENT_POST_MORTEM, audit + assert audit.store_total > 0, ( + f"INTAKE AUDIT POSITIVE CONTROL -- {r.sweep_mode}@N={r.count} sent {r.sent} message(s) " + f"and the store sweep read {audit.store_total} row(s): {audit.summary()}. A clean set " + f"comparison over an empty read says nothing about intake." + ) + + async def test_connscale_smoke_end_to_end() -> None: # Reserve a contiguous inbound-port block (BACKLOG #1014). The sweep's max connection count # needs that many contiguous inbound ports, and the engine binds base_port + i for each. A @@ -213,10 +252,28 @@ async def test_connscale_smoke_end_to_end() -> None: } # (2) No-loss at each N (sent == engine_read, engine_written == sink_received, backlog drained). + # UNCHANGED, deliberately. This is the COUNT check, and (2b) below does not replace it: a step + # whose counts do not reconcile still fails here exactly as before. What changed is that its + # failure message now carries the per-message attribution appended by `_build_record`, so a CI + # reader gets a verdict instead of a number they cannot act on. for r in report.records: assert r.sent > 0, r assert r.no_loss.ok, (r.sweep_mode, r.count, r.no_loss.detail) + # (2b) BACKLOG #1292 -- the PER-MESSAGE intake audit, asserted INDEPENDENTLY of (2). + # + # It is strictly ADDITIONAL, not a restatement: (2) compares counts and forgives an unconfirmed + # send, so a message the engine ACCEPT-ACKed and then has no row for passes (2) whenever it also + # happened to be excused as a timeout. This asserts the thing (2) cannot: that no accept-ACKed + # message is absent from the engine's own committed store. + # + # PROBE_UNUSABLE is required to be ABSENT rather than tolerated. Unlike wall #4's OS probe there + # is no known legitimate degrade path here -- the read is a paged query against a quiesced, + # step-exclusive store -- so a probe that could not answer is a defect to classify deliberately, + # not a gap to excuse. `store_total > 0` is the sweep's positive control: a query returning + # nothing would otherwise score a clean set comparison over an empty set. + _assert_intake_audit(report.records) + # (3) Curve monotonicity smoke (a LOOSE >= per mode; CI runners are noisy): FD count + empty-claims # at N=24 >= N=12. Asserted via the report's monotonicity SLOs. slo_by_name = {c.name: c for c in report.slos} From 1fa7460beca240dff2e5d2afa3cc69bfe239288c Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 20:28:00 -0500 Subject: [PATCH 06/11] fix(1292): the intake audit could clear intake over an empty comparison, and blamed the gauge by guessing The adversarial review the wip commit said this needed has now run: six dimensions, each finding handed to a refuter. 17 findings raised, 8 survived, 9 refuted as overstated or wrong. Then a quality pass found a further defect in my own fix for one of them. Every fix below is verified red-first -- the guard is broken, the test is watched to fail, the guard restored. THE HIGH ONE. judge()'s vacuity guard tested `ledger.total` (confirmed + unconfirmed) while every finding branch consumes only `ledger.confirmed`. A step where nothing was ever ACK-confirmed therefore passed all six blindness guards and returned a CONCLUSIVE verdict -- "the shortfall is not in intake" -- computed over a ZERO-ELEMENT comparison, with a green SLO row and all four smoke assertions passing. The existing positive control guards the STORE side; nothing guarded the LEDGER side. It is reachable on the harness's own headline fault, because the runner's excusal clamps `excused` to 0 over budget. This is the blind-but-green direction the whole ordering exists to prevent, and it cleared intake on precisely the step the reconcile calls a possible accepted-and-dropped. THE ONE I GOT WRONG FIRST, RECORDED BECAUSE THE SHAPE MATTERS. SAMPLING_LAG named the `engine_read` gauge by pure elimination. My first correction inferred the missing state inside judge() by asking `read_short <= len(ledger.unconfirmed)`. That inference holds only OVER budget. IN budget `excused == unconfirmed`, so the unconfirmed sends are ALREADY subtracted out of `read_short` and any residue is confirmed sends the gauge did not count -- a real finding my predicate silenced. And over-budget needs `timeouts > 3/4 sent`, so the case I broke is the COMMON one. Measured against the real producer arithmetic (sent=100, timeouts=5, read=93): old predicate `0 < 2 <= 5` fires and silences; corrected predicate names the gauge. The fix is at the producer instead -- `_unexplained_shortfall` computes `sent - unconfirmed - read`, which means the same thing in both worlds, and judge() consumes it rather than reconstructing budget arithmetic it does not own. An absent value falls back to naming the gauge, so the unsafe direction is never the default. ALSO FIXED, all confirmed by the review: - engine_suspect now requires the POST-MORTEM moment. A live sweep pages over a store still being written and can manufacture INVARIANT_SUSPECT; the prose already hedged that, but the machine surface did not, and only a distant call site's choice of field kept it honest. - the live moment reports an absent row WITHOUT the engine conclusion (`_conclusion`), so a console line or JSON artifact cannot be quoted as an invariant violation the post-mortem does not support. - the live NOT_RUN reason said "no intake shortfall to attribute" when the audit was merely DISABLED -- a false statement about the run, in the field whose only job is attribution, on exactly the step someone opens the artifact to read. - two docstrings promised full control ids "stay in the harness log line". Nothing logs one. False in both directions: a reader finds none, and a maintainer reconciling prose against code would make it true by logging exactly what the PHI rule exists to keep out. TWO TESTS THAT PASSED WITH THE FEATURE BROKEN, which is the defect class this branch has now been corrected for three times. The overflow test asserted only the verdict against an empty snapshot, so a neighbouring guard produced the same PROBE_UNUSABLE and it passed with `if ledger.overflow:` deleted outright. CORRELATION_SUSPECT had a positive test but no negative control, so dropping the store-membership half of its predicate kept the suite green while turning every ordinary NAK into a standing finding. Both now assert the discriminating detail and both go red when broken. BEHAVIOUR CHANGE, STATED SO IT IS NOT A SURPRISE: a step where nothing was ever confirmed now returns PROBE_UNUSABLE, which is not `conclusive`, so `_assert_intake_audit` FAILS there. It previously passed with a vacuous clean verdict. That is the intended direction -- the item's own text says a probe that cannot answer is a defect to classify deliberately, not a gap to excuse -- but it does mean this test can now red where it was falsely green. Quality pass also collapsed three near-identical `_build` call sites into a `_matched` closure beside the existing `_unusable` one, hoisted the duplicated counts, and gave the two audit moments one spelling of the disabled reason so they cannot drift into disagreeing about why nothing ran. VERIFIED: ruff check + format, mypy strict, 33 intake-audit tests, 241 connscale tests, and the connscale end-to-end smoke (2 passed) -- which exercises the new verdict semantics for real rather than at unit level. Red-first on all six new or repaired guards, including one that reproduces the predicate bug I introduced. Glyph scan clean over 403 added lines against a 736-hit positive control. The ledger stays untouched, as on every commit on this branch. Co-Authored-By: Claude Opus 5 --- docs/LOAD-TESTING.md | 21 ++- harness/load/connscale/intake_audit.py | 186 ++++++++++++++++------ harness/load/connscale/report.py | 4 +- harness/load/connscale/runner.py | 51 ++++++- tests/test_connscale_intake_audit.py | 204 ++++++++++++++++++++++++- 5 files changed, 403 insertions(+), 63 deletions(-) diff --git a/docs/LOAD-TESTING.md b/docs/LOAD-TESTING.md index dc495d2d..525838ea 100644 --- a/docs/LOAD-TESTING.md +++ b/docs/LOAD-TESTING.md @@ -306,17 +306,28 @@ headroom denominator (in + out events, not messages). Exit codes match `--load`. own store, per message, whether that row is there. Its verdict is on the console, in the JSON artifact under `records[].intake_audit`, and appended to a failing `no_loss` detail: - `INTAKE_COMPLETE` — every confirmed send has a row. - - `SAMPLING_LAG` — a shortfall was reported and every confirmed send has a row anyway, so the - shortfall is in the gauge (sample attribution or per-inbound sum coverage). A harness defect. + - `SAMPLING_LAG` — a shortfall was reported, every confirmed send has a row anyway, **and the + shortfall is larger than the never-confirmed sends can account for**, so the unexplained + remainder is in the gauge (sample attribution or per-inbound sum coverage). A harness defect. + - `UNCONFIRMED_SHORTFALL` — the shortfall is no larger than the set of sends the harness never got + a response frame for, so it implicates **neither** intake **nor** the gauge. Split out from + `SAMPLING_LAG` because the excusal clamps its allowance to zero once the unconfirmed count + exceeds its budget, and the shortfall then consists of sends the engine may never have received; + blaming the gauge for those accused an instrument that was exactly right, and contradicted the + `no_loss` line this verdict is appended to, which already calls that step a systemic no-ACK fault. - `INVARIANT_SUSPECT` — a send the engine accept-ACKed has no row in its own stopped, committed store. The engine branch: on a deployment an acknowledged message would be lost at intake. The verdict names the sequence numbers, so it is reproducible rather than statistical. - `CORRELATION_SUSPECT` — only *rejected* sends are unmatched. Not an engine finding: several NAK paths record their row with a NULL control id, so a rejected message is expected to be unmatchable by control id. - - `PROBE_UNUSABLE` — the audit could not answer (its own read came back empty, was truncated, or - the send ledger was incomplete). Deliberately **not** rendered as "everything is missing", and - deliberately **not** a pass either. + - `PROBE_UNUSABLE` — the audit could not answer. At least: its own read came back empty or + truncated, the send ledger was incomplete, or **nothing was ever confirmed**, which leaves the + compared set empty. That last one is the ledger-side positive control and it is not redundant + with the store-side one — a ledger holding only unconfirmed sends is non-empty by total and + still compares nothing, so without it the audit returned a conclusive "not in intake" computed + over zero elements. Deliberately **not** rendered as "everything is missing", and deliberately + **not** a pass either. It runs at two moments: LIVE (engine still up, only on a shortfall) and POST-MORTEM (engine stopped, always). The post-mortem one is authoritative — a live read can be explained away as diff --git a/harness/load/connscale/intake_audit.py b/harness/load/connscale/intake_audit.py index bedabb83..5238b36a 100644 --- a/harness/load/connscale/intake_audit.py +++ b/harness/load/connscale/intake_audit.py @@ -38,7 +38,12 @@ PHI. ``report.py`` states the rule for this artifact family: metrics and metadata only, never message bodies and never control-id lists. So the audit reports SEQUENCE NUMBERS (dense integers minted by the harness's own counter, meaningless outside the run) and the DISTINCT MSA-1 codes involved -- both -sufficient to act on, neither a control-id list. Full control ids stay in the harness log line. +sufficient to act on, neither a control-id list. **Control ids are not emitted ANYWHERE -- not to the +artifact, not to the log.** Said explicitly because the earlier wording here promised they "stay in +the harness log line", which was false in both directions: a reader who went looking for them found +none, and a maintainer reconciling the prose against the code would have made it true by logging +them, which is precisely what the rule above forbids. A run is reproduced from the seqs and the +profile, never from an identifier list. """ from __future__ import annotations @@ -65,6 +70,13 @@ #: A shortfall was reported, yet every confirmed send HAS a row -> the ``engine_read`` gauge, not the #: engine, is short. A harness/instrument defect (sample attribution or sum coverage). VERDICT_SAMPLING_LAG: Final = "SAMPLING_LAG" +#: A shortfall was reported and it is wholly accounted for by sends that were NEVER CONFIRMED, so it +#: implicates neither intake nor the gauge. Split out from :data:`VERDICT_SAMPLING_LAG` because the +#: runner's excusal clamps ``excused`` to 0 once the unconfirmed count exceeds its budget, and the +#: shortfall handed here then consists of sends the engine may never have seen. Blaming the gauge for +#: those named an instrument that was exactly right, and CONTRADICTED the reconcile text this verdict +#: is appended to -- which already calls that step a systemic no-ACK fault. +VERDICT_UNCONFIRMED_SHORTFALL: Final = "UNCONFIRMED_SHORTFALL" #: A send the engine ACCEPT-ACKed has no ``messages`` row -> the count-and-log invariant would be #: broken. The engine branch, and the only one that justifies the P1. VERDICT_INVARIANT_SUSPECT: Final = "INVARIANT_SUSPECT" @@ -203,14 +215,23 @@ def conclusive(self) -> bool: return self.verdict in ( VERDICT_INTAKE_COMPLETE, VERDICT_SAMPLING_LAG, + VERDICT_UNCONFIRMED_SHORTFALL, VERDICT_INVARIANT_SUSPECT, VERDICT_CORRELATION_SUSPECT, ) @property def engine_suspect(self) -> bool: - """Is this the branch that implicates the ENGINE (vs the harness or the probe)?""" - return self.verdict == VERDICT_INVARIANT_SUSPECT + """Is this the branch that implicates the ENGINE (vs the harness or the probe)? + + THE MOMENT IS PART OF THE CLAIM, not a caveat on it. A LIVE sweep pages over a store still + being written and can miss a row that is present, so a live INVARIANT_SUSPECT is not by + itself an engine finding -- ``_conclusion`` already says so in the prose. Requiring the + post-mortem here makes the machine surface agree with that text BY CONSTRUCTION rather than + by the SLO gate happening to read the post-mortem field, which is an invariant maintained by + a distant call site and would break silently if another reader picked the live one. + """ + return self.verdict == VERDICT_INVARIANT_SUSPECT and self.moment == MOMENT_POST_MORTEM def summary(self) -> str: """One line a CI reader can act on without re-running anything.""" @@ -261,6 +282,27 @@ def not_run(reason: str, *, moment: str = MOMENT_POST_MORTEM) -> IntakeAudit: ) +def _conclusion(moment: str) -> str: + """What an accept-ACKed-but-absent row is ALLOWED to conclude, which depends on the moment. + + Only the post-mortem may state the engine finding. ``sweep_store`` pages with ``ORDER BY + received_at DESC`` + OFFSET, so a row committed while a LIVE sweep is walking shifts the window + and a genuinely present row can go unread -- the module docstring records this as known and + deliberate, and it manufactures exactly this verdict. The live text therefore reports the same + observation without the conclusion, so a console line or a JSON artifact cannot be quoted as an + invariant violation the post-mortem beside it does not support. + """ + if moment == MOMENT_POST_MORTEM: + return ( + "the engine was STOPPED and its store committed when this was read, so on a deployment " + "the count-and-log invariant would not hold for those messages" + ) + return ( + "NOT an engine finding on its own: this LIVE read pages over a store still being written " + "and can miss a row that is present, so it stands only if the post-mortem reproduces it" + ) + + def judge( ledger: IntakeLedger, snapshot: StoreSnapshot, @@ -268,6 +310,7 @@ def judge( moment: str, sent: int, read_short: int, + unexplained_short: int | None = None, ) -> IntakeAudit: """Turn a ledger + one store read into a verdict. Pure -- the whole decision table, unit-testable. @@ -276,22 +319,33 @@ def judge( blind is ruled out BEFORE a null is allowed to mean anything: 1. the sender-side ledger is overflowed / non-unique / empty -> PROBE_UNUSABLE. - 2. the store sweep failed or was truncated -> PROBE_UNUSABLE. - 3. the store sweep read ZERO rows against a non-empty ledger -> PROBE_UNUSABLE. This is the - positive control, and it is checked HERE so a broken query renders as "unusable" and never as - "every message is missing" -- the worst possible false positive to hang a P1 on. - 4. an ACCEPT-ACKed send with no row -> INVARIANT_SUSPECT. Checked BEFORE the partial-ledger guard + 2. NOTHING was ever confirmed -> PROBE_UNUSABLE. The ledger-side positive control, and separate + from step 1 on purpose: the compared set is ``confirmed``, so a ledger holding only + unconfirmed sends is non-empty by ``total`` and still compares NOTHING. + 3. the store sweep failed or was truncated -> PROBE_UNUSABLE. + 4. the store sweep read ZERO rows against a non-empty ledger -> PROBE_UNUSABLE. The store-side + positive control, checked HERE so a broken query renders as "unusable" and never as "every + message is missing" -- the worst possible false positive to hang a P1 on. + 5. an ACCEPT-ACKed send with no row -> INVARIANT_SUSPECT. Checked BEFORE the partial-ledger guard below: a short ledger under-reports, so a finding inside it is still a real finding. - 5. only REJECT-ACKed sends unmatched -> CORRELATION_SUSPECT (see the module docstring). - 6. the ledger did not account for every send -> PROBE_UNUSABLE, because a null over a partial - ledger proves nothing. This is step 4's mirror image, and why the two are split rather than + 6. only REJECT-ACKed sends unmatched -> CORRELATION_SUSPECT (see the module docstring). + 7. the ledger did not account for every send -> PROBE_UNUSABLE, because a null over a partial + ledger proves nothing. This is step 5's mirror image, and why the two are split rather than both being checked up front. - 7. a shortfall with every confirmed send present -> SAMPLING_LAG: the gauge is short, not intake. - 8. otherwise INTAKE_COMPLETE. + 8. a shortfall with NOTHING left unexplained once the never-confirmed sends are set aside -> + UNCONFIRMED_SHORTFALL. ``unexplained_short`` is supplied by the PRODUCER, which alone knows + whether its excusal was clamped; this step must not infer it from the unconfirmed count, + because in-budget those sends are already subtracted out of ``read_short`` and the guess + silences a real gauge finding on the common path. + 9. a shortfall with an unexplained remainder, every confirmed send present -> SAMPLING_LAG: that + remainder is in the gauge, not intake. + 10. otherwise INTAKE_COMPLETE. """ confirmed = ledger.confirmed + unconfirmed_count = len(ledger.unconfirmed) ledger_total = ledger.total store_ids = snapshot.control_ids + unexplained = read_short if unexplained_short is None else unexplained_short def _unusable(detail: str) -> IntakeAudit: return _build( @@ -322,6 +376,22 @@ def _unusable(detail: str) -> IntakeAudit: f"the send ledger recorded NOTHING against {sent} counted send(s) -- the sender-side " f"instrument did not run, so a clean set comparison here would be vacuous" ) + if not confirmed and sent > 0: + # THE POSITIVE CONTROL FOR THE LEDGER SIDE, and it must test `confirmed` rather than + # `ledger_total`: every finding branch below iterates `confirmed` and NOTHING reads + # `unconfirmed` except as a count, so a ledger holding only unconfirmed sends compares an + # EMPTY set and every verdict it could reach would be true of nothing. The guard above does + # not cover this -- it fires only when the ledger is empty outright. Reachable on the + # harness's own headline fault: when the runner's excusal goes over budget it clamps + # `excused` to 0, so a step where no send was ever ACKed arrives here with a large + # `read_short`, and without this it returned a CONCLUSIVE "not in intake" over zero + # elements -- clearing intake on exactly the step the reconcile calls a possible + # accepted-and-dropped. + return _unusable( + f"NO send was ever confirmed against {sent} counted send(s) ({len(ledger.unconfirmed)} " + f"unconfirmed) -- the compared set is empty, so no verdict here could distinguish a " + f"clean intake from a lost one" + ) if snapshot.error is not None: return _unusable(f"the store sweep failed: {snapshot.error}") if snapshot.truncated: @@ -352,6 +422,25 @@ def _unusable(detail: str) -> IntakeAudit: ) late_unconfirmed = sum(1 for cid in ledger.unconfirmed if cid in store_ids) + def _matched(verdict: str, detail: str) -> IntakeAudit: + """The three MATCHED outcomes -- every confirmed send accounted for -- differ only in verdict + and prose. Collapsed for the same reason ``_unusable`` above is: three adjacent hand-rolled + blocks differing in one constant make a divergence in a copied argument read as normal, and + that divergence is not hypothetical here (``_unusable`` deliberately passes + ``late_unconfirmed=0`` while these pass the computed value).""" + return _build( + moment=moment, + verdict=verdict, + read_short=read_short, + sent=sent, + ledger=ledger, + snapshot=snapshot, + missing_accepted=(), + missing_rejected=(), + late_unconfirmed=late_unconfirmed, + detail=detail, + ) + if missing_accepted: partial = ( "" @@ -370,8 +459,7 @@ def _unusable(detail: str) -> IntakeAudit: late_unconfirmed=late_unconfirmed, detail=( f"{len(missing_accepted)} send(s) the engine ACCEPT-ACKed have no messages row in " - f"its own store ({snapshot.total} row(s) present){partial} -- on a deployment the " - f"count-and-log invariant would not hold for those messages" + f"its own store ({snapshot.total} row(s) present){partial} -- {_conclusion(moment)}" ), ) if missing_rejected: @@ -397,37 +485,37 @@ def _unusable(detail: str) -> IntakeAudit: f"comparison over a partial ledger cannot exclude a loss among the " f"{sent - ledger_total} it never saw" ) + # NAMING THE GAUGE IS A POSITIVE CLAIM, so it is made only for the part of the shortfall no + # excusal can forgive -- and that part is COMPUTED BY THE PRODUCER, never inferred here. The + # runner's excusal clamps `excused` to 0 over budget and then hands on a bare int, so + # `read_short` alone cannot say which world it describes: in-budget the unconfirmed sends are + # already subtracted out of it, over-budget they are still inside it. Guessing from the + # unconfirmed COUNT gets the common in-budget case backwards and silences a real gauge finding. + # `None` means the producer did not say; then every missing message is the gauge's to answer + # for, which is the pre-existing behaviour and errs toward a HARNESS finding rather than + # toward silence. + if read_short > 0 and unexplained <= 0: + return _matched( + VERDICT_UNCONFIRMED_SHORTFALL, + f"engine_read is short by {read_short}, and once the {unconfirmed_count} never-" + f"confirmed send(s) are set aside NOTHING is left unaccounted for -- so the shortfall " + f"implicates NEITHER intake NOR the engine_read gauge. All {len(confirmed)} confirmed " + f"send(s) have a messages row ({snapshot.total} row(s) present, {late_unconfirmed} " + f"unconfirmed send(s) arrived anyway)", + ) if read_short > 0: - return _build( - moment=moment, - verdict=VERDICT_SAMPLING_LAG, - read_short=read_short, - sent=sent, - ledger=ledger, - snapshot=snapshot, - missing_accepted=(), - missing_rejected=(), - late_unconfirmed=late_unconfirmed, - detail=( - f"engine_read is short by {read_short} yet all {len(confirmed)} confirmed send(s) " - f"HAVE a messages row ({snapshot.total} row(s) present) -- the shortfall is in the " - f"engine_read gauge (sample attribution or per-inbound sum coverage), not in intake" - ), + return _matched( + VERDICT_SAMPLING_LAG, + f"engine_read is short by {read_short}, of which {unexplained} remain(s) unaccounted " + f"for after the {unconfirmed_count} never-confirmed send(s) are set aside, yet all " + f"{len(confirmed)} confirmed send(s) HAVE a messages row ({snapshot.total} row(s) " + f"present) -- that remainder is in the engine_read gauge (sample attribution or " + f"per-inbound sum coverage), not in intake", ) - return _build( - moment=moment, - verdict=VERDICT_INTAKE_COMPLETE, - read_short=read_short, - sent=sent, - ledger=ledger, - snapshot=snapshot, - missing_accepted=(), - missing_rejected=(), - late_unconfirmed=late_unconfirmed, - detail=( - f"all {len(confirmed)} confirmed send(s) have a messages row; {snapshot.total} row(s) " - f"present, {late_unconfirmed} excused send(s) arrived anyway" - ), + return _matched( + VERDICT_INTAKE_COMPLETE, + f"all {len(confirmed)} confirmed send(s) have a messages row; {snapshot.total} row(s) " + f"present, {late_unconfirmed} excused send(s) arrived anyway", ) @@ -472,6 +560,7 @@ async def run_intake_audit( moment: str, sent: int, read_short: int, + unexplained_short: int | None = None, ) -> IntakeAudit: """Read the store once through ``reader`` and judge. @@ -482,7 +571,14 @@ async def run_intake_audit( snapshot = await reader() except Exception as exc: # noqa: BLE001 - any reader failure is a probe outcome, not a run failure snapshot = StoreSnapshot(frozenset(), 0, error=f"{type(exc).__name__}: {exc}") - audit = judge(ledger, snapshot, moment=moment, sent=sent, read_short=read_short) + audit = judge( + ledger, + snapshot, + moment=moment, + sent=sent, + read_short=read_short, + unexplained_short=unexplained_short, + ) if audit.verdict != VERDICT_INTAKE_COMPLETE: log.warning("%s", audit.summary()) return audit diff --git a/harness/load/connscale/report.py b/harness/load/connscale/report.py index 193d2666..075e8670 100644 --- a/harness/load/connscale/report.py +++ b/harness/load/connscale/report.py @@ -10,7 +10,9 @@ That rule is why the BACKLOG #1292 intake audit reports **sequence numbers**, not the control ids it actually matched on: a seq is a dense integer minted by the harness's own counter and meaningless outside the run, so it identifies the message for a follow-up without putting a list of message -identifiers into a shared artifact. The control ids stay in the harness log line. +identifiers into a shared artifact. The control ids are emitted NOWHERE -- not here and not to the +log; the previous wording sent readers to a log line that never carried them, and invited a +maintainer to make the sentence true by logging exactly what this rule exists to keep out. The thundering-herd measurement is reported **explicitly and separated** (critic must-change #3): the ``fixed_aggregate`` sweep (constant R across N) IS the herd measurement, so the report carries the diff --git a/harness/load/connscale/runner.py b/harness/load/connscale/runner.py index 9a749e54..4e780932 100644 --- a/harness/load/connscale/runner.py +++ b/harness/load/connscale/runner.py @@ -89,6 +89,10 @@ _STOP_GRACE = 5.0 _SETTLE = 0.5 # let final ACKs/arrivals settle before the truly-final engine sample _HEALTH_TIMEOUT = 30.0 +# One spelling, because both audit moments report it and they must not drift into disagreeing about +# why nothing ran -- a reader comparing the two moments of a disabled step reads the difference as +# meaningful. +_AUDIT_DISABLED = "intake audit disabled for this profile" _PORTS_READY_TIMEOUT = 60.0 # waiting for the engine to report all N inbound rows (N can be large) # A single trivial ADT type — the connscale graph routes every message identically, so the mix only # needs to drive ONE generated type (the wall is per-connection machinery, not message-type spread). @@ -479,22 +483,35 @@ async def _run_one_step( read_short = _read_shortfall( metrics.counters, poller.baseline, poller.final, unconfirmed_budget=count ) - audit_live = intake_audit.not_run( - "no intake shortfall to attribute at this moment", moment=intake_audit.MOMENT_LIVE - ) - if ledger is not None and read_short > 0: + # The part no excusal forgives, computed HERE because only this side knows whether the + # excusal was clamped. The audit must not re-derive it -- see `_unexplained_shortfall`. + unexplained_short = _unexplained_shortfall(metrics.counters, poller.baseline, poller.final) + # The two NOT_RUN reasons answer different questions, and each is stated at the branch it + # describes. The disabled one has to be reachable on a step that DID have a shortfall: "no + # shortfall to attribute" printed on a failing step is a false statement about the run, in + # the one field whose entire job is attribution, on exactly the step someone opens the + # artifact to read. + if ledger is None: + audit_live = intake_audit.not_run(_AUDIT_DISABLED, moment=intake_audit.MOMENT_LIVE) + elif read_short > 0: audit_live = await intake_audit.run_intake_audit( ledger, _store_reader(node_env, metrics.counters.sent), moment=intake_audit.MOMENT_LIVE, sent=metrics.counters.sent, read_short=read_short, + unexplained_short=unexplained_short, + ) + else: + audit_live = intake_audit.not_run( + "no intake shortfall to attribute at this moment", + moment=intake_audit.MOMENT_LIVE, ) # MOMENT 2, POST-MORTEM. Stop the engine FIRST, so the store is committed and quiesced: with # no process running, "we sampled too early" is no longer available as an explanation, which # is what separates outcome 1 (sample lag) from outcome 2 (sum coverage). `stop()` is # idempotent, so the `finally` below still runs it on every other path. - audit_final = intake_audit.not_run("intake audit disabled for this profile") + audit_final = intake_audit.not_run(_AUDIT_DISABLED) if ledger is not None: with contextlib.suppress(Exception): await node.stop() @@ -512,6 +529,7 @@ async def _run_one_step( moment=intake_audit.MOMENT_POST_MORTEM, sent=metrics.counters.sent, read_short=read_short, + unexplained_short=unexplained_short, ) return _build_record( claim_mode=claim_mode, @@ -989,6 +1007,29 @@ def _read_shortfall( ) +def _unexplained_shortfall( + c: Counters, base: EngineSample | None, final: EngineSample | None +) -> int: + """The part of the shortfall NO excusal can forgive -- ``sent - unconfirmed - engine_read``. + + THE PRODUCER OWNS THIS BECAUSE ONLY THE PRODUCER CAN COMPUTE IT. ``_read_shortfall`` subtracts + ``excused``, which ``_excusal`` CLAMPS TO 0 over budget, and it then returns a bare int -- so the + clamp state is destroyed one line after being computed. A consumer handed only that int cannot + tell the two worlds apart: in-budget the unconfirmed sends are ALREADY subtracted out, so a + residual shortfall is a genuine gauge finding, while over-budget the same number silently + contains them. Comparing the shortfall against the unconfirmed COUNT to guess which world it is + gets the common (in-budget) case backwards and would silence a real gauge finding. + + Subtracting the UNCLAMPED ``c.timeouts`` makes the quantity mean the same thing in both worlds, + so the audit never has to reconstruct budget arithmetic it does not own. Uses ``c.timeouts`` -- + the same input ``_excusal`` calls ``unconfirmed`` -- rather than the ledger, keeping one + definition of the population. + """ + if base is None or final is None: + return 0 + return c.sent - c.timeouts - (final.read - base.read) + + def _reconcile( c: Counters, base: EngineSample | None, diff --git a/tests/test_connscale_intake_audit.py b/tests/test_connscale_intake_audit.py index 4d2998aa..394bb395 100644 --- a/tests/test_connscale_intake_audit.py +++ b/tests/test_connscale_intake_audit.py @@ -12,7 +12,9 @@ The three planted worlds mirror the three ways this can go, and they are deliberately not variations of one: -* the rows ARE all there while a shortfall is reported -> SAMPLING_LAG (harness/instrument) +* the rows ARE all there, a shortfall is reported, and + part of it survives setting the never-confirmed + sends aside -> SAMPLING_LAG (harness/instrument) * an ACCEPT-ACKed row is genuinely absent -> INVARIANT_SUSPECT (the engine branch) * the probe's own read comes back empty -> PROBE_UNUSABLE (the null guard) @@ -37,6 +39,7 @@ VERDICT_NOT_RUN, VERDICT_PROBE_UNUSABLE, VERDICT_SAMPLING_LAG, + VERDICT_UNCONFIRMED_SHORTFALL, IntakeAudit, IntakeLedger, StoreSnapshot, @@ -199,6 +202,189 @@ def test_absent_rejected_message_is_correlation_suspect_not_loss() -> None: assert audit.missing_codes == ("AE",) +def test_a_rejected_send_that_IS_stored_is_not_a_correlation_finding() -> None: + """The negative control for the branch above, and it is the one that pins the MEMBERSHIP test. + + The NULL-control-id NAK limbs are only SOME of them: a limb that rejects AFTER parsing MSH-10 + writes a row that DOES carry the id, and the sweep finds it. Without this, dropping the + ``cid not in store_ids`` half of the predicate -- leaving a bare ``not rec.accepted`` -- kept the + whole suite green while turning every ordinary NAK into a standing CORRELATION_SUSPECT that + ``report.py`` prints on each clean step, and which the smoke assertion cannot catch because that + verdict is conclusive and not engine_suspect. This is the false-alarm direction. + """ + led = _ledger(accepted=3, rejected=1) + audit = judge( + led, StoreSnapshot(_all_ids(led), total=4), moment=MOMENT_POST_MORTEM, sent=4, read_short=0 + ) + + assert audit.verdict == VERDICT_INTAKE_COMPLETE + assert audit.missing_rejected_total == 0 and audit.missing_accepted_total == 0 + + +# --- the LEDGER-side positive control, and the shortfall it must not misattribute ---------------- + + +def test_a_ledger_with_nothing_confirmed_is_unusable_not_clean() -> None: + """THE BLIND-BUT-GREEN CASE, and the one the store-side control could not see. + + ``confirmed`` is the only set compared; ``unconfirmed`` is read as a count and never searched. + So a step where no send was ever ACKed compares an EMPTY set, and every guard keyed on + ``ledger.total`` -- which counts both -- waves it through. It is reachable on the harness's own + headline fault: the runner's excusal clamps ``excused`` to 0 once the unconfirmed count exceeds + its budget, so such a step arrives here with a large ``read_short``. + + Before the ledger-side control this returned a CONCLUSIVE verdict stating the shortfall was + "not in intake" -- computed over zero elements, rendering a green SLO row, and passing all four + smoke assertions -- on precisely the step the reconcile calls a possible accepted-and-dropped. + """ + led = _ledger(accepted=0, unconfirmed=5) + snap = StoreSnapshot(frozenset({"OTHER0", "OTHER1", "OTHER2"}), total=3) + + audit = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=5, read_short=2) + + assert audit.verdict == VERDICT_PROBE_UNUSABLE + assert "NO send was ever confirmed" in audit.detail + # The properties that actually protect the run: an empty comparison must not be readable as an + # answer, and must never clear the engine. + assert not audit.conclusive + assert not audit.engine_suspect + + +def test_a_shortfall_inside_the_unconfirmed_set_does_not_accuse_the_gauge() -> None: + """A shortfall made ENTIRELY of never-confirmed sends implicates neither intake nor the gauge. + + ``_excusal`` clamps ``excused`` to 0 when the unconfirmed count exceeds its budget, so + ``read_short`` then carries sends the engine may never have received. Calling that SAMPLING_LAG + accused an ``engine_read`` gauge that matched the store exactly, and the sentence was appended + verbatim to the reconcile's own "systemic no-ACK fault" line -- one failure message contradicting + itself and pointing at an enginepoll bug that does not exist. + """ + led = _ledger(accepted=2, unconfirmed=8) + stored = frozenset(led.confirmed) + # The OVER-BUDGET world: excusal clamped to 0, so read_short = sent - read = 8, and once the 8 + # never-confirmed sends are set aside nothing is unexplained (10 - 8 - 2 = 0). + audit = judge( + led, + StoreSnapshot(stored, total=2), + moment=MOMENT_POST_MORTEM, + sent=10, + read_short=8, + unexplained_short=0, + ) + + assert audit.verdict == VERDICT_UNCONFIRMED_SHORTFALL + assert audit.conclusive and not audit.engine_suspect + # The regression guard keys on the ACCUSATION, not on the words "engine_read gauge" -- this + # detail names the gauge inside a NEGATION ("implicates NEITHER intake NOR the engine_read + # gauge"), so a bare substring test would answer a different question than the one asked. + # "sample attribution" is the diagnosis unique to SAMPLING_LAG, and is what must be absent. + assert "sample attribution" not in audit.detail + assert "never-confirmed" in audit.detail + + +def test_a_shortfall_larger_than_the_unconfirmed_set_still_names_the_gauge() -> None: + """The complement, so the split above cannot be satisfied by never returning SAMPLING_LAG. + + One more missing than the never-confirmed sends can account for, with every confirmed send + present, leaves a remainder nothing else explains -- and THAT is a real gauge finding. + """ + led = _ledger(accepted=2, unconfirmed=8) + audit = judge( + led, + StoreSnapshot(frozenset(led.confirmed), total=2), + moment=MOMENT_POST_MORTEM, + sent=10, + read_short=9, + unexplained_short=1, + ) + + assert audit.verdict == VERDICT_SAMPLING_LAG + assert "sample attribution" in audit.detail + + +def test_an_IN_BUDGET_shortfall_is_a_gauge_finding_even_though_sends_went_unconfirmed() -> None: + """THE REGRESSION THAT AN INFERRED PREDICATE GETS BACKWARDS, and it is the COMMON path. + + A first cut at the split above asked ``read_short <= len(ledger.unconfirmed)`` and read a True as + "the excusal was clamped". That inference only holds OVER budget. In budget ``excused == + unconfirmed``, so the never-confirmed sends are ALREADY subtracted out of ``read_short`` and any + residue is confirmed sends the gauge did not count -- a genuine SAMPLING_LAG. Because + over-budget needs ``timeouts > 3/4 sent``, the in-budget world here is the ordinary one, so the + inferred predicate silenced a real gauge finding on the path most runs take. + + Numbers are the real arithmetic: sent=100, timeouts=5, engine_read=93. ``_excusal`` is in budget + (5 <= max(24, 75)) so ``excused``=5 and ``read_short`` = 100-5-93 = 2, while the unconfirmed + count is 5 -- and 2 <= 5, which is exactly the shape the bad predicate accepted. The producer's + ``unexplained`` = 100-5-93 = 2 is positive, so the gauge is correctly named. + """ + led = _ledger(accepted=95, unconfirmed=5) + audit = judge( + led, + StoreSnapshot(frozenset(led.confirmed), total=95), + moment=MOMENT_POST_MORTEM, + sent=100, + read_short=2, + unexplained_short=2, + ) + + assert audit.verdict == VERDICT_SAMPLING_LAG + assert "sample attribution" in audit.detail + + +def test_the_unexplained_remainder_comes_from_the_producer_not_the_ledger() -> None: + """The two worlds are INDISTINGUISHABLE from inside judge(), which is why it must not guess. + + Identical ledger, identical store, identical ``read_short`` -- only the producer's + ``unexplained_short`` differs, and the verdict flips. That is the whole argument for passing it: + no function of the ledger alone could separate these two. + """ + + def _verdict(unexplained: int) -> str: + led = _ledger(accepted=2, unconfirmed=8) + return judge( + led, + StoreSnapshot(frozenset(led.confirmed), total=2), + moment=MOMENT_POST_MORTEM, + sent=10, + read_short=8, + unexplained_short=unexplained, + ).verdict + + assert _verdict(0) == VERDICT_UNCONFIRMED_SHORTFALL + assert _verdict(3) == VERDICT_SAMPLING_LAG + + +def test_only_the_post_mortem_moment_states_the_engine_conclusion() -> None: + """The same absent row concludes DIFFERENT things at the two moments, and the text must say so. + + ``sweep_store`` pages ``ORDER BY received_at DESC`` + OFFSET, so a row committed while a LIVE + sweep walks shifts the window and a genuinely present row can go unread -- documented as known + and deliberate, and it manufactures exactly this verdict. The machine gates already read only the + post-mortem, but the live detail is printed to the console and stored in the JSON artifact, where + an unhedged "the count-and-log invariant would not hold" is quotable as an engine finding that + the post-mortem beside it may not support. + """ + led = _ledger(accepted=3) + snap = StoreSnapshot(frozenset(list(led.confirmed)[1:]), total=2) + + live = judge(led, snap, moment=MOMENT_LIVE, sent=3, read_short=1) + post = judge(led, snap, moment=MOMENT_POST_MORTEM, sent=3, read_short=1) + + # Both SEE it -- the hedge must not suppress the finding, only its conclusion. + assert live.verdict == post.verdict == VERDICT_INVARIANT_SUSPECT + assert live.missing_accepted_total == post.missing_accepted_total == 1 + + assert "count-and-log invariant would not hold" in post.detail + assert "STOPPED" in post.detail + assert "count-and-log invariant would not hold" not in live.detail + assert "post-mortem reproduces it" in live.detail + + # The MACHINE surface must agree with the prose by construction, not because the SLO gate + # happens to read the post-mortem field. A live sweep can manufacture this verdict; only the + # post-mortem may carry it into `engine_suspect`, which is what fails the run. + assert post.engine_suspect and not live.engine_suspect + + # --- the positive controls ------------------------------------------------------------------------ @@ -261,12 +447,16 @@ def test_ledger_overflow_and_duplicates_are_unusable() -> None: for i in range(4): small.record_confirmed(f"C{i}", i, "AA", accepted=True) assert small.overflow == 2 - assert ( - judge( - small, StoreSnapshot(frozenset(), 0), moment=MOMENT_LIVE, sent=4, read_short=0 - ).verdict - == VERDICT_PROBE_UNUSABLE - ) + # The snapshot trips NO OTHER GUARD -- rows present, nothing truncated, no error, and every id + # the ledger did manage to record IS in the store -- and the DETAIL is asserted, not just the + # verdict. Both matter: with the overflow guard deleted this world still reaches + # PROBE_UNUSABLE via the partial-ledger guard ("accounted 2 of 4"), so a verdict-only assertion + # passed with the guard under test entirely removed. Overflow is step 1 of the blindness + # ordering, and a step whose test cannot fail is not covering it. + snap = StoreSnapshot(frozenset(small.confirmed), total=len(small.confirmed)) + overflowed = judge(small, snap, moment=MOMENT_LIVE, sent=4, read_short=0) + assert overflowed.verdict == VERDICT_PROBE_UNUSABLE + assert "overflowed" in overflowed.detail dup = IntakeLedger() dup.record_confirmed("SAME", 0, "AA", accepted=True) From 08d1ea481d8bdfd4a51646bcef2d168d0cf06506 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 20 Aug 2026 23:54:33 -0500 Subject: [PATCH 07/11] test(321): per-class coverage that exercises the LOADED token set instead of a monkeypatched one BACKLOG #321, BUILD HALF ONLY. Owner-directed. The floor raise in security.yml is NOT here -- see the bottom of this message. THE DEFECT. Every per-class test in tests/test_scan_forbidden.py runs behind the `sf` fixture, which monkeypatches synthetic values over FORBIDDEN / ESTATE_TOKENS / SITE_CODE_RE / _SITE_CODE_FILE. They prove the machinery matches a pattern someone handed it, and never touch the load path. With no prefix loaded both site detectors fall back to `_NEVER` (scan_forbidden.py:194), an empty negative lookahead that matches nothing anywhere -- so a blind scanner and a clean tree are the same green tick, and the suite that looks like per-class coverage CANNOT FAIL when the real token set is wrong. That is the shape of the original defect, reproduced inside its own test suite. TWO ARMS, split by what each is allowed to touch. BEHAVIOURAL: pins the source to the committed synthetic example and drives the REAL pipeline -- MEFOR_FORBIDDEN_TOKENS -> _resolve_token_text -> _parse_tokens -> compilation -> scan_file. Nothing is monkeypatched onto the globals, and probes are DERIVED from what loaded, so a set that loads blind never reaches an assertion: the derivation fails first. REAL SET: runs only where a real source is configured, and asserts STRUCTURE ONLY -- present, not the sentinel, every class counted. It never reads, builds with, or reports a real token. Proving the scanner catches a real token would require putting one in this file, which is exactly the disclosure the scanner exists to prevent (CLAUDE.md sec. 9); the test would become the leak. A HOLE I FOUND IN MY OWN FIRST CUT AND CLOSED. A configured-but-MANGLED source leaves TOKENS_PRESENT false, identically to having no source at all -- so skipping on that alone turned the documented cutover-mangling case (headers lost, comments only, a BOM before the first section) into a green tick. Only the ABSENCE of a source is now a skip; a source that exists and parsed to nothing FAILS. AND ONE THE RED-FIRST PASS FOUND IN MY OWN TEST, recorded because it is this item's exact subject. With FORBIDDEN forced empty, the [names] arm still PASSED -- green against the very class it names. The sets OVERLAP BY DESIGN (a customer name is typically in [names] AND [estate]), so the probe word drawn from [names] was also an estate token and the estate detector produced the hit. The probe is now filtered to a candidate no other detector can explain, and the arm goes red as it should. An over-determined assertion is not coverage, which is the whole reason this item exists. ASSERTED DELIBERATELY, AND NOT: - the estate arm asserts the scan_file PATH, not a count. [estate_body_only] tokens are held out of _ESTATE_FILE_RES and never enter scan_file, while raising the `estate` count identically -- so a count cannot tell a token the file scanner sees from one it does not. - never reason TEXT. The scanner substitutes a generic reason when a reason would itself match a detector, so asserting wording reads the substituted value rather than the finding. - a negative control per class, so an arm cannot be satisfied by a detector that flags everything, and a negative control on the blind state itself, so the guard cannot be asserting something vacuously true. RED-FIRST, ALL FIVE, each broken in scan_forbidden.py and watched to fail, then restored: site prefixes forced to the sentinel; estate held out of the file scan; names loaded empty; the site detector widened to any six-digit run; the blind fallback made unreachable. NOT DONE, AND NOT MINE TO DO: MEFOR_MIN_DETECTORS in .github/workflows/security.yml stays at names=7,estate=13,site_prefixes=1. Raising it to 8/14/2 hard-fails a required check until the owner updates BOTH the Actions and the Dependabot secrets -- both, or every Dependabot PR fails. I will ask rather than infer that from any message. VERIFIED: ruff check + format clean; 102 tests across the three scan_forbidden suites; glyph scan 0 over the new file against a 736-hit positive control. No token value appears in this file, in any assertion message, or in this commit. Co-Authored-By: Claude Opus 5 --- tests/test_scan_forbidden_loaded_set.py | 289 ++++++++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 tests/test_scan_forbidden_loaded_set.py diff --git a/tests/test_scan_forbidden_loaded_set.py b/tests/test_scan_forbidden_loaded_set.py new file mode 100644 index 00000000..eb0ed79b --- /dev/null +++ b/tests/test_scan_forbidden_loaded_set.py @@ -0,0 +1,289 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""BACKLOG #321, build half -- per-class coverage that exercises the LOADED token set. + +WHY THIS FILE EXISTS, AND WHY THE EXISTING PER-CLASS TESTS DO NOT COVER IT. Every per-class test in +``test_scan_forbidden.py`` runs behind the ``sf`` fixture, which monkeypatches SYNTHETIC values over +``FORBIDDEN`` / ``ESTATE_TOKENS`` / ``SITE_CODE_RE`` / ``_SITE_CODE_FILE``. Those tests prove the +MACHINERY matches a pattern someone handed it. They never touch the load path, so they say nothing +about whether a real token set arrives compiled and able to match. + +THE FAILURE MODE THAT MAKES THAT A DEFECT RATHER THAN A GAP. With no prefix loaded, ``SITE_CODE_RE`` +and ``_SITE_CODE_FILE`` both fall back to ``_NEVER`` (``scan_forbidden.py:194``), an empty negative +lookahead that matches NOTHING ANYWHERE. A blind scanner and a scanner with nothing to report are +the same green tick. So the suite that looks like per-class coverage cannot fail when the real set is +wrong -- which is the shape of the original defect, reproduced inside its own tests. + +THE TWO ARMS, and the split is about what each is allowed to touch: + +* THE BEHAVIOURAL ARM pins the source to the COMMITTED SYNTHETIC EXAMPLE and drives the REAL + pipeline -- ``MEFOR_FORBIDDEN_TOKENS`` -> ``_resolve_token_text`` -> ``_parse_tokens`` -> + compilation -> ``scan_file``. Nothing is monkeypatched onto the globals. Probes are DERIVED from + what actually loaded, so a set that loads blind cannot reach the assertion: the derivation itself + fails first. +* THE REAL-SET ARM runs only where a real (non-synthetic) source is configured, and asserts + STRUCTURE ONLY -- present, not ``_NEVER``, counted. It never reads, builds with, or reports a real + token, because this repository's forbidden-content guard exists to keep exactly those values out of + files like this one (CLAUDE.md sec. 9). A test that embedded one to prove the scanner catches it + would be the leak it is testing for. + +TWO THINGS THIS FILE DELIBERATELY DOES NOT ASSERT: + +* NOT REASON TEXT. The scanner substitutes a generic reason when a reason string would itself match a + detector, so an assertion on reason wording reads the substituted value rather than the finding. + These tests assert that a hit OCCURRED, on a probe line constructed so nothing else could have + produced it. +* NOT A BARE DETECTOR COUNT. A token added under ``[estate_body_only]`` raises the ``estate`` count + identically to one under ``[estate]`` while never entering ``scan_file``. Counting alone therefore + cannot tell a token that is scanned from one that is merely listed, so the estate arm asserts the + ``scan_file`` path itself. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts" / "security")) + +import scan_forbidden as sfm # noqa: E402 + +pytestmark = pytest.mark.tooling + +EXAMPLE = Path(sfm.__file__).parent / "scan-tokens.local.txt.example" + + +def _load(monkeypatch: pytest.MonkeyPatch, source: str | None) -> Any: + """Drive the REAL load path and hand back the module. + + Deliberately NOT a monkeypatch of the globals: the point of this file is that everything from + ``_resolve_token_text`` through pattern compilation actually runs. + """ + if source is None: + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", "") + else: + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", source) + sfm.reload_tokens() + return sfm + + +@pytest.fixture +def example(monkeypatch: pytest.MonkeyPatch) -> Any: + """The committed synthetic example, loaded through the real pipeline. + + Restored afterwards by reloading from the ambient environment, so a real local token file is not + left displaced for the rest of the session. + """ + mod = _load(monkeypatch, str(EXAMPLE)) + yield mod + monkeypatch.undo() + sfm.reload_tokens() + + +def _word_probes_for_names(text: str) -> list[str]: + """Words a ``\\b``-anchored ``[names]`` entry is expected to match, RECOVERED FROM THE SOURCE. + + A ``[names]`` entry is ``regex | reason | flags``, so the loaded table holds compiled patterns and + the literal is gone -- and a regex cannot be inverted into a matching string in general. Rather + than type a probe here (which would drift silently from the file it is meant to exercise), this + recovers the plain-word entries, which are the shape the class is overwhelmingly made of, and + ignores the rest. Recovering NOTHING is treated as a failure by the caller, so a source whose + shape changed cannot quietly turn this into a no-op. + """ + import re as _re + + probes: list[str] = [] + in_names = False + for raw in text.splitlines(): + line = raw.strip() + if line.startswith("["): + in_names = line.lower().startswith("[names]") + continue + if not in_names or not line or line.startswith("#"): + continue + pattern = line.split("|")[0].strip() + if m := _re.fullmatch(r"\\b([A-Za-z][A-Za-z0-9]*)\\b", pattern): + probes.append(m.group(1)) + return probes + + +# --- the anti-vacuity guards: these run FIRST because every assertion below is worthless without --- + + +def test_the_example_load_produces_non_blind_detectors(example: Any) -> None: + """THE GUARD THE REST OF THIS FILE STANDS ON. + + ``_NEVER`` matches nothing anywhere, so a scanner that loaded nothing reports a clean tree and a + scanner with nothing to find reports a clean tree. Asserting the detectors are not the sentinel is + what makes every "no hit" result below mean something. + """ + assert example.TOKENS_PRESENT, "the example did not load as a usable token source" + assert example.SITE_CODE_RE is not example._NEVER + assert example._SITE_CODE_FILE is not example._NEVER + + counts = example.loaded_token_counts() + for section in ("names", "estate", "site_prefixes"): + assert counts[section] > 0, f"class {section!r} loaded ZERO detectors from the example" + # estate_file_scanned is the subset that scan_file can actually reach. Zero here with a non-zero + # estate count means every token is body-only, which no file-scan assertion could detect. + assert counts["estate_file_scanned"] > 0 + + +def test_a_source_that_loads_nothing_is_reported_blind_rather_than_clean( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """THE NEGATIVE CONTROL FOR THE GUARD ABOVE -- it must be able to observe the blind state. + + Without this, the guard could be asserting a condition that is simply always true, and would pass + just as happily if ``_NEVER`` were unreachable. This pins that the blind state EXISTS and is + distinguishable, which is the whole premise of the file. + """ + mod = _load(monkeypatch, None) + try: + assert not mod.TOKENS_PRESENT + assert mod.SITE_CODE_RE is mod._NEVER + assert mod._SITE_CODE_FILE is mod._NEVER + counts = mod.loaded_token_counts() + assert counts["names"] == 0 and counts["estate"] == 0 and counts["site_prefixes"] == 0 + finally: + monkeypatch.undo() + sfm.reload_tokens() + + +# --- per class, over what actually loaded --------------------------------------------------------- + + +def test_the_loaded_names_class_matches_a_token_it_loaded(example: Any, tmp_path: Path) -> None: + """Class [names], driven end-to-end rather than through a handed-in pattern. + + The probe is recovered FROM THE SOURCE that loaded rather than typed here, so it cannot drift away + from the file it is meant to exercise, and a source that failed to load reaches no assertion at + all: there is nothing to recover a probe from. + """ + probes = _word_probes_for_names(EXAMPLE.read_text(encoding="utf-8")) + assert probes, "recovered no plain-word [names] entry to probe with -- the source shape changed" + + # THE PROBE MUST BE ATTRIBUTABLE TO THIS CLASS ALONE, and the first candidate is not: the sets + # OVERLAP BY DESIGN (a customer name is typically in [names] AND [estate]), so a word drawn from + # [names] is often an estate token too, and the estate detector then produces the hit. Measured: + # with `FORBIDDEN` forced empty this test still PASSED on the first candidate -- green against the + # very class it names. Discard any candidate another detector can explain. + def _line(word: str) -> str: + return f"contact {word} about the interface" + + estate_pats = [pat for _token, pat in example._ESTATE_FILE_RES] + usable = [ + w + for w in probes + if not any(p.search(_line(w)) for p in estate_pats) + and not example._SITE_CODE_FILE.search(_line(w)) + ] + assert usable, ( + "every recovered [names] probe is also matched by another class, so no hit here could be " + "attributed to [names] -- this arm cannot be made to mean anything against this source" + ) + + probe = tmp_path / "note.md" + probe.write_text(_line(usable[0]) + "\n", encoding="utf-8") + hits = example.scan_file(probe) + + assert hits, "a loaded [names] token in a file body produced no hit" + + +def test_the_loaded_estate_class_is_caught_on_the_FILE_SCAN_path( + example: Any, tmp_path: Path +) -> None: + """Class [estate], asserted through ``scan_file`` -- NOT through a count and NOT through scan_text. + + THE DISTINCTION IS THE POINT. ``[estate_body_only]`` tokens are excluded from ``_ESTATE_FILE_RES`` + and therefore never enter ``scan_file`` at all, while still raising the ``estate`` detector count + exactly as a scanned token does. So a test that watched the count could not tell a token the file + scanner can see from one it cannot, and the leak this class exists for is a token sitting in a + tracked file. + + The probe butts the token against identifier characters on an otherwise-unremarkable line: estate + patterns run LAST in ``scan_file`` and only on a line no other detector flagged, so this shape is + both the case only estate can reach and the one that keeps the hit attributable. + """ + scanned = [token for token, _pat in example._ESTATE_FILE_RES] + assert scanned, "no estate token is file-scanned, so this path cannot be exercised" + + probe = tmp_path / "config.txt" + probe.write_text(f"OB_{scanned[0]}_ORU\n", encoding="utf-8") + hits = example.scan_file(probe) + + assert hits, "a file-scanned estate token butted against identifier characters produced no hit" + + +def test_the_loaded_site_prefix_class_matches_a_prefix_it_loaded( + example: Any, tmp_path: Path +) -> None: + """Class [site_prefix], built from the prefix that actually loaded plus a four-digit run. + + ``SITE_CODE_RE`` is the detector that falls back to ``_NEVER``, so this is the class where a + silent load failure is indistinguishable from a clean tree. + """ + assert example._SITE_PREFIXES, "no site prefix loaded, so this class cannot be exercised" + code = f"{example._SITE_PREFIXES[0]}0000" + + probe = tmp_path / "note.md" + probe.write_text(f"the record was filed under {code} last week\n", encoding="utf-8") + hits = example.scan_file(probe) + + assert hits, "a site code built from a loaded prefix produced no hit" + + +def test_a_digit_run_with_no_loaded_prefix_is_not_flagged(example: Any, tmp_path: Path) -> None: + """The per-class negative control: the site detector must not match any six-digit run. + + Without this the class arm above is satisfied by a detector that flags everything, which is the + other way an instrument stops discriminating. + """ + probe = tmp_path / "note.md" + probe.write_text("order 4815162342 shipped\n", encoding="utf-8") + + assert example.scan_file(probe) == [] + + +# --- the real set, when one is configured --------------------------------------------------------- + + +def test_a_configured_REAL_token_set_is_loaded_and_not_blind() -> None: + """The arm that covers the environment the gate actually protects. + + STRUCTURE ONLY, AND THAT IS NOT A SHORTCUT. Asserting a real token matches would require putting + one in this file, which is precisely the disclosure the scanner exists to prevent (CLAUDE.md + sec. 9) -- the test would become the leak. What is checkable without handling a value is that the + set LOADED, that no class fell back to the sentinel, and that every class is populated. That is + the blind-set failure, which is the one this item is about. + + Skips where no real set is configured, and says which state it saw: a silent skip here would be + indistinguishable from a pass, and this is the arm most likely to be silently absent in CI. + """ + sfm.reload_tokens() + # A SOURCE THAT EXISTS BUT PARSED TO NOTHING IS A FAILURE, NOT A SKIP -- and separating the two is + # the point. Both states leave TOKENS_PRESENT false, so skipping on that alone would turn the + # documented mangling case (headers lost, comments only, a BOM ahead of the first section -- the + # cutover runbook has the owner paste a whole file into a secret box) into a green tick, which is + # the vacuous pass this item exists to remove. Only the ABSENCE of any source is a legitimate skip. + configured = sfm._resolve_token_text() is not None + if not configured: + pytest.skip("no token source configured in this environment") + assert sfm.TOKENS_PRESENT, ( + "a token source IS configured but parsed to zero detectors -- the source is present and " + "unusable, which reports identically to having none" + ) + if sfm.is_synthetic_token_set(): + pytest.skip("token source is the shipped synthetic example, not a real set") + + counts = sfm.loaded_token_counts() + assert sfm.SITE_CODE_RE is not sfm._NEVER + assert sfm._SITE_CODE_FILE is not sfm._NEVER + for section in ("names", "estate", "site_prefixes"): + assert counts[section] > 0, f"real token set loaded ZERO detectors for class {section!r}" + assert counts["estate_file_scanned"] > 0, ( + "every real estate token is body-only, so scan_file covers none of them" + ) From 27631c88380860a8dae19068c191d402f04bf0b4 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 21 Aug 2026 05:17:44 -0500 Subject: [PATCH 08/11] fix(test): a pid that is FREE is not a pid that stays free (BACKLOG #1303) Three test files each spawned `cmd /c exit`, waited for it to EXIT, slept, and returned its pid as "free". The pid is free at the moment it returns and NOTHING KEEPS IT FREE: between that return and the moment the tool under test reads the record, the OS may hand it to a new process. NOT A FLAKE RULING, AND THE DISTINCTION IS DELIBERATE. Intermittency was never the evidence. The race is visible in the CONSTRUCTION -- a pid is acquired, released, and then relied upon across a gap nothing holds. I refused to call the ubuntu SIGSEGV timing-dependent from intermittency alone earlier tonight; this one meets the stricter standard, which is why it gets the label and that one does not. THE COMMENT IS THE BEST EVIDENCE IN THE ITEM. One copy read: time.sleep(0.3) # let the OS reap it before we claim the pid is gone That states the intent exactly and the mechanism does the opposite. Reaping does not RESERVE a pid, it RELEASES it for reuse -- so the sleep WIDENS the window it appears to guard. The hazard was reasoned about and the direction inverted. THE PATH TO THE OBSERVED FAILURE, traced through the real fence rather than guessed. Test-Record- Liveness (scripts/coord/session-registry.ps1:181) reports DEAD when `Get-Process -Id` finds nothing, and DEAD vetoes nothing. A REUSED pid IS running, and a test record carries no `startedAt` for the reuse fence to consult, so the verdict becomes UNVERIFIED -- which DOES veto (occupancy.ps1:75). The occupant list comes back non-empty and `assert d["Occupants"] == []` fires. Seen on windows-2025 in run 32268545492, beside (not caused by) an unrelated crash in the same run; `cmd /c exit` is Windows-only, which matches where it appeared. THE FIX: one `tests/_dead_pid.py` returning 2147483647. Int32.MaxValue -- inside the `[int]` cast the fence performs, NON-ZERO so it takes the liveness path rather than the UNREADABLE shortcut a falsy pid triggers, and structurally unassignable (Linux caps pids at pid_max, ceiling ~2^22; Windows pids are multiples of 4 far below 2^31). Dead BY CONSTRUCTION rather than by timing. THE CONSTRAINT THAT SHAPED IT, AND IT IS PROVEN RATHER THAN ASSERTED. The test asserts BOTH `Occupants == []` AND `Decision == "SKIP"` -- a dead record is neither a veto nor a permission. Any remedy that stubbed, mocked or forced the liveness verdict would make it pass while exercising NOTHING, converting a loud false-failure into a quiet always-pass. The real `Get-Process` call still runs and still returns "not running" on its own. VERIFIED: adding DEAD to occupancy.ps1's veto states turns the test RED, and restoring it turns it green -- so the veto path is still covered. That is the direction this fix moves, stated because a fix here could so easily have moved the other way. DUPLICATED SHAPE, NOT ONE SITE: three definitions, four call sites, across test_worktree_prune_merged, test_coord_presence and test_session_registry -- now one shared helper. An earlier count of mine said six call sites; that used `grep -c '_find_free_pid()'`, which also matches the DEFINITION line and inflated every file by one. A peer had the right number and deferred to mine as "a different question"; it was not, it was wrong. VERIFIED: ruff check + format clean; 98 tests across the three suites (79 + 19); red-first on the veto path as above. Ledger entry allocated from this worktree and landed in the same commit. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 19 +++++++++ tests/_dead_pid.py | 66 +++++++++++++++++++++++++++++ tests/test_coord_presence.py | 14 ++---- tests/test_session_registry.py | 15 ++----- tests/test_worktree_prune_merged.py | 13 ++---- 5 files changed, 95 insertions(+), 32 deletions(-) create mode 100644 tests/_dead_pid.py diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index de08b41f..ce2da524 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -11364,3 +11364,22 @@ _FHIR_ID_RE.fullmatch("abc\n") -> False the fix **Cluster:** Session coordination / silent-loss transport. **Priority:** P2. **Verdict:** build. **Severity:** no product effect, no PHI effect, no deployment axis (sec. 0) -- developer coordination only. The cost is that a seat's entire output can vanish while every instrument on the path reports success, and the recipient then reasons from a silence it has no way to distinguish from an idle lane. + +## 1303. three test files pick a free pid and rely on it staying free, so a loaded runner reuses it and a DEAD record reads as a veto + +> ✅ **SHIPPED 2026-08-21 -- three copies of a `_find_free_pid` helper replaced by one `tests/_dead_pid.py` returning a pid that CANNOT be assigned, so deadness holds by construction instead of by timing. The fix was constrained to keep the test able to FAIL, and that was proven rather than asserted.** Diagnosed from run `32268545492`'s sibling failure on `windows-2025`: `tests/test_worktree_prune_merged.py:753`, `assert d["Occupants"] == []`, an ordinary exit-1 assertion sitting beside an unrelated crash in the same run. + +> **THE RACE IS VISIBLE IN THE CONSTRUCTION, WHICH IS WHY THIS IS NOT A FLAKE RULING.** The helper spawned `cmd /c exit`, waited for it to EXIT, slept, and returned its pid as "free". The pid is free at the moment it returns and **nothing keeps it free** -- between that return and the moment the tool reads the record, the OS may hand it to a new process. Intermittency was never the evidence; the acquire-release-then-rely sequence is. + +> **THE COMMENT IS THE BEST EVIDENCE IN THE ITEM.** One copy carried `time.sleep(0.3) # let the OS reap it before we claim the pid is gone`. That states the intent exactly and the mechanism does the opposite: **reaping does not RESERVE a pid, it RELEASES it for reuse**, so the sleep WIDENS the window it appears to guard. The hazard was reasoned about and the direction inverted. + +> **THE PATH TO THE FAILURE, traced through the real fence.** `Test-RecordLiveness` ([`scripts/coord/session-registry.ps1:181`](../scripts/coord/session-registry.ps1)) reports **DEAD** when `Get-Process -Id` finds nothing -- and DEAD vetoes nothing. A REUSED pid **is** running, and a test record carries no `startedAt` for the reuse fence to consult, so the verdict becomes **UNVERIFIED** -- which **does** veto (`occupancy.ps1:75`). The occupant list then comes back non-empty and an assertion that a dead record is "not a veto" fires. `cmd /c exit` is Windows-only, matching where it was seen; pid reuse needs pid churn, and that tier spawns pwsh/git children constantly on a runner whose pid space recycles far faster than a developer box. + +> **WHY `2147483647`.** `Int32.MaxValue`: inside the `[int]` cast the fence performs, **non-zero** so it takes the liveness path rather than the `UNREADABLE` shortcut a falsy pid triggers, and **structurally unassignable** -- Linux caps pids at `pid_max` (ceiling ~2^22) and Windows pids are multiples of 4 far below 2^31. + +> **THE CONSTRAINT THAT SHAPED THE FIX, and it is the reason to prefer this over the obvious remedy.** The test asserts BOTH `Occupants == []` AND `Decision == "SKIP"` -- a dead record is *neither* a veto *nor* a permission. **Any remedy that stubbed, mocked or forced the liveness verdict would make the test pass while exercising nothing**, converting a loud false-failure into a quiet always-pass. The real `Get-Process` call still runs and still returns "not running" on its own. **PROVEN, not asserted:** adding `DEAD` to `occupancy.ps1`'s veto states turns the test RED, and removing it turns it green again -- so the veto path is still covered. + +> **SIZE, corrected against an earlier miscount.** THREE definitions and FOUR call sites, across `test_worktree_prune_merged.py`, `test_coord_presence.py` and `test_session_registry.py`. An initial count said six call sites; that used `grep -c '_find_free_pid()'`, which also matches the DEFINITION line (`def _find_free_pid() -> int:`) and so inflated every file by one. + +**Cluster:** CI reliability / test determinism. **Priority:** P2. **Verdict:** build. +**Severity:** no product effect, no PHI effect, no deployment axis (sec. 0) -- test-suite determinism only. The cost is a required context redding on a race whose failure looks like a real occupancy veto, and a duplicated shape that would have been fixed one site at a time. diff --git a/tests/_dead_pid.py b/tests/_dead_pid.py new file mode 100644 index 00000000..c77b1294 --- /dev/null +++ b/tests/_dead_pid.py @@ -0,0 +1,66 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""A pid whose DEADNESS HOLDS -- BACKLOG #1303. + +THE DEFECT THIS REPLACES. Three test files each carried their own copy of:: + + proc = subprocess.Popen(["cmd", "/c", "exit"], ...) + proc.wait(timeout=30) + time.sleep(0.3) # let the OS reap it before we claim the pid is gone + return proc.pid + +It spawns a process, waits for it to EXIT, and returns its pid as "free". The pid is free at the +moment it returns and **nothing keeps it free**: between that return and the moment the tool under +test reads the record, the OS may hand the pid to a new process. + +**THE COMMENT STATES THE INTENT AND THE MECHANISM DOES THE OPPOSITE.** Reaping does not RESERVE a pid, +it RELEASES it for reuse -- so the sleep widens the window it appears to guard. The hazard was +reasoned about and the direction was inverted, which is why this is a defect rather than an oversight. + +HOW IT SURFACED. `Test-RecordLiveness` (`scripts/coord/session-registry.ps1:181`) reads +`Get-Process -Id `: not running is **DEAD**, which vetoes nothing. But a REUSED pid IS running, +and a test record carries no ``startedAt`` for the reuse fence to check, so the verdict becomes +**UNVERIFIED** -- and UNVERIFIED *does* veto. The occupant list then comes back non-empty and an +assertion that a dead record is "not a veto" fails. Observed on `windows-2025`, run `32268545492`:: + + assert d["Occupants"] == [] + AssertionError: assert [{'Short': 'e...-clean', ...}] == [] + +`cmd /c exit` is Windows-only, which matches where it was seen. Pid reuse needs pid churn, and that +tier spawns pwsh/git children constantly, on a runner whose pid space recycles far faster than a +developer box -- which is why it reproduces there and not locally. + +WHY THIS VALUE. ``2147483647`` is ``Int32.MaxValue``. It is: + +* **within ``[int]``**, which `Test-RecordLiveness` casts to (``$procId = [int]$Record.pid``); +* **non-zero**, so it takes the liveness path rather than the ``UNREADABLE`` shortcut that a falsy + pid triggers -- a record with no pid is deliberately NOT dead there; +* **structurally unassignable**: Linux caps pids at ``/proc/sys/kernel/pid_max`` (default 4194304, + ceiling ~2^22) and Windows pids are multiples of 4 far below 2^31. + +So it is dead **by construction rather than by timing**, and it stays dead however loaded the host is. + +WHAT THIS DELIBERATELY DOES *NOT* DO, and it is the point. It does not stub, mock or force the +liveness verdict. The real `Get-Process` call still runs and still returns "not running" on its own. +A remedy that short-circuited the verdict would make every caller pass while testing NOTHING -- and +the assertions this feeds exist to prove that a dead record is *neither* a veto *nor* a permission +(``Occupants == []`` AND ``Decision == "SKIP"``). Converting a loud false-failure into a quiet +always-pass would be worse than the flake it replaces. +""" + +from __future__ import annotations + +from typing import Final + +#: See the module docstring for why this specific value, and why it is not merely "a big number". +NEVER_LIVE_PID: Final = 2147483647 + + +def never_live_pid() -> int: + """A pid no process can hold, so a record written with it reads DEAD at any later moment. + + Callable rather than a bare constant so call sites read as an intent ("give me a pid that cannot + be alive") rather than as a magic literal, and so a future platform that needs a different value + has one place to change. + """ + return NEVER_LIVE_PID diff --git a/tests/test_coord_presence.py b/tests/test_coord_presence.py index 83a1f08f..c3c87e74 100644 --- a/tests/test_coord_presence.py +++ b/tests/test_coord_presence.py @@ -29,6 +29,8 @@ import pytest +from tests._dead_pid import never_live_pid + PRESENCE = Path(__file__).resolve().parents[1] / "scripts" / "coord" / "presence.ps1" pytestmark = pytest.mark.skipif( @@ -159,7 +161,7 @@ def test_pid_reuse_is_not_reported_live(repo: Path, config_root: Path) -> None: def test_dead_pid_is_excluded_by_default_and_shown_with_all(repo: Path, config_root: Path) -> None: - dead = _find_free_pid() + dead = never_live_pid() write_session(config_root, pid=dead, cwd=repo, session_id="dddddddd-4444") assert run_presence(repo, config_root) == [] @@ -273,16 +275,6 @@ def test_the_human_table_names_its_columns_so_the_id_cannot_read_as_a_sha( ) -def _find_free_pid() -> int: - """A pid that is not currently running -- start a process, note its pid, wait for it to exit.""" - proc = subprocess.Popen( - ["cmd", "/c", "exit"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL - ) - proc.wait(timeout=30) - time.sleep(0.3) # let the OS reap it before we claim the pid is gone - return proc.pid - - def test_outside_a_repo_the_json_roster_carries_an_unavailable_receipt(tmp_path: Path) -> None: """ "I could not look" must not render as "nobody is live" on the machine-readable channel. diff --git a/tests/test_session_registry.py b/tests/test_session_registry.py index e21587e9..f962c3f6 100644 --- a/tests/test_session_registry.py +++ b/tests/test_session_registry.py @@ -30,6 +30,8 @@ import pytest +from tests._dead_pid import never_live_pid + REGISTRY = Path(__file__).resolve().parents[1] / "scripts" / "coord" / "session-registry.ps1" pytestmark = pytest.mark.skipif( @@ -108,7 +110,7 @@ def test_recycled_pid_is_not_live(config_root: Path) -> None: def test_dead_pid_is_dead(config_root: Path) -> None: - write_session(config_root, pid=_find_free_pid(), session_id="cccccccc-3333") + write_session(config_root, pid=never_live_pid(), session_id="cccccccc-3333") assert liveness(config_root, "cccccccc")["State"] == "DEAD" @@ -154,15 +156,6 @@ def test_malformed_record_does_not_break_the_lookup(config_root: Path) -> None: def test_prefix_match_reports_the_most_alive_candidate(config_root: Path) -> None: """Deciding whether it is safe to disturb something: an ambiguous prefix must not resolve to the dead one and green-light the move.""" - write_session(config_root, pid=_find_free_pid(), session_id="7777abcd-8888") + write_session(config_root, pid=never_live_pid(), session_id="7777abcd-8888") write_session(config_root, pid=os.getpid(), session_id="7777efgh-9999") assert liveness(config_root, "7777")["State"] == "LIVE" - - -def _find_free_pid() -> int: - proc = subprocess.Popen( - ["cmd", "/c", "exit"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL - ) - proc.wait(timeout=30) - time.sleep(0.3) - return proc.pid diff --git a/tests/test_worktree_prune_merged.py b/tests/test_worktree_prune_merged.py index 26110968..094ecf5d 100644 --- a/tests/test_worktree_prune_merged.py +++ b/tests/test_worktree_prune_merged.py @@ -48,6 +48,8 @@ import pytest +from tests._dead_pid import never_live_pid + SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "worktree" / "prune-merged.ps1" pytestmark = pytest.mark.skipif( @@ -744,7 +746,7 @@ def test_a_record_with_no_cwd_is_unplaceable_and_refuses_too(fx: Fixture, sleepe def test_dead_record_is_not_a_veto_and_not_a_permission(fx: Fixture, sleeper: int) -> None: """Liveness may only VETO. A DEAD verdict must not authorise the removal by itself.""" - dead = _find_free_pid() + dead = never_live_pid() fx.write_session(pid=dead, cwd=fx.sibling("clean"), session_id="eeeeeeee-5555") live_record(fx, sleeper, fx.primary) # keeps the fence available @@ -1618,15 +1620,6 @@ def test_a_name_that_matches_nothing_does_not_exit_green(fx: Fixture, sleeper: i ) -def _find_free_pid() -> int: - proc = subprocess.Popen( - ["cmd", "/c", "exit"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL - ) - proc.wait(timeout=30) - time.sleep(0.3) - return proc.pid - - # -------------------------------------------------------------------------------------------------- # Coordination claims stranded by a removal (BACKLOG #345) # From ba010f4387645592bb8e06ed953f18b68ecab440 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 21 Aug 2026 05:36:30 -0500 Subject: [PATCH 09/11] fix(coord): refuse a wrong-namespace -ToSessionId at SEND, not at the inbox (BACKLOG #1302) An id-addressed message could strand silently. `mail-drain.ps1:852` compares the recorded `to.sessionId` against the reading session's harness id with `-ne`; an id from another namespace never matches, so the message sat in the inbox until it was swept to expired/ -- with the send path printing `Queued 1 message(s)` the whole time. THE ASYMMETRY WAS THE DEFECT, AND IT IS WHY THE GUARD IS ON THE SEND SIDE. The drain already reported its half ("N message(s) are addressed to a different session id and were left in the inbox"), so the RECIPIENT was told. The SENDER was told nothing -- and the sender is the only party who can correct the id. I did not touch the drain's filter: it is CORRECT. A worktree outlives its occupant, so an id-addressed note must not reach a stranger, and loosening the match to "fix" delivery would trade a silent non-delivery for a silent MIS-delivery, which is worse. THE SHAPE. A harness session id is a bare UUID (the drain reads `$hook.session_id`). The MCP namespace prefixes its own as `local_`. Two id spaces for one session, compared literally. MEASURED, AND I WAS THE ONE WHO CAUSED IT. Six of my own messages to the dispatcher stranded for a whole session -- a level report, a CI mechanism diagnosis, two unprompted self-retractions and a request to pull two never-started items. The recipient read my lane as silent and wrote "level unreported" three times. They were found only by opening the box by hand, and were due to expire with neither end told. PARTIAL CONTROL, AND THE ITEM SAYS SO RATHER THAN LEAVING IT TO BE DISCOVERED. This catches a wrong-NAMESPACE id. It does NOT catch a correctly-shaped but STALE one -- an id belonging to a session that has ended fails identically and just as silently. A pass at send is not a promise of delivery. THE MUST-NOT-TRIP ARM IS IN THE SAME TEST AS THE REFUSAL, deliberately: a guard that rejected everything would otherwise pass by satisfying one half. No `-ToSessionId` at all is the ordinary broadcast and still sends; a genuine bare-UUID id still sends. RED-FIRST IN BOTH DIRECTIONS, each broken then restored -- disabling the guard reddens the refusal test, widening it to reject everything reddens the must-not-trip test. The refusal names the REMEDY, not just the rejection: the sender's next move is to drop the flag and address by worktree path, and a message that only said "invalid" would leave them hunting an id. VERIFIED: ruff clean; 103 tests across the two mail suites; red-first both ways as above. The #1302 banner was flipped by lifting the closed-alphabet character from an already-closed item and asserting its membership before use -- never typed (sec. 11). Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 13 +++++++++- scripts/coord/mail.ps1 | 34 +++++++++++++++++++++++++ tests/test_session_mail.py | 51 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index ce2da524..07d7e0f4 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -11347,7 +11347,18 @@ _FHIR_ID_RE.fullmatch("abc\n") -> False the fix ## 1302. mail.ps1 accepts an MCP-namespace session id in -ToSessionId and the message becomes silently undeliverable, expiring with neither sender nor recipient told -> 🔢 **Re-scored 2026-08-20 -> P2.** Value **5/10** · Difficulty **2/10** · _fill-in_. The validation gap stands -- mail.ps1:106 takes -ToSessionId unvalidated, :240 stores it, and mail-drain.ps1:852 string-compares it against the harness id read at :490, so a wrong-namespace id is filtered on every pass -- but the item's reporting claim does not: mail-drain.ps1:825-836 writes an 'expired-unshown' receipt on the sweep, mail.ps1:484-487 already tells the sender to read exactly that file and what the disposition means, and mail-drain.ps1:1014 reports the filtered count to the recipient on every drain. Value 5 rather than the filed 7 or the scorer's 6 because the item's value argument was built on those instruments being absent and all three ship, leaving a detection DELAY and an ambiguous no-receipt reading rather than a silent loss, on developer coordination with no product or PHI axis. Difficulty 2 and arguably generous: mail.ps1:153 already dot-sources mail-claim.ps1, whose Test-SessionId at :209 is the exact UUID shape check the fix needs, so the remainder is a post-binding refusal plus a must-not-trip arm for the no-sessionId broadcast case. _(was 7/10 · 2/10.)_ +> ✅ **SHIPPED 2026-08-21 -- `mail.ps1` now REFUSES a `-ToSessionId` that is not a harness session id, at SEND, before any message is written. The sender is the only party who can correct the id, and was the only party never told.** The drain already reported its half (*"N message(s) are addressed to a different session id and were left in the inbox"*); the send path printed `Queued 1 message(s)` and nothing else. **That asymmetry was the defect, and it is why the guard sits on the send side rather than in the drain -- the drain's filter is CORRECT: a worktree outlives its occupant, so an id-addressed note must not reach a stranger.** + +> **THE SHAPE.** A harness session id is a bare UUID (the drain reads `$hook.session_id`); the MCP namespace prefixes its own as `local_`. Two id spaces for one session, compared with `-ne` at [`mail-drain.ps1:852`](../scripts/hooks/mail-drain.ps1), so a wrong-namespace id never matches and the message waits in the inbox until it is swept to `expired/` -- silently in both directions. + +> **MEASURED, NOT HYPOTHETICAL.** Six messages from one seat -- a level report, a CI mechanism diagnosis, two unprompted self-retractions and a request to pull two never-started items -- stranded for a whole session while the recipient read that lane as silent and wrote "level unreported" three times. They were found only by opening the box by hand. + +> **PARTIAL CONTROL, RECORDED RATHER THAN DISCOVERED LATER.** This catches a wrong-NAMESPACE id. It does **not** catch a correctly-shaped but **STALE** one -- an id belonging to a session that has ended fails identically and just as silently. A pass at send is not a promise of delivery, and nothing here should be read as one. + +> **THE MUST-NOT-TRIP ARM IS ASSERTED IN THE SAME TEST AS THE REFUSAL**, so a guard that rejected everything could not pass by satisfying one half: a message with NO `-ToSessionId` is the ordinary broadcast and still sends, and a genuine bare-UUID id still sends. **RED-FIRST IN BOTH DIRECTIONS:** disabling the guard reddens the refusal test, and widening it to reject everything reddens the must-not-trip test. + +> **Re-scored 2026-08-20 -> P2.** Value **5/10** · Difficulty **2/10** · _fill-in_. The validation gap stands -- mail.ps1:106 takes -ToSessionId unvalidated, :240 stores it, and mail-drain.ps1:852 string-compares it against the harness id read at :490, so a wrong-namespace id is filtered on every pass -- but the item's reporting claim does not: mail-drain.ps1:825-836 writes an 'expired-unshown' receipt on the sweep, mail.ps1:484-487 already tells the sender to read exactly that file and what the disposition means, and mail-drain.ps1:1014 reports the filtered count to the recipient on every drain. Value 5 rather than the filed 7 or the scorer's 6 because the item's value argument was built on those instruments being absent and all three ship, leaving a detection DELAY and an ambiguous no-receipt reading rather than a silent loss, on developer coordination with no product or PHI axis. Difficulty 2 and arguably generous: mail.ps1:153 already dot-sources mail-claim.ps1, whose Test-SessionId at :209 is the exact UUID shape check the fix needs, so the remainder is a post-binding refusal plus a must-not-trip arm for the no-sessionId broadcast case. _(was 7/10 · 2/10.)_ + > > **Filed 2026-08-21 -- not started. The one place in this transport where a message is genuinely LOST rather than late, and both ends read it as delivered.** `scripts/coord/mail.ps1:106` declares `-ToSessionId` as a bare `[string]` with **no validation**, and `:240` writes it into the message as `sessionId`. The drain then compares that value against the **harness** session id -- `scripts/hooks/mail-drain.ps1:852`, against `$sessionId` sourced at `:490` from `$hook.session_id`. **Three id namespaces exist for one session -- registry, MCP and harness -- and only one of them can ever match.** > **THE MECHANISM, verified in code rather than inferred from the symptom.** An MCP id is `local_`-prefixed; a harness id is a bare UUID. The comparison at `:852` is a string inequality, so a `local_` id can never equal the value it is tested against, on any drain, ever. The message is skipped every pass, stays in `inbox/`, and expires. **Nothing errors at send time, nothing errors at drain time, and nothing reports the expiry to either party.** `docs/WORKTREES.md` already records that a registry id and an MCP id for one session **shared no characters** -- so the namespaces are known to be disjoint, and nothing acts on that knowledge at the point where it matters. diff --git a/scripts/coord/mail.ps1 b/scripts/coord/mail.ps1 index 96010c4b..412028f5 100644 --- a/scripts/coord/mail.ps1 +++ b/scripts/coord/mail.ps1 @@ -440,6 +440,40 @@ if ($Send) { } } + # BACKLOG #1302 -- FAIL THE SENDER, WHO CAN FIX IT, RATHER THAN THE RECIPIENT, WHO CANNOT. + # + # A `-ToSessionId` from the wrong namespace is compared literally against the reading session's + # harness id (`mail-drain.ps1`: `[string]$m.to.sessionId -ne $sessionId`), never matches, and the + # message sits in the inbox until it is swept to expired/. MEASURED: six messages from one seat -- + # a level report, a CI mechanism diagnosis, two unprompted self-retractions and a request to pull + # two items -- stranded for a whole session while the recipient read that lane as silent. The send + # path printed `Queued 1 message(s)` for every one of them. + # + # THE ASYMMETRY IS THE DEFECT, and it is why this check goes HERE. The drain ALREADY reports its + # side ("N message(s) are addressed to a different session id and were left in the inbox"), so the + # recipient is told. The SENDER is told nothing, and the sender is the only party who can correct + # the id. + # + # THE SHAPE: a harness session id is a bare UUID (the drain reads `$hook.session_id`). The MCP + # namespace prefixes its own (`local_`), and that is exactly the shape that stranded them -- + # two id spaces for one session, compared with `-ne`. + # + # PARTIAL CONTROL, AND RECORDING THAT IS PART OF THE FIX. This catches a wrong-NAMESPACE id. It + # does NOT catch a correctly-shaped but STALE one -- an id belonging to a session that has since + # ended fails identically and just as silently. A pass here is not a promise of delivery. + # + # MUST NOT TRIP ON THE ORDINARY CASE: no `-ToSessionId` at all is the normal broadcast, and it has + # to keep delivering untouched. The guard is scoped to a value the caller actually supplied. + if ($ToSessionId -and $ToSessionId -notmatch '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$') { + throw ( + "-ToSessionId '$ToSessionId' is not a harness session id, so the drain would compare it " + + "against the reading session's id, never match, and leave the message in the inbox until " + + "it expired -- with neither end told. A harness session id is a bare UUID; an id carrying " + + "a namespace prefix such as 'local_' belongs to a different id space. Send with -To " + + " and omit -ToSessionId unless you have the harness id." + ) + } + # PER-TARGET, NOT ALL-OR-NOTHING. Now that a publish can genuinely fail -- the verify is real, so a # move that did not happen is reported instead of assumed -- a broadcast that aborted on target 1 # would hide targets 2..N, and one that swallowed the failure would put the defect back at the diff --git a/tests/test_session_mail.py b/tests/test_session_mail.py index 8cf7aa75..b80ec300 100644 --- a/tests/test_session_mail.py +++ b/tests/test_session_mail.py @@ -436,6 +436,57 @@ def _send(repo: Path, body: str) -> subprocess.CompletedProcess[str]: ) # fmt: skip +def _send_addressed(repo: Path, session_id: str | None) -> subprocess.CompletedProcess[str]: + """Send with an optional ``-ToSessionId``. ``None`` omits the flag entirely (the ordinary case).""" + args = [ + "pwsh", "-NoProfile", "-NonInteractive", "-File", str(MAIL), + "-Send", "-MailRoot", str(mail_root(repo)), "-To", str(repo), "-Body", "probe", + ] # fmt: skip + if session_id is not None: + args += ["-ToSessionId", session_id] + return subprocess.run( + args, cwd=str(repo), capture_output=True, text=True, timeout=TIMEOUT, check=False + ) + + +def test_a_wrong_namespace_session_id_is_refused_at_SEND(repo: Path) -> None: + """BACKLOG #1302 -- the sender is the only party who can fix the id, so the sender is told. + + An id from the wrong namespace is compared literally against the reading session's harness id + (`mail-drain.ps1`), never matches, and the message sits in the inbox until it is swept to + `expired/`. MEASURED: six messages from one seat stranded for a whole session while the send path + printed `Queued 1 message(s)` for every one of them, and the recipient read that lane as silent. + + The drain already reports ITS side. This asserts the half that was missing. + """ + bad = _send_addressed(repo, "local_2b3b416c-1d0c-4e81-987c-bb19e590045d") + + assert bad.returncode != 0, f"a wrong-namespace id must be refused at send: {bad.stdout}" + # The refusal has to name the REMEDY, not merely the rejection -- the sender's next move is to drop + # the flag, and a message that only says "invalid" leaves them guessing at which id to hunt for. + assert "-ToSessionId" in bad.stderr + assert "omit -ToSessionId" in bad.stderr + + +def test_the_ordinary_broadcast_and_a_real_harness_id_both_still_send(repo: Path) -> None: + """THE MUST-NOT-TRIP ARM, and the reason the guard is scoped to a supplied value. + + Two ways this fix could have been worse than the defect. Refusing a message with NO + ``-ToSessionId`` would break the ORDINARY broadcast, which is most traffic on this channel. And + refusing a genuine harness id would make the flag unusable for the case it exists to serve -- mail + that is only meaningful to one session, where a worktree outliving its occupant would otherwise + hand a note to a stranger. + + Asserted in the SAME test as a pair, so a guard that accidentally rejected everything cannot pass + by satisfying one half. + """ + broadcast = _send_addressed(repo, None) + assert broadcast.returncode == 0, f"no -ToSessionId is the ordinary case: {broadcast.stderr}" + + real = _send_addressed(repo, "177a513c-60f4-49af-8cd4-465ff4f9118d") + assert real.returncode == 0, f"a bare-UUID harness id must send: {real.stderr}" + + def test_the_send_line_arm_refuses_at_the_boundary_and_passes_one_below(repo: Path) -> None: """The adjacent pair, not a 300-char probe. From 31005c2f03d8c6ba0b5bf4bdb1b828eab07f324d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 21 Aug 2026 07:50:39 -0500 Subject: [PATCH 10/11] ci(security): raise the leak-gate detector floor to 8/14/2, in BOTH workflows (BACKLOG #321) The owner refreshed MEFOR_FORBIDDEN_TOKENS in both secret stores, so the floor can now assert the larger set. VERIFIED BY ME rather than taken from a relay -- names and dates only, no values, which is all `gh secret list` exposes: Actions MEFOR_FORBIDDEN_TOKENS 2026-08-21T12:45:15Z Dependabot MEFOR_FORBIDDEN_TOKENS 2026-08-21T12:45:23Z EIGHT SECONDS APART, so the half-done state this change was held for did not occur. If Actions had been updated and Dependabot had not, every Dependabot PR would hard-fail a required check. My stated constraint was "only after BOTH, and I will ask rather than infer" -- both are updated and the measurement is mine. RAISED IN TWO PLACES, NOT ONE. The release named `security.yml`. `branch-leak-scan.yml:88` carried the SAME literal and nobody named it. Raising only one would have left a second gate passing on the old floor -- a partial raise that reads as done. There are now zero occurrences of the old triple under .github/workflows/. PRE-FLIGHT BEFORE RAISING A FLOOR THAT HARD-FAILS A REQUIRED CHECK, counts only: names 8 estate 14 estate_file_scanned 13 site_prefixes 2 synthetic=False The real set satisfies 8/14/2 exactly, and `estate_file_scanned` at 13 matches the documented 12->13 move -- so the added token is FILE-SCANNED rather than body-only, which is the half that matters. NOT INERT, AND THAT IS CHECKED RATHER THAN ASSUMED. `token_floor_failure` passes at 8/14/2 and FAILS at 9/15/3 naming each short section ("names 8<9, estate 14<15, site_prefixes 2<3"). A floor that cannot fail is not a floor. EXPECT COLLATERAL HITS ON THE FIRST FULL SWEEP AND DO NOT READ THEM AS FINDINGS. The added site prefix is two digits, so it matches any delimited six-digit run in a 10000-wide band -- synthetic MRNs, sentinel ids, clamp ceilings. Triage noise, anticipated before the value was written. VERIFIED: 146 tests across the scanner + token-source + CI-pinning suites; 120 more across the workflow-lint and lockstep suites. No token value appears in this change, in any test, or in this message. Co-Authored-By: Claude Opus 5 --- .github/workflows/branch-leak-scan.yml | 2 +- .github/workflows/security.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/branch-leak-scan.yml b/.github/workflows/branch-leak-scan.yml index 68228981..ba94449b 100644 --- a/.github/workflows/branch-leak-scan.yml +++ b/.github/workflows/branch-leak-scan.yml @@ -85,7 +85,7 @@ jobs: run: | if [ -n "$MEFOR_FORBIDDEN_TOKENS" ]; then export MEFOR_REQUIRE_TOKENS=1 - export MEFOR_MIN_DETECTORS=names=7,estate=13,site_prefixes=1 + export MEFOR_MIN_DETECTORS=names=8,estate=14,site_prefixes=2 echo "token list loaded from the MEFOR_FORBIDDEN_TOKENS secret (fail-closed, per-section floor)." else # A push event cannot come from a fork, so unlike security.yml there is no legitimate diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 3dfab839..826450da 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -671,7 +671,7 @@ jobs: # the env var first, so that file would never be read -- it would only drop the full real # token list into the job workspace for every later step to see. export MEFOR_REQUIRE_TOKENS=1 - export MEFOR_MIN_DETECTORS=names=7,estate=13,site_prefixes=1 + export MEFOR_MIN_DETECTORS=names=8,estate=14,site_prefixes=2 echo "token list loaded from the MEFOR_FORBIDDEN_TOKENS secret (fail-closed, per-section floor)." elif [ "$IS_FORK_PR" = "true" ]; then echo "fork PR -- the secret is unavailable BY DESIGN; structural-only scan." From 029f27ee7c02971c2349035e555bbd3e44739d4a Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 21 Aug 2026 11:21:02 -0500 Subject: [PATCH 11/11] fix(release): hash-pin the signing toolchain through uv.lock, not an inline pin (BACKLOG #332) `release.yml:255` ran `pip install "sigstore==4.4.0"` inside the job holding `id-token: write`. The very next commands sign the wheel, write the SLSA attestation and publish to PyPI, so anything that executes there runs with the credential that signs the artifacts -- a backdoored wheel produced at that point carries a VALID Sigstore bundle and VALID provenance, and every downstream verifier reports success. The compromise defeats the controls it is standing next to. TWO DEFECTS, AND A VERSION BUMP FIXES NEITHER: 1. The pin covered the TOP package only. ~30 transitive dependencies floated, unhashed, resolved at signing time. 2. NO Dependabot ecosystem parses an inline `pip install X==Y` in a workflow `run:` block -- dependabot.yml registers uv, github-actions and npm. So the pin had no updater, no trigger and no owner. Its own comment said "Re-evaluate to 4.5.0 once it has aged past the window" and nothing would ever have prompted that; it was 19 days overdue. test_ci_venv_pinning.py already worded the class: "a stale pin rots invisibly and a DELETED pin is invisible twice over." Routing it through uv.lock fixes both: the transitives are hash-pinned, and the group sits under the `uv` ecosystem Dependabot already watches. THE RECORDED DECISION WAS SPENT, NOT OVERRIDDEN. ADR 0034:350 deliberately kept sigstore OUT of the lock because routing it resolves 4.5.0, then <48h old against dependabot's 5-day cooldown -- pinning the SIGNING toolchain to a fresher artifact than the repo's own update policy allows would invert that policy at its highest-privilege point. That reasoning was CORRECT when written on 2026-07-29. MEASURED from PyPI's version-specific endpoint: 4.5.0 published 2026-07-28T07:34:00Z, so the window closed 2026-08-02, nineteen days before this. The ADR's own residual row still needs amending to say so -- that is the owner's, and it is NOT done here. GATE 2 PASSED, WHICH IS WHY THIS COULD LAND AT ALL. The known failure mode is a new group forcing a re-resolve across every other artifact -- semgrep was excluded by decision for exactly that. Measured: `uv lock` ADDED sigstore 4.5.0 plus 10 transitives and updated NOTHING, and re-running all six existing exports left them byte-identical (hashes checked against a pre-mutation snapshot). Verified with a detector control: perturbing one lock makes the same `git diff --exit-code` report 1. SEVEN-PLACE LOCKSTEP, NOT SIX. The structural test walked me through every site: the export, the DEP-1 `git diff --exit-code` set, the resync's export, its `git diff --quiet` short-circuit, and its `git add`. Missing any one leaves a Dependabot PR red with no bot-reachable path to green -- which is what happened to constraints.lock between #1193 and its fix. THE GUARD MOVED, IT WAS NOT DELETED. `sigstore` leaves RELEASE_PINNED_TOOLS because there is no longer an inline target to find. Deleting a row there is the exact regression that tuple exists to catch, so it is replaced by a stronger one: the lock exists, pins sigstore with `==`, EVERY requirement carries a hash, and release.yml installs from it with `--require-hashes`. All four in one test, and the hash check carries its own control because a detector that cannot see an unhashed requirement makes "0 unhashed" meaningless. RED-FIRST ON ALL THREE ARMS: reverting to the inline pin fails it, removing the lock fails it, and stripping the hashes fails it. The third arm initially PASSED -- because my mutation silently did nothing against a CRLF file, not because the guard was blind. A mutation that does not apply and a guard that does not catch produce the identical green, so the strip now asserts 193 -> 0 hashes before the test is allowed to mean anything. NOT DONE, deliberately: step 6's `build` and `cyclonedx-bom` are a separate change -- cyclonedx-bom is half of the byte-identity pair test_sbom_install_is_byte_identical_in_release_and_security enforces, so both halves must move together. And release.yml runs only on a tag push, so per ADR 0034 the first real exercise of this path is a `workflow_dispatch` dry-run before the next tag. VERIFIED: ruff clean; 56 tests across the pinning, lockstep and scanner suites; 255 more across the workflow/release/sbom suites. Co-Authored-By: Claude Opus 5 --- .github/workflows/dependabot-lock-resync.yml | 11 +- .github/workflows/release.yml | 33 ++- .github/workflows/security.yml | 7 +- ci/locks/release-tools.lock | 271 +++++++++++++++++++ pyproject.toml | 18 ++ tests/test_ci_venv_pinning.py | 67 ++++- uv.lock | 156 +++++++++++ 7 files changed, 548 insertions(+), 15 deletions(-) create mode 100644 ci/locks/release-tools.lock diff --git a/.github/workflows/dependabot-lock-resync.yml b/.github/workflows/dependabot-lock-resync.yml index 6ce3d315..23396d40 100644 --- a/.github/workflows/dependabot-lock-resync.yml +++ b/.github/workflows/dependabot-lock-resync.yml @@ -1,14 +1,14 @@ name: Dependabot lock resync -# Re-exports the SIX committed "uv export" artifacts (requirements.lock + +# Re-exports the SEVEN committed "uv export" artifacts (requirements.lock + # docker/locks/requirements-core.lock + docker/locks/requirements-sqlserver.lock + the HASHLESS # constraints.lock + the two PEP 735 CI-toolchain locks ci/locks/ci-scanners.lock and -# ci/locks/ci-quality.lock) on a Dependabot PR that touched uv.lock / pyproject.toml, and commits them +# ci/locks/ci-quality.lock, and the release-signing lock ci/locks/release-tools.lock) on a Dependabot PR that touched uv.lock / pyproject.toml, and commits them # back to the PR branch so the DEP-1 drift gate (security.yml -> pip-audit job, step "Check the # lockfile is in sync with pyproject (DEP-1)") goes green WITHOUT a human re-export. # # WHY: the native "uv" Dependabot ecosystem regenerates uv.lock + pyproject.toml in its PR, but NOT -# the exported lock artifacts. The DEP-1 gate re-runs "uv lock --check" + all six "uv export"s and +# the exported lock artifacts. The DEP-1 gate re-runs "uv lock --check" + all seven "uv export"s and # "git diff --exit-code"s the result, so a Dependabot uv PR would otherwise leave the exports stale # and red the gate. This workflow runs the IDENTICAL commands and pushes the refreshed exports onto # the Dependabot branch. @@ -137,6 +137,7 @@ jobs: # four above: the gate diffs them, so the bot must re-export them or the PR has no path green. uv export --only-group ci-scanners --format requirements.txt -o ci/locks/ci-scanners.lock uv export --only-group ci-quality --format requirements.txt -o ci/locks/ci-quality.lock + uv export --only-group release-tools --format requirements.txt -o ci/locks/release-tools.lock - name: Commit and push the resynced locks if: steps.creds.outputs.present == 'true' @@ -148,11 +149,11 @@ jobs: set -euo pipefail git config user.name 'dependabot[bot]' git config user.email '49699333+dependabot[bot]@users.noreply.github.com' - if git diff --quiet -- requirements.lock docker/locks/requirements-core.lock docker/locks/requirements-sqlserver.lock constraints.lock ci/locks/ci-scanners.lock ci/locks/ci-quality.lock; then + if git diff --quiet -- requirements.lock docker/locks/requirements-core.lock docker/locks/requirements-sqlserver.lock constraints.lock ci/locks/ci-scanners.lock ci/locks/ci-quality.lock ci/locks/release-tools.lock; then echo 'Exported lock files already in sync; nothing to push.' exit 0 fi - git add requirements.lock docker/locks/requirements-core.lock docker/locks/requirements-sqlserver.lock constraints.lock ci/locks/ci-scanners.lock ci/locks/ci-quality.lock + git add requirements.lock docker/locks/requirements-core.lock docker/locks/requirements-sqlserver.lock constraints.lock ci/locks/ci-scanners.lock ci/locks/ci-quality.lock ci/locks/release-tools.lock git commit -m 'chore(deps): resync exported lock files (DEP-1)' # Push with the persisted App-token credential (NOT GITHUB_TOKEN). An App-token push emits a # synchronize event so the required checks (DEP-1, ci.yml) re-run on the new commit; a diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fa67a985..0896d686 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -245,14 +245,31 @@ jobs: run: | # PINNED — the sharpest of these installs: this step is unconditional (every tag AND every # dispatch) and the very next command signs the release artifacts with the job's OIDC identity, - # the same identity that publishes to PyPI below. 4.4.0, not the newer 4.5.0: .github/ - # dependabot.yml sets a 5-day supply-chain cooldown to dodge a package compromised shortly - # after publish, and 4.5.0 is <48h old — hard-pinning the SIGNING toolchain to a fresher - # artifact than the repo's own routine-update policy allows inverts that policy at the highest- - # privilege point in the pipeline. Re-evaluate to 4.5.0 once it has aged past the window. - # NOTE: this pins the TOP only; sigstore's ~30 transitive deps still float at signing time. - # Closing the Scorecard alert outright needs the hashed release-tools lock (ADR 0034 option B). - python -m pip install "sigstore==4.4.0" + # the same identity that publishes to PyPI below — so ANY code that executes here runs with + # the credential that signs the artifacts. A backdoored wheel produced at this point carries a + # VALID Sigstore bundle and VALID SLSA provenance, and every downstream verifier reports + # success: the compromise defeats the controls it is standing next to. + # + # HASHED LOCK, not an inline pin (BACKLOG #332). The previous form was + # `pip install "sigstore==4.4.0"`, which had TWO defects that a version bump does not fix: + # 1. It pinned the TOP package only. ~30 transitive dependencies still floated, unhashed, + # resolved at signing time. + # 2. NO Dependabot ecosystem parses an inline `pip install X==Y` inside a workflow `run:` + # block (.github/dependabot.yml registers uv, github-actions, npm). So the pin had no + # updater, no trigger and no owner — its own comment said "re-evaluate once it has aged + # past the window" and nothing would ever have prompted that. It was 19 days overdue when + # this landed. tests/test_ci_venv_pinning.py words the class: "a stale pin rots invisibly + # and a DELETED pin is invisible twice over." + # Routing it through `uv.lock` fixes both: the transitives are hash-pinned, and the group is + # under the `uv` ecosystem Dependabot already watches. + # + # ON THE VERSION: the lock resolves sigstore 4.5.0, which the old inline pin deliberately + # avoided. That choice was CORRECT when made — dependabot.yml sets a 5-day supply-chain + # cooldown and 4.5.0 was then <48h old, so pinning the SIGNING toolchain to a fresher artifact + # than the repo's own update policy allows would have inverted that policy at its + # highest-privilege point. MEASURED: 4.5.0 published 2026-07-28T07:34:00Z, so the window + # closed 2026-08-02. The objection is SPENT, not overridden. + python -m pip install --require-hashes -r ci/locks/release-tools.lock # Sign the wheel + sdist AND the SBOM + VEX, so an operator can verify the provenance of the # bill-of-materials and the exploitability assessment too — not just the code artifacts (ADR 0149). python -m sigstore sign dist/*.tar.gz dist/*.whl \ diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 826450da..14be6f58 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -94,7 +94,12 @@ jobs: # stay out of the four exports above (and out of the SBOM / image locks / audited runtime). uv export --only-group ci-scanners --format requirements.txt -o ci/locks/ci-scanners.lock uv export --only-group ci-quality --format requirements.txt -o ci/locks/ci-quality.lock - git diff --exit-code -- requirements.lock docker/locks/requirements-core.lock docker/locks/requirements-sqlserver.lock constraints.lock ci/locks/ci-scanners.lock ci/locks/ci-quality.lock + # The RELEASE SIGNING toolchain (BACKLOG #332). Same mechanism as the two above, and it is + # here rather than inline in release.yml because release.yml runs ONLY on a tag push -- so a + # drift that lived there would first be observed during a release. Exported on every + # security run instead, where it is cheap to catch. + uv export --only-group release-tools --format requirements.txt -o ci/locks/release-tools.lock + git diff --exit-code -- requirements.lock docker/locks/requirements-core.lock docker/locks/requirements-sqlserver.lock constraints.lock ci/locks/ci-scanners.lock ci/locks/ci-quality.lock ci/locks/release-tools.lock - name: Install from the hashed lockfile (DEP-1) run: | # --require-hashes enforces a hash for every requirement (the lockfile carries them): a diff --git a/ci/locks/release-tools.lock b/ci/locks/release-tools.lock new file mode 100644 index 00000000..da25a39f --- /dev/null +++ b/ci/locks/release-tools.lock @@ -0,0 +1,271 @@ +# This file was autogenerated by uv via the following command: +# uv export --only-group release-tools --format requirements.txt -o ci/locks/release-tools.lock +annotated-types==0.7.0 \ + --hash=sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53 \ + --hash=sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89 + # via pydantic +certifi==2026.6.17 \ + --hash=sha256:024c88eeec92ca068db80f02b8b07c9cef7b9fe261d1d535abfd5abd6f6af432 \ + --hash=sha256:2227dcbaafe0d2f59279d1762ddddc37783ed4354594f194ffc31d20f41fc3db + # via requests +cffi==2.0.0 ; platform_python_implementation != 'PyPy' \ + --hash=sha256:087067fa8953339c723661eda6b54bc98c5625757ea62e95eb4898ad5e776e9f \ + --hash=sha256:0a1527a803f0a659de1af2e1fd700213caba79377e27e4693648c2923da066f9 \ + --hash=sha256:12873ca6cb9b0f0d3a0da705d6086fe911591737a59f28b7936bdfed27c0d47c \ + --hash=sha256:1fc9ea04857caf665289b7a75923f2c6ed559b8298a1b8c49e59f7dd95c8481e \ + --hash=sha256:203a48d1fb583fc7d78a4c6655692963b860a417c0528492a6bc21f1aaefab25 \ + --hash=sha256:24b6f81f1983e6df8db3adc38562c83f7d4a0c36162885ec7f7b77c7dcbec97b \ + --hash=sha256:28a3a209b96630bca57cce802da70c266eb08c6e97e5afd61a75611ee6c64592 \ + --hash=sha256:38100abb9d1b1435bc4cc340bb4489635dc2f0da7456590877030c9b3d40b0c1 \ + --hash=sha256:44d1b5909021139fe36001ae048dbdde8214afa20200eda0f64c068cac5d5529 \ + --hash=sha256:6c6c373cfc5c83a975506110d17457138c8c63016b563cc9ed6e056a82f13ce4 \ + --hash=sha256:737fe7d37e1a1bffe70bd5754ea763a62a066dc5913ca57e957824b72a85e205 \ + --hash=sha256:7553fb2090d71822f02c629afe6042c299edf91ba1bf94951165613553984512 \ + --hash=sha256:7a66c7204d8869299919db4d5069a82f1561581af12b11b3c9f48c584eb8743d \ + --hash=sha256:7cc09976e8b56f8cebd752f7113ad07752461f48a58cbba644139015ac24954c \ + --hash=sha256:92b68146a71df78564e4ef48af17551a5ddd142e5190cdf2c5624d0c3ff5b2e8 \ + --hash=sha256:9a67fc9e8eb39039280526379fb3a70023d77caec1852002b4da7e8b270c4dd9 \ + --hash=sha256:afb8db5439b81cf9c9d0c80404b60c3cc9c3add93e114dcae767f1477cb53775 \ + --hash=sha256:b1e74d11748e7e98e2f426ab176d4ed720a64412b6a15054378afdb71e0f37dc \ + --hash=sha256:c654de545946e0db659b3400168c9ad31b5d29593291482c43e3564effbcee13 \ + --hash=sha256:d68b6cef7827e8641e8ef16f4494edda8b36104d79773a334beaa1e3521430f6 \ + --hash=sha256:d9b97165e8aed9272a6bb17c01e3cc5871a594a446ebedc996e2397a1c1ea8ef \ + --hash=sha256:dbd5c7a25a7cb98f5ca55d258b103a2054f859a46ae11aaf23134f9cc0d356ad \ + --hash=sha256:fc33c5141b55ed366cfaad382df24fe7dcbc686de5be719b207bb248e3053dc5 + # via cryptography +charset-normalizer==3.4.7 \ + --hash=sha256:03853ed82eeebbce3c2abfdbc98c96dc205f32a79627688ac9a27370ea61a49c \ + --hash=sha256:0c96c3b819b5c3e9e165495db84d41914d6894d55181d2d108cc1a69bfc9cce0 \ + --hash=sha256:1c2aed2e5e41f24ea8ef1590b8e848a79b56f3a5564a65ceec43c9d692dc7d8a \ + --hash=sha256:2d6eb928e13016cea4f1f21d1e10c1cebd5a421bc57ddf5b1142ae3f86824fab \ + --hash=sha256:3534e7dcbdcf757da6b85a0bbf5b6868786d5982dd959b065e65481644817a18 \ + --hash=sha256:3c9a494bc5ec77d43cea229c4f6db1e4d8fe7e1bbffa8b6f0f0032430ff8ab44 \ + --hash=sha256:3dce51d0f5e7951f8bb4900c257dad282f49190fdbebecd4ba99bcc41fef404d \ + --hash=sha256:54523e136b8948060c0fa0bc7b1b50c32c186f2fceee897a495406bb6e311d2b \ + --hash=sha256:5b77459df20e08151cd6f8b9ef8ef1f961ef73d85c21a555c7eed5b79410ec10 \ + --hash=sha256:67f6279d125ca0046a7fd386d01b311c6363844deac3e5b069b514ba3e63c246 \ + --hash=sha256:6c114670c45346afedc0d947faf3c7f701051d2518b943679c8ff88befe14f8e \ + --hash=sha256:715479b9a2802ecac752a3b0efa2b0b60285cf962ee38414211abdfccc233b41 \ + --hash=sha256:733784b6d6def852c814bce5f318d25da2ee65dd4839a0718641c696e09a2960 \ + --hash=sha256:752a45dc4a6934060b3b0dab47e04edc3326575f82be64bc4fc293914566503e \ + --hash=sha256:7579e913a5339fb8fa133f6bbcfd8e6749696206cf05acdbdca71a1b436d8e72 \ + --hash=sha256:8778f0c7a52e56f75d12dae53ae320fae900a8b9b4164b981b9c5ce059cd1fcb \ + --hash=sha256:8d828b6667a32a728a1ad1d93957cdf37489c57b97ae6c4de2860fa749b8fc1e \ + --hash=sha256:92a0a01ead5e668468e952e4238cccd7c537364eb7d851ab144ab6627dbbe12f \ + --hash=sha256:a180c5e59792af262bf263b21a3c49353f25945d8d9f70628e73de370d55e1e1 \ + --hash=sha256:a5fe03b42827c13cdccd08e6c0247b6a6d4b5e3cdc53fd1749f5896adcdc2356 \ + --hash=sha256:a89c23ef8d2c6b27fd200a42aa4ac72786e7c60d40efdc76e6011260b6e949c4 \ + --hash=sha256:ae89db9e5f98a11a4bf50407d4363e7b09b31e55bc117b4f7d80aab97ba009e5 \ + --hash=sha256:bd6c2a1c7573c64738d716488d2cdd3c00e340e4835707d8fdb8dc1a66ef164e \ + --hash=sha256:c03a41a8784091e67a39648f70c5f97b5b6a37f216896d44d2cdcb82615339a0 \ + --hash=sha256:c35abb8bfff0185efac5878da64c45dafd2b37fb0383add1be155a763c1f083d \ + --hash=sha256:c36c333c39be2dbca264d7803333c896ab8fa7d4d6f0ab7edb7dfd7aea6e98c0 \ + --hash=sha256:c45e9440fb78f8ddabcf714b68f936737a121355bf59f3907f4e17721b9d1aae \ + --hash=sha256:ce3412fbe1e31eb81ea42f4169ed94861c56e643189e1e75f0041f3fe7020abe \ + --hash=sha256:cf1493cd8607bec4d8a7b9b004e699fcf8f9103a9284cc94962cb73d20f9d4a3 \ + --hash=sha256:d6038d37043bced98a66e68d3aa2b6a35505dc01328cd65217cefe82f25def44 \ + --hash=sha256:e74327fb75de8986940def6e8dee4f127cc9752bee7355bb323cc5b2659b6d46 \ + --hash=sha256:e8ac484bf18ce6975760921bb6148041faa8fef0547200386ea0b52b5d27bf7b \ + --hash=sha256:effc3f449787117233702311a1b7d8f59cba9ced946ba727bdc329ec69028e24 \ + --hash=sha256:fbccdc05410c9ee21bbf16a35f4c1d16123dcdeb8a1d38f33654fa21d0234f79 + # via requests +cryptography==50.0.0 \ + --hash=sha256:031e2d5dd4bb9caa3ca9c82e5a197fd8ae680232cee62603d1a813f3f07e3d03 \ + --hash=sha256:06a32a980526a6ab9a4b9bf8f7385800791e2bb960903cb6b530e4817509a3b7 \ + --hash=sha256:07949c449a1abcf60d1ee6e88956d89404c7df3c8258f46589e912988e551987 \ + --hash=sha256:105110f43a471dbd0060b9c9516cb8a6a79233631a04cc2ba16f28323ac6e025 \ + --hash=sha256:11b74db56cdbe3cdee6e3f6982ecb70334fa10dce99ed58bf7894aaaa3b2a037 \ + --hash=sha256:12b9c6996425c76ea6c457ace4f3073e715b8c545add07cd1a8f3a4f90691269 \ + --hash=sha256:1489e263a8048bb8b6a8bac662eb2d402ea5d2b7b4699b72f385f1e2772db105 \ + --hash=sha256:19736989797678c6af1e55cd49055cdbcb55d8f6b5583ac5335f933aba9101dc \ + --hash=sha256:1b4a266766514614f8aa60416e71f2fc6e575d36e7bdc90f644fadb2f4b75b95 \ + --hash=sha256:2a8183b489dc1f7f80f135780fadc1108f14b31b8a40411c7a5b17425f65f28b \ + --hash=sha256:37fdb0d0111f1e2ff07139dfb79f1b49531f8e213c46f1163dd7642979b58c47 \ + --hash=sha256:3f5735ffe4996d28b809371756219f5354864902a3b9e7c0b9ee87041209fc9c \ + --hash=sha256:49e7d93abdbd2990caced757e5fade25302f719c3c8fb6e6fff2dde98999fc41 \ + --hash=sha256:65c2c3add92b45fd0709db8594536aea39c2a67af0e27ffcf049c498501140b7 \ + --hash=sha256:6ba6a53445bd3cfa809ef3ef5f1589aa6ba08784a1d962bf47d0940e871dab1c \ + --hash=sha256:6e7d61120573a7f2cd94cc095f9e81f6967c61ccdf194285aa143ecec8e0b708 \ + --hash=sha256:7cec5b856506da6defb290f30c9ee687d5f5e8cb0bd3f6459dde43b0b4fa40ef \ + --hash=sha256:80b63928fa35083b33966ce1efb70e5b9607181e49dcd1c22c8c005e319f667f \ + --hash=sha256:82148ec5bddac30b51a5b3c1945075f896fa022cb93f8e4a01e9f6ee95292c5f \ + --hash=sha256:828743d939e9629bc267b8e2d08d8bb67cd4319c771a33d4b18b22dd8fb7440a \ + --hash=sha256:8d89f3976b10b4ce31118de72329025f70d2c6ead14a8217c5514dd2c6d5a78f \ + --hash=sha256:900131fafd8aead39ac7dd3a7e833be754c17a95cfd91221636949fe4eb0aa8a \ + --hash=sha256:910e1d2668e7de9648f2bcee30e180db2a6b15c30f887d7c4c93ddf96e3992e3 \ + --hash=sha256:9aa87839c383bdbab6ef865787a1fb877af8dd03464c4400322726feaaadfc6d \ + --hash=sha256:a1b30560f2acc95aa8b2e06e716a13dbfc97314747b80d9707e307f77b40d6b3 \ + --hash=sha256:a91296cb61e8df6f86d0c19cc4068228da256bf59bf86049fbd821084565327f \ + --hash=sha256:b42a28c1844fd9de8f3f7d540e36b66f3a9c83fceac7170ebc7a6a19edd9dcae \ + --hash=sha256:bd1c592e4d5974f0d08d4888e432157adba757c66da0246918e43677fafa2d30 \ + --hash=sha256:c87f62a3d3b9888ed0fdde100ec06aa61ca9cd44bad9057d1dff9a516b5f5bb9 \ + --hash=sha256:ccdc4a71a4dabae05de219404f9f4abc38e3b58422177ff93d0da05967dafa07 \ + --hash=sha256:d24fead1d4d076e1bfb006dcec392074a3cd8d7b4fc8a595aa64073b2b7a96ba \ + --hash=sha256:d58c3db7cd6eed54e6c06744db55456b65ebd7492ddeae9c1e93cfca7aa857d3 \ + --hash=sha256:d764dcf130c428ef66786f866dd750f53182bc608813489915e9fc106bb0c82f \ + --hash=sha256:df2a58a472f332225671c35b0a830208b86d004f82baa8530fa3782c85646533 \ + --hash=sha256:e722f16708d854fe924790e051061f6704a472c3bac347b6fd88033ea8dd0dc5 \ + --hash=sha256:ecfed7367f965a0328cfbdd70da860f15441f002f613185668c6e6ebf5a0ac11 \ + --hash=sha256:eeac2acb5a20ed25e0ad6d1df9891a520b78b404266b6d11778f25d5d691a6c9 \ + --hash=sha256:f59e38625469987d7ef6d495323c55e7db6c212eaf6112267e0d3b565a2e9c9f \ + --hash=sha256:f89831ef99dd7dd169ab06d63a831adb9e20a87aac6d380266bbda5823349169 \ + --hash=sha256:fd9192b7b70c573d7f214eb1ae35e00d359f6f5e4b27c7e21e30de1fc6204645 + # via + # pyopenssl + # rfc3161-client + # sigstore +dnspython==2.8.0 \ + --hash=sha256:01d9bbc4a2d76bf0db7c1f729812ded6d912bd318d3b1cf81d30c0f845dbf3af \ + --hash=sha256:181d3c6996452cb1189c4046c61599b84a5a86e099562ffde77d26984ff26d0f + # via email-validator +email-validator==2.3.0 \ + --hash=sha256:80f13f623413e6b197ae73bb10bf4eb0908faf509ad8362c5edeb0be7fd450b4 \ + --hash=sha256:9fc05c37f2f6cf439ff414f8fc46d917929974a82244c20eb10231ba60c54426 + # via pydantic +id==1.6.1 \ + --hash=sha256:d0732d624fb46fd4e7bc4e5152f00214450953b9e772c182c1c22964def1a069 \ + --hash=sha256:f5ec41ed2629a508f5d0988eda142e190c9c6da971100612c4de9ad9f9b237ca + # via sigstore +idna==3.18 \ + --hash=sha256:7f952cbe720b688055e3f87de14f5c3e5fdaa8bc3928985c4077ca689de849a2 \ + --hash=sha256:ffb385a7e039654cef1ab9ef32c6fafe283c0c0467bba1d9029738ce4a14a848 + # via + # email-validator + # requests +markdown-it-py==4.2.0 \ + --hash=sha256:04a21681d6fbb623de53f6f364d352309d4094dd4194040a10fd51833e418d49 \ + --hash=sha256:9f7ebbcd14fe59494226453aed97c1070d83f8d24b6fc3a3bcf9a38092641c4a + # via rich +mdurl==0.1.2 \ + --hash=sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8 \ + --hash=sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba + # via markdown-it-py +platformdirs==4.11.0 \ + --hash=sha256:0555d18370482847566ffabcaa53ad7c6c1c29f195989ae1ed634a05f76ea1e0 \ + --hash=sha256:360ccded2b7fce0af0ff80cc8f5942a1c5d99b0e856033acb030bfc634709e74 + # via sigstore +pyasn1==0.6.4 \ + --hash=sha256:9c447d8431c947fe4c8febc4ed9e760bc29011a5b01e5c74b67025bd9fb8ce81 \ + --hash=sha256:deda9277cfd454080ec40b207fb6df82206a3a2688735233cdcd8d3d565f088b + # via sigstore +pycparser==3.0 ; implementation_name != 'PyPy' and platform_python_implementation != 'PyPy' \ + --hash=sha256:600f49d217304a5902ac3c37e1281c9fe94e4d0489de643a9504c5cdfdfc6b29 \ + --hash=sha256:b727414169a36b7d524c1c3e31839a521725078d7b2ff038656844266160a992 + # via cffi +pydantic==2.13.4 \ + --hash=sha256:45a282cde31d808236fd7ea9d919b128653c8b38b393d1c4ab335c62924d9aba \ + --hash=sha256:c40756b57adaa8b1efeeced5c196f3f3b7c435f90e84ea7f443901bec8099ef6 + # via + # sigstore + # sigstore-models + # sigstore-rekor-types +pydantic-core==2.46.4 \ + --hash=sha256:0cbe8b01f948de4286c74cdd6c667aceb38f5c1e26f0693b3983d9d74887c65e \ + --hash=sha256:1a7dd0b3ee80d90150e3495a3a13ac34dbcbfd4f012996a6a1d8900e91b5c0fb \ + --hash=sha256:2108ba5c1c1eca18030634489dc544844144ee36357f2f9f780b93e7ddbb44b5 \ + --hash=sha256:23ace664830ee0bfe014a0c7bc248b1f7f25ed7ad103852c317624a1083af462 \ + --hash=sha256:29c61fc04a3d840155ff08e475a04809278972fe6aef51e2720554e96367e34b \ + --hash=sha256:372429a130e469c9cd698925ce5fc50940b7a1336b0d82038e63d5bbc4edc519 \ + --hash=sha256:3bf92c5d0e00fefaab325a4d27828fe6b6e2a21848686b5b60d2d9eeb09d76c6 \ + --hash=sha256:3ecbc122d18468d06ca279dc26a8c2e2d5acb10943bb35e36ae92096dc3b5565 \ + --hash=sha256:3fb702cd90b0446a3a1c5e470bfa0dd23c0233b676a9099ddcc964fa6ca13898 \ + --hash=sha256:428e04521a40150c85216fc8b85e8d39fece235a9cf5e383761238c7fa9b96fb \ + --hash=sha256:4fcbe087dbc2068af7eda3aa87634eba216dbda64d1ae73c8684b621d33f6596 \ + --hash=sha256:5a4330cdbc57162e4b3aa303f588ba752257694c9c9be3e7ebb11b4aca659b5d \ + --hash=sha256:617d7e2ca7dcb8c5cf6bcb8c59b8832c94b36196bbf1cbd1bfb56ed341905edd \ + --hash=sha256:62f875393d7f270851f20523dd2e29f082bcc82292d66db2b64ea71f64b6e1c1 \ + --hash=sha256:7027560ee92211647d0d34e3f7cd6f50da56399d26a9c8ad0da286d3869a53f3 \ + --hash=sha256:7283d57845ecf5a163403eb0702dfc220cc4fbdd18919cb5ccea4f95ee1cdab4 \ + --hash=sha256:7a5f930472650a82629163023e630d160863fce524c616f4e5186e5de9d9a49b \ + --hash=sha256:811ff8e9c313ab425368bcbb36e5c4ebd7108c2bbf4e4089cfbb0b01eff63fac \ + --hash=sha256:85bb3611ff1802f3ee7fdd7dbff26b56f343fb432d57a4728fdd49b6ef35e2f4 \ + --hash=sha256:8daafc69c93ee8a0204506a3b6b30f586ef54028f52aeeeb5c4cfc5184fd5914 \ + --hash=sha256:97e7cf2be5c77b7d1a9713a05605d49460d02c6078d38d8bef3cbe323c548424 \ + --hash=sha256:af8244b2bef6aaad6d92cda81372de7f8c8d36c9f0c3ea36e827c60e7d9467a0 \ + --hash=sha256:b8458003118a712e66286df6a707db01c52c0f52f7db8e4a38f0da1d3b94fc4e \ + --hash=sha256:bfec22eab3c8cc2ceec0248aec886624116dc079afa027ecc8ad4a7e62010f8a \ + --hash=sha256:c1b3f518abeca3aa13c712fd202306e145abf59a18b094a6bafb2d2bbf59192c \ + --hash=sha256:c50f2528cf200c5eed56faf3f4e22fcd5f38c157a8b78576e6ba3168ec35f000 \ + --hash=sha256:cd2213145bcc2ba85884d0ac63d222fece9209678f77b9b4d76f054c561adb28 \ + --hash=sha256:ce5c1d2a8b27468f433ca974829c44060b8097eedc39933e3c206a90ee49c4a9 \ + --hash=sha256:e846ae7835bf0703ae43f534ab79a867146dadd59dc9ca5c8b53d5c8f7c9ef02 \ + --hash=sha256:f99626688942fb746e545232e7726926f3be91b5975f8b55327665fafda991c7 \ + --hash=sha256:fc3e9034a63de20e15e8ade85358bc6efc614008cab72898b4b4952bea0509ff + # via pydantic +pygments==2.20.0 \ + --hash=sha256:6757cd03768053ff99f3039c1a36d6c0aa0b263438fcab17520b30a303a82b5f \ + --hash=sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176 + # via rich +pyjwt==2.13.0 \ + --hash=sha256:41571c89ca91598c79e8ef18a2d07367d4810fbbd6f637794879baf1b7703423 \ + --hash=sha256:66adcc2aff09b3f1bbd95fc1e1577df8ac8723c978552fd43304c8a290ac5728 + # via sigstore +pyopenssl==26.4.0 \ + --hash=sha256:28dfcce0162b9211413e26dfbfdf1d24317fbeba18fc93c12400a1856b2a0bc7 \ + --hash=sha256:f0eb0cb2d581d3ad2b9c489468485e7f2ab6727d08401bcf9d824c3caddf3c1c + # via sigstore +requests==2.34.2 \ + --hash=sha256:2a0d60c172f83ac6ab31e4554906c0f3b3588d37b5cb939b1c061f4907e278e0 \ + --hash=sha256:f288924cae4e29463698d6d60bc6a4da69c89185ad1e0bcc4104f584e960b9ed + # via sigstore +rfc3161-client==1.0.8 \ + --hash=sha256:29cebab8dadec88b85eac43505db186749e23ddf1c7699ffa08f7cbdc86a52fe \ + --hash=sha256:38c5c278c7d4605e9c166a332c13b286475730b750109235b547f7d1b21c5c6f \ + --hash=sha256:3951db9573e4f6b4a1a62ad4f0074683610714625bf50c010739ffa568f23beb \ + --hash=sha256:4bda5a2bc6947c16b6f8df90ff0e99cb333d78ab1465517f637d313d75703651 \ + --hash=sha256:4bdb12618f98ee634d3625208f4f1c3cdde2306a8187a7416bfa47d45cad3dba \ + --hash=sha256:51adc82dbd04d2b88e3a17f524f0e57d0270d7276887a5800c81833f79fb4f4a \ + --hash=sha256:55cd9366f20dcea8dc65f93b08d12607c071015d2f5b5d24129128f04643a77d \ + --hash=sha256:5c1889d6bae269dc0f1e418f82e554b413066b5f8dd864f9367cf1ffb3d0a312 \ + --hash=sha256:7f8b82c97c1935a09376591b45bd81aa57a4232f4d446eee3a44c025ef7c4d5a \ + --hash=sha256:9826227dc04e1a86f2598f9c1829f078afc35f558987df77578ea1508d1460a1 \ + --hash=sha256:9d382372e7fdfde592584f985fb1063d1506ae0573df45fcb2b41e2ed82e3431 \ + --hash=sha256:d3c25311c67a7daeef990fb5b94eaed706135c6a6fd98c6e382bbc857faa4214 \ + --hash=sha256:e95ca8a64fddfdd639e09e48bab4722f31b26ce099067ad2bb8e85f88fcc707b + # via sigstore +rfc8785==0.1.4 \ + --hash=sha256:520d690b448ecf0703691c76e1a34a24ddcd4fc5bc41d589cb7c58ec651bcd48 \ + --hash=sha256:e545841329fe0eee4f6a3b44e7034343100c12b4ec566dc06ca9735681deb4da + # via sigstore +rich==15.0.0 \ + --hash=sha256:33bd4ef74232fb73fe9279a257718407f169c09b78a87ad3d296f548e27de0bb \ + --hash=sha256:edd07a4824c6b40189fb7ac9bc4c52536e9780fbbfbddf6f1e2502c31b068c36 + # via sigstore +securesystemslib==1.4.0 \ + --hash=sha256:a0743a3d978cf26e98a70a57e3fbd5a18e0a74c20cabe615f6a55b02ef0272b3 \ + --hash=sha256:faea87be0f9c4b4277a5fa1b54bf9bfd807be9a94ab11be6c557dc8b75c43285 + # via tuf +sigstore==4.5.0 \ + --hash=sha256:020d3e07f622b2916bf453e66ff6ff0711e1fdc5ab69e8bd8902f71d9fcb316f \ + --hash=sha256:f045b207f2e12605cf775ec38e89c5eda625d71ffa7830477db65e47ec2bc8b2 +sigstore-models==0.0.6 \ + --hash=sha256:5201a68f4d7d0f8bec1e2f4378eb646b084c52609a4e31db8c385095fff68b2e \ + --hash=sha256:c766c09470c2a7e8a4a333c893f07e2001c56a3ff1757b1a246119f53169a849 + # via sigstore +sigstore-rekor-types==0.0.18 \ + --hash=sha256:19aef25433218ebf9975a1e8b523cc84aaf3cd395ad39a30523b083ea7917ec5 \ + --hash=sha256:b62bf38c5b1a62bc0d7fe0ee51a0709e49311d137c7880c329882a8f4b2d1d78 + # via sigstore +tuf==7.0.0 \ + --hash=sha256:572bdbdc9ff4a82278a0d4773e6100863b9b33023f27575e84ca65b486dd0d79 \ + --hash=sha256:9d2e6723538e0d5a3e482b6de805fcfe64481448d5853039ba6b06ba541efd7f + # via sigstore +typing-extensions==4.15.0 \ + --hash=sha256:0cea48d173cc12fa28ecabc3b837ea3cf6f38c6d1136f85cbaaf598984861466 \ + --hash=sha256:f0fa19c6845758ab08074a0cfa8b7aecb71c999ca73d62883bc25cc018c4e548 + # via + # pydantic + # pydantic-core + # sigstore-models + # typing-inspection +typing-inspection==0.4.2 \ + --hash=sha256:4ed1cacbdc298c220f1bd249ed5287caa16f34d44ef4e9c3d0cbad5b521545e7 \ + --hash=sha256:ba561c48a67c5958007083d386c3295464928b01faa735ab8547c5692e87f464 + # via pydantic +urllib3==2.7.0 \ + --hash=sha256:231e0ec3b63ceb14667c67be60f2f2c40a518cb38b03af60abc813da26505f4c \ + --hash=sha256:9fb4c81ebbb1ce9531cce37674bbc6f1360472bc18ca9a553ede278ef7276897 + # via + # id + # requests + # tuf diff --git a/pyproject.toml b/pyproject.toml index d0d82603..97d0c9a0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -276,6 +276,24 @@ ci-scanners = [ "pip-audit==2.10.1", "zizmor==1.29.0", ] +# The RELEASE SIGNING toolchain (BACKLOG #332). Exact `==`, and the version is the contract for the +# same reason the scanners' are: this runs in the job holding `id-token: write`, and whatever executes +# there signs the wheel, writes the SLSA attestation and publishes to PyPI. +# sigstore 4.5.0, not the 4.4.0 currently pinned inline at `release.yml:255`. That pin chose the +# OLDER release deliberately -- `dependabot.yml`'s 5-day supply-chain cooldown, and 4.5.0 +# was then <48h old, so hard-pinning the SIGNING toolchain to a fresher artifact than the +# repo's own update policy allows would have inverted that policy at the highest-privilege +# point in the pipeline. MEASURED: 4.5.0 published 2026-07-28T07:34:00Z, so the cooldown +# closed 2026-08-02 -- the objection is SPENT, not overridden, and `release.yml`'s own +# comment says "Re-evaluate to 4.5.0 once it has aged past the window." +# The point of the group is NOT the version. An inline `pip install X==Y` pins only the TOP package -- +# ~30 transitives still float, unhashed, resolved at signing time -- and NO Dependabot ecosystem parses +# an inline install inside a workflow `run:` block, so the pin has no updater, no trigger and no owner. +# Routing it through `uv.lock` hashes the transitives AND puts it under the `uv` ecosystem Dependabot +# already watches. Non-default, like its siblings, so it stays out of the runtime exports and the SBOM. +release-tools = [ + "sigstore==4.5.0", +] # The ADVISORY measurement tools (quality-advisory.yml). Exact where something PARSES the tool's # output, a floor where nothing does: # diff-cover `==`: the inline-annotation surface depends on this version's diff --git a/tests/test_ci_venv_pinning.py b/tests/test_ci_venv_pinning.py index 2f081925..a6ce15e5 100644 --- a/tests/test_ci_venv_pinning.py +++ b/tests/test_ci_venv_pinning.py @@ -202,7 +202,13 @@ def test_scratch_venvs_do_not_hide_an_unpinned_pip_fetch(workflow: str) -> None: #: Tools whose release-path pin must EXIST — the non-vacuity backstop for the scan above. Deleting a #: step would otherwise make the scan pass by finding nothing left to check. RELEASE_PINNED_TOOLS = ( - ("release.yml", "sigstore"), + # `sigstore` IS NOT HERE ANY MORE, and it was not dropped -- it MOVED. BACKLOG #332 routed it + # through `ci/locks/release-tools.lock`, so there is no longer an inline `pip install sigstore==` + # target for the scan above to find. The backstop that entry provided is preserved, and + # strengthened, by `test_the_release_signing_toolchain_is_installed_from_a_hashed_lock` below: + # deleting the lock install fails there instead. Removing an entry from this tuple WITHOUT a + # replacement guard is the exact regression the comment above warns about, so the two changes + # belong in one commit and this note is what makes that reviewable. ("release.yml", "build"), ("release.yml", "pip"), ("release.yml", "cyclonedx-bom"), @@ -980,3 +986,62 @@ def test_constraints_lock_still_carries_the_packaging_pin() -> None: f"resolves its pin with `sed … | head -1`, so zero lines hard-fail the next tag push and " f"two would silently pick the first." ) + + +# --- the release SIGNING toolchain, moved from an inline pin into a hashed lock (BACKLOG #332) ----- + + +def test_the_release_signing_toolchain_is_installed_from_a_hashed_lock() -> None: + """The replacement for ``sigstore``'s entry in ``RELEASE_PINNED_TOOLS``, and it is stronger. + + The inline pin it replaces covered the TOP package only -- ~30 transitive dependencies still + floated, unhashed, and resolved at signing time, inside the job holding ``id-token: write``. So + the old guard could pass while the thing it was protecting was wide open. + + THREE ASSERTIONS, EACH CLOSING A DIFFERENT WAY THIS GOES BACK: + + 1. the lock EXISTS and pins ``sigstore`` exactly -- not a floor, not a range; + 2. every requirement in it carries a hash -- a lock without hashes is a version pin wearing a + lock's filename, and ``--require-hashes`` would reject it at install time rather than here; + 3. ``release.yml`` actually installs FROM it WITH ``--require-hashes`` -- a lock nothing installs + from is decoration, which is the vacuity the tuple's own comment warns about. + """ + lock = _REPO / "ci" / "locks" / "release-tools.lock" + assert lock.is_file(), ( + "ci/locks/release-tools.lock is missing -- the signing toolchain is unpinned" + ) + + body = lock.read_text(encoding="utf-8") + assert re.search(r"(?mi)^sigstore==", body), ( + "release-tools.lock does not pin `sigstore` with `==` -- a range at the signing step is what " + "this lock exists to remove" + ) + # JOIN CONTINUATIONS FIRST. `uv export` writes a requirement as `name==version \` followed by its + # own `--hash=` lines, so a per-LINE check reports EVERY requirement as unhashed -- the hash is + # never on the line that names the package. Measured while writing this: the line-local form + # reported 31 of 31 unhashed against a lock carrying 193 hashes. Only the logical requirement + # answers "pinned AND hashed". + blocks = [b.strip() for b in re.split(r"\n(?=\S)", body) if b.strip()] + requirements = [b for b in blocks if re.match(r"^[A-Za-z0-9._-]+==", b)] + assert requirements, "no requirement blocks found -- the lock is empty or its format changed" + + def _without_hash(items: list[str]) -> list[str]: + return [b for b in items if "--hash=" not in b] + + # CONTROL: a detector that cannot SEE an unhashed requirement makes "0 unhashed" meaningless. + assert _without_hash(["fakepkg==1.0.0"]) == ["fakepkg==1.0.0"], ( + "the hash detector cannot identify an unhashed requirement, so its verdict on the real lock " + "would be indistinguishable from a blind pass" + ) + unhashed = [b.splitlines()[0][:60] for b in _without_hash(requirements)] + assert not unhashed, ( + f"{len(unhashed)} requirement(s) in release-tools.lock carry no hash, e.g. {unhashed[:3]} -- " + f"`--require-hashes` would fail the release rather than this test, which is far later" + ) + + release = (_WORKFLOWS / "release.yml").read_text(encoding="utf-8") + assert "--require-hashes -r ci/locks/release-tools.lock" in release, ( + "release.yml no longer installs the signing toolchain from the hashed lock. If that was " + "deliberate, restore an equivalent guard in the same commit -- otherwise the lock is inert " + "and the signing step is back to resolving unhashed dependencies at tag time." + ) diff --git a/uv.lock b/uv.lock index 045490bd..452deac9 100644 --- a/uv.lock +++ b/uv.lock @@ -542,6 +542,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/63/c8/5345dfb7c955639454fffc767c3eaf9863d79a1cdb0081392a71102fedfa/diff_cover-10.5.0-py3-none-any.whl", hash = "sha256:5619f924e838b8f8c4c7b243b32a6068d8c449c1766c6f6a1110b152195ed9b9", size = 61184, upload-time = "2026-08-08T17:25:18.337Z" }, ] +[[package]] +name = "dnspython" +version = "2.8.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/8c/8b/57666417c0f90f08bcafa776861060426765fdb422eb10212086fb811d26/dnspython-2.8.0.tar.gz", hash = "sha256:181d3c6996452cb1189c4046c61599b84a5a86e099562ffde77d26984ff26d0f", size = 368251, upload-time = "2025-09-07T18:58:00.022Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ba/5a/18ad964b0086c6e62e2e7500f7edc89e3faa45033c71c1893d34eed2b2de/dnspython-2.8.0-py3-none-any.whl", hash = "sha256:01d9bbc4a2d76bf0db7c1f729812ded6d912bd318d3b1cf81d30c0f845dbf3af", size = 331094, upload-time = "2025-09-07T18:57:58.071Z" }, +] + [[package]] name = "elementpath" version = "5.1.3" @@ -551,6 +560,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/37/11/317824213350e6bf7af8aa4238cb78bec9ca014b89fdd5e25283f1c40d98/elementpath-5.1.3-py3-none-any.whl", hash = "sha256:c46f5e0e36c149b892308843e1e394bdee17ceafbc18cfa39250403dd8475a4e", size = 260297, upload-time = "2026-06-28T11:24:00.522Z" }, ] +[[package]] +name = "email-validator" +version = "2.3.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "dnspython" }, + { name = "idna" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/f5/22/900cb125c76b7aaa450ce02fd727f452243f2e91a61af068b40adba60ea9/email_validator-2.3.0.tar.gz", hash = "sha256:9fc05c37f2f6cf439ff414f8fc46d917929974a82244c20eb10231ba60c54426", size = 51238, upload-time = "2025-08-26T13:09:06.831Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/de/15/545e2b6cf2e3be84bc1ed85613edd75b8aea69807a71c26f4ca6a9258e82/email_validator-2.3.0-py3-none-any.whl", hash = "sha256:80f13f623413e6b197ae73bb10bf4eb0908faf509ad8362c5edeb0be7fd450b4", size = 35604, upload-time = "2025-08-26T13:09:05.858Z" }, +] + [[package]] name = "execnet" version = "2.1.2" @@ -744,6 +766,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/55/33/71e45a6bd6875f44a26f99da31c63b6840123e88bedf2c0b1ce429b8be12/hvac-2.4.0-py3-none-any.whl", hash = "sha256:008db5efd8c2f77bd37d2368ea5f713edceae1c65f11fd608393179478649e0f", size = 155921, upload-time = "2025-10-30T12:57:46.253Z" }, ] +[[package]] +name = "id" +version = "1.6.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "urllib3" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/6d/04/c2156091427636080787aac190019dc64096e56a23b7364d3c1764ee3a06/id-1.6.1.tar.gz", hash = "sha256:d0732d624fb46fd4e7bc4e5152f00214450953b9e772c182c1c22964def1a069", size = 18088, upload-time = "2026-02-04T16:19:41.26Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/42/77/de194443bf38daed9452139e960c632b0ef9f9a5dd9ce605fdf18ca9f1b1/id-1.6.1-py3-none-any.whl", hash = "sha256:f5ec41ed2629a508f5d0988eda142e190c9c6da971100612c4de9ad9f9b237ca", size = 14689, upload-time = "2026-02-04T16:19:40.051Z" }, +] + [[package]] name = "idna" version = "3.18" @@ -1092,6 +1126,9 @@ ci-scanners = [ { name = "pip-audit" }, { name = "zizmor" }, ] +release-tools = [ + { name = "sigstore" }, +] [package.metadata] requires-dist = [ @@ -1153,6 +1190,7 @@ ci-scanners = [ { name = "pip-audit", specifier = "==2.10.1" }, { name = "zizmor", specifier = "==1.29.0" }, ] +release-tools = [{ name = "sigstore", specifier = "==4.5.0" }] [[package]] name = "msgpack" @@ -1571,6 +1609,11 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/fd/7b/122376b1fd3c62c1ed9dc80c931ace4844b3c55407b6fb2d199377c9736f/pydantic-2.13.4-py3-none-any.whl", hash = "sha256:45a282cde31d808236fd7ea9d919b128653c8b38b393d1c4ab335c62924d9aba", size = 472262, upload-time = "2026-05-06T13:43:02.641Z" }, ] +[package.optional-dependencies] +email = [ + { name = "email-validator" }, +] + [[package]] name = "pydantic-core" version = "2.46.4" @@ -1630,6 +1673,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f4/7e/a72dd26f3b0f4f2bf1dd8923c85f7ceb43172af56d63c7383eb62b332364/pygments-2.20.0-py3-none-any.whl", hash = "sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176", size = 1231151, upload-time = "2026-03-29T13:29:30.038Z" }, ] +[[package]] +name = "pyjwt" +version = "2.13.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/3b/81/58d0ac84e1ef3a3843791d6954d94c0b33d526c75eeb1efbce9d0a4c4077/pyjwt-2.13.0.tar.gz", hash = "sha256:41571c89ca91598c79e8ef18a2d07367d4810fbbd6f637794879baf1b7703423", size = 107515, upload-time = "2026-05-21T19:54:36.618Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a3/5e/ecf12fdb62546d64385c158514e9b2b671f7832108ef2ecd2020ce0af2d1/pyjwt-2.13.0-py3-none-any.whl", hash = "sha256:66adcc2aff09b3f1bbd95fc1e1577df8ac8723c978552fd43304c8a290ac5728", size = 31274, upload-time = "2026-05-21T19:54:35.362Z" }, +] + [[package]] name = "pynacl" version = "1.6.2" @@ -1939,6 +1991,38 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a0/f4/c67b0b3f1b9245e8d266f0f112c500d50e5b4e83cb6f3b71b6528104182a/requests-2.34.2-py3-none-any.whl", hash = "sha256:2a0d60c172f83ac6ab31e4554906c0f3b3588d37b5cb939b1c061f4907e278e0", size = 73075, upload-time = "2026-05-14T19:25:26.443Z" }, ] +[[package]] +name = "rfc3161-client" +version = "1.0.8" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/18/a6/cf05ce2b73da1e7c876c8992035bcbede938483f16ad04f0bda34c39b299/rfc3161_client-1.0.8.tar.gz", hash = "sha256:4bda5a2bc6947c16b6f8df90ff0e99cb333d78ab1465517f637d313d75703651", size = 107402, upload-time = "2026-07-29T13:54:05.93Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ca/0a/a06fa0af9676fa8ae6962def859cb1966fb1caf0be507d5abcc22994ca13/rfc3161_client-1.0.8-cp39-abi3-macosx_10_12_x86_64.whl", hash = "sha256:3951db9573e4f6b4a1a62ad4f0074683610714625bf50c010739ffa568f23beb", size = 2108437, upload-time = "2026-07-29T13:53:44.887Z" }, + { url = "https://files.pythonhosted.org/packages/38/59/62123806432e3fa42c06e74ee66b0dbdce66fa5cc0582d478de4ef6934e3/rfc3161_client-1.0.8-cp39-abi3-macosx_11_0_arm64.whl", hash = "sha256:4bdb12618f98ee634d3625208f4f1c3cdde2306a8187a7416bfa47d45cad3dba", size = 2437251, upload-time = "2026-07-29T13:53:46.697Z" }, + { url = "https://files.pythonhosted.org/packages/8c/d6/64bf6b0af601e3a532d1800180058088a086bca6bd8119acd576dba3c8b7/rfc3161_client-1.0.8-cp39-abi3-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:29cebab8dadec88b85eac43505db186749e23ddf1c7699ffa08f7cbdc86a52fe", size = 2648056, upload-time = "2026-07-29T13:53:48.384Z" }, + { url = "https://files.pythonhosted.org/packages/59/66/b739e4c8778e8221eb58e09de47543fa07da039204ef2c87d3aa6e3ec4d4/rfc3161_client-1.0.8-cp39-abi3-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:38c5c278c7d4605e9c166a332c13b286475730b750109235b547f7d1b21c5c6f", size = 2047923, upload-time = "2026-07-29T13:53:50.08Z" }, + { url = "https://files.pythonhosted.org/packages/03/2b/fdbcf18d362eb7e149d4a6552ccb3e2c8c40d058cb8a44b2f305d6f16a2d/rfc3161_client-1.0.8-cp39-abi3-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:9826227dc04e1a86f2598f9c1829f078afc35f558987df77578ea1508d1460a1", size = 2416368, upload-time = "2026-07-29T13:53:51.667Z" }, + { url = "https://files.pythonhosted.org/packages/ce/f5/6e0775d3219d791cac11e30b5a3071d62a2a7fe21bbfffa801903a15ab12/rfc3161_client-1.0.8-cp39-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:55cd9366f20dcea8dc65f93b08d12607c071015d2f5b5d24129128f04643a77d", size = 2410483, upload-time = "2026-07-29T13:53:53.327Z" }, + { url = "https://files.pythonhosted.org/packages/a4/f1/99638e03afae594418f9e9c27aeb189fe6dabc3d853cd48b36a1cf61e0a4/rfc3161_client-1.0.8-cp39-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:e95ca8a64fddfdd639e09e48bab4722f31b26ce099067ad2bb8e85f88fcc707b", size = 2993036, upload-time = "2026-07-29T13:53:55.074Z" }, + { url = "https://files.pythonhosted.org/packages/ee/a6/d5f7964d180b6ecf659d989d3474edaa7ef721a0038562b226ec89fc8811/rfc3161_client-1.0.8-cp39-abi3-musllinux_1_2_armv7l.whl", hash = "sha256:7f8b82c97c1935a09376591b45bd81aa57a4232f4d446eee3a44c025ef7c4d5a", size = 2353705, upload-time = "2026-07-29T13:53:57.184Z" }, + { url = "https://files.pythonhosted.org/packages/23/fa/ad5e2a354d01d2f1cedaa5ea24548f7cf71cef427715bf3104f3f81c461d/rfc3161_client-1.0.8-cp39-abi3-musllinux_1_2_i686.whl", hash = "sha256:51adc82dbd04d2b88e3a17f524f0e57d0270d7276887a5800c81833f79fb4f4a", size = 2564143, upload-time = "2026-07-29T13:53:59.098Z" }, + { url = "https://files.pythonhosted.org/packages/5d/49/141ecbee41a7cef5d819ad692305d99775af3ecb5b145d323dfd8458f32f/rfc3161_client-1.0.8-cp39-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:d3c25311c67a7daeef990fb5b94eaed706135c6a6fd98c6e382bbc857faa4214", size = 2639490, upload-time = "2026-07-29T13:54:00.832Z" }, + { url = "https://files.pythonhosted.org/packages/65/53/0de416266baddfac030b80602a8034e0133673deb18d4934623f78087d44/rfc3161_client-1.0.8-cp39-abi3-win32.whl", hash = "sha256:9d382372e7fdfde592584f985fb1063d1506ae0573df45fcb2b41e2ed82e3431", size = 1991195, upload-time = "2026-07-29T13:54:02.859Z" }, + { url = "https://files.pythonhosted.org/packages/76/57/da3c2f7784d1e7b7c48f8ce2e391314f6e50a14f5eb1a9d9460bb7b4fc8e/rfc3161_client-1.0.8-cp39-abi3-win_amd64.whl", hash = "sha256:5c1889d6bae269dc0f1e418f82e554b413066b5f8dd864f9367cf1ffb3d0a312", size = 2426065, upload-time = "2026-07-29T13:54:04.48Z" }, +] + +[[package]] +name = "rfc8785" +version = "0.1.4" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/ef/2f/fa1d2e740c490191b572d33dbca5daa180cb423c24396b856f5886371d8b/rfc8785-0.1.4.tar.gz", hash = "sha256:e545841329fe0eee4f6a3b44e7034343100c12b4ec566dc06ca9735681deb4da", size = 14321, upload-time = "2024-09-27T16:33:31.206Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/4d/78/119878110660b2ad709888c8a1614fce7e2fab39080ab960656dc8605bf6/rfc8785-0.1.4-py3-none-any.whl", hash = "sha256:520d690b448ecf0703691c76e1a34a24ddcd4fc5bc41d589cb7c58ec651bcd48", size = 9240, upload-time = "2024-09-27T16:33:29.683Z" }, +] + [[package]] name = "rich" version = "15.0.0" @@ -1977,6 +2061,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/57/c9/e69b1ff4c8b69093ef08b8919ab767af0569666865b39c30a8795d88d3c6/ruff-0.15.22-py3-none-win_arm64.whl", hash = "sha256:e1168075b72158510839f250027659cdd78476f40507dd517892304c41318661", size = 11298172, upload-time = "2026-07-16T15:14:10.51Z" }, ] +[[package]] +name = "securesystemslib" +version = "1.4.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/b8/11/9623c61604f9b8955248d43fc6a75658bb687c0d3ab65b032b2e43613bd5/securesystemslib-1.4.0.tar.gz", hash = "sha256:faea87be0f9c4b4277a5fa1b54bf9bfd807be9a94ab11be6c557dc8b75c43285", size = 934332, upload-time = "2026-05-27T08:01:53.21Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f9/29/3ff76b9d90ce4482dc8e8d216dc3a6b6d0c934c52f68b0738e2c99ca685f/securesystemslib-1.4.0-py3-none-any.whl", hash = "sha256:a0743a3d978cf26e98a70a57e3fbd5a18e0a74c20cabe615f6a55b02ef0272b3", size = 871457, upload-time = "2026-05-27T08:01:51.093Z" }, +] + [[package]] name = "setproctitle" version = "1.3.7" @@ -2031,6 +2124,56 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/50/20/16b2e170786aa6140344d3aac887fbd266717421ab4a79c0020df0e19726/signxml-5.1.0-py3-none-any.whl", hash = "sha256:f9d815164e35c8451295fc532d5292d2b10202a85a845c17c802fb15da8783c6", size = 62628, upload-time = "2026-07-05T02:03:01.556Z" }, ] +[[package]] +name = "sigstore" +version = "4.5.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, + { name = "id" }, + { name = "platformdirs" }, + { name = "pyasn1" }, + { name = "pydantic" }, + { name = "pyjwt" }, + { name = "pyopenssl" }, + { name = "requests" }, + { name = "rfc3161-client" }, + { name = "rfc8785" }, + { name = "rich" }, + { name = "sigstore-models" }, + { name = "sigstore-rekor-types" }, + { name = "tuf" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/18/e0/279419065e2d7102413605b3456122adbbccbc42e010b499c7b882fc01f8/sigstore-4.5.0.tar.gz", hash = "sha256:020d3e07f622b2916bf453e66ff6ff0711e1fdc5ab69e8bd8902f71d9fcb316f", size = 90969, upload-time = "2026-07-28T07:34:01.717Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/bf/7f/51dc313c06dd3ec6c5b308e334492cfdea0b02c9184a0da7b2db8c2a30a1/sigstore-4.5.0-py3-none-any.whl", hash = "sha256:f045b207f2e12605cf775ec38e89c5eda625d71ffa7830477db65e47ec2bc8b2", size = 111724, upload-time = "2026-07-28T07:34:00.211Z" }, +] + +[[package]] +name = "sigstore-models" +version = "0.0.6" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pydantic" }, + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/c6/ed/5c0ff809f90b19f4e971e17c1ed11f4df60082c6010b32a82054087e91e0/sigstore_models-0.0.6.tar.gz", hash = "sha256:c766c09470c2a7e8a4a333c893f07e2001c56a3ff1757b1a246119f53169a849", size = 7037, upload-time = "2025-11-26T19:18:12.61Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/12/dc/7a19864a7bc9f43121ab459e12768ab0080590e3e1b60a29b2f2eb01fb85/sigstore_models-0.0.6-py3-none-any.whl", hash = "sha256:5201a68f4d7d0f8bec1e2f4378eb646b084c52609a4e31db8c385095fff68b2e", size = 13213, upload-time = "2025-11-26T19:18:11.365Z" }, +] + +[[package]] +name = "sigstore-rekor-types" +version = "0.0.18" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pydantic", extra = ["email"] }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b4/54/102e772445c5e849b826fbdcd44eb9ad7b3d10fda17b08964658ec7027dc/sigstore_rekor_types-0.0.18.tar.gz", hash = "sha256:19aef25433218ebf9975a1e8b523cc84aaf3cd395ad39a30523b083ea7917ec5", size = 15687, upload-time = "2024-11-22T13:59:54.009Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/45/7c/f0b4e19fd424df4cc964f5d454e1d814fd2dc3b386342e6040441024318f/sigstore_rekor_types-0.0.18-py3-none-any.whl", hash = "sha256:b62bf38c5b1a62bc0d7fe0ee51a0709e49311d137c7880c329882a8f4b2d1d78", size = 20610, upload-time = "2024-11-22T13:59:52.684Z" }, +] + [[package]] name = "six" version = "1.17.0" @@ -2155,6 +2298,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/19/97/56608b2249fe206a67cd573bc93cd9896e1efb9e98bce9c163bcdc704b88/truststore-0.10.4-py3-none-any.whl", hash = "sha256:adaeaecf1cbb5f4de3b1959b42d41f6fab57b2b1666adb59e89cb0b53361d981", size = 18660, upload-time = "2025-08-12T18:49:01.46Z" }, ] +[[package]] +name = "tuf" +version = "7.0.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "securesystemslib" }, + { name = "urllib3" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/aa/40/25ceaf7f02e18b0d99150d94e200929351a542479c54abb7b92e1fd74b10/tuf-7.0.0.tar.gz", hash = "sha256:9d2e6723538e0d5a3e482b6de805fcfe64481448d5853039ba6b06ba541efd7f", size = 272032, upload-time = "2026-05-18T08:28:57.408Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/43/c7/301f699ad9427bb0e06935ed56850dd26eecb2b3e8e07b80311875eae676/tuf-7.0.0-py3-none-any.whl", hash = "sha256:572bdbdc9ff4a82278a0d4773e6100863b9b33023f27575e84ca65b486dd0d79", size = 55277, upload-time = "2026-05-18T08:28:56.123Z" }, +] + [[package]] name = "typing-extensions" version = "4.15.0"