From 55a7df226e368f4d78c9e3015296aacc3767d8ea Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 21 Aug 2026 21:24:35 +0200 Subject: [PATCH] test(785): pin the workflow's TRIGGERS, and sweep the retracted figure by subject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifth cold review returned MERGEABLE, having reproduced all five of the previous commit's measurements exactly and verified by execution that all twelve parametrised cases decide for their labelled reason rather than incidentally. Three residual items, none of which was coverage claimed and not delivered. The `on:` trigger was an uncovered channel. `_active_runs` started at `jobs:`, so a workflow whose triggers no longer include `pull_request` left a perfectly healthy `decisions-guard` job gating nothing while the proof stayed green. Nothing claimed to cover it, which is why it was LOW rather than a false coverage claim — but it is two lines and it is now the ninth disablement case. `on` parses to the BOOLEAN True in YAML 1.1, not the string `"on"`, so the key is looked up both ways. Verified which branch is live rather than assuming: pyyaml produces `True` here, so a lookup of `"on"` alone would have yielded an empty trigger set and fired the assertion on a correct workflow — a false red that the anti-vacuity check (the unmodified workflow must still hold) is what catches. `script.replace("python3", sys.executable)` was unbounded, having been `count=1` on a single extracted line before the withdrawal. It rewrote every occurrence including inside a path, so a step spelled `/usr/bin/python3` would have become nonsense and failed with a red blaming the catalog guard. Now substituted only where `python3` is a bare command word. And the retracted "1,488 records" figure survived in a second place — the leak guard's docstring — because the previous sweep went by file rather than by subject. Replaced with the reproduction and the figure that survives it, the same wording the inventory carries. `stale-comments-sweep-by-subject-not-verb`. Two channels remain uncovered and are deliberately not guessed at: a job `if:` whose expression is false only at run time, and `continue-on-error` as a non-literal expression. `_falsey`'s docstring already declines these as undecidable outside the runner, and no row, test name or docstring claims them. Refs: #785 Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/test_build_catalog_check_path.py | 22 ++++++++++++++++++- .../tests/test_worktree_ownership_guard.py | 5 +++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/scripts/tests/test_build_catalog_check_path.py b/scripts/tests/test_build_catalog_check_path.py index 2ed5cafd8..8a0ae5fde 100644 --- a/scripts/tests/test_build_catalog_check_path.py +++ b/scripts/tests/test_build_catalog_check_path.py @@ -169,6 +169,18 @@ def _active_runs(workflow: Path | None = None) -> list[str]: stepping around it. No shell parsing, and the ambiguous cases resolve by execution. """ doc = yaml.safe_load((workflow or WORKFLOW).read_text()) + + # The workflow's TRIGGERS, before its jobs. A `decisions-guard` job that is perfectly healthy + # gates nothing if the workflow stopped running on pull requests, and starting at `jobs:` cannot + # see that. Note `on` parses to the boolean True in YAML 1.1 (the Norway problem's cousin), so + # the key is looked up both ways rather than assumed. + triggers = (doc or {}).get("on", (doc or {}).get(True)) or {} + names = set(triggers) if isinstance(triggers, dict) else {triggers} if isinstance(triggers, str) else set(triggers) + assert "pull_request" in names, ( + "pr-checks.yml no longer runs on `pull_request`, so NO gate in it — including the catalog " + f"guard — fires on a PR. Triggers found: {sorted(str(n) for n in names)}" + ) + jobs = (doc or {}).get("jobs") or {} job = jobs.get("decisions-guard") assert job is not None, ( @@ -250,7 +262,11 @@ def _ci_check_command(workflow: Path | None = None) -> list[str]: "the catalog step's script interpolates an Actions expression, which cannot be expanded " f"outside the runner — this proof would be executing something else: {script!r}" ) - return ["bash", "-c", script.replace("python3", sys.executable)] + # Substitute the interpreter only where `python3` is a bare command word. A plain + # `str.replace` rewrites EVERY occurrence, including inside a path — `/usr/bin/python3` would + # become `/usr/bin//bin/python3` and fail with ENOENT, a red blaming the catalog guard for + # something this line did. + return ["bash", "-c", re.sub(r"(? bool: @@ -345,6 +361,10 @@ _STEP = ( 1, ), ), + ( + "the WORKFLOW no longer runs on pull requests", + lambda s: s.replace("on:\n pull_request:", "on:\n workflow_dispatch:", 1), + ), ( "the command survives only as HEREDOC DATA", lambda s: s.replace( diff --git a/scripts/tests/test_worktree_ownership_guard.py b/scripts/tests/test_worktree_ownership_guard.py index 60f6dab12..9a461c352 100644 --- a/scripts/tests/test_worktree_ownership_guard.py +++ b/scripts/tests/test_worktree_ownership_guard.py @@ -63,8 +63,9 @@ def _env() -> dict: to the REAL `$HOME/.cache/ersatztv/hook-fire/` log instead of the fixture's tmp dir. That is not untidiness: it is #776's defect reintroduced in the file that is meant to prove #776's hooks, and it corrupts the `hook-fire-log.sh report` surface this repo cites as the observability claim - for every guard still graded NONE. Measured before the fix: 1,488 records for the two fake - session ids below in the production log. + for every guard still graded NONE. The reproduction, rather than a figure whose evidence has + since been deleted: reintroduce the snapshot and run this file, then count records for the two + synthetic session ids below — 58 per run, on macOS and Linux alike. """ return {**os.environ, "CLAUDE_PROJECT_DIR": str(REPO_ROOT)}